[bug#50239] [PATCH core-updates-frozen] gnu: diffutils: Fix signal processing.

[Thiago Jung Bauermann via Guix-patches via <guix-patches@gnu.org>]

Hi Ludo,

Em segunda-feira, 6 de setembro de 2021, às 08:55:52 -03, Ludovic Courtès 
escreveu:
> Hi,
> 
> Thiago Jung Bauermann <bauermann@kolabnow.com> skribis:
> > This fixes the build of diffutils on powerpc64le-linux, which currently
> > fails more often than not. The patch I’m adding here isn’t being
> > shipped by Debian and hasn’t been seen by upstream yet. I just brought
> > it to their attention here:
> > 
> > https://debbugs.gnu.org/cgi/bugreport.cgi?bug=34519#11
> 
> Thanks for working on this!

You’re welcome. Thank you for reviewing the patches!

> > I’m not familiar with the diffutils code base, but FWIW I analysed
> > the patch and it looks very reasonable to me. To be honest I’m not
> > sure if it completely fixes the race condition or just makes it much
> > less likely to happen, but in any case I can’t hit the race condition
> > anymore.
> > 
> > In addition, since all it does is add a new call to the function which
> > checks and processes any pending signal, I don’t think it can cause
> > any harm.
> > 
> > Finally, this patch is based on top of the one which updates diffutils
> > to version 3.8:
> > 
> > https://issues.guix.gnu.org/50233
> > 
> > The fix works equally well in version 3.7 so if you think it’s not
> > worth updating diffutils I can rebase this patch on top of current
> > ‘core-updates-frozen’.
> 
> Normally we won’t update diffutils on ‘core-updates-frozen’.

Yes, it makes sense.

> Thus, could you adjust this patch so that (1) it applies on
> ‘core-updates-frozen’, and (2) it doesn’t change derivations on other
> platforms (thus, the patch needs to be applied from a build phase)?

Sure, no problem. I’ll do that. Thanks for the tip about having to apply 
the patch from a build phase.

> Bonus point if there’s an upstream patch to use, as Maxime suggested.

There isn’t one for now. We have three alternatives if the situation 
doesn’t change by the time ‘core-updates-frozen’ gets merged:

1. Apply this patch.

2. Disable the ‘colors’ test. This is what Debian is doing. It’s a bit 
unfortunate in that we’re disabling a test that just did its job of finding 
a bug, but on the other hand the bug is minor (the terminal is left in a 
wrong state if ‘diff’ is interrupted in the middle of printing colored 
output) and easy to work around (just run ‘reset’ if that happens).

3. Do nothing. If the diffutils build fails on the CI, restart it until it 
succeeds. :-)

That is my personal order of preference, but option 1 carries a slight risk 
in that we would be shipping a patch that isn’t “battle tested”, so the 
other options are very reasonable too.

-- 
Thanks,
Thiago