[bug#35866] [PATCH] gnu: Add qtwebengine.

[Pierre Neidhardt <mail@ambrevar.xyz>]

[-- Attachment #1: Type: text/plain, Size: 5890 bytes --]

Looking good!

Comments below:

> qtwebengine now inherits qtsvg
>
> This also uses substitute-keyword-arguments to only replace configure phase.

I think those 2 lines can be omitted or maybe rephrased.  We didn't have
qtwebengine before, so you'd need to explain what's up with qtsvg, why
it was problematic to use it.

> +(define-public qtwebengine
> +  (package (inherit qtsvg)
> +    (name "qtwebengine")
> +    (version (package-version qtbase))
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri
> +        (string-append "https://download.qt.io/official_releases/qt/"
> +                       (substring version 0 4)

Use (version-major+minor ...) or similar.

> +                       "/" version "/submodules/"
> +                       (string-append name "-everywhere-src-" version)
> +                       ".tar.xz"))
> +       (sha256
> +        (base32
> +         "08c60nh95m98mcqk444axs76xi6m9x0wvdxrzk9c2cxwqdbz59fa"))))
> +    (build-system gnu-build-system)
> +    (native-inputs
> +     `(("bison" ,bison)
> +       ("flex" ,flex)
> +       ("gperf" ,gperf)
> +       ("ninja" ,ninja)
> +       ("perl" ,perl)
> +       ("pkg-config" ,pkg-config)
> +       ("python-2" ,python-2)
> +       ("ruby" ,ruby)
> +       ))

Dangling parentheses.

> +    (inputs
> +     `(("alsa-lib" ,alsa-lib)
> +       ("atk" ,atk)
> +       ("cups-minimal" ,cups-minimal)
> +       ("dbus" ,dbus)
> +       ("ffmpeg" ,ffmpeg)
> +       ("fontconfig" ,fontconfig)
> +       ("harbuzz" ,harfbuzz)
> +       ("icu4c" ,icu4c)
> +       ("jsoncpp" ,jsoncpp)
> +       ("lcms" ,lcms)
> +       ("libcap" ,libcap)
> +       ("libevent" ,libevent)
> +       ("libgcrypt" ,libgcrypt)
> +       ("libjpeg" ,libjpeg-turbo)
> +       ;; libsrtp not found by qmake. seems to not have config.tests?
> +       ("libsrtp" ,libsrtp)

What does it entail?  Is libsrtp ignored then?
If there is a problem, please add a "FIXME:" comment or similar.

> +       ;; FIXME: error: ?struct vpx_svc_ref_frame_config? has no member named ?frame_flags?
> +       ;; ("libvpx" ,libvpx)
> +       ("libwebp" ,libwebp)
> +       ("libx11" ,libx11)
> +       ("libxcb" ,libxcb)
> +       ("libxcomposite" ,libxcomposite)
> +       ("libxcursor" ,libxcursor)
> +       ("libxi" ,libxi)
> +       ("libxkbcommon" ,libxkbcommon)
> +       ;; libxml not found due to icu not enabled in libxml?

You mean libxml2?
Is it ignored then?
Same as above, prefix with a FIXME if needed.

> +       ("libxml2" ,libxml2)
> +       ("libxrandr" ,libxrandr)
> +       ("libxrender" ,libxrender)
> +       ("libxslt" ,libxslt)
> +       ("libxtst" ,libxtst)
> +       ("mesa" ,mesa)
> +       ("minizip" ,minizip)
> +       ("nss" ,nss)
> +       ("opus" ,opus)
> +       ("pciutils" ,pciutils)
> +       ("protobuf" ,protobuf)
> +       ;; ("pulseaudio" ,pulseaudio)

Please explain why you left pulseaudio commented out.

> +       ("qtbase" ,qtbase)
> +       ("qtdeclarative" ,qtdeclarative)
> +       ("qtmultimedia" ,qtmultimedia)
> +       ("qtwebchannel" ,qtwebchannel)
> +       ("re2" ,re2)
> +       ("snappy" ,snappy)
> +       ("udev" ,eudev)
> +       ("xcb-util" ,xcb-util)))
> +    (arguments
> +     (substitute-keyword-arguments (package-arguments qtsvg)
> +       ((#:phases phases)
> +        `(modify-phases ,phases
> +           (replace 'configure
> +             (lambda* (#:key inputs outputs #:allow-other-keys)
> +               ;; Avoids potential race conditions
> +               (setenv "PYTHONDONTWRITEBYTECODE" "1")
> +               ;; Make build output less verbose
> +               (setenv "NINJAFLAGS" "-k1")
> +               ;; Use this output translations not qtbase

This comment is not very clear, maybe it could be rephrased, e.g.

"Replace the qtbase translations with those from qtwebengine.

> +               (substitute* "src/core/web_engine_library_info.cpp"
> +                 (("QLibraryInfo::location\\(QLibraryInfo::TranslationsPath\\)")
> +                  (format #f "QLatin1String(\"~a\")" (string-append (assoc-ref outputs "out") "/share/qt5/translations")))
> +                 (("QLibraryInfo::location\\(QLibraryInfo::DataPath\\)")
> +                  (format #f "QLatin1String(\"~a\")" (string-append (assoc-ref outputs "out") "/share/qt5"))))
> +               ;; Substitute full dynamic path for nss
> +               (substitute* "src/3rdparty/chromium/crypto/nss_util.cc"
> +                 (("libnssckbi.so")
> +                  (string-append (assoc-ref inputs "nss") "/lib/nss/libnssckbi.so")))
> +               ;; Substitute full dynamic path for udev
> +               (substitute* "src/3rdparty/chromium/device/udev_linux/udev1_loader.cc"
> +                 (("libudev.so.1")
> +                  (string-append (assoc-ref inputs "udev") "/lib/libudev.so.1")))
> +               ;; Valid QT_BUILD_PARTS variables are:
> +               ;; libs tools tests examples demos docs translations
> +               (invoke "qmake" "QT_BUILD_PARTS = libs tools" "--"
> +                       "--webengine-printing-and-pdf=no"
> +                       "--webengine-ffmpeg=system"
> +                       "--webengine-icu=system"
> +                       "--webengine-pepper-plugins=no")))))
> +       ((#:tests? _ #f) #f)))

No tests?

> +    (native-search-paths
> +     (list (search-path-specification
> +            (file-type 'regular)
> +            (separator #f)
> +            (variable "QTWEBENGINEPROCESS_PATH")
> +            (files '("lib/qt5/libexec/QtWebEngineProcess")))))
> +    (home-page "https://www.qt.io")
> +    (synopsis "Qt WebEngine Module")

Probably no capital for "module."

Great work, thanks again!

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]