From mboxrd@z Thu Jan 1 00:00:00 1970 From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) Subject: Re: [PATCH] gnu: kdbusaddons: Embed path to kdeinit5, avoid dependency cycles. Date: Mon, 19 Dec 2016 15:19:22 +0100 Message-ID: <87d1go3st1.fsf@gnu.org> References: <20161212.192812.1004299089367170098.thomas.danckaert@aeronomie.be> <8760mlfa84.fsf@gnu.org> <20161216.154211.1017954800346642936.thomas.danckaert@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:50693) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cIymo-0006qS-UW for guix-devel@gnu.org; Mon, 19 Dec 2016 09:19:32 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cIymj-0007K0-JG for guix-devel@gnu.org; Mon, 19 Dec 2016 09:19:30 -0500 In-Reply-To: <20161216.154211.1017954800346642936.thomas.danckaert@gmail.com> (Thomas Danckaert's message of "Fri, 16 Dec 2016 15:42:11 +0100 (CET)") List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: Thomas Danckaert Cc: guix-devel@gnu.org Hi Thomas, Thomas Danckaert skribis: > From: ludo@gnu.org (Ludovic Court=C3=A8s) > 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 =E2=80=98QT_PLUGIN_PATH=E2=80= =99 and >> =E2=80=98QML2_IMPORT_PATH=E2=80=99 search paths of =E2=80=98kinit=E2=80= =99; or (2) add a =E2=80=9Cprofile >> hook=E2=80=9D >> that creates a file containing the search path, and patch kinit to >> honor >> that file somehow. >> >> Option 1 sounds better, but =E2=80=98QT_PLUGIN_PATH=E2=80=99 really belo= ngs 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=E2=80=99re right. Search paths only work for direct dependenci= es. > So, provided option (2) can somehow find the paths of dependencies as > well, maybe that's better? (Are there examples of such a =E2=80=9Cprofile > hook=E2=80=9D somewhere? I didn't find anything in the manual). It=E2=80=99s not documented yet, but see (guix profiles) for examples. The difficulty in writing these hooks is that you don=E2=80=99t want the ho= ok to pull in KDE (or GNOME, or GHC, etc.) if the user doesn=E2=80=99t have any K= DE 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 =E2=80=98kdbusaddons=E2=80=99 doesn=E2=80=99t have any patches, yo= u can omit this >> =E2=80=98source=E2=80=99 >> 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=E2=80=99t it enough to do: >> >> (define kinit-bootstrap >> ((package-input-rewriting `((,kdbusaddons >> . ,kdbusaddons-bootstrap))) >> kinit)) >> >> ? Remember that =E2=80=98package-input-rewriting=E2=80=99 replaces inpu= ts >> 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 =E2=80=9Crecursive=E2=80=9D... would have saved me a lot of work (yo= u should have > seen the first versions of this patch O_o). Well, glad you like it! ;-) >> You can check with =E2=80=98guix graph -e '(@ (gnu packages kde) >> kdeinit-bootstrap)'=E2=80=99 >> whether you=E2=80=99re really getting what you want. > > tangentially: this seems to work only if I make the kinit-bootstrap > package a public variable? Right. If it=E2=80=99s 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 =E2=80=9Cpath=E2=80=9D traditionally means =E2=80=9Csearch path.= =E2=80=9D > > 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 =E2=80=9Cstore directory=E2=80=9D instead of =E2=80=9Cstor= e file name=E2=80=9D) I often use =E2=80=9Cstore item=E2=80=9D. > From e6c66e9d1daafee8907fa03db2f4c11104aab2b5 Mon Sep 17 00:00:00 2001 > From: Thomas Danckaert > 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=E2=80=99.