Re: good patch vs. sed fixes

Hi,

On Thu, 21 Nov 2024, Xi Ruoyao wrote:

> On Thu, 2024-11-21 at 03:40 +0100, Uwe Düffert wrote:
>>
>> I'm not sure it is worth the effort, but from time to time I stumble
>> across the "problem" that certain sed or patch commands would apply
>> multiple times although they should not. I'm fully aware this is
>> unsupported "deviating from the book" but it happens. I'm trying to build
>> some not-yet-in-the-book new version, something fails, I fix the problem
>> and try again, then some unexpected "double sed" or "double patch"
>> silently breaks stuff. Sure, I should have done it cleanly from scratch,
>> but the nature of the fixed problem suggested I could just repeat the
>> build...
> Then why not just issue "./mach build"?  Why the sed is ever executed
> again?
For some reason, a fresh version of a package does not build with 
original instructions, so I need to modify something. If this was only 
package sources, ./mach build (in case of firefox) would be OK. But 
usually I would have to recompile a dependency or add a patch to my 
build script or add/remove an option and reconfigure, so usually need to 
re-run more then just ./mach build (or the like). My buildscript also 
logs all output which simplifies finding the source of a build failure.
So there are cases regulary where I'm tempted to just re-run the build 
script, e.g. if configure fails early and that is easily fixable. A 
unified patch usually would not hurt in such sceanrio, in contrast it 
would tell me via "reversed or previous applied patch" that everything is 
as it should. When my firefox build bailed out with a poppler problem I 
already had applied the sed multiple times without being aware and would 
not spot the difference anymore, the line had multiple CLASS_CHARACTER 
instead of one, presumable successfully applied, still build broke. I 
surely tried pure ./mach build a few times at that point, because I 
wanted to identify the cause before starting from scratch until I finally 
noticed what the multi-sed actually did silently.

>> [...]
>> I'm not proposing using some more official patch such as
>> https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=d9712b1358dc1b64933c3090d72153aae3270f25 
>> in this case, I'd just like to mention that this patch 1) Bmakes the
>> idea more clear (there are not just 5 more CLASS_CHARACTERs but they do
>> have names)
> The names are not relevant.
I fully agree. It would only have made it easier for me to see what 
actually happened. Just as the output of patch would have done.

> [...]
> Sometimes I do deliberately make a sed no-op if applied twice.  But it's
> only a bonus, not something we need to guarantee.  And sometimes such a
> bonus is a bad thing: when we update the package to a new version where
> the fix is already upstream, the editor can easily forget to remove the
> sed which is no-op now.  (OTOH if it's just "double applied" the build
> fails and the editor immediately notice it.)
Wow. I like that argument. It nearly convinced me that "breakage by 
double-sed" is actually a good thing. But after thinking further about 
it, my actual problem with those seds is their silent execution, for 
success as well as for failure. A sed could easily "apply a second time" 
although it only got executed once when upstream already fixed it the same 
way. Adding a missing #include for c++17 compatibility for example would 
not be spotted that way, as it does not hurt to apply it multiple times. 
In contrast to a unified patch that would most likely report it has 
nothing to do...

Uwe

-- 
http://lists.linuxfromscratch.org/sympa/info/blfs-dev
Unsubscribe: See the above information page