[bug#39102] [PATCH v2 2/2 staging] gnu: qtbase: Open links properly without xdg-utils in profile

[Jakub Kądziołka]

* gnu/packages/qt.scm (qtbase)[inputs]: Add XDG-UTILS.
  [arguments](patch-xdg-open): New phase.
---

On Sun, Jan 12, 2020 at 08:44:43PM +0100, Marius Bakke wrote:
> Jakub Kądziołka <address@hidden> writes:
> 
> > * gnu/packages/patches/qtbase-use-xdg-open-in-store.patch: New file.
> > * gnu/packages/qt.scm (qtbase)[source][patches]: Apply the patch.
> >   [inputs]: Add a dependency on xdg-utils to get its store path.
> >   [arguments]: Add a new phase to patch the path into the source code.
> 
> This patch does a lot.  :-)

Yeah, for some reason I thought a patch like this might as well not
leave any dead code behind.

> With this patch, BROWSER and DEFAULT_BROWSER would no longer be
> consulted, right?

BROWSER is checked by xdg-open anyway. DEFAULT_BROWSER would indeed be
no longer consulted.

> Does checkExecutable work with absolute file names?  I.e. could we get
> away by simply patching "xdg-open" with its store file name?

That's what I considered doing at first, but when I drilled a few
methods into the code, I decided that it's very unlikely to work and I
don't want to check it empirically due to the painfully long
compile-times.

> Probably should change the default browsers while at it, though.  :-)

I don't think there's much point, as that code becomes dead when
xdg-open is always found.

> Wrt the easy substitution, I think we should try and avoid introducing
> changes to source code that depend on magic from #:phases.  That way
> people will still be (mostly) able to build Qt manually using the Guix
> source.  In this case, maybe defaulting to just "xdg-open" is enough?

Perhaps applying the patch in a phase instead of (source _) would help?

> In short, I'm looking for an easier way to achieve the same goal,
> without the rather intrusive patch.

See PATCHv2, which uses a much more minimal approach, but leaves some
dead stuff that isn't likely to be optimized by the compiler. Caring
about this might not be rational, now that I think about it...

On Mon, Jan 13, 2020 at 09:53:12AM +0200, Efraim Flashner wrote:
| Looking at the patch, I'm not in love with how there's a default list of
| browsers to look for. Looking at the code, it seems that if there's
| xdg-open available then open browser from the pre-defined list.

You seem to be misreading the code. If xdg-open is available, it is
used, the browser list is only used when xdg-open isn't found.

| I think our best bet would be to [...] change the list of *browsers[] to
| ones we actually have in Guix.

As mentioned above, the list would never be read, since xdg-open would
always be found.
---
 gnu/packages/qt.scm | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/gnu/packages/qt.scm b/gnu/packages/qt.scm
index 514577678e..8dc771a5f8 100644
--- a/gnu/packages/qt.scm
+++ b/gnu/packages/qt.scm
@@ -14,6 +14,7 @@
 ;;; Copyright © 2019, 2020 Marius Bakke <address@hidden>
 ;;; Copyright © 2018 John Soo <address@hidden>
 ;;; Copyright © 2020 Mike Rosset <address@hidden>
+;;; Copyright © 2020 Jakub Kądziołka <address@hidden>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -363,6 +364,7 @@ developers using C++ or QML, a CSS & JavaScript like 
language.")
     (build-system gnu-build-system)
     (propagated-inputs
      `(("mesa" ,mesa)
+       ;; Use which the package, not the function
        ("which" ,(@ (gnu packages base) which))))
     (inputs
      `(("alsa-lib" ,alsa-lib)
@@ -407,6 +409,7 @@ developers using C++ or QML, a CSS & JavaScript like 
language.")
        ("xcb-util-keysyms" ,xcb-util-keysyms)
        ("xcb-util-renderutil" ,xcb-util-renderutil)
        ("xcb-util-wm" ,xcb-util-wm)
+       ("xdg-utils" ,xdg-utils)
        ("zlib" ,zlib)))
     (native-inputs
      `(("bison" ,bison)
@@ -428,6 +431,14 @@ developers using C++ or QML, a CSS & JavaScript like 
language.")
                             "qmake/library/qmakebuiltins.cpp")
                           (("/bin/sh") (which "sh")))
              #t))
+         (add-after 'configure 'patch-xdg-open
+           (lambda _
+             (substitute* 
'("src/platformsupport/services/genericunix/qgenericunixservices.cpp")
+                          (("^.*const char \\*browsers.*$" all)
+                           (string-append "*browser = QStringLiteral(\""
+                                          (which "xdg-open")
+                                          "\"); return true; \n" all)))
+             #t))
          (replace 'configure
            (lambda* (#:key outputs #:allow-other-keys)
              (let ((out (assoc-ref outputs "out")))
-- 
2.24.1