bug#51559: Webkit fails to build

bug#51559: Webkit fails to build

[Liliana Marie Prikler]

Copypasta of the relevant log
(/var/log/guix/drvs/pr/nl8g1ky1z3l3dfp4llh50w5qzd2qz4-webkitgtk-
2.34.1.drv.bz2):

-- Checking for module 'egl'
--   Found egl, version 20.2.4
-- Found EGL: /gnu/store/jf269s6clr6r57p8v5c3c1qkyra6apq2-mesa-
20.2.4/include  
-- Checking for module 'gl'
--   Found gl, version 20.2.4
-- Found OpenGL: /gnu/store/jf269s6clr6r57p8v5c3c1qkyra6apq2-mesa-
20.2.4/include  
-- Looking for include file GL/glx.h
-- Looking for include file GL/glx.h - found
-- Checking for module 'glesv2'
--   Found glesv2, version 20.2.4
-- Found OpenGLES2: /gnu/store/jf269s6clr6r57p8v5c3c1qkyra6apq2-mesa-
20.2.4/lib/libGLESv2.so (found version "3.2") 
-- Checking for module 'gtk+-3.0'
--   Found gtk+-3.0, version 3.24.24
-- Checking for module 'gtk+-unix-print-3.0'
--   Found gtk+-unix-print-3.0, version 3.24.24
-- Found GTK: 3.24.24 (Required is at least version "3.22.0") 
-- Could NOT find LibSoup: Found unsuitable version "", but required is
at least "2.99.9" (found LIBSOUP_INCLUDE_DIRS-NOTFOUND)
CMake Error at Source/cmake/OptionsGTK.cmake:212 (message):
  libsoup 3 is required.  Enable USE_SOUP2 to use libsoup 2 (disables
HTTP/2)
Call Stack (most recent call first):
  Source/cmake/WebKitCommon.cmake:220 (include)
  CMakeLists.txt:20 (include)


-- Configuring incomplete, errors occurred!
See also "/tmp/guix-build-webkitgtk-2.34.1.drv-
0/build/CMakeFiles/CMakeOutput.log".
See also "/tmp/guix-build-webkitgtk-2.34.1.drv-
0/build/CMakeFiles/CMakeError.log".
command "cmake" "../webkitgtk-2.34.1" "-DCMAKE_BUILD_TYPE=Release" "-
DCMAKE_INSTALL_PREFIX=/gnu/store/4vchdgd2c3kvgyf8s9z54985vf80gsaa-
webkitgtk-2.34.1" "-DCMAKE_INSTALL_LIBDIR=lib" "-
DCMAKE_INSTALL_RPATH_USE_LINK_PATH=TRUE" "-
DCMAKE_INSTALL_RPATH=/gnu/store/4vchdgd2c3kvgyf8s9z54985vf80gsaa-
webkitgtk-2.34.1/lib" "-DCMAKE_VERBOSE_MAKEFILE=ON" "-DPORT=GTK" "-
DENABLE_GTKDOC=ON" "-DENABLE_GAMEPAD=OFF" "-DUSE_SYSTEMD=OFF" "-
DLIB_INSTALL_DIR=/gnu/store/4vchdgd2c3kvgyf8s9z54985vf80gsaa-webkitgtk-
2.34.1/lib" "-DUSE_GSTREAMER_GL=OFF" failed with status 1

bug#51559: Webkit fails to build

[Maxim Cournoyer]

Hello,

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

