[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] gnu: kdbusaddons: Embed path to kdeinit5, avoid dependency c
|
From: |
Thomas Danckaert |
|
Subject: |
Re: [PATCH] gnu: kdbusaddons: Embed path to kdeinit5, avoid dependency cycles. |
|
Date: |
Fri, 16 Dec 2016 15:42:11 +0100 (CET) |
From: address@hidden (Ludovic Courtès)
Subject: Re: [PATCH] gnu: kdbusaddons: Embed path to kdeinit5, avoid
dependency cycles.
Date: Thu, 15 Dec 2016 17:06:19 +0100
Other options that come to mind: (1) make ‘QT_PLUGIN_PATH’ and
‘QML2_IMPORT_PATH’ search paths of ‘kinit’; or (2) add a “profile
hook”
that creates a file containing the search path, and patch kinit to
honor
that file somehow.
Option 1 sounds better, but ‘QT_PLUGIN_PATH’ really belongs to Qt,
not
to kinit.
I think adding QT_PLUGIN_PATH to kinit (and a number of other KDE
packages) could be justified. KDE applications heavily rely on
QT_PLUGIN_PATH (and use a different path than the default path used
by Qt: ${PREFIX}/lib/plugins instead of ${PREFIX}/plugins). See for
example the section on environment variables at
https://community.kde.org/Guidelines_and_HOWTOs/Build_from_source
However, I'm not sure it's enough: as far as I understand,
search-paths will only work for packages directly installed in a
user's profile, but KDE applications often need plugins provided by
their dependencies (e.g. for kdevelop, plugins from kdevplatform and
kinit must be found on the QT_PLUGIN_PATH, but a user who installs
kdevelop shouldn't be forced to install kinit and kdevplatform in
their profile?). (Perhaps I misunderstand how search-paths works)
So, provided option (2) can somehow find the paths of dependencies as
well, maybe that's better? (Are there examples of such a “profile
hook” somewhere? I didn't find anything in the manual).
+;; This version of kdbusaddons does not use kinit as an input,
and is used to
+;; build kinit-bootstrap, as well as bootstrap versions of all
kinit
+;; dependencies which also rely on kdbusaddons.
+(define kdbusaddons-bootstrap
+ (package
+ (inherit kdbusaddons)
+ (source (origin
+ (inherit (package-source kdbusaddons))
+ (patches '())))
Since ‘kdbusaddons’ doesn’t have any patches, you can omit this
‘source’
field.
This commit adds a patch to kdbusaddons (the one that adds the kinit
store directory), so I think does become necessary?
[...]
Isn’t it enough to do:
(define kinit-bootstrap
((package-input-rewriting `((,kdbusaddons .
,kdbusaddons-bootstrap)))
kinit))
? Remember that ‘package-input-rewriting’ replaces inputs
recursively.
Yes, ..., yes it is :-) I had this nagging feeling that tracking the
dependency chain like that could be automated, and therefore probably
already _had_ been automated in guix :-) I didn't pay attention to the
word “recursive”... would have saved me a lot of work (you should have
seen the first versions of this patch O_o).
You can check with ‘guix graph -e '(@ (gnu packages kde)
kdeinit-bootstrap)'’
whether you’re really getting what you want.
tangentially: this seems to work only if I make the kinit-bootstrap
package a public variable?
diff --git a/gnu/packages/patches/kdbusaddons-kinit-path.patch
b/gnu/packages/patches/kdbusaddons-kinit-path.patch
new file mode 100644
index 0000000..97c7319
--- /dev/null
+++ b/gnu/packages/patches/kdbusaddons-kinit-path.patch
@@ -0,0 +1,15 @@
+Add placeholder for kinit's store path.
s/path/file name/ please. :-)
In GNU “path” traditionally means “search path.”
I'm happy to comply, but note that the info manual does talk about
store paths. Should this be changed?
https://www.gnu.org/software/guix/manual/html_node/The-Store.html
(Also, I chose “store directory” instead of “store file name”)
Thomas
>From e6c66e9d1daafee8907fa03db2f4c11104aab2b5 Mon Sep 17 00:00:00 2001
From: Thomas Danckaert <address@hidden>
Date: Tue, 6 Dec 2016 14:55:39 +0100
Subject: [PATCH] gnu: kdbusaddons: Embed kinit store dir, avoid dependency
cycles.
kdbusaddons needs to know the location of the kdeinit5 executable,
provided by kinit. kinit depends on kdbusaddons, so we add bootstrap
versions of all packages in the dependency chain from kinit to
kdbusaddons to avoid cyclic dependencies.
* gnu/packages/kde-frameworks.scm (kinit-bootstrap,
kdbusaddons-bootstrap): New variables.
(kdbusaddons)[inputs]: Add kinit-bootstrap.
[source,arguments]: Add patch and substitution to embed
kinit-bootstrap's store directory in the code.
* gnu/packages/patches/kdbusaddons-kinit-file-name.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
---
gnu/local.mk | 1 +
gnu/packages/kde-frameworks.scm | 36 ++++++++++++++++++++--
.../patches/kdbusaddons-kinit-file-name.patch | 15 +++++++++
3 files changed, 49 insertions(+), 3 deletions(-)
create mode 100644 gnu/packages/patches/kdbusaddons-kinit-file-name.patch
diff --git a/gnu/local.mk b/gnu/local.mk
index c6cb55b..f7b661c 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -639,6 +639,7 @@ dist_patch_DATA =
\
%D%/packages/patches/isl-0.11.1-aarch64-support.patch \
%D%/packages/patches/jbig2dec-ignore-testtest.patch \
%D%/packages/patches/jq-CVE-2015-8863.patch \
+ %D%/packages/patches/kdbusaddons-kinit-file-name.patch \
%D%/packages/patches/khmer-use-libraries.patch \
%D%/packages/patches/kmod-module-directory.patch \
%D%/packages/patches/kobodeluxe-paths.patch \
diff --git a/gnu/packages/kde-frameworks.scm b/gnu/packages/kde-frameworks.scm
index 8b84133..94145fb 100644
--- a/gnu/packages/kde-frameworks.scm
+++ b/gnu/packages/kde-frameworks.scm
@@ -25,6 +25,7 @@
#:use-module ((guix licenses) #:prefix license:)
#:use-module (guix packages)
#:use-module (guix utils)
+ #:use-module (gnu packages)
#:use-module (gnu packages acl)
#:use-module (gnu packages admin)
#:use-module (gnu packages attr)
@@ -50,7 +51,8 @@
#:use-module (gnu packages version-control)
#:use-module (gnu packages web)
#:use-module (gnu packages xml)
- #:use-module (gnu packages xorg))
+ #:use-module (gnu packages xorg)
+ #:use-module (srfi srfi-1))
(define-public extra-cmake-modules
(package
@@ -516,7 +518,8 @@ many more.")
name "-" version ".tar.xz"))
(sha256
(base32
- "07mzb1xr8wyiid25p8kg6mjp6vq8ngvv1ikhq75zvd2cbax530c8"))))
+ "07mzb1xr8wyiid25p8kg6mjp6vq8ngvv1ikhq75zvd2cbax530c8"))
+ (patches (search-patches "kdbusaddons-kinit-file-name.patch"))))
(build-system cmake-build-system)
(native-inputs
`(("extra-cmake-modules" ,extra-cmake-modules)
@@ -524,10 +527,18 @@ many more.")
("qttools" ,qttools)))
(inputs
`(("qtbase" ,qtbase)
- ("qtx11extras" ,qtx11extras)))
+ ("qtx11extras" ,qtx11extras)
+ ("kinit" ,kinit-bootstrap))) ;; kinit-bootstrap: kinit package which
does not depend on kdbusaddons.
(arguments
`(#:phases
(modify-phases %standard-phases
+ (add-before
+ 'configure 'patch-source
+ (lambda* (#:key inputs #:allow-other-keys)
+ ;; look for the kdeinit5 executable in kinit's store directory,
+ ;; instead of the current application's directory:
+ (substitute* "src/kdeinitinterface.cpp"
+ (("@SUBSTITUTEME@") (assoc-ref inputs "kinit")))))
(replace 'check
(lambda _
(setenv "DBUS_FATAL_WARNINGS" "0")
@@ -2866,3 +2877,22 @@ setUrl, setUserAgent and call.")
script engines.")
;; dual licensed
(license (list license:gpl2+ license:lgpl2.1+))))
+
+;; This version of kdbusaddons does not use kinit as an input, and is used to
+;; build kinit-bootstrap, as well as bootstrap versions of all kinit
+;; dependencies which also rely on kdbusaddons.
+(define kdbusaddons-bootstrap
+ (package
+ (inherit kdbusaddons)
+ (source (origin
+ (inherit (package-source kdbusaddons))
+ (patches '())))
+ (inputs (alist-delete "kinit" (package-inputs kdbusaddons)))
+ (arguments
+ (substitute-keyword-arguments (package-arguments kdbusaddons)
+ ((#:phases phases)
+ `(modify-phases ,phases
+ (delete 'patch-source)))))))
+
+(define kinit-bootstrap
+ ((package-input-rewriting `((,kdbusaddons . ,kdbusaddons-bootstrap))) kinit))
diff --git a/gnu/packages/patches/kdbusaddons-kinit-file-name.patch
b/gnu/packages/patches/kdbusaddons-kinit-file-name.patch
new file mode 100644
index 0000000..ffed88e
--- /dev/null
+++ b/gnu/packages/patches/kdbusaddons-kinit-file-name.patch
@@ -0,0 +1,15 @@
+Add placeholder for kinit's store file name.
+
+diff --git a/src/kdeinitinterface.cpp b/src/kdeinitinterface.cpp
+index 22fa5e5..3d40937 100644
+--- a/src/kdeinitinterface.cpp
++++ b/src/kdeinitinterface.cpp
+@@ -52,7 +52,7 @@ void KDEInitInterface::ensureKdeinitRunning()
+ // If not found in system paths, search other paths
+ if (srv.isEmpty()) {
+ const QStringList searchPaths = QStringList()
+- << QCoreApplication::applicationDirPath() // then look where our
application binary is located
++ << QString::fromUtf8("@SUBSTITUTEME@/bin") // using
QStringLiteral would be more efficient, but breaks guix store reference
detection.
+ << QLibraryInfo::location(QLibraryInfo::BinariesPath); // look
where exec path is (can be set in qt.conf)
+ srv = QStandardPaths::findExecutable(QStringLiteral("kdeinit5"),
searchPaths);
+ if (srv.isEmpty()) {
--
2.7.4