GNU bug report logs - #42735
[PATCH] pre-inst-env: don't use GUIX_PACKAGE_PATH

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 42735 in the body.
You can then email your comments to 42735 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Denis 'GNUtoo' Carikli <GNUtoo <at> cyberdimension.org>
To: guix-patches <at> gnu.org
Cc: Denis 'GNUtoo' Carikli <GNUtoo <at> cyberdimension.org>
Subject: [PATCH] pre-inst-env: don't use GUIX_PACKAGE_PATH
Date: Fri,  7 Aug 2020 04:21:42 +0200

./pre-inst-env is supposed to use only the packages definitions that are in
the guix source tree and not the host packages.

However if GUIX_PACKAGE_PATH is set, it will use host packages as well.

In addition, when packages are defined in both the guix source tree and in
GUIX_PACKAGE_PATH, GUIX_PACKAGE_PATH will take the precedence and guix
will print warnings like that:
    guix build: warning: ambiguous package specification `libsamsung-ipc'
    guix build: warning: choosing libsamsung-ipc <at> 0.1 from
                         /home/[...]/.config/guix/local/replicant.scm:31:2

That situation can happen when working in a new package in
GUIX_PACKAGE_PATH and then importing the package in the guix source tree to
add it upstream.

Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo <at> cyberdimension.org>
---
 build-aux/pre-inst-env.in | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/build-aux/pre-inst-env.in b/build-aux/pre-inst-env.in
index e0aa7fe868..698a7994fb 100644
--- a/build-aux/pre-inst-env.in
+++ b/build-aux/pre-inst-env.in
@@ -59,4 +59,8 @@ export NIX_HASH
 GUIX_UNINSTALLED=1
 export GUIX_UNINSTALLED
 
+# Make sure we don't use local package definitions
+GUIX_PACKAGE_PATH=""
+export GUIX_PACKAGE_PATH
+
 exec "$@"
-- 
2.28.0

Message #8 received at 42735 <at> debbugs.gnu.org (full text, mbox):

From: Brett Gilio <brettg <at> gnu.org>
To: Denis 'GNUtoo' Carikli <GNUtoo <at> cyberdimension.org>
Cc: 42735 <at> debbugs.gnu.org
Subject: Re: [bug#42735] [PATCH] pre-inst-env: don't use GUIX_PACKAGE_PATH
Date: Thu, 06 Aug 2020 22:46:52 -0500

Denis 'GNUtoo' Carikli <GNUtoo <at> cyberdimension.org> writes:

>  
> +# Make sure we don't use local package definitions
> +GUIX_PACKAGE_PATH=""
> +export GUIX_PACKAGE_PATH
> +
>  exec "$@"

Wouldn't it make more sense to use `unset` instead of defining an empty
variable?

Message #11 received at 42735 <at> debbugs.gnu.org (full text, mbox):

From: Denis 'GNUtoo' Carikli <GNUtoo <at> cyberdimension.org>
To: Brett Gilio <brettg <at> gnu.org>
Cc: 42735 <at> debbugs.gnu.org
Subject: Re: [bug#42735] [PATCH] pre-inst-env: don't use GUIX_PACKAGE_PATH
Date: Fri, 7 Aug 2020 08:02:04 +0200

[Message part 1 (text/plain, inline)]

On Thu, 06 Aug 2020 22:46:52 -0500
Brett Gilio <brettg <at> gnu.org> wrote:

> Denis 'GNUtoo' Carikli <GNUtoo <at> cyberdimension.org> writes:
> 
> >  
> > +# Make sure we don't use local package definitions
> > +GUIX_PACKAGE_PATH=""
> > +export GUIX_PACKAGE_PATH
> > +
> >  exec "$@"
> 
> Wouldn't it make more sense to use `unset` instead of defining an
> empty variable?
I used export and defined an empty variable because I don't know how to
write portable shell scripts and export was already used in
build-aux/pre-inst-env.in.

Is unset portable? If so should I resend a patch with unset instead?

Denis.

[Message part 2 (application/pgp-signature, inline)]

Message #14 received at 42735 <at> debbugs.gnu.org (full text, mbox):

From: Mathieu Othacehe <othacehe <at> gnu.org>
To: Denis 'GNUtoo' Carikli <GNUtoo <at> cyberdimension.org>
Cc: 42735 <at> debbugs.gnu.org
Subject: Re: [bug#42735] [PATCH] pre-inst-env: don't use GUIX_PACKAGE_PATH
Date: Fri, 07 Aug 2020 10:41:52 +0200

Hello Denis,

> ./pre-inst-env is supposed to use only the packages definitions that are in
> the guix source tree and not the host packages.
>
> However if GUIX_PACKAGE_PATH is set, it will use host packages as well.

Let say you have a custom my-hello package inheriting from hello, and
you want to build it on top of a recent Guix checkout, then you probably
want GUIX_PACKAGE_PATH to cooperate with pre-inst-env.

Same goes if you have a manifest mixing Guix packages and custom
packages accessed by GUIX_PACKAGE_PATH, and you want to test your
manifest on top of a Guix checkout using pre-inst-env.

Thanks,

Mathieu

Message #17 received at 42735 <at> debbugs.gnu.org (full text, mbox):

From: Denis 'GNUtoo' Carikli <GNUtoo <at> cyberdimension.org>
To: Mathieu Othacehe <othacehe <at> gnu.org>
Cc: 42735 <at> debbugs.gnu.org
Subject: Re: [bug#42735] [PATCH] pre-inst-env: don't use GUIX_PACKAGE_PATH
Date: Sun, 9 Aug 2020 11:09:39 +0200

[Message part 1 (text/plain, inline)]

On Fri, 07 Aug 2020 10:41:52 +0200
Mathieu Othacehe <othacehe <at> gnu.org> wrote:
> Let say you have a custom my-hello package inheriting from hello, and
> you want to build it on top of a recent Guix checkout, then you
> probably want GUIX_PACKAGE_PATH to cooperate with pre-inst-env.
> 
> Same goes if you have a manifest mixing Guix packages and custom
> packages accessed by GUIX_PACKAGE_PATH, and you want to test your
> manifest on top of a Guix checkout using pre-inst-env.
Thanks for the explanation.

What would be the way for both use cases to be addressed?

Should a warning be emitted when packages come from GUIX_PACKAGE_PATH
with ./pre-inst-env ?

What about something that would look like that:
>    guix build: warning: GUIX_PACKAGE_PATH is set
>    guix build: warning: choosing my-hello <at> 0.1 from
>                         /home/[...]/.config/guix/local/custom.scm:31:2
>    guix build: warning: If you don't want to use
>                         packages from GUIX_PACKAGE_PATH you can run
>                         'unset GUIX_PACKAGE_PATH' before running
>                         pre-inst-env

The issue is if it picks dependencies from GUIX_PACKAGE_PATH which are
not in Guix source code yet. In that case it might be more complicated
to make sure that people will see the warning as it might be hidden in
the huge build log.

Another option which can also be combined with the previous one would be
to warn people in the manual.

Note that I'm also fine with the status quo, but if it's not too
complicated to improve the situation in a way that doesn't break
existing use cases it would probably make sense to do it.

Denis.

[Message part 2 (application/pgp-signature, inline)]

Message #22 received at 42735-done <at> debbugs.gnu.org (full text, mbox):

From: Oleg Pykhalov <go.wigust <at> gmail.com>
To: Denis 'GNUtoo' Carikli <GNUtoo <at> cyberdimension.org>
Cc: 42735-done <at> debbugs.gnu.org
Subject: Re: [bug#42735] [PATCH] pre-inst-env: don't use GUIX_PACKAGE_PATH
Date: Fri, 14 Aug 2020 13:00:16 +0300

[Message part 1 (text/plain, inline)]

Hi,

Denis 'GNUtoo' Carikli <GNUtoo <at> cyberdimension.org> writes:

[…]

> Should a warning be emitted when packages come from GUIX_PACKAGE_PATH
> with ./pre-inst-env ?

I don't think a user needs another warning for an environment variable
which he setted by himself.  Also, user could use ‘-L’ flag instead of
‘GUIX_PACKAGE_PATH’.


Currently,

with ‘hello’ package in GUIX_PACKAGE_PATH:
--8<---------------cut here---------------start------------->8---
oleg <at> guixsd ~/src/guix-master$ GUIX_PACKAGE_PATH=/home/oleg/src/guix-wigust/guix ./pre-inst-env guix build --no-grafts --no-offload hello
guix build: warning: ambiguous package specification `hello'
guix build: warning: choosing hello <at> 2.10 from /home/oleg/src/guix-wigust/guix/wigust/packages/games.scm:174:2
/gnu/store/a462kby1q51ndvxdv3b6p0rsixxrgx1h-hello-2.10
--8<---------------cut here---------------end--------------->8---

