From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:470:142:3::10]:47443) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iqy4i-00044a-3m for guix-patches@gnu.org; Mon, 13 Jan 2020 06:40:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iqy4g-0002lx-NB for guix-patches@gnu.org; Mon, 13 Jan 2020 06:40:04 -0500 Received: from debbugs.gnu.org ([209.51.188.43]:52745) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1iqy4g-0002lm-Ih for guix-patches@gnu.org; Mon, 13 Jan 2020 06:40:02 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1iqy4g-0007xV-DY for guix-patches@gnu.org; Mon, 13 Jan 2020 06:40:02 -0500 Subject: [bug#39102] [PATCH v2 2/2 staging] gnu: qtbase: Open links properly without xdg-utils in profile References: <20200112154353.3xfdivef3fewlqx4@zdrowyportier.kadziolka.net> In-Reply-To: <20200112154353.3xfdivef3fewlqx4@zdrowyportier.kadziolka.net> Resent-Message-ID: Date: Mon, 13 Jan 2020 12:39:45 +0100 From: Jakub =?UTF-8?Q?K=C4=85dzio=C5=82ka?= Message-ID: <20200113113945.xrozaipgq65yxwbz@zdrowyportier.kadziolka.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: 39102@debbugs.gnu.org, mbakke@fastmail.com, efraim@flashner.co.il * 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 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 ;;; Copyright © 2018 John Soo ;;; Copyright © 2020 Mike Rosset +;;; Copyright © 2020 Jakub Kądziołka ;;; ;;; 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