Subject: [bug#54703] [PATCH v2 2/3] gnu: webkitgtk: Update to 2.36.0 and enable extra features.

From: Liliana Marie Prikler <[email-masked]>
Date: Tue, 05 Apr 2022 07:59:34 +0200

Am Montag, dem 04.04.2022 um 20:27 -0400 schrieb Maxim Cournoyer:
> * gnu/packages/webkit.scm (webkitgtk): Update to 2.36.0.
> [source]: Remove the webkitgtk-bind-all-fonts.patch and
> webkitgtk-canonicalize-paths.patch patches.
I assume they were upstreamed?
> [arguments]: Use gexp.
> [tests?]: Update comment.
> [build-type]: Add explanatory comment.
> [configure-flags]: Enable gamepad support,  remove nonexistent
> USE_SYSTEMD option, enable accelerated 2D canvas and mini-browser.
> [phases]{do-not-disable-new-dtags}: New phase.
> {help-cmake-find-elogind}: Likewise.
> {patch-gtk-doc-scan}: Simplify to avoid for-each.  Locate
> docbookx.dtd via
> search-input-file.
> {embed-absolute-wpebackend-reference}: Use search-input-file.
> [native-inputs]: Use new style.
> [inputs]: Likewise.  Add elogind and libmanette.
> [description]: Adjust indentation.
> (webkitgtk-with-libsoup2): Re-format comment, fixing spacing.
> [configure-flags]: Use gexp.
> [propagated-inputs]: Use modify-inputs to replace libsoup with
> libsoup-minimal-2.
> * gnu/packages/patches/webkitgtk-bind-all-fonts.patch: Delete file.
> * gnu/packages/patches/webkitgtk-canonicalize-paths.patch: Likewise.
> * gnu/local.mk: De-register them.
> ---
>  gnu/local.mk                                  |   2 -
>  .../patches/webkitgtk-bind-all-fonts.patch    |  25 --
>  .../webkitgtk-canonicalize-paths.patch        |  66 -----
>  gnu/packages/webkit.scm                       | 232 ++++++++++------
> --
>  4 files changed, 124 insertions(+), 201 deletions(-)
>  delete mode 100644 gnu/packages/patches/webkitgtk-bind-all-
> fonts.patch
>  delete mode 100644 gnu/packages/patches/webkitgtk-canonicalize-
> paths.patch
> 
> diff --git a/gnu/local.mk b/gnu/local.mk
> index aac7362475..7f9c2fe1a1 100644
> --- a/gnu/local.mk
> +++ b/gnu/local.mk
> @@ -1931,9 +1931,7 @@ dist_patch_DATA
> =                                         \
>    %D%/packages/patches/vte-CVE-2012-2738-
> pt2.patch                     \
>    %D%/packages/patches/vtk-fix-freetypetools-build-
> failure.patch       \
>    %D%/packages/patches/warsow-qfusion-fix-bool-return-
> type.patch       \
> -  %D%/packages/patches/webkitgtk-bind-all-fonts.patch          \
>    %D%/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch \
> -  %D%/packages/patches/webkitgtk-canonicalize-paths.patch      \
>    %D%/packages/patches/webrtc-audio-processing-big-
> endian.patch        \
>    %D%/packages/patches/websocketpp-fix-for-cmake-3.15.patch    \
>    %D%/packages/patches/wicd-bitrate-none-fix.patch             \
> diff --git a/gnu/packages/patches/webkitgtk-bind-all-fonts.patch
> b/gnu/packages/patches/webkitgtk-bind-all-fonts.patch
> deleted file mode 100644
> index 27013180c4..0000000000
> --- a/gnu/packages/patches/webkitgtk-bind-all-fonts.patch
> +++ /dev/null
> @@ -1,25 +0,0 @@
> -Upstream commit:
> https://github.com/WebKit/WebKit/commit/31ac354cbeecf866f9a38f7b2f8f59f7975d3f6a
> -
> -diff --git
> a/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp
> b/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp
> -index ecc804663784..8de174be3c0e 100644
> ---- a/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp
> -+++ b/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp
> -@@ -288,6 +288,7 @@ static void bindFonts(Vector<CString>& args)
> -     const char* homeDir = g_get_home_dir();
> -     const char* dataDir = g_get_user_data_dir();
> -     const char* cacheDir = g_get_user_cache_dir();
> -+    const char* const * dataDirs = g_get_system_data_dirs();
> - 
> -     // Configs can include custom dirs but then we have to parse
> them...
> -     GUniquePtr<char> fontConfig(g_build_filename(configDir,
> "fontconfig", nullptr));
> -@@ -304,6 +305,10 @@ static void bindFonts(Vector<CString>& args)
> -     bindIfExists(args, fontHomeConfigDir.get());
> -     bindIfExists(args, fontData.get());
> -     bindIfExists(args, fontHomeData.get());
> -+    for (auto* dataDir = dataDirs; dataDir && *dataDir; dataDir++)
> {
> -+        GUniquePtr<char> fontDataDir(g_build_filename(*dataDir,
> "fonts", nullptr));
> -+        bindIfExists(args, fontDataDir.get());
> -+    }
> -     bindIfExists(args, "/var/cache/fontconfig"); // Used by Debian.
> - }
> - 
> diff --git a/gnu/packages/patches/webkitgtk-canonicalize-paths.patch
> b/gnu/packages/patches/webkitgtk-canonicalize-paths.patch
> deleted file mode 100644
> index 741d534831..0000000000
> --- a/gnu/packages/patches/webkitgtk-canonicalize-paths.patch
> +++ /dev/null
> @@ -1,66 +0,0 @@
> -Upstream commit:
> https://github.com/WebKit/WebKit/commit/6a87eb254ef57a986a1a6ce9a3a4b66928afeb65
> -
> -diff --git
> a/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp
> b/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp
> -index ecc804663784..a2a1c9d7a4dd 100644
> ---- a/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp
> -+++ b/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp
> -@@ -27,7 +27,6 @@
> - #include <seccomp.h>
> - #include <sys/ioctl.h>
> - #include <sys/mman.h>
> --#include <unistd.h>
> - #include <wtf/FileSystem.h>
> - #include <wtf/UniStdExtras.h>
> - #include <wtf/glib/GRefPtr.h>
> -@@ -165,6 +164,15 @@ enum class BindFlags {
> -     Device,
> - };
> - 
> -+static void bindSymlinksRealPath(Vector<CString>& args, const char*
> path, const char* bindOption = "--ro-bind")
> -+{
> -+    WTF::String realPath = FileSystem::realPath(path);
> -+    if (path != realPath) {
> -+        CString rpath = realPath.utf8();
> -+        args.appendVector(Vector<CString>({ bindOption,
> rpath.data(), rpath.data() }));
> -+    }
> -+}
> -+
> - static void bindIfExists(Vector<CString>& args, const char* path,
> BindFlags bindFlags = BindFlags::ReadOnly)
> - {
> -     if (!path || path[0] == '\0')
> -@@ -177,7 +185,16 @@ static void bindIfExists(Vector<CString>& args,
> const char* path, BindFlags bind
> -         bindType = "--ro-bind-try";
> -     else
> -         bindType = "--bind-try";
> --    args.appendVector(Vector<CString>({ bindType, path, path }));
> -+
> -+    // Canonicalize the source path, otherwise a symbolic link
> could
> -+    // point to a location outside of the namespace.
> -+    bindSymlinksRealPath(args, path, bindType);
> -+
> -+    // As /etc is exposed wholesale, do not layer extraneous bind
> -+    // directives on top, which could fail in the presence of
> symbolic
> -+    // links.
> -+    if (!g_str_has_prefix(path, "/etc/"))
> -+        args.appendVector(Vector<CString>({ bindType, path, path
> }));
> - }
> - 
> - static void bindDBusSession(Vector<CString>& args, bool
> allowPortals)
> -@@ -410,17 +427,6 @@ static void bindV4l(Vector<CString>& args)
> -     }));
> - }
> - 
> --static void bindSymlinksRealPath(Vector<CString>& args, const char*
> path)
> --{
> --    char realPath[PATH_MAX];
> --
> --    if (realpath(path, realPath) && strcmp(path, realPath)) {
> --        args.appendVector(Vector<CString>({
> --            "--ro-bind", realPath, realPath,
> --        }));
> --    }
> --}
> --
> - // Translate a libseccomp error code into an error message.
> libseccomp
> - // mostly returns negative errno values such as -ENOMEM, but some
> - // standard errno values are used for non-standard purposes where
> their
> diff --git a/gnu/packages/webkit.scm b/gnu/packages/webkit.scm
> index f79d6bd4f6..f453372b1e 100644
> --- a/gnu/packages/webkit.scm
> +++ b/gnu/packages/webkit.scm
> @@ -27,6 +27,7 @@ (define-module (gnu packages webkit)
>    #:use-module ((guix licenses) #:prefix license:)
>    #:use-module (guix packages)
>    #:use-module (guix download)
> +  #:use-module (guix gexp)
>    #:use-module (guix utils)
>    #:use-module (guix build utils)
>    #:use-module (guix build-system cmake)
> @@ -41,6 +42,7 @@ (define-module (gnu packages webkit)
>    #:use-module (gnu packages flex)
>    #:use-module (gnu packages fontutils)
>    #:use-module (gnu packages freedesktop)
> +  #:use-module (gnu packages games)
>    #:use-module (gnu packages gcc)
>    #:use-module (gnu packages gettext)
>    #:use-module (gnu packages ghostscript)
> @@ -239,121 +241,135 @@ (define-public wpewebkit
>  (define-public webkitgtk
>    (package
>      (name "webkitgtk")
> -    (version "2.34.6")
> +    (version "2.36.0")
>      (source (origin
>                (method url-fetch)
>                (uri (string-append
> "https://www.webkitgtk.org/releases/"
>                                    "webkitgtk-" version ".tar.xz"))
>                (sha256
>                 (base32
> -               
> "1xn1hhd0qaxmjf6vy6664i4mmmjsw9zgrr4w8ni3415d981zvj3b"))
> -              (patches (search-patches "webkitgtk-bind-all-
> fonts.patch"
> -                                       "webkitgtk-adjust-bubblewrap-
> paths.patch"
> -                                       "webkitgtk-canonicalize-
> paths.patch"))))
> +               
> "0kf6avqlq9f74lwzji7l3xlnng5yqarc4ykwsmfmy8q5y6hwqxxq"))
> +              (patches (search-patches
> +                        "webkitgtk-adjust-bubblewrap-
> paths.patch"))))
>      (build-system cmake-build-system)
>      (outputs '("out" "doc" "debug"))
>      (arguments
> -     `(#:tests? #f ; no tests
> -       #:build-type "Release" ; turn off debugging symbols to save
> space
> -       #:configure-flags (list
> -                          "-DPORT=GTK"
> -                          "-DENABLE_GTKDOC=ON" ; No doc by default
> -                          ;; Requires libmanette, new dependency
> added in 2.32.0.
> -                          ;; TODO Decide if we should enable this
> -                          "-DENABLE_GAMEPAD=OFF"
> -                          "-DUSE_SYSTEMD=OFF"
> -                          (string-append ; uses lib64 by default
> -                           "-DLIB_INSTALL_DIR="
> -                           (assoc-ref %outputs "out") "/lib"))
> -       #:make-flags
> -       ;; Never build with unsupported -j1:
> https://issues.guix.gnu.org/47964#5
> -       (list "-j" (number->string (max 2 (parallel-job-count))))
> -       #:phases
> -       (modify-phases %standard-phases
> -         (add-after 'unpack 'configure-bubblewrap-store-directory
> -           (lambda _
> -             ;; This phase is a corollary to 'webkitgtk-share-
> store.patch' to
> -             ;; avoid hard coding /gnu/store, for users with other
> prefixes.
> -             (let ((store-directory (%store-directory)))
> -               (substitute*
> -                  
> "Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp"
> -                 (("@storedir@") store-directory)))))
> -         (add-after 'unpack 'patch-gtk-doc-scan
> -           (lambda* (#:key inputs #:allow-other-keys)
> -             (for-each (lambda (file)
> -                         (substitute* file
> -                          
> (("http://www.oasis-open.org/docbook/xml/4.1.2/docbookx.dtd")
> -                            (string-append (assoc-ref inputs
> "docbook-xml")
> -                                          
> "/xml/dtd/docbook/docbookx.dtd"))))
> -                       (find-files "Source" "\\.sgml$"))))
> -         (add-after 'unpack 'embed-absolute-wpebackend-reference
> -           (lambda* (#:key inputs #:allow-other-keys)
> -             (let ((wpebackend-fdo (assoc-ref inputs "wpebackend-
> fdo")))
> -               (substitute*
> "Source/WebKit/UIProcess/glib/WebProcessPoolGLib.cpp"
> -                 (("libWPEBackend-fdo-([\\.0-9]+)\\.so" all version)
> -                  (string-append wpebackend-fdo "/lib/" all))))))
> -         ,@(if (target-x86-64?)
> -               '()
> -               '((add-after 'unpack 'disable-sse2
> -                   (lambda _
> -                     (substitute*
> "Source/cmake/WebKitCompilerFlags.cmake"
> -                       (("WTF_CPU_X86 AND NOT CMAKE_CROSSCOMPILING")
> -                        "FALSE"))))))
> -         (add-after 'install 'move-doc-files
> -           (lambda* (#:key outputs #:allow-other-keys)
> -             (let ((out (assoc-ref outputs "out"))
> -                   (doc (assoc-ref outputs "doc")))
> -               (mkdir-p (string-append doc "/share"))
> -               (rename-file (string-append out "/share/gtk-doc")
> -                            (string-append doc "/share/gtk-
> doc"))))))))
> +     (list
> +      ;; The release archive doesn't include the resources/sources
> needed to
> +      ;; run the various regression tests.
> +      #:tests? #f
> +      ;; When building using the default RelWithDebInfo build type,
> the final
> +      ;; binaries require 20 GiB of memory to link (even with
> ld.gold or lld)
> +      ;; and produce 4.6 GiB of debug symbols.
> +      #:build-type "Release"
> +      #:configure-flags #~(list
> +                           "-DPORT=GTK"
> +                           "-DENABLE_GTKDOC=ON" ;disabled by default
> +                           "-DENABLE_ACCELERATED_2D_CANVAS=ON"
> ;disabled by default
> +                           "-DENABLE_MINIBROWSER=ON" ;disabled by
> default
Rather than saying "disabled by default" we should probably say why
we're enabling them if upstream thinks that's unwise to do.
> +                           ;; The default lib installation prefix is
> lib64.
> +                           (string-append "-DLIB_INSTALL_DIR="
> #$output "/lib"))
> +      ;; The build may fail with -j1 (see:
> +      ;; https://bugs.webkit.org/show_bug.cgi?id=195251).
> +      #:make-flags #~(list "-j" (number->string (max 2 (parallel-
> job-count))))
> +      #:phases
> +      #~(modify-phases %standard-phases
> +          (add-after 'unpack 'configure-bubblewrap-store-directory
> +            (lambda _
> +              ;; This phase is a corollary to 'webkitgtk-share-
> store.patch' to
> +              ;; avoid hard coding /gnu/store, for users with other
> prefixes.
That patch no longer exists AFAICS, so the comment needs to be updated.
> +              (let ((store-directory (%store-directory)))
> +                (substitute*
> +                   
> "Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp"
> +                  (("@storedir@") store-directory)))))
> +          (add-after 'unpack 'do-not-disable-new-dtags
> +            ;; Ensure the linker uses new dynamic tags as this is
> what Guix
> +            ;; uses and validates in the validate-runpath phase.
> +            (lambda _
> +              (substitute* "Source/cmake/OptionsCommon.cmake"
> +                (("if \\(LD_SUPPORTS_DISABLE_NEW_DTAGS\\)")
> +                 "if (FALSE)"))))
> +          (add-after 'unpack 'help-cmake-find-elogind
> +            (lambda _
> +              (substitute* "Source/cmake/FindJournald.cmake"
> +                ;; Otherwise, CMake would throw an error because it
> relies on
> +                ;; the pkg-config search to locate headers.
> +                (("pkg_check_modules\\(PC_SYSTEMD QUIET libsystemd")
> +                 "pkg_check_modules(PC_SYSTEMD QUIET libelogind"))))
> +          (add-after 'unpack 'patch-gtk-doc-scan
> +            (lambda* (#:key native-inputs inputs #:allow-other-keys)
> +              (substitute* (find-files "Source" "\\.sgml$")
> +               
> (("http://www.oasis-open.org/docbook/xml/4.1.2/docbookx.dtd")
> +                 (search-input-file (or native-inputs inputs)
> +                                   
> "xml/dtd/docbook/docbookx.dtd")))))
> +          (add-after 'unpack 'embed-absolute-wpebackend-reference
> +            (lambda* (#:key inputs #:allow-other-keys)
> +              (let ((wpebackend-fdo (assoc-ref inputs "wpebackend-
> fdo")))
> +                (substitute*
> "Source/WebKit/UIProcess/glib/WebProcessPoolGLib.cpp"
> +                  (("libWPEBackend-fdo-[\\.0-9]+\\.so" all)
> +                   (search-input-file inputs (string-append "lib/"
> all)))))))
> +          #$@(if (target-x86-64?)
> +                 '()
> +                 '((add-after 'unpack 'disable-sse2
> +                     (lambda _
> +                       (substitute*
> "Source/cmake/WebKitCompilerFlags.cmake"
> +                         (("WTF_CPU_X86 AND NOT
> CMAKE_CROSSCOMPILING")
> +                          "FALSE"))))))

> +          (add-after 'install 'move-doc-files
> +            (lambda* (#:key outputs #:allow-other-keys)
> +              (let ((doc (assoc-ref outputs "doc")))
> +                (mkdir-p (string-append doc "/share"))
> +                (rename-file (string-append #$output "/share/gtk-
> doc")
> +                             (string-append doc "/share/gtk-
> doc"))))))))
>      (native-inputs
> -     `(("bison" ,bison)
> -       ("gettext" ,gettext-minimal)
> -       ("glib:bin" ,glib "bin") ; for glib-mkenums, etc.
> -       ("gobject-introspection" ,gobject-introspection)
> -       ("gperf" ,gperf)
> -       ("perl" ,perl)
> -       ("pkg-config" ,pkg-config)
> -       ("python" ,python-wrapper)
> -       ("gtk-doc" ,gtk-doc/stable) ; For documentation generation
> -       ("docbook-xml" ,docbook-xml) ; For documentation generation
> -       ("ruby" ,ruby)))
> +     (list bison
> +           gettext-minimal
> +           `(,glib "bin")               ;for glib-mkenums, etc.
> +           gobject-introspection
> +           gperf
> +           perl
> +           pkg-config
> +           python-wrapper
> +           gtk-doc/stable               ;for documentation
> generation
> +           docbook-xml                  ;for documentation
> generation
I think "for documentation" would be enough here.  Small nitpick:
What's it with all the lacking spaces after the semicolons?
> +           ruby))
>      (propagated-inputs
>       (list gtk+ libsoup))
>      (inputs
> -     `(("at-spi2-core" ,at-spi2-core)
> -       ("bubblewrap" ,bubblewrap)
> -       ("enchant" ,enchant)
> -       ("geoclue" ,geoclue)
> -       ("gst-plugins-base" ,gst-plugins-base)
> -       ("gtk+-2" ,gtk+-2)
> -       ("harfbuzz" ,harfbuzz)
> -       ("hyphen" ,hyphen)
> -       ("icu4c" ,icu4c)
> -       ("lcms" ,lcms)
> -       ("libgcrypt" ,libgcrypt)
> -       ("libjpeg" ,libjpeg-turbo)
> -       ("libnotify" ,libnotify)
> -       ("libpng" ,libpng)
> -       ("libseccomp" ,libseccomp)
> -       ("libsecret" ,libsecret)
> -       ("libtasn1" ,libtasn1)
> -       ("libwebp" ,libwebp)
> -       ("libwpe" ,libwpe)
> -       ("libxcomposite" ,libxcomposite)
> -       ("libxml2" ,libxml2)
> -       ("libxslt" ,libxslt)
> -       ("libxt" ,libxt)
> -       ("mesa" ,mesa)
> -       ("openjpeg" ,openjpeg)
> -       ("sqlite" ,sqlite)
> -       ("woff2" ,woff2)
> -       ("wpebackend-fdo" ,wpebackend-fdo)
> -       ("xdg-dbus-proxy" ,xdg-dbus-proxy)))
> +     (list at-spi2-core
> +           bubblewrap
> +           elogind
> +           enchant
> +           geoclue
> +           gst-plugins-base
> +           gtk+-2
> +           harfbuzz
> +           hyphen
> +           icu4c
> +           lcms
> +           libgcrypt
> +           libjpeg-turbo
> +           libmanette
> +           libnotify
> +           libpng
> +           libseccomp
> +           libsecret
> +           libtasn1
> +           libwebp
> +           libwpe
> +           libxcomposite
> +           libxml2
> +           libxslt
> +           libxt
> +           mesa
> +           openjpeg
> +           sqlite
> +           woff2
> +           wpebackend-fdo
> +           xdg-dbus-proxy))
>      (home-page "https://www.webkitgtk.org/")
>      (synopsis "Web content engine for GTK+")
> -    (description
> -     "WebKitGTK+ is a full-featured port of the WebKit rendering
> engine,
> +    (description "WebKitGTK+ is a full-featured port of the WebKit
> rendering engine,
>  suitable for projects requiring any kind of web integration, from
> hybrid
>  HTML/CSS applications to full-fledged web browsers.  WebKitGTK+
> video playing
>  capabilities can be extended through the use of GStreamer plugins
> (not
> @@ -366,17 +382,17 @@ (define-public webkitgtk
>                     license:bsd-2
>                     license:bsd-3))))
>  
> -;;; Required by gnome-online-accounts; as webkitgtk 2.34 propagates
> libsoup 3,
> -;;; which causes the build to fail.
> -;;; Also required by e.g. emacs-next-pgtk,  emacs-xwidgets, and some
> other
> -;;; Gnome packages for webkit2gtk-4.0. See also the upstream tracker
> for
> -;;; libsoup 3: https://gitlab.gnome.org/GNOME/libsoup/-/issues/218
> +;;; Required by gnome-online-accounts as webkitgtk propagates
> libsoup 3, which
> +;;; causes the build to fail.  Also required by e.g. emacs-next-
> pgtk,
> +;;; emacs-xwidgets, and some other GNOME packages for webkit2gtk-
> 4.0.  See
> +;;; also the upstream tracker for libsoup 3:
> +;;; https://gitlab.gnome.org/GNOME/libsoup/-/issues/218
>  (define-public webkitgtk-with-libsoup2
>    (package/inherit webkitgtk
>      (name "webkitgtk-with-libsoup2")
>      (arguments (substitute-keyword-arguments (package-arguments
> webkitgtk)
>                   ((#:configure-flags flags)
> -                  `(cons "-DUSE_SOUP2=ON" ,flags))))
> +                  #~(cons "-DUSE_SOUP2=ON" #$flags))))
>      (propagated-inputs
> -     (alist-replace "libsoup" (list libsoup-minimal-2)
> -                    (package-propagated-inputs webkitgtk)))))
> +     (modify-inputs (package-propagated-inputs webkitgtk)
> +       (replace "libsoup" libsoup-minimal-2)))))
Otherwise LGTM.

You might however want to wait for more comments, particularly lfam's
security advice :)