without ‘hello’ package in GUIX_PACKAGE_PATH:
--8<---------------cut here---------------start------------->8---
oleg <at> guixsd ~/src/guix-master$ GUIX_PACKAGE_PATH=/home/oleg/src/guix-wigust/guix ./pre-inst-env guix build --no-grafts --no-offload hello
/gnu/store/a462kby1q51ndvxdv3b6p0rsixxrgx1h-hello-2.10
--8<---------------cut here---------------end--------------->8---

> The issue is if it picks dependencies from GUIX_PACKAGE_PATH which are
> not in Guix source code yet. In that case it might be more complicated
> to make sure that people will see the warning as it might be hidden in
> the huge build log.
>
> Another option which can also be combined with the previous one would be
> to warn people in the manual.

It's already documented, isn't it?

https://www.gnu.org/software/guix/manual/html_node/Package-Modules.html
--8<---------------cut here---------------start------------->8---
 -- Environment Variable: GUIX_PACKAGE_PATH
     This is a colon-separated list of directories to search for
     additional package modules.  Directories listed in this variable
     take precedence over the own modules of the distribution.
--8<---------------cut here---------------end--------------->8---

I'll close the bug.  Feel free to reopen if I miss something.


Regards,
Oleg.

[signature.asc (application/pgp-signature, inline)]

Message #25 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ricardo Wurmus <rekado <at> elephly.net>
To: Mathieu Othacehe <othacehe <at> gnu.org>
Cc: guix-patches <at> gnu.org, 42735 <at> debbugs.gnu.org,
 Denis 'GNUtoo' Carikli <GNUtoo <at> cyberdimension.org>
Subject: Re: [bug#42735] [PATCH] pre-inst-env: don't use GUIX_PACKAGE_PATH
Date: Fri, 14 Aug 2020 13:16:09 +0200

Mathieu Othacehe <othacehe <at> gnu.org> writes:

> Hello Denis,
>
>> ./pre-inst-env is supposed to use only the packages definitions that are in
>> the guix source tree and not the host packages.
>>
>> However if GUIX_PACKAGE_PATH is set, it will use host packages as well.
>
> Let say you have a custom my-hello package inheriting from hello, and
> you want to build it on top of a recent Guix checkout, then you probably
> want GUIX_PACKAGE_PATH to cooperate with pre-inst-env.

That’s primarily how I use GUIX_PACKAGE_PATH: for development.  If you
have GUIX_PACKAGE_PATH set for any other reasons you should consider
using channels instead.

-- 
Ricardo

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.