> Copypasta of the relevant log
> (/var/log/guix/drvs/pr/nl8g1ky1z3l3dfp4llh50w5qzd2qz4-webkitgtk-
> 2.34.1.drv.bz2):
>
> -- Checking for module 'egl'
> --   Found egl, version 20.2.4
> -- Found EGL: /gnu/store/jf269s6clr6r57p8v5c3c1qkyra6apq2-mesa-
> 20.2.4/include  
> -- Checking for module 'gl'
> --   Found gl, version 20.2.4
> -- Found OpenGL: /gnu/store/jf269s6clr6r57p8v5c3c1qkyra6apq2-mesa-
> 20.2.4/include  
> -- Looking for include file GL/glx.h
> -- Looking for include file GL/glx.h - found
> -- Checking for module 'glesv2'
> --   Found glesv2, version 20.2.4
> -- Found OpenGLES2: /gnu/store/jf269s6clr6r57p8v5c3c1qkyra6apq2-mesa-
> 20.2.4/lib/libGLESv2.so (found version "3.2") 
> -- Checking for module 'gtk+-3.0'
> --   Found gtk+-3.0, version 3.24.24
> -- Checking for module 'gtk+-unix-print-3.0'
> --   Found gtk+-unix-print-3.0, version 3.24.24
> -- Found GTK: 3.24.24 (Required is at least version "3.22.0") 
> -- Could NOT find LibSoup: Found unsuitable version "", but required is
> at least "2.99.9" (found LIBSOUP_INCLUDE_DIRS-NOTFOUND)
> CMake Error at Source/cmake/OptionsGTK.cmake:212 (message):
>   libsoup 3 is required.  Enable USE_SOUP2 to use libsoup 2 (disables
> HTTP/2)
> Call Stack (most recent call first):
>   Source/cmake/WebKitCommon.cmake:220 (include)
>   CMakeLists.txt:20 (include)
>
>
> -- Configuring incomplete, errors occurred!
> See also "/tmp/guix-build-webkitgtk-2.34.1.drv-
> 0/build/CMakeFiles/CMakeOutput.log".
> See also "/tmp/guix-build-webkitgtk-2.34.1.drv-
> 0/build/CMakeFiles/CMakeError.log".
> command "cmake" "../webkitgtk-2.34.1" "-DCMAKE_BUILD_TYPE=Release" "-
> DCMAKE_INSTALL_PREFIX=/gnu/store/4vchdgd2c3kvgyf8s9z54985vf80gsaa-
> webkitgtk-2.34.1" "-DCMAKE_INSTALL_LIBDIR=lib" "-
> DCMAKE_INSTALL_RPATH_USE_LINK_PATH=TRUE" "-
> DCMAKE_INSTALL_RPATH=/gnu/store/4vchdgd2c3kvgyf8s9z54985vf80gsaa-
> webkitgtk-2.34.1/lib" "-DCMAKE_VERBOSE_MAKEFILE=ON" "-DPORT=GTK" "-
> DENABLE_GTKDOC=ON" "-DENABLE_GAMEPAD=OFF" "-DUSE_SYSTEMD=OFF" "-
> DLIB_INSTALL_DIR=/gnu/store/4vchdgd2c3kvgyf8s9z54985vf80gsaa-webkitgtk-
> 2.34.1/lib" "-DUSE_GSTREAMER_GL=OFF" failed with status 1

That's fixed on core-updates-frozen-batched-changes, having upgraded
webkitgtk to 2.34.4 and libsoup to 3.0.1.

Closing.

Maxim

bug#51559: Webkit fails to build

[Mark H Weaver]

reopen 51559
thanks

Hi Maxim,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
> That's fixed on core-updates-frozen-batched-changes, having upgraded
> webkitgtk to 2.34.4 and libsoup to 3.0.1.
>
> Closing.

Unless I'm mistaken, there still remains the problem that 'webkitgtk'
and all of its dependents are currently broken on 'master', so I'm
reopening the bug.

      Mark

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

bug#51559: Webkit fails to build

[Maxim Cournoyer]

Hello,

Mark H Weaver <mhw@netris.org> writes:

> reopen 51559
> thanks
>
> Hi Maxim,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>> That's fixed on core-updates-frozen-batched-changes, having upgraded
>> webkitgtk to 2.34.4 and libsoup to 3.0.1.
>>
>> Closing.
>
> Unless I'm mistaken, there still remains the problem that 'webkitgtk'
> and all of its dependents are currently broken on 'master', so I'm
> reopening the bug.

Thanks Mark (also for the fix) -- I had mistakenly thought the issue was
about core-updates-frozen.

Note that on the core-updates-frozen-batched-changes branch webkitgtk
(with libsoup2 or 3) could still be built with GCC (version 10).

Thanks,

Maxim

bug#51559: Webkit fails to build

[Liliana Marie Prikler]

Hi,

Am Donnerstag, den 04.11.2021, 14:23 -0400 schrieb Maxim Cournoyer:
> Note that on the core-updates-frozen-batched-changes branch webkitgtk
> (with libsoup2 or 3) could still be built with GCC (version 10).
Should we try building webkitgtk and its dependants with GCC 10 on
master or should we rather wait for the core-updates-frozen merge?  Do
we have an ETA?

CC'd lfam due to security relevance

Cheers

bug#51559: Webkit fails to build

[Mark H Weaver]

Hi,

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

> Am Donnerstag, den 04.11.2021, 14:23 -0400 schrieb Maxim Cournoyer:
>> Note that on the core-updates-frozen-batched-changes branch webkitgtk
>> (with libsoup2 or 3) could still be built with GCC (version 10).
> Should we try building webkitgtk and its dependants with GCC 10 on
> master

I've already verified that webkitgtk-2.34.1 can be successfully built
with GCC 10, because I tried it on my private branch (simply because I
already have a built GCC 10 on my system, and I don't use substitutes).
However, I ran into the same problem where programs linked to webkitgtk
failed.

My knowledge of C++ is weak (I avoid it like the plague) but I guess
this might be because GCC 10 and GCC 7 use different versions of the
standard C++ library, which are not ABI compatible with each other.

Since GCC 10 is the default compiler on 'core-updates-frozen', I think
it's quite likely that webkitgtk-2.34.1 and its programs can be built
successfully on 'core-updates-frozen' with its default compiler.  That
would need to be verified, of course.

     Regards,
       Mark

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

bug#51559: Webkit fails to build

[Maxim Cournoyer]

Hi Mark,

Mark H Weaver <mhw@netris.org> writes:

> Hi,
>
> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
>
>> Am Donnerstag, den 04.11.2021, 14:23 -0400 schrieb Maxim Cournoyer:
>>> Note that on the core-updates-frozen-batched-changes branch webkitgtk
>>> (with libsoup2 or 3) could still be built with GCC (version 10).
>> Should we try building webkitgtk and its dependants with GCC 10 on
>> master
>
> I've already verified that webkitgtk-2.34.1 can be successfully built
> with GCC 10, because I tried it on my private branch (simply because I
> already have a built GCC 10 on my system, and I don't use substitutes).
> However, I ran into the same problem where programs linked to webkitgtk
> failed.
>
> My knowledge of C++ is weak (I avoid it like the plague) but I guess
> this might be because GCC 10 and GCC 7 use different versions of the
> standard C++ library, which are not ABI compatible with each other.
>
> Since GCC 10 is the default compiler on 'core-updates-frozen', I think
> it's quite likely that webkitgtk-2.34.1 and its programs can be built
> successfully on 'core-updates-frozen' with its default compiler.  That
> would need to be verified, of course.

It's already been verified on the core-updates-frozen-batched-changes
branch (which had webkitgtk-2.34.1 before master).

Thanks,

Maxim

bug#51559: Webkit fails to build

[Liliana Marie Prikler]

Hi,

Am Donnerstag, den 04.11.2021, 17:58 -0400 schrieb Mark H Weaver:
> However, I ran into the same problem where programs linked to
> webkitgtk failed.
> 
> My knowledge of C++ is weak (I avoid it like the plague) but I guess
> this might be because GCC 10 and GCC 7 use different versions of the
> standard C++ library, which are not ABI compatible with each other.
Yes, you will have to add GCC 10 to the packages that fail to link. 
Maxim pointed out, that it already woks on core-updates-frozen-batched-
changes, what we need to check is which packages will require GCC 10 on
master.

Cheers

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

[Liliana Marie Prikler]

* 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)
+               (substitute* "Source/cmake/DetectSSE2.cmake"
+                 (("CHECK_FOR_SSE2\\(\\)") "")))))
          (add-before 'configure 'prepare-build-environment
            (lambda* (#:key inputs #:allow-other-keys)
              (setenv "CC" "clang")
-- 
2.33.1

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

[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'.

Suggestion:
  (unless ,(target-x86-64?)
    (substitute* ...))

Greetings,
Maxime.

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

[Liliana Marie Prikler]

Hi,

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

> Suggestion:
>   (unless ,(target-x86-64?)
>     (substitute* ...))
target-x86-64 does not exist on master yet, but that'd probably work
for c-u-frozen, yeah.

Greetings,
Liliana

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

[Maxime Devos]

Liliana Marie Prikler schreef op vr 05-11-2021 om 23:19 [+0100]:
> > > [...]
> > > +         (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?  If so, I could write (or
> target system), no?  Packages on master sadly follow wildly different
> conventions and I picked one at random.

Looking at (guix build system gnu), there are three system/target-like
arguments:

a. system, which is set to (%current-system)
b. build, which is set to the GNU triplet equivalent (%current-system) 
c. target, which is set to (%current-target-system) (#f when compiling
natively)

So it appears system and target correspond to (%current-system) and
(%current-target-system), so (or target system) should work, I think.

I prefer ,(target-x86-64?) myself though, because it avoids having
to remember that 'target' is not set when compiling natively. I think
'target' should always be set, because compiling natively is ‘merely’
a special case of cross-compiling, and (or target system)/(or
(%current-target-system) (%current-system)) seems a bit repetitive
to me). That would probably break some package definitions though,
and not really related to webkitgtk.

Also, that the arch comes first in GNU triplets and nix system strings,
seems a bit an implementation detail to me. Not terribly important /
YMMV though.

Greetings,
Maxime.

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

[Mark H Weaver]

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>.

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

[Liliana Marie Prikler]

* gnu/packages/webkit.scm (webkitgtk)[#:phases]: Add ‘disable-sse2’ if the
current (target) system is not an x86_64 one.
---
Hi Maxime,

I tried conditionally disabling SSE2 on the host side, but for some reason this
patch would cause a rebuild for me.  Did I do something wrong?

 gnu/packages/webkit.scm | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/webkit.scm b/gnu/packages/webkit.scm
index e54b680855..ca50ab37a8 100644
--- a/gnu/packages/webkit.scm
+++ b/gnu/packages/webkit.scm
@@ -241,7 +241,7 @@ (define-public webkitgtk
     (build-system cmake-build-system)
     (outputs '("out" "doc"))
     (arguments
-     '(#:tests? #f ; no tests
+     `(#:tests? #f ; no tests
        #:build-type "Release" ; turn off debugging symbols to save space
        #:configure-flags (list
                           "-DPORT=GTK"
@@ -295,6 +295,13 @@ (define-public webkitgtk
                  (("libWPEBackend-fdo-([\\.0-9]+)\\.so" all version)
                   (string-append wpebackend-fdo "/lib/" all)))
                #t)))
+         ,@(if (string-prefix? "x68_64" (or (%current-target-system)
+                                            (%current-system)))
+               '()
+               '((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")
-- 
2.33.1

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

[Mark H Weaver]

Hi Liliana,

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

> diff --git a/gnu/packages/webkit.scm b/gnu/packages/webkit.scm
> index e54b680855..ca50ab37a8 100644
> --- a/gnu/packages/webkit.scm
> +++ b/gnu/packages/webkit.scm
> @@ -241,7 +241,7 @@ (define-public webkitgtk
>      (build-system cmake-build-system)
>      (outputs '("out" "doc"))
>      (arguments
> -     '(#:tests? #f ; no tests
> +     `(#:tests? #f ; no tests
>         #:build-type "Release" ; turn off debugging symbols to save space
>         #:configure-flags (list
>                            "-DPORT=GTK"
> @@ -295,6 +295,13 @@ (define-public webkitgtk
>                   (("libWPEBackend-fdo-([\\.0-9]+)\\.so" all version)
>                    (string-append wpebackend-fdo "/lib/" all)))
>                 #t)))
> +         ,@(if (string-prefix? "x68_64" (or (%current-target-system)

The problem is here _______________^^^^^^.
That should be "x86_64", not "x68_64".

It's rebuilding on your system because it's erroneously adding the new
phase.  Otherwise, your commit looks good to me.

     Thank you!
        Mark

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

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

[Mark H Weaver]

Hi Liliana,

I have one more comment:

Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
> @@ -241,7 +241,7 @@ (define-public webkitgtk
>      (build-system cmake-build-system)
>      (outputs '("out" "doc"))
>      (arguments
> -     '(#:tests? #f ; no tests
> +     `(#:tests? #f ; no tests
>         #:build-type "Release" ; turn off debugging symbols to save space
>         #:configure-flags (list
>                            "-DPORT=GTK"

You'll need to remove this hunk in order to apply this to 'master',
since I've already made the change above in commit
4fd12fb00d595b6f6a5d5cc734863f4c4bc5f46c.

> @@ -295,6 +295,13 @@ (define-public webkitgtk
>                   (("libWPEBackend-fdo-([\\.0-9]+)\\.so" all version)
>                    (string-append wpebackend-fdo "/lib/" all)))
>                 #t)))
> +         ,@(if (string-prefix? "x68_64" (or (%current-target-system)
> +                                            (%current-system)))
> +               '()
> +               '((add-after 'unpack 'disable-sse2
> +                   (lambda _
> +                     (substitute* "Source/cmake/DetectSSE2.cmake"
> +                       (("CHECK_FOR_SSE2\\(\\)") ""))))))

As a general recommendation, it would be preferable to add the phase
only on the systems that need it (32-bit x86), instead of adding it on
every system except x86_64.  The code above will force rebuilds on every
system other than x86_64, whereas if the test was done as I suggested,
rebuilds would only happen on 32-bit x86 systems.

However, in this particular case it hardly matters, since our libsoup
package currently fails to build on any system that's not x86-based.

      Thanks,
        Mark

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

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

[Liliana Marie Prikler]

Hi,

Am Samstag, den 06.11.2021, 04:55 -0400 schrieb Mark H Weaver:
> Hi Liliana,
> 
> I have one more comment:
> 
> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
> > @@ -241,7 +241,7 @@ (define-public webkitgtk
> >      (build-system cmake-build-system)
> >      (outputs '("out" "doc"))
> >      (arguments
> > -     '(#:tests? #f ; no tests
> > +     `(#:tests? #f ; no tests
> >         #:build-type "Release" ; turn off debugging symbols to save
> > space
> >         #:configure-flags (list
> >                            "-DPORT=GTK"
> 
> You'll need to remove this hunk in order to apply this to 'master',
> since I've already made the change above in commit
> 4fd12fb00d595b6f6a5d5cc734863f4c4bc5f46c.
git pull did that automatically (I love rebasing pulls).

> > @@ -295,6 +295,13 @@ (define-public webkitgtk
> >                   (("libWPEBackend-fdo-([\\.0-9]+)\\.so" all
> > version)
> >                    (string-append wpebackend-fdo "/lib/" all)))
> >                 #t)))
> > +         ,@(if (string-prefix? "x68_64" (or (%current-target-
> > system)
> > +                                            (%current-system)))
> > +               '()
> > +               '((add-after 'unpack 'disable-sse2
> > +                   (lambda _
> > +                     (substitute* "Source/cmake/DetectSSE2.cmake"
> > +                       (("CHECK_FOR_SSE2\\(\\)") ""))))))
> 
> As a general recommendation, it would be preferable to add the phase
> only on the systems that need it (32-bit x86), instead of adding it
> on every system except x86_64.  The code above will force rebuilds on
> every system other than x86_64, whereas if the test was done as I
> suggested, rebuilds would only happen on 32-bit x86 systems.
I chose this variant, because x86_64 is the only architecture in which
we allow sse2 as of right now.  Better be safe than allowing for a
potential "i786" in the future :P

> However, in this particular case it hardly matters, since our libsoup
> package currently fails to build on any system that's not x86-based.
That sounds awful.  Is there a related bug?

I've pushed my patch with the spelling error corrected.  This is the
last webkitgtk-related issue for now, right?

Thanks,
Liliana

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

[Mark H Weaver]

Hi Liliana,

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

> Am Samstag, den 06.11.2021, 04:55 -0400 schrieb Mark H Weaver:
>> However, in this particular case it hardly matters, since our libsoup
>> package currently fails to build on any system that's not x86-based.
> That sounds awful.  Is there a related bug?

I don't know of one off-hand, but I haven't looked.

> I've pushed my patch with the spelling error corrected.  This is the
> last webkitgtk-related issue for now, right?

I believe so.  Thanks again for pushing these issues toward resolution.

     Regards,
       Mark

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