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

[Maxime Devos]

Thiago Jung Bauermann via Guix-patches via schreef op za 28-08-2021 om 13:43 
[-0300]:
> [...]
> diffutils has a race condition in its signal processing code which is easy
> to trigger on powerpc64le-linux. More often than not, it causes the
> ‘colors’ test to fail and therefore the build of the package fails as well.
> [...]

Adding this patch makes sense to me too, though I don't have a powerpc64le-linux
to test this on.

> Add the patch proposed in Debian bug 922552 which fixes the problem.

Do you think upstream will have its own working patch soonish?
(See <https://debbugs.gnu.org/cgi/bugreport.cgi?bug=36488#41>, for people
who aren't Thiago Jung Bauermann.)  If so, it might make sense to wait a little
for an ‘official’ patch.

> * gnu/packages/patches/diffutils-fix-signal-processing.patch: New file.
> * gnu/local.mk (dist_patch_DATA): Add it.
> * gnu/packages/base.scm (diffutils)[source]: Use it.
> ---
> 
> Hello,
> 
> 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
> 
> 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’.
> 
>  gnu/local.mk                                  |  1 +
>  gnu/packages/base.scm                         |  3 +-
>  .../diffutils-fix-signal-processing.patch     | 60 +++++++++++++++++++
>  3 files changed, 63 insertions(+), 1 deletion(-)
>  create mode 100644 gnu/packages/patches/diffutils-fix-signal-processing.patch
> 
> diff --git a/gnu/local.mk b/gnu/local.mk
> index 11b002b66e72..bc385eecc592 100644
> --- a/gnu/local.mk
> +++ b/gnu/local.mk
> @@ -962,6 +962,7 @@ dist_patch_DATA =                                         
> \
>    %D%/packages/patches/desmume-gcc6-fixes.patch                      \
>    %D%/packages/patches/desmume-gcc7-fixes.patch                      \
>    %D%/packages/patches/dfu-programmer-fix-libusb.patch               \
> +  %D%/packages/patches/diffutils-fix-signal-processing.patch \
>    %D%/packages/patches/diffutils-gets-undeclared.patch               \
>    %D%/packages/patches/disarchive-cross-compilation.patch    \
>    %D%/packages/patches/dkimproxy-add-ipv6-support.patch              \
> diff --git a/gnu/packages/base.scm b/gnu/packages/base.scm
> index 2c648953ae39..0e3b346b93a0 100644
> --- a/gnu/packages/base.scm
> +++ b/gnu/packages/base.scm
> @@ -272,7 +272,8 @@ differences.")
>                                  version ".tar.xz"))
>              (sha256
>               (base32
> -              "1v4g8gi0lgakqa7iix8s4fq7lq6l92vw3rjd9wfd2rhjng8xggd6"))))
> +              "1v4g8gi0lgakqa7iix8s4fq7lq6l92vw3rjd9wfd2rhjng8xggd6"))
> +            (patches (search-patches 
> "diffutils-fix-signal-processing.patch"))))
>     (build-system gnu-build-system)
>     (native-inputs (list perl))
>     (synopsis "Comparing and merging files")
> diff --git a/gnu/packages/patches/diffutils-fix-signal-processing.patch 
> b/gnu/packages/patches/diffutils-fix-signal-processing.patch
> new file mode 100644
> index 000000000000..24130bd4c37a
> --- /dev/null
> +++ b/gnu/packages/patches/diffutils-fix-signal-processing.patch
> @@ -0,0 +1,60 @@
> +Author: Frédéric Bonnard <frediz@debian.org>
> +
> +Obtained from:
> +
> +https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=922552#19
> +
> +and slightly adapted to apply on v3.8.
> +
> +Fixes bug reported upstream at:
> +
> +https://debbugs.gnu.org/cgi/bugreport.cgi?bug=34519
> +
> +diff --git a/src/diff.c b/src/diff.c
> +index 9938daa0c8fd..2bc443f1ca70 100644
> +--- a/src/diff.c
> ++++ b/src/diff.c
> +@@ -1453,6 +1453,8 @@ compare_files (struct comparison const *parent,
> +         }
> +     }
> + 
> ++  final_process_signals ();
> ++
> +   /* Now the comparison has been done, if no error prevented it,
> +      and STATUS is the value this function will return.  */
> + 
> +diff --git a/src/diff.h b/src/diff.h
> +index 27362c010fd2..28c89b0797ef 100644
> +--- a/src/diff.h
> ++++ b/src/diff.h
> +@@ -390,6 +390,7 @@ extern enum changes analyze_hunk (struct change *, lin 
> *, lin *, lin *, lin *);
> + extern void begin_output (void);
> + extern void debug_script (struct change *);
> + extern void fatal (char const *) __attribute__((noreturn));
> ++extern void final_process_signals (void);
> + extern void finish_output (void);
> + extern void message (char const *, char const *, char const *);
> + extern void message5 (char const *, char const *, char const *,
> +diff --git a/src/util.c b/src/util.c
> +index 4348757e1507..8954197f33fc 100644
> +--- a/src/util.c
> ++++ b/src/util.c
> +@@ -237,6 +237,18 @@ process_signals (void)
> +     }
> + }
> + 
> ++/* Process remaining signals once before exit  */
> ++void
> ++final_process_signals (void)
> ++{
> ++  static int last = 1;
> ++
> ++  if (last) {
> ++    process_signals ();
> ++    last = 0;
> ++  }
> ++}
> ++
> + static void
> + install_signal_handlers (void)
> + {
> 
> 
>

signature.asc
Description: This is a digitally signed message part