Re: good patch vs. sed fixes

Hi,

On Sat, 23 Nov 2024, Rainer Fiebig wrote:

> Am 22.11.24 um 00:06 schrieb Uwe DÃŒffert ([email protected] via
> blfs-dev Mailing List):
>> 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.
> Sorry, but your approach is wrong. [...]
I'm aware of that and mentioned it from the beginning. I just told the 
story how I happend to stumble across that "issue". I'm in no way trying 
to imply you should do it like me or the book should be robust against 
that.

> I also have a problem with the subject of your post as it seems to
> indicate that a patch would generally be superior to a sed-statement.
I'm sorry for that, this wasn't my intention. I now realize this can be 
mis-read, the idea behind it was "good (patch vs. sed) fixes". There are 
valid reasons for both, I just wanted to share my thoughts about what 
works well, for patch as well as for sed.

> The problem rather seems
> to be that you do not fully understand how sed works or what the
> statements do _before_ you apply them.  Using the seds with "-i.orig" or
> "-i.bkp" and then comparing the two files before actually building 
> the package might be helpful in this respect.
No, *my* problem was that sed *unnoticedly* applied multiple times. 
Here this surely was my fault. I'm not blaming the sed. I do feel stupid 
for not recognizing it earlier, I easily could have by looking at the sed 
more closely or the result of it or any backup.

I decided to write about it nevertheless, because this also could have 
happened when a new upstream version fixes something in similar way, I 
wrongly keep applying the sed - once, so it is still basically 
double-applied in total. If it breaks the build I will notice, but it 
could also introduce way more subtile mis-behavior. That may be not a 
problem for the book at all because editors better check than me whether 
the fix is still needed and applies correctly. For me, that job would be 
easier if either such sed/patch only applies once or like patch reports 
when it did not apply.

Uwe

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