Package: guix-patches;
Reported by: Jakub Kądziołka <kuba <at> kadziolka.net>
Date: Sun, 12 Jan 2020 15:45:02 UTC
Severity: normal
Tags: patch
Done: Marius Bakke <mbakke <at> fastmail.com>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 39102 in the body.
You can then email your comments to 39102 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
View this report as an mbox folder, status mbox, maintainer mbox
guix-patches <at> gnu.org:bug#39102; Package guix-patches.
(Sun, 12 Jan 2020 15:45:02 GMT) Full text and rfc822 format available.Jakub Kądziołka <kuba <at> kadziolka.net>:guix-patches <at> gnu.org.
(Sun, 12 Jan 2020 15:45:02 GMT) Full text and rfc822 format available.Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Jakub Kądziołka <kuba <at> kadziolka.net> To: guix-patches <at> gnu.org Subject: [PATCH 1/2] gnu: xdg-utils: Don't use propagated inputs. Date: Sun, 12 Jan 2020 16:43:53 +0100
* gnu/packages/freedesktop.scm (xdg-utils)
[propagated-inputs, inputs]: Add awk, coreutils, grep, inetutils and
sed; make the dependencies not propagated.
[arguments](patch-hardcoded-paths): Move to after 'install to make
wrap-program function correctly. Wrap the installed programs.
---
gnu/packages/freedesktop.scm | 43 +++++++++++++++++++++++++++---------
1 file changed, 33 insertions(+), 10 deletions(-)
diff --git a/gnu/packages/freedesktop.scm b/gnu/packages/freedesktop.scm
index 7066685dee..06006ed53d 100644
--- a/gnu/packages/freedesktop.scm
+++ b/gnu/packages/freedesktop.scm
@@ -16,6 +16,7 @@
;;; Copyright © 2018 Stefan Stefanović <stefanx2ovic <at> gmail.com>
;;; Copyright © 2019 Reza Alizadeh Majd <r.majd <at> pantherx.org>
;;; Copyright © 2019 Guillaume Le Vaillant <glv <at> posteo.net>
+;;; Copyright © 2020 Jakub Kądziołka <kuba <at> kadziolka.net>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -47,6 +48,7 @@
#:use-module (gnu packages acl)
#:use-module (gnu packages admin)
#:use-module (gnu packages autotools)
+ #:use-module (gnu packages base)
#:use-module (gnu packages bash)
#:use-module (gnu packages boost)
#:use-module (gnu packages check)
@@ -55,6 +57,7 @@
#:use-module (gnu packages disk)
#:use-module (gnu packages docbook)
#:use-module (gnu packages documentation)
+ #:use-module (gnu packages gawk)
#:use-module (gnu packages gettext)
#:use-module (gnu packages ghostscript)
#:use-module (gnu packages gl)
@@ -107,20 +110,40 @@
("libxslt" ,libxslt)
("w3m" ,w3m)
("xmlto" ,xmlto)))
- (propagated-inputs
- `(("perl-file-mimeinfo" ,perl-file-mimeinfo) ; for mimeopen fallback
- ("xprop" ,xprop) ; for Xfce detecting
- ("xset" ,xset))) ; for xdg-screensaver
+ (inputs
+ `(("PATH:awk" ,gawk)
+ ("PATH:coreutils" ,coreutils)
+ ("PATH:grep" ,grep)
+ ("PATH:inetutils" ,inetutils) ; xdg-screensaver uses `hostname'
+ ("PATH:perl-file-mimeinfo" ,perl-file-mimeinfo) ; for mimeopen fallback
+ ("PATH:sed" ,sed)
+ ("PATH:xprop" ,xprop) ; for Xfce detecting
+ ("PATH:xset" ,xset))) ; for xdg-screensaver
(arguments
`(#:tests? #f ; no check target
#:phases
(modify-phases %standard-phases
- (add-after 'unpack 'patch-hardcoded-paths
- (lambda _
- (substitute* "scripts/xdg-mime.in"
- (("/usr/bin/file") (which "file")))
- (substitute* "scripts/xdg-open.in"
- (("/usr/bin/printf") (which "printf")))
+ (add-after 'install 'patch-hardcoded-paths
+ (lambda* (#:key inputs outputs #:allow-other-keys)
+ (with-directory-excursion (string-append (assoc-ref outputs "out")
+ "/bin")
+ (substitute* "xdg-mime"
+ (("/usr/bin/file") (which "file")))
+ (substitute* "xdg-open"
+ (("/usr/bin/printf") (which "printf")))
+ (let ((path-ext
+ (map (lambda (package)
+ (string-append package "/bin"))
+ (cons (assoc-ref outputs "out")
+ (map cdr
+ (filter (lambda (package)
+ (string-prefix? "PATH:" (car package)))
+ inputs))))))
+ (for-each
+ (lambda (script)
+ (wrap-program script
+ `("PATH" ":" prefix ,path-ext)))
+ (find-files "."))))
#t))
(add-before 'build 'locate-catalog-files
(lambda* (#:key inputs #:allow-other-keys)
--
2.24.1
guix-patches <at> gnu.org:bug#39102; Package guix-patches.
(Sun, 12 Jan 2020 15:48:02 GMT) Full text and rfc822 format available.Message #8 received at 39102 <at> debbugs.gnu.org (full text, mbox):
From: Jakub Kądziołka <kuba <at> kadziolka.net> To: 39102 <at> debbugs.gnu.org Subject: [PATCH 2/2 staging] gnu: qtbase: Open links properly without xdg-utils in profile Date: Sun, 12 Jan 2020 16:47:09 +0100
* 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.
---
.../qtbase-use-xdg-open-in-store.patch | 103 ++++++++++++++++++
gnu/packages/qt.scm | 10 +-
2 files changed, 112 insertions(+), 1 deletion(-)
create mode 100644 gnu/packages/patches/qtbase-use-xdg-open-in-store.patch
diff --git a/gnu/packages/patches/qtbase-use-xdg-open-in-store.patch b/gnu/packages/patches/qtbase-use-xdg-open-in-store.patch
new file mode 100644
index 0000000000..505fff13c2
--- /dev/null
+++ b/gnu/packages/patches/qtbase-use-xdg-open-in-store.patch
@@ -0,0 +1,103 @@
+To make Qt respect the user's settings for the preferred browser, use xdg-open
+even if it is not installed in the profile. The patch makes Qt always use
+xdg-open, and makes the path used easy to substitute when building.
+========================================================================
+--- qtbase-everywhere-src-5.12.6.orig/src/platformsupport/services/genericunix/qgenericunixservices.cpp 2020-01-01 20:47:16.775671802 +0100
++++ qtbase-everywhere-src-5.12.6/src/platformsupport/services/genericunix/qgenericunixservices.cpp 2020-01-01 20:56:22.167662035 +0100
+@@ -117,47 +117,6 @@
+ return QByteArrayLiteral("UNKNOWN");
+ }
+
+-static inline bool checkExecutable(const QString &candidate, QString *result)
+-{
+- *result = QStandardPaths::findExecutable(candidate);
+- return !result->isEmpty();
+-}
+-
+-static inline bool detectWebBrowser(const QByteArray &desktop,
+- bool checkBrowserVariable,
+- QString *browser)
+-{
+- const char *browsers[] = {"google-chrome", "firefox", "mozilla", "opera"};
+-
+- browser->clear();
+- if (checkExecutable(QStringLiteral("xdg-open"), browser))
+- return true;
+-
+- if (checkBrowserVariable) {
+- QByteArray browserVariable = qgetenv("DEFAULT_BROWSER");
+- if (browserVariable.isEmpty())
+- browserVariable = qgetenv("BROWSER");
+- if (!browserVariable.isEmpty() && checkExecutable(QString::fromLocal8Bit(browserVariable), browser))
+- return true;
+- }
+-
+- if (desktop == QByteArray("KDE")) {
+- // Konqueror launcher
+- if (checkExecutable(QStringLiteral("kfmclient"), browser)) {
+- browser->append(QLatin1String(" exec"));
+- return true;
+- }
+- } else if (desktop == QByteArray("GNOME")) {
+- if (checkExecutable(QStringLiteral("gnome-open"), browser))
+- return true;
+- }
+-
+- for (size_t i = 0; i < sizeof(browsers)/sizeof(char *); ++i)
+- if (checkExecutable(QLatin1String(browsers[i]), browser))
+- return true;
+- return false;
+-}
+-
+ static inline bool launch(const QString &launcher, const QUrl &url)
+ {
+ const QString command = launcher + QLatin1Char(' ') + QLatin1String(url.toEncoded());
+@@ -297,6 +256,8 @@
+ return result;
+ }
+
++static QString xdg_open = QStringLiteral("@@GUIX:XDG-OPEN@@");
++
+ bool QGenericUnixServices::openUrl(const QUrl &url)
+ {
+ if (url.scheme() == QLatin1String("mailto")) {
+@@ -320,11 +281,7 @@
+ }
+ #endif
+
+- if (m_webBrowser.isEmpty() && !detectWebBrowser(desktopEnvironment(), true, &m_webBrowser)) {
+- qWarning("Unable to detect a web browser to launch '%s'", qPrintable(url.toString()));
+- return false;
+- }
+- return launch(m_webBrowser, url);
++ return launch(xdg_open, url);
+ }
+
+ bool QGenericUnixServices::openDocument(const QUrl &url)
+@@ -337,11 +294,7 @@
+ }
+ #endif
+
+- if (m_documentLauncher.isEmpty() && !detectWebBrowser(desktopEnvironment(), false, &m_documentLauncher)) {
+- qWarning("Unable to detect a launcher for '%s'", qPrintable(url.toString()));
+- return false;
+- }
+- return launch(m_documentLauncher, url);
++ return launch(xdg_open, url);
+ }
+
+ #else
+diff -ur qtbase-everywhere-src-5.12.6.orig/src/platformsupport/services/genericunix/qgenericunixservices_p.h qtbase-everywhere-src-5.12.6/src/platformsupport/services/genericunix/qgenericunixservices_p.h
+--- qtbase-everywhere-src-5.12.6.orig/src/platformsupport/services/genericunix/qgenericunixservices_p.h 2020-01-01 20:47:16.775671802 +0100
++++ qtbase-everywhere-src-5.12.6/src/platformsupport/services/genericunix/qgenericunixservices_p.h 2020-01-01 20:54:12.135664364 +0100
+@@ -65,10 +65,6 @@
+
+ bool openUrl(const QUrl &url) override;
+ bool openDocument(const QUrl &url) override;
+-
+-private:
+- QString m_webBrowser;
+- QString m_documentLauncher;
+ };
+
+ QT_END_NAMESPACE
diff --git a/gnu/packages/qt.scm b/gnu/packages/qt.scm
index 514577678e..0b1d372d8b 100644
--- a/gnu/packages/qt.scm
+++ b/gnu/packages/qt.scm
@@ -14,6 +14,7 @@
;;; Copyright © 2019, 2020 Marius Bakke <mbakke <at> fastmail.com>
;;; Copyright © 2018 John Soo <jsoo1 <at> asu.edu>
;;; Copyright © 2020 Mike Rosset <mike.rosset <at> gmail.com>
+;;; Copyright © 2020 Jakub Kądziołka <kuba <at> kadziolka.net>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -349,7 +350,8 @@ developers using C++ or QML, a CSS & JavaScript like language.")
(base32
"09wz7zs1x5mpgs2y4xnl2zv3naqls4sz6v2arwl1fz2dsx4jddba"))
;; Use TZDIR to avoid depending on package "tzdata".
- (patches (search-patches "qtbase-use-TZDIR.patch"))
+ (patches (search-patches "qtbase-use-TZDIR.patch"
+ "qtbase-use-xdg-open-in-store.patch"))
(modules '((guix build utils)))
(snippet
;; corelib uses bundled harfbuzz, md4, md5, sha3
@@ -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,11 @@ 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")
+ (("@@GUIX:XDG-OPEN@@") (which "xdg-open")))
+ #t))
(replace 'configure
(lambda* (#:key outputs #:allow-other-keys)
(let ((out (assoc-ref outputs "out")))
--
2.24.1
guix-patches <at> gnu.org:bug#39102; Package guix-patches.
(Sun, 12 Jan 2020 17:04:01 GMT) Full text and rfc822 format available.Message #11 received at 39102 <at> debbugs.gnu.org (full text, mbox):
From: Jakub Kądziołka <kuba <at> kadziolka.net> To: 39102 <at> debbugs.gnu.org Subject: [PATCH v2 1/2] gnu: xdg-utils: Don't use propagated inputs. Date: Sun, 12 Jan 2020 18:03:12 +0100
Sending a revision to the first patch after feedback on IRC. Changes:
- Remove the `string-prefix? "PATH:"' hack, list the packages
explicitly.
- Use cute to make the code easier to read.
---
* gnu/packages/freedesktop.scm (xdg-utils)
[propagated-inputs, inputs]: Add awk, coreutils, grep and sed, make
the dependencies not propagated.
[arguments](patch-hardcoded-paths): Move to after 'install to make
wrap-program function correctly. Wrap the installed programs.
---
gnu/packages/freedesktop.scm | 40 ++++++++++++++++++++++++++++--------
1 file changed, 32 insertions(+), 8 deletions(-)
diff --git a/gnu/packages/freedesktop.scm b/gnu/packages/freedesktop.scm
index 7066685dee..9e1bf730d2 100644
--- a/gnu/packages/freedesktop.scm
+++ b/gnu/packages/freedesktop.scm
@@ -16,6 +16,7 @@
;;; Copyright © 2018 Stefan Stefanović <stefanx2ovic <at> gmail.com>
;;; Copyright © 2019 Reza Alizadeh Majd <r.majd <at> pantherx.org>
;;; Copyright © 2019 Guillaume Le Vaillant <glv <at> posteo.net>
+;;; Copyright © 2020 Jakub Kądziołka <kuba <at> kadziolka.net>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -47,6 +48,7 @@
#:use-module (gnu packages acl)
#:use-module (gnu packages admin)
#:use-module (gnu packages autotools)
+ #:use-module (gnu packages base)
#:use-module (gnu packages bash)
#:use-module (gnu packages boost)
#:use-module (gnu packages check)
@@ -55,6 +57,7 @@
#:use-module (gnu packages disk)
#:use-module (gnu packages docbook)
#:use-module (gnu packages documentation)
+ #:use-module (gnu packages gawk)
#:use-module (gnu packages gettext)
#:use-module (gnu packages ghostscript)
#:use-module (gnu packages gl)
@@ -107,20 +110,41 @@
("libxslt" ,libxslt)
("w3m" ,w3m)
("xmlto" ,xmlto)))
- (propagated-inputs
- `(("perl-file-mimeinfo" ,perl-file-mimeinfo) ; for mimeopen fallback
+ (inputs
+ `(("awk" ,gawk)
+ ("coreutils" ,coreutils)
+ ("grep" ,grep)
+ ("inetutils" ,inetutils) ; xdg-screensaver uses `hostname'
+ ("perl-file-mimeinfo" ,perl-file-mimeinfo) ; for mimeopen fallback
+ ("sed" ,sed)
("xprop" ,xprop) ; for Xfce detecting
("xset" ,xset))) ; for xdg-screensaver
(arguments
`(#:tests? #f ; no check target
+ #:modules ((srfi srfi-26)
+ (guix build utils)
+ (guix build gnu-build-system))
#:phases
(modify-phases %standard-phases
- (add-after 'unpack 'patch-hardcoded-paths
- (lambda _
- (substitute* "scripts/xdg-mime.in"
- (("/usr/bin/file") (which "file")))
- (substitute* "scripts/xdg-open.in"
- (("/usr/bin/printf") (which "printf")))
+ (add-after 'install 'patch-hardcoded-paths
+ (lambda* (#:key inputs outputs #:allow-other-keys)
+ (with-directory-excursion (string-append (assoc-ref outputs "out")
+ "/bin")
+ (substitute* "xdg-mime"
+ (("/usr/bin/file") (which "file")))
+ (substitute* "xdg-open"
+ (("/usr/bin/printf") (which "printf")))
+ (let ((path-ext
+ (map (cute string-append <> "/bin")
+ (cons (assoc-ref outputs "out")
+ (map (cute assoc-ref inputs <>)
+ '("awk" "coreutils" "grep" "inetutils"
+ "perl-file-mimeinfo" "sed" "xprop"
+ "xset"))))))
+ (for-each
+ (cute wrap-program <>
+ `("PATH" ":" prefix ,path-ext))
+ (find-files "."))))
#t))
(add-before 'build 'locate-catalog-files
(lambda* (#:key inputs #:allow-other-keys)
--
2.24.1
guix-patches <at> gnu.org:bug#39102; Package guix-patches.
(Sun, 12 Jan 2020 19:31:01 GMT) Full text and rfc822 format available.Message #14 received at 39102 <at> debbugs.gnu.org (full text, mbox):
From: Marius Bakke <mbakke <at> fastmail.com> To: Jakub Kądziołka <kuba <at> kadziolka.net>, 39102 <at> debbugs.gnu.org Subject: Re: [bug#39102] [PATCH v2 1/2] gnu: xdg-utils: Don't use propagated inputs. Date: Sun, 12 Jan 2020 20:29:59 +0100
[Message part 1 (text/plain, inline)]
Jakub Kądziołka <kuba <at> kadziolka.net> writes: > Sending a revision to the first patch after feedback on IRC. Changes: > - Remove the `string-prefix? "PATH:"' hack, list the packages > explicitly. > - Use cute to make the code easier to read. > > --- > * gnu/packages/freedesktop.scm (xdg-utils) > [propagated-inputs, inputs]: Add awk, coreutils, grep and sed, make > the dependencies not propagated. > [arguments](patch-hardcoded-paths): Move to after 'install to make > wrap-program function correctly. Wrap the installed programs. FYI: The stray '---' here made git disregard the actual commit message, and instead only added the four lines above. The other way around would be perfect. :-) I've applied this patch, with the changes below (leaving the original patch-hardcoded-paths phase intact, renaming the new phase accordingly; and added a let binding for "out"). I'll address the Qt patch in a separate message.
[diff (text/x-patch, inline)]
diff --git a/gnu/packages/freedesktop.scm b/gnu/packages/freedesktop.scm
index 9e1bf730d2..ed221439b4 100644
--- a/gnu/packages/freedesktop.scm
+++ b/gnu/packages/freedesktop.scm
@@ -122,29 +122,15 @@
(arguments
`(#:tests? #f ; no check target
#:modules ((srfi srfi-26)
- (guix build utils)
- (guix build gnu-build-system))
+ ,@%gnu-build-system-modules)
#:phases
(modify-phases %standard-phases
- (add-after 'install 'patch-hardcoded-paths
- (lambda* (#:key inputs outputs #:allow-other-keys)
- (with-directory-excursion (string-append (assoc-ref outputs "out")
- "/bin")
- (substitute* "xdg-mime"
- (("/usr/bin/file") (which "file")))
- (substitute* "xdg-open"
- (("/usr/bin/printf") (which "printf")))
- (let ((path-ext
- (map (cute string-append <> "/bin")
- (cons (assoc-ref outputs "out")
- (map (cute assoc-ref inputs <>)
- '("awk" "coreutils" "grep" "inetutils"
- "perl-file-mimeinfo" "sed" "xprop"
- "xset"))))))
- (for-each
- (cute wrap-program <>
- `("PATH" ":" prefix ,path-ext))
- (find-files "."))))
+ (add-after 'unpack 'patch-hardcoded-paths
+ (lambda _
+ (substitute* "scripts/xdg-mime.in"
+ (("/usr/bin/file") (which "file")))
+ (substitute* "scripts/xdg-open.in"
+ (("/usr/bin/printf") (which "printf")))
#t))
(add-before 'build 'locate-catalog-files
(lambda* (#:key inputs #:allow-other-keys)
@@ -168,6 +154,21 @@
"/manpages/docbook.xsl man")))
(setenv "STYLESHEET"
(string-append xsldoc "/html/docbook.xsl"))
+ #t)))
+ (add-after 'install 'wrap-executables
+ (lambda* (#:key inputs outputs #:allow-other-keys)
+ (let ((out (assoc-ref outputs "out")))
+ (with-directory-excursion (string-append out "/bin")
+ (let ((path-ext
+ (map (cute string-append <> "/bin")
+ (cons out
+ (map (cute assoc-ref inputs <>)
+ '("awk" "coreutils" "grep" "inetutils"
+ "perl-file-mimeinfo" "sed" "xprop"
+ "xset"))))))
+ (for-each (cute wrap-program <>
+ `("PATH" ":" prefix ,path-ext))
+ (find-files "."))))
#t))))))
(home-page "https://www.freedesktop.org/wiki/Software/xdg-utils/")
(synopsis "Freedesktop.org scripts for desktop integration")
[signature.asc (application/pgp-signature, inline)]
guix-patches <at> gnu.org:bug#39102; Package guix-patches.
(Sun, 12 Jan 2020 19:45:02 GMT) Full text and rfc822 format available.Message #17 received at 39102 <at> debbugs.gnu.org (full text, mbox):
From: Marius Bakke <mbakke <at> fastmail.com> To: Jakub Kądziołka <kuba <at> kadziolka.net>, 39102 <at> debbugs.gnu.org Cc: Efraim Flashner <efraim <at> flashner.co.il> Subject: Re: [bug#39102] [PATCH 2/2 staging] gnu: qtbase: Open links properly without xdg-utils in profile Date: Sun, 12 Jan 2020 20:44:43 +0100
[Message part 1 (text/plain, inline)]
Jakub Kądziołka <kuba <at> kadziolka.net> 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. :-) With this patch, BROWSER and DEFAULT_BROWSER would no longer be consulted, right? Does checkExecutable work with absolute file names? I.e. could we get away by simply patching "xdg-open" with its store file name? Probably should change the default browsers while at it, though. :-) 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? In short, I'm looking for an easier way to achieve the same goal, without the rather intrusive patch. Copying Efraim as our resident Qt expert.
[signature.asc (application/pgp-signature, inline)]
guix-patches <at> gnu.org:bug#39102; Package guix-patches.
(Mon, 13 Jan 2020 07:54:01 GMT) Full text and rfc822 format available.Message #20 received at 39102 <at> debbugs.gnu.org (full text, mbox):
From: Efraim Flashner <efraim <at> flashner.co.il> To: Marius Bakke <mbakke <at> fastmail.com> Cc: Jakub Kądziołka <kuba <at> kadziolka.net>, 39102 <at> debbugs.gnu.org Subject: Re: [bug#39102] [PATCH 2/2 staging] gnu: qtbase: Open links properly without xdg-utils in profile Date: Mon, 13 Jan 2020 09:53:12 +0200
[Message part 1 (text/plain, inline)]
On Sun, Jan 12, 2020 at 08:44:43PM +0100, Marius Bakke wrote:
> Jakub Kądziołka <kuba <at> kadziolka.net> 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. :-)
>
> With this patch, BROWSER and DEFAULT_BROWSER would no longer be
> consulted, right?
>
> Does checkExecutable work with absolute file names? I.e. could we get
> away by simply patching "xdg-open" with its store file name? Probably
> should change the default browsers while at it, though. :-)
>
> 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?
>
> In short, I'm looking for an easier way to achieve the same goal,
> without the rather intrusive patch.
>
> Copying Efraim as our resident Qt expert.
I disagree about being a qt expert, I just like fixing packages :)
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.
What is the code around m_documentLauncher? Does that really need to be
removed?
I think our best bet would be to substitute xdg-open with the actual
xdg-open binary around line 130 and to change the list of *browsers[] to
ones we actually have in Guix. If we switched the list to {"icecat",
"next", "chromium", "netsurf"} and made the substitution on xdg-open
we'd be in a much better place than we are now.
As it currently stands I know I don't have BROWSER or DEFAULT_BROWSER
defined and we don't have any Debian-like package named
'default-browser' or similar that we could throw in.
--
Efraim Flashner <efraim <at> flashner.co.il> אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D 14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted
[signature.asc (application/pgp-signature, inline)]
guix-patches <at> gnu.org:bug#39102; Package guix-patches.
(Mon, 13 Jan 2020 11:40:02 GMT) Full text and rfc822 format available.Message #23 received at 39102 <at> debbugs.gnu.org (full text, mbox):
From: Jakub Kądziołka <kuba <at> kadziolka.net> To: 39102 <at> debbugs.gnu.org, mbakke <at> fastmail.com, efraim <at> flashner.co.il Subject: [PATCH v2 2/2 staging] gnu: qtbase: Open links properly without xdg-utils in profile Date: Mon, 13 Jan 2020 12:39:45 +0100
* 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 <kuba <at> kadziolka.net> 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 <mbakke <at> fastmail.com>
;;; Copyright © 2018 John Soo <jsoo1 <at> asu.edu>
;;; Copyright © 2020 Mike Rosset <mike.rosset <at> gmail.com>
+;;; Copyright © 2020 Jakub Kądziołka <kuba <at> kadziolka.net>
;;;
;;; 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
Marius Bakke <mbakke <at> fastmail.com>:Jakub Kądziołka <kuba <at> kadziolka.net>:Message #28 received at 39102-done <at> debbugs.gnu.org (full text, mbox):
From: Marius Bakke <mbakke <at> fastmail.com> To: Jakub Kądziołka <kuba <at> kadziolka.net>, 39102-done <at> debbugs.gnu.org, efraim <at> flashner.co.il Subject: Re: [PATCH v2 2/2 staging] gnu: qtbase: Open links properly without xdg-utils in profile Date: Mon, 13 Jan 2020 22:43:01 +0100
[Message part 1 (text/plain, inline)]
Jakub Kądziołka <kuba <at> kadziolka.net> writes: > * 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 <kuba <at> kadziolka.net> 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. Right, thanks for clarifying. >> 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... Heh. Considering possible side effects is good, even if the effects are not very worrying. :-) > 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. OK. The new patch is much less intimidating, I will apply it shortly. (by the way, please attach full patches instead of plain diffs, so we can 'git am' straight from our MUA instead of having to mangle it) Thank you!
[signature.asc (application/pgp-signature, inline)]
guix-patches <at> gnu.org:bug#39102; Package guix-patches.
(Mon, 13 Jan 2020 22:32:02 GMT) Full text and rfc822 format available.Message #31 received at 39102-done <at> debbugs.gnu.org (full text, mbox):
From: Marius Bakke <mbakke <at> fastmail.com> To: Jakub Kądziołka <kuba <at> kadziolka.net> Cc: 39102-done <at> debbugs.gnu.org Subject: Re: [PATCH v2 2/2 staging] gnu: qtbase: Open links properly without xdg-utils in profile Date: Mon, 13 Jan 2020 23:31:10 +0100
[Message part 1 (text/plain, inline)]
Jakub Kądziołka <kuba <at> kadziolka.net> writes: > On Mon, Jan 13, 2020 at 10:43:01PM +0100, Marius Bakke wrote: >> (by the way, please attach full patches instead of plain diffs, so we >> can 'git am' straight from our MUA instead of having to mangle it) > > I did! This is how format-patch generated the email, with commentary > added like you've suggested before: > >> FYI: The stray '---' here made git disregard the actual commit message, >> and instead only added the four lines above. The other way around would >> be perfect. :-) > > How should my email have looked? Ha, you're right, my bad! I had not even tried applying the patch, but it did indeed work out of the box. So I pushed it as-is in 6e332fd3706fbe81c67b50c9d6b27df18f363c34. I need to adjust my mental patch parser ;-) Thank you!
[signature.asc (application/pgp-signature, inline)]
Debbugs Internal Request <help-debbugs <at> gnu.org>
to internal_control <at> debbugs.gnu.org.
(Tue, 11 Feb 2020 12:24:04 GMT) Full text and rfc822 format available.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.