[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] gnu: libffi: Add unreleased patch to fix float128 on powerpc
From: |
Mark H Weaver |
Subject: |
Re: [PATCH] gnu: libffi: Add unreleased patch to fix float128 on powerpc64le. |
Date: |
Tue, 22 Dec 2020 00:15:25 -0500 |
Hi,
There's a problem with the following commit:
> commit 7eaa2f24ea77cddbb4bbc2d6a6905673a36f8f99
> Author: John Doe <dftxbs3e@free.fr>
> Date: Tue Dec 15 10:23:44 2020 +0100
>
> gnu: libffi: Add unreleased patch to fix float128 on powerpc64le.
>
> Fixes <https://bugs.gnu.org/45252>.
>
> * gnu/packages/patches/libffi-float128-powerpc64le.patch: Import patch file
> from <https://github.com/libffi/libffi/pull/561.patch>.
> * gnu/packages/libffi.scm (libffi)[arguments]: Apply patch conditionally for
> powerpc64le-* systems in a phase.
> [inputs]: Add patch as input conditionally for powerpc64le-* systems.
> * gnu/local.mk (dist_patch_DATA): Add patch file to build system.
>
> Signed-off-by: Chris Marusich <cmmarusich@gmail.com>
The problem is in how the 'patch' program is invoked, here:
> diff --git a/gnu/packages/libffi.scm b/gnu/packages/libffi.scm
> index d324892330..66239e0363 100644
> --- a/gnu/packages/libffi.scm
> +++ b/gnu/packages/libffi.scm
[...]
> @@ -67,13 +68,28 @@
> "powerpc-patch")))
> (invoke "patch" "--batch" "-p1"
> "-i" patch))))))
> + '())
> + ,@(if (string-prefix? "powerpc64le-" (or (%current-target-system)
> + (%current-system)))
> + '(#:phases (modify-phases %standard-phases
> + (add-after 'unpack 'apply-patch2
> + (lambda* (#:key inputs #:allow-other-keys)
> + (let ((patch (assoc-ref inputs
> + "powerpc64le-patch")))
> + (invoke "patch" "--batch" "-p1"
> + "-i" patch))))))
> '())))
When invoking 'patch' in Guix, you should *always* use "--force" instead
of "--batch". There's a crucial difference between these two options:
If 'patch' finds that the given patch has already been applied, then
"--batch" will automatically *revert* the patch, whereas "--force" will
raise an error. Here's the relevant section of the 'diffutils' manual:
> 10.11.2 Inhibiting Keyboard Input
> ---------------------------------
>
> There are two ways you can prevent 'patch' from asking you any
> questions. The '--force' ('-f') option assumes that you know what you
> are doing. It causes 'patch' to do the following:
>
> * Skip patches that do not contain file names in their headers.
>
> * Patch files even though they have the wrong version for the
> 'Prereq:' line in the patch;
>
> * Assume that patches are not reversed even if they look like they
> are.
>
> The '--batch' ('-t') option is similar to '-f', in that it suppresses
> questions, but it makes somewhat different assumptions:
>
> * Skip patches that do not contain file names in their headers (the
> same as '-f').
>
> * Skip patches for which the file has the wrong version for the
> 'Prereq:' line in the patch;
>
> * Assume that patches are reversed if they look like they are.
Now consider what will happen when we upgrade 'libffi' to a newer
version that already includes this fix. If the Guix developer who
performs the upgrade forgets to remove this patch, the 'patch'
invocation above will start silently re-inserting the old bug.
We ran into this exact problem in the early years of Guix, and
henceforth changed all of the invocations of 'patch' to use '--force'.
Can we fix this right away, before many powerpc64le-* binaries are built
on top of it?
In any case, thanks very much for working on the powerpc64le port!
Regards,
Mark
- Re: [PATCH] gnu: libffi: Add unreleased patch to fix float128 on powerpc64le.,
Mark H Weaver <=