[PATCH] gnu: kdbusaddons: Embed path to kdeinit5, avoid dependency cycles.

[PATCH] gnu: kdbusaddons: Embed path to kdeinit5, avoid dependency cycles.

[Thomas Danckaert]

[-- Attachment #1: Type: Text/Plain, Size: 1902 bytes --]

Hi,

kdbusaddons is responsible for (re-)starting kdeinit5 (a daemon that 
speeds up startup of KDE applications by forking itself into a new 
process) when it is not running.  However, kdbusaddons is also a 
build dependency of kinit.

This patch adds a kinit-bootstrap package, makes it  an input of 
kdbusaddons, and embeds the store path of the kdeinit5 executable (an 
output of kinit-bootstrap) in the kdbusaddons source.  Without this 
patch, applications using kdbusaddons will always need a PATH 
environment variable in order to find kdeinit5.

Because kinit depends on kdbusaddons via a number of intermediate 
packages, I had to add kdbusaddons-bootstrap and a few other extra 
packages to avoid dependency cycles.  This patch would trigger a 
build of around ~30 packages.

A remaining issue with kdeinit is, that it needs to run in an 
environment with all the required environment variables (such as 
QT_PLUGIN_PATH and QML2_IMPORT_PATH) for all possible KDE 
applications it has to start (and any libraries they depend on), in 
order to find dependencies at runtime, such as plugins and Qml 
modules.  If a user wants to run a KDE application which relies on 
kdeinit, but kdeinit is already running without all the required 
environment vars, the application will not find all the plugins or 
Qml modules it needs.  (In a conventional system, kde applications 
just need to know one central directory where they will find all KDE 
plugins present on the system)

I don't know what would be a good solution.  A “kde-master” package, 
which has references to all packaged KDE applications (and some 
libraries which provide plugins) as inputs, and just adds the 
required paths to the user's profile (not very modular, but it could 
work)? Could a kdeinit service could help with this in GuixSD 
(haven't really looked at services yet)?

Thomas

[-- Attachment #2: 0001-gnu-kdbusaddons-Embed-path-to-kdeinit5-avoid-depende.patch --]
[-- Type: Text/X-Patch, Size: 7460 bytes --]

From ba729cd9a2bbbc98bd308702ded837cae5980281 Mon Sep 17 00:00:00 2001
From: Thomas Danckaert <thomas.danckaert@gmail.com>
Date: Tue, 6 Dec 2016 14:55:39 +0100
Subject: [PATCH] gnu: kdbusaddons: Embed path to kdeinit5, 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, kbookmarks-bootstrap, kglobalaccel-bootstrap,
  kio-bootstrap, kservice-bootstrap, ktextwidgets-bootstrap,
  kwallet-bootstrap, kxmlgui-bootstrap): New variables.
  (kdbusaddons)[inputs]: Add kinit-bootstrap.
  [source,arguments]: Add patch and substitution to embed
  kinit-bootstrap's store path in the code.
* gnu/packages/patches/kdbusaddons-kinit-path.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
---
 gnu/local.mk                                      |  1 +
 gnu/packages/kde-frameworks.scm                   | 68 ++++++++++++++++++++++-
 gnu/packages/patches/kdbusaddons-kinit-path.patch | 15 +++++
 3 files changed, 81 insertions(+), 3 deletions(-)
 create mode 100644 gnu/packages/patches/kdbusaddons-kinit-path.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 55dee48..b42258e 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-path.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..ea53e04 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-path.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 path,
+            ;; 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,54 @@ 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 kservice-bootstrap
+  ((package-input-rewriting `((,kdbusaddons . ,kdbusaddons-bootstrap))) kservice))
+
+(define ktextwidgets-bootstrap
+  ((package-input-rewriting `((,kservice . ,kservice-bootstrap))) ktextwidgets))
+
+(define kwallet-bootstrap
+  ((package-input-rewriting `((,kdbusaddons . ,kdbusaddons-bootstrap)
+                              (,kservice . ,kservice-bootstrap))) kwallet))
+
+(define kglobalaccel-bootstrap
+  ((package-input-rewriting `((,kdbusaddons . ,kdbusaddons-bootstrap)
+                               (,kservice . ,kservice-bootstrap))) kglobalaccel))
+
+(define kxmlgui-bootstrap
+  ((package-input-rewriting `((,kglobalaccel . ,kglobalaccel-bootstrap)
+                              (,ktextwidgets . ,ktextwidgets-bootstrap))) kxmlgui))
+
+(define kbookmarks-bootstrap
+  ((package-input-rewriting `((,kxmlgui . ,kxmlgui-bootstrap))) kbookmarks))
+
+(define kio-bootstrap
+  ((package-input-rewriting `((,kservice . ,kservice-bootstrap)
+                              (,kxmlgui . ,kxmlgui-bootstrap)
+                              (,kbookmarks . ,kbookmarks-bootstrap)
+                              (,kdbusaddons . ,kdbusaddons-bootstrap)
+                              (,kwallet . ,kwallet-bootstrap)
+                              (,ktextwidgets . ,ktextwidgets-bootstrap))) kio))
+
+(define kinit-bootstrap
+  ((package-input-rewriting `((,kio . ,kio-bootstrap)
+                              (,kservice . ,kservice-bootstrap)
+                              (,kbookmarks . ,kbookmarks-bootstrap)
+                              (,kxmlgui . ,kxmlgui-bootstrap))) kinit))
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.
+
+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

Re: [PATCH] gnu: kdbusaddons: Embed path to kdeinit5, avoid dependency cycles.

[Ludovic Courtès]

Hello!

Thomas Danckaert <thomas.danckaert@aeronomie.be> skribis:

> A remaining issue with kdeinit is, that it needs to run in an
> environment with all the required environment variables (such as
> QT_PLUGIN_PATH and QML2_IMPORT_PATH) for all possible KDE applications
> it has to start (and any libraries they depend on), in order to find
> dependencies at runtime, such as plugins and Qml modules.  If a user
> wants to run a KDE application which relies on kdeinit, but kdeinit is
> already running without all the required environment vars, the
> application will not find all the plugins or Qml modules it needs.
> (In a conventional system, kde applications just need to know one
> central directory where they will find all KDE plugins present on the
> system)

OK.

> I don't know what would be a good solution.  A “kde-master” package,
> which has references to all packaged KDE applications (and some
> libraries which provide plugins) as inputs, and just adds the required
> paths to the user's profile (not very modular, but it could work)?
> Could a kdeinit service could help with this in GuixSD (haven't really
> looked at services yet)?

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.

Thoughts?

> From ba729cd9a2bbbc98bd308702ded837cae5980281 Mon Sep 17 00:00:00 2001
> From: Thomas Danckaert <thomas.danckaert@gmail.com>
> Date: Tue, 6 Dec 2016 14:55:39 +0100
> Subject: [PATCH] gnu: kdbusaddons: Embed path to kdeinit5, 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, kbookmarks-bootstrap, kglobalaccel-bootstrap,
>   kio-bootstrap, kservice-bootstrap, ktextwidgets-bootstrap,
>   kwallet-bootstrap, kxmlgui-bootstrap): New variables.
>   (kdbusaddons)[inputs]: Add kinit-bootstrap.
>   [source,arguments]: Add patch and substitution to embed
>   kinit-bootstrap's store path in the code.
> * gnu/packages/patches/kdbusaddons-kinit-path.patch: New file.
> * gnu/local.mk (dist_patch_DATA): Add it.

[...]

> +;; 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.

> +(define kservice-bootstrap
> +  ((package-input-rewriting `((,kdbusaddons . ,kdbusaddons-bootstrap))) kservice))
> +
> +(define ktextwidgets-bootstrap
> +  ((package-input-rewriting `((,kservice . ,kservice-bootstrap))) ktextwidgets))
> +
> +(define kwallet-bootstrap
> +  ((package-input-rewriting `((,kdbusaddons . ,kdbusaddons-bootstrap)
> +                              (,kservice . ,kservice-bootstrap))) kwallet))
> +
> +(define kglobalaccel-bootstrap
> +  ((package-input-rewriting `((,kdbusaddons . ,kdbusaddons-bootstrap)
> +                               (,kservice . ,kservice-bootstrap))) kglobalaccel))
> +
> +(define kxmlgui-bootstrap
> +  ((package-input-rewriting `((,kglobalaccel . ,kglobalaccel-bootstrap)
> +                              (,ktextwidgets . ,ktextwidgets-bootstrap))) kxmlgui))
> +
> +(define kbookmarks-bootstrap
> +  ((package-input-rewriting `((,kxmlgui . ,kxmlgui-bootstrap))) kbookmarks))
> +
> +(define kio-bootstrap
> +  ((package-input-rewriting `((,kservice . ,kservice-bootstrap)
> +                              (,kxmlgui . ,kxmlgui-bootstrap)
> +                              (,kbookmarks . ,kbookmarks-bootstrap)
> +                              (,kdbusaddons . ,kdbusaddons-bootstrap)
> +                              (,kwallet . ,kwallet-bootstrap)
> +                              (,ktextwidgets . ,ktextwidgets-bootstrap))) kio))
> +
> +(define kinit-bootstrap
> +  ((package-input-rewriting `((,kio . ,kio-bootstrap)
> +                              (,kservice . ,kservice-bootstrap)
> +                              (,kbookmarks . ,kbookmarks-bootstrap)
> +                              (,kxmlgui . ,kxmlgui-bootstrap))) kinit))

Isn’t it enough to do:

  (define kinit-bootstrap
    ((package-input-rewriting `((,kdbusaddons . ,kdbusaddons-bootstrap)))
     kinit))

?  Remember that ‘package-input-rewriting’ replaces inputs recursively.

You can check with ‘guix graph -e '(@ (gnu packages kde) kdeinit-bootstrap)'’
whether you’re really getting what you want.

> 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.”

Thanks!

Ludo’.

Re: [PATCH] gnu: kdbusaddons: Embed path to kdeinit5, avoid dependency cycles.

[Thomas Danckaert]

[-- Attachment #1: Type: Text/Plain, Size: 3777 bytes --]

From: ludo@gnu.org (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

[-- Attachment #2: 0001-gnu-kdbusaddons-Embed-kinit-store-dir-avoid-dependen.patch --]
[-- Type: Text/X-Patch, Size: 5856 bytes --]

From e6c66e9d1daafee8907fa03db2f4c11104aab2b5 Mon Sep 17 00:00:00 2001
From: Thomas Danckaert <thomas.danckaert@gmail.com>
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

Re: [PATCH] gnu: kdbusaddons: Embed path to kdeinit5, avoid dependency cycles.

[Ludovic Courtès]

Hi Thomas,

Thomas Danckaert <thomas.danckaert@gmail.com> skribis:

> From: ludo@gnu.org (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)

I think you’re right.  Search paths only work for direct dependencies.

> 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).

It’s not documented yet, but see (guix profiles) for examples.

The difficulty in writing these hooks is that you don’t want the hook to
pull in KDE (or GNOME, or GHC, etc.) if the user doesn’t have any KDE
application installed.  So that needs extra care.

>>> +;; 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?

Oh, OK.

>> [...]
>>
>> 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).

Well, glad you like it!  ;-)

>> 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?

Right.  If it’s private, the you need to use two at signs: @@.

>>> 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?

It should.  Someday!  :-)

> (Also, I chose “store directory” instead of “store file name”)

I often use “store item”.

> From e6c66e9d1daafee8907fa03db2f4c11104aab2b5 Mon Sep 17 00:00:00 2001
> From: Thomas Danckaert <thomas.danckaert@gmail.com>
> 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.

Go for it!

Thanks!

Ludo’.