[PATCH] pre-inst-env: don't use GUIX_PACKAGE_PATH

  • Done
  • quality assurance status badge
Details
5 participants
  • Denis 'GNUtoo' Carikli
  • Brett Gilio
  • Oleg Pykhalov
  • Mathieu Othacehe
  • Ricardo Wurmus
Owner
unassigned
Submitted by
Denis 'GNUtoo' Carikli
Severity
normal
D
D
Denis 'GNUtoo' Carikli wrote on 7 Aug 2020 04:21
(address . guix-patches@gnu.org)(name . Denis 'GNUtoo' Carikli)(address . GNUtoo@cyberdimension.org)
20200807022142.26296-1-GNUtoo@cyberdimension.org
./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@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@cyberdimension.org>
---
build-aux/pre-inst-env.in | 4 ++++
1 file changed, 4 insertions(+)

Toggle diff (15 lines)
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
B
B
Brett Gilio wrote on 7 Aug 2020 05:46
(name . Denis 'GNUtoo' Carikli)(address . GNUtoo@cyberdimension.org)(address . 42735@debbugs.gnu.org)
87o8nni0cz.fsf@gnu.org
Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org> writes:

Toggle quote (7 lines)
>
> +# 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?
D
D
Denis 'GNUtoo' Carikli wrote on 7 Aug 2020 08:02
(name . Brett Gilio)(address . brettg@gnu.org)(address . 42735@debbugs.gnu.org)
20200807080204.0b2f9b9d@primarylaptop.localdomain
On Thu, 06 Aug 2020 22:46:52 -0500
Brett Gilio <brettg@gnu.org> wrote:

Toggle quote (11 lines)
> Denis 'GNUtoo' Carikli <GNUtoo@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.
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEeC+d2+Nrp/PU3kkGX138wUF34mMFAl8s7lwACgkQX138wUF3
4mM6eA//cKAfT/kgMNsYchvV2Koet5Kloyf/zx1f63QkwdietPJFLaVf5majabhx
NvQu0RBH8cSay5pKed+fEmSWWFVRCJAIymzzafo2A6pGzJB3naQYxGtUQUY3K62Y
98j6q72B3wlz+RNJmCJwzcxFU+4UOb7yCsvFIX5B+0WexRGqKJYzYfx9FK04LyUq
IRaUNQ86028qtrrxV2QYyWrRDY7l9JTupmGv+4qZIaJfInlhrpY6r9Imrg7T8xiG
FBOUVtwFlQ/F8hjk/y0/bOgby4cxMQe2kgcMdKLBzSRU8TwAs7+PgqYbQSPM+p0L
Q5siitiAI/3IjpF3HYVvpRVfFjmBeD4QeUc7sy8L+s/C4AVmqkLYOI90U4Wgwm7N
wEgbbCKQT/xeaNu2M1KdjzAkFyZGIDiHRu0BGvJC8xqp5ogA94BOc4095yGU70yI
a6PGkB332SmAbg6aVEqggbHAl+gxVibuhdZphQBZSC33aZUB055vZ2Tk/Z/jvlOW
xDibm6ilPY3cBHoDCgQkbDVHweqvTSbmbcq5gu4EwWUcZa9O6AT3LWCnKwnXQlLn
qMwMQeex5y1krY50ffJPqFFLzRsxIYsZ2m7r0bnDDHahXP2yslfK+fstLRXspVJP
XrKgckaD2XAlVaAzsTgONSA4aJGlyit6NhXjwNgGmpkXeDD8Hgo=
=uvBH
-----END PGP SIGNATURE-----


M
M
Mathieu Othacehe wrote on 7 Aug 2020 10:41
(name . Denis 'GNUtoo' Carikli)(address . GNUtoo@cyberdimension.org)(address . 42735@debbugs.gnu.org)
87mu36onjj.fsf@gnu.org
Hello Denis,

Toggle quote (5 lines)
> ./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
D
D
Denis 'GNUtoo' Carikli wrote on 9 Aug 2020 11:09
(name . Mathieu Othacehe)(address . othacehe@gnu.org)(address . 42735@debbugs.gnu.org)
20200809110939.407b0bb9@primarylaptop.localdomain
On Fri, 07 Aug 2020 10:41:52 +0200
Mathieu Othacehe <othacehe@gnu.org> wrote:
Toggle quote (7 lines)
> 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:
Toggle quote (8 lines)
> guix build: warning: GUIX_PACKAGE_PATH is set
> guix build: warning: choosing my-hello@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.
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEeC+d2+Nrp/PU3kkGX138wUF34mMFAl8vvVQACgkQX138wUF3
4mPTRQ//WYGSrgkkVctf7YmWqK0C0AQK4OI2oHm0aes74xDUNLrib6hV3I9w7iNq
qDdXF9Xn/QW08yNw/TYFQe150ltKnF7e9MubkbjK+PSujx+hrmLK0R+oMFcnfJgi
u0cwSV8ST19BxtkDS1TVtFvf93kbGgQ9Ag5zTDq5nH3NvYl2ZBiwYDYc5yFM3GKj
kV7qWijl2Coa1v822yL5dHN1HKBmgpbqHUpexpq01NlnEfDb+tZNFikZsa+tcoHc
IK6vXTkJBLG0nKmv835v+6DWOJrESAetXxVMYlfG1nahXUGF+Ajqi2fxApZnwzGU
79b8GRPolSkMN7gaksiXzU0WZsGr/tZPIXsyVy2UzdNmjHt2Fd9shTLw0lR56ZZo
NSVdQxH1d/LNQt/6KdW809cK4nXkPiZYsLcOwD5SAwpFfLrCNGihgUcFrZpHngUP
tEJfmHTqDI2GyjXVRpJ81Imhd+D1a/3LwjqWzlKbDj45l57L3NO3EJb+lv/dGH15
OUzO96Cx7w9DC0y83I/MLnHU3eJSCJk7bulqnZYmeiDa4hj8EzJBmnggfYk1x03o
c0fi//+0knGclt8NITuoSPrGZiXm29cTbQNY1IIlyT/ENduavSv4kQIvjDoXSQLR
5XD8a1bh94tz7SehSXS3YP/Hnrl2jLEHnjFHzkYEQRa6qfDeCJA=
=vvTS
-----END PGP SIGNATURE-----


O
O
Oleg Pykhalov wrote on 14 Aug 2020 12:00
(name . Denis 'GNUtoo' Carikli)(address . GNUtoo@cyberdimension.org)(address . 42735-done@debbugs.gnu.org)
87lfihsg27.fsf@gmail.com
Hi,

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

[…]

Toggle quote (3 lines)
> 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:
Toggle snippet (6 lines)
oleg@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@2.10 from /home/oleg/src/guix-wigust/guix/wigust/packages/games.scm:174:2
/gnu/store/a462kby1q51ndvxdv3b6p0rsixxrgx1h-hello-2.10

without ‘hello’ package in GUIX_PACKAGE_PATH:
Toggle snippet (4 lines)
oleg@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

Toggle quote (8 lines)
> 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?

Toggle snippet (6 lines)
-- 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.

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


Regards,
Oleg.
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEcjhxI46s62NFSFhXFn+OpQAa+pwFAl82YLAACgkQFn+OpQAa
+pzL0Q/+KW2sLToAcjSjGm+yQv+4iwdepo/Zb5f0QkjeGhl6DpJDnFHV5FB7mqQz
4nznrC88NmpUMLrxAOjI+NHgVGLPRDdx26gMNu8nG4vHdL/D0UjzkMvtGfTUbZlu
+eT2i3aElqfz3Hl9nnE+14uxhG+MlTWCq7JIOn6636O+b+Im0ynVKGNH+Eb1c/6r
AMuTgOn7+IT5kzuqWduSblNsWrunis8vkCaMjSFyCjV79XuIL/jybRgXFJ+WB0MO
tvs2WF+SYacKev2DpAcwbt6+AwTSfxkuI6qYEc/zETxwAjSYbcGnA4uJAvTlFScx
zJ3A8dAk7WTuTDxVCxCPs4mkDBzS7r4AbHIzZBwaIag6ZHKgHjcnqOQ3b0R+7fOQ
WeVLbQCVxsRe8mGATRbvUOAgQa2qTH4D+GjVn57Y/9F2OEIa4vjtM9vwknXBCcGE
47RGfdCPo7iNMjeCtIsdgtljyGw8vDQRXX6sP21KwcALrU56UZjFyzQDHQqM1hnB
oMhZvswxWGXeb0cM3tO0ibgNsgDDyfO/CSQIfhZxSJA7rQ/2tAjamIQjoIr59VXh
/iT/awabGJsd7UTq5vUKSBSPO/7LDA5c2PvuAjKh9yEfL080GgGgLB+oYLEx4x0p
rDpmfiO/NFikvYzQaK8i8KT+q5mOCKttA73hZIK2d7tQH+zou7U=
=7gdY
-----END PGP SIGNATURE-----

Closed
R
R
Ricardo Wurmus wrote on 14 Aug 2020 13:16
(name . Mathieu Othacehe)(address . othacehe@gnu.org)
87h7t532bq.fsf@elephly.net
Mathieu Othacehe <othacehe@gnu.org> writes:

Toggle quote (11 lines)
> 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
?