bug#51559: [PATCH] gnu: webkitgtk: Disable SSE2 when not on x86_64.

[Mark H Weaver <mhw@netris.org>]

Hi Liliana,

Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

> Am Freitag, den 05.11.2021, 20:15 +0000 schrieb Maxime Devos:
>> Liliana Marie Prikler schreef op vr 05-11-2021 om 21:06 [+0100]:
>> > * gnu/packages/webkit.scm (webkitgtk)[#:phases]: Add ‘disable-
>> > sse2’.
>> > ---
>> >  gnu/packages/webkit.scm | 5 +++++
>> >  1 file changed, 5 insertions(+)
>> > 
>> > diff --git a/gnu/packages/webkit.scm b/gnu/packages/webkit.scm
>> > index e54b680855..45150e185f 100644
>> > --- a/gnu/packages/webkit.scm
>> > +++ b/gnu/packages/webkit.scm
>> > @@ -295,6 +295,11 @@ (define-public webkitgtk
>> >                   (("libWPEBackend-fdo-([\\.0-9]+)\\.so" all
>> > version)
>> >                    (string-append wpebackend-fdo "/lib/" all)))
>> >                 #t)))
>> > +         (add-after 'unpack 'disable-sse2
>> > +           (lambda* (#:key system #:allow-other-keys)
>> > +             (unless (string-prefix? "x86_64" system)
>> 
>> IIUC, when cross-compiling, you need to look at 'target', not
>> 'system'.  At least, that's the case if %current-system/%current-
>> target-system is used. Not sure about 'system' and 'target'.
> Do target and system follow the same rules?

Unfortunately not.  It's a bit of a mess, IMO.

%current-system is a Nix system string, e.g. "x86_64-linux",
"i686-linux", "aarch64-linux", etc.

%current-target-system is either #f (if not cross-compiling) or a GNU
triplet, e.g. "x86_64-unknown-linux-gnu".

It is quite common to write code like:

__ (or %current-target-system
______ %current-system)

but then you must then write code that copes properly with either Nix
system strings or GNU triplets.

> If so, I could write (or target system), no?  Packages on master sadly
> follow wildly different conventions and I picked one at random.

A nice touch would be to use quasiquotation to perform the target checks
in the host-side code, maybe something like this (after adding
'target-x86-32?' from 'core-updates-frozen' to (guix utils)):

--8<---------------cut here---------------start------------->8---
diff --git a/gnu/packages/webkit.scm b/gnu/packages/webkit.scm
index 358254f23f..439341c222 100644
--- a/gnu/packages/webkit.scm
+++ b/gnu/packages/webkit.scm
@@ -292,6 +292,12 @@ acceleration in mind, leveraging common 3D graphics APIs for best performance.")
                  (("libWPEBackend-fdo-([\\.0-9]+)\\.so" all version)
                   (string-append wpebackend-fdo "/lib/" all)))
                #t)))
+         ,@(if (target-x86-32?)
+               '((add-after 'unpack 'disable-sse2
+                   (lambda _
+                     (substitute* "Source/cmake/DetectSSE2.cmake"
+                       (("CHECK_FOR_SSE2\\(\\)") "")))))
+               '())
          (add-before 'configure 'prepare-build-environment
            (lambda* (#:key inputs #:allow-other-keys)
              (setenv "CC" "clang")
--8<---------------cut here---------------end--------------->8---

The nice thing about this approach is that it does not cause rebuilds on
other systems.  This benefit is especially useful for core packages,
where the cost of rebuilds can be prohibitive, but where it is sometimes
necessary to add more target-specific bits like this when Guix is
bootstrapped on a new platform.

What do you think?

Anyway, thanks for working on it.

      Regards,
        Mark

-- 
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>.