[PATCH] guix: Add lint check for guix as propagated-input

  • Open
  • quality assurance status badge
Details
4 participants
  • Karl Hallsby
  • Ludovic Courtès
  • Christopher Baines
  • Simon Tournier
Owner
unassigned
Submitted by
Karl Hallsby
Severity
normal
K
K
Karl Hallsby wrote on 25 Jul 2023 20:04
(address . guix-patches@gnu.org)(name . Karl Hallsby)(address . karl@hallsby.com)
388dc9bec6dabc97092eb2009fed304c3f84c6fc.1690308209.git.karl@hallsby.com
* guix/lint.scm (new check): Run lint check warning user if the provided
package uses guix as a propagated-input.

Passing guix as a propagated-input is problematic when users install a package
into their profiles. This can cause the guix propagated by the package to be
used in preference of the real one in $HOME/.config/. It was first noticed on
---
guix/lint.scm | 14 ++++++++++++++
1 file changed, 14 insertions(+)

Toggle diff (36 lines)
diff --git a/guix/lint.scm b/guix/lint.scm
index d173563e51..5fae34ca22 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -575,6 +575,16 @@ (define (check-input-labels package)
(inputs ,package-inputs)
(propagated-inputs ,package-propagated-inputs))))
+(define (check-guix-propagated-inputs package)
+ (if (and (not (null? (package-propagated-inputs package)))
+ (not (memq (@ (gnu packages package-management) guix)
+ (package-propagated-inputs package))))
+ (list
+ (make-warning package
+ (G_ "are you sure guix should be a propagated-input?")
+ #:field 'propagated-inputs))
+ '()))
+
(define (report-wrap-program-error package wrapper-name)
"Warn that \"bash-minimal\" is missing from 'inputs', while WRAPPER-NAME
requires it."
@@ -1884,6 +1894,10 @@ (define %local-checkers
(name 'input-labels)
(description "Identify input labels that do not match package names")
(check check-input-labels))
+ (lint-checker
+ (name 'warn-guix-propagated-inputs)
+ (description "Emit warning if guix package is propagated-input")
+ (check check-guix-propagated-inputs))
(lint-checker
(name 'wrapper-inputs)
(description "Make sure 'wrap-program' can finds its interpreter.")

base-commit: 9ff1e7652a407b88a3eeeab6a67261f6fee40807
--
2.40.1
C
C
Christopher Baines wrote on 25 Jul 2023 20:26
(name . Karl Hallsby)(address . karl@hallsby.com)(address . 64861@debbugs.gnu.org)
87pm4f7skl.fsf@cbaines.net
Karl Hallsby <karl@hallsby.com> writes:

Toggle quote (12 lines)
> * guix/lint.scm (new check): Run lint check warning user if the provided
> package uses guix as a propagated-input.
>
> Passing guix as a propagated-input is problematic when users install a package
> into their profiles. This can cause the guix propagated by the package to be
> used in preference of the real one in $HOME/.config/. It was first noticed on
> IRC with https://logs.guix.gnu.org/guix/2023-07-22.log#044534, and reproduced
> with a different package https://logs.guix.gnu.org/guix/2023-07-25.log#054737.
> ---
> guix/lint.scm | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)

I'm not sure this lint warning is helpful as in both cases mentioned in
the IRC links you provided (guile-imanifest and guix-data-service),
these packages provide Guile modules that depend on Guix, and therefore
the guix package is expected in their propagated inputs.

If you remove guix or make it a input instead of a propagated input,
then this breaks using the package, for example, with this change:

modified gnu/packages/guile-xyz.scm
@@ -2098,8 +2098,10 @@ (define-public guile-imanifest
(build-system guile-build-system)
(native-inputs
(list guile-3.0))
+ (inputs
+ (list guix))
(propagated-inputs
- (list guile-readline guile-colorized guix))
+ (list guile-readline guile-colorized))
(synopsis "Interactive Guix manifests")
(description "This package provides functions to generate Guix manifests

Using guile-imanifest breaks:

→ ./pre-inst-env guix environment --pure --ad-hoc guile guile-imanifest -- guile -c "(use-modules (imanifest))"
Backtrace:
...

ice-9/boot-9.scm:3330:6: In procedure resolve-interface:
no code for module (guix profiles)


I think some other approach is needed to avoid people having problems
with the guix package appearing in their users profile.
-----BEGIN PGP SIGNATURE-----

iQKlBAEBCgCPFiEEPonu50WOcg2XVOCyXiijOwuE9XcFAmTAFJpfFIAAAAAALgAo
aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF
ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcRHG1haWxAY2Jh
aW5lcy5uZXQACgkQXiijOwuE9XchPw/+IlvgRWwEQFH8HzbVdE1XmUYRQ94BsCu8
3QCq406XzvVdUXO94rmy4jqtEqQgZk9AR63Qa0dj8mx6YaNvWo/Hf0pyb2FihnID
g/Hu7M6jrI6sx0MUXJ0jRNRUDZgO2U4PrxokHtRzs8jsPOmTeuNSSWS1+lHwoQt5
1jk3vD4HxZ3dH5oKCdYOZ61WpOsL+0IUSI9vFsRh3mYF6I+tsQMBIY7WN9iOOCpy
YAaChBiNXuR3MpgWvi6f3QZCcrBv6DdQitmvP1YJqxazuNXOOa5mv5bId+fZ/yF1
Gs0hKk4rd7aDagspEKjcvzqbMWAcAAVnII08Y8w0I1hm9GxrbDEDRv70aZy410Tb
fuGVcBzOsLHJqiKGcvoh7MPoz29JmKrg1mu83Sd4UFL3vluLwtoSq35xHaYVtkzJ
Krr4p/FPgag39r80fZaDSWSQwzR+jEufCiEkOhVXKvvg2EqQeqUj4Fu0D26srDbd
oArse7NwoGHEv2s8qaB4o0u7vl0OxFq46qO++Xuf4/Zh3c90re2RYJofehIRNuY7
fIcpqiG7KT5IId7LbQsFVyP3yaLz2mbfzpqMYOBYp9hOrM92PSLDDP2vwQwF4DV+
0a4N/v/54+/hMH2MBL87EFFjL8qEMRojRTRhb1NlDF7/SQ0y7YYHef2PpI/3Sjta
Qq4dCatz01c=
=P4R2
-----END PGP SIGNATURE-----

K
K
Karl G. Hallsby wrote on 25 Jul 2023 20:41
(name . Christopher Baines)(address . mail@cbaines.net)(address . 64861@debbugs.gnu.org)
874jlr96d5.fsf@hallsby.com
Christopher Baines <mail@cbaines.net> writes:

Toggle quote (53 lines)
> [[PGP Signed Part:Undecided]]
>
> Karl Hallsby <karl@hallsby.com> writes:
>
>> * guix/lint.scm (new check): Run lint check warning user if the provided
>> package uses guix as a propagated-input.
>>
>> Passing guix as a propagated-input is problematic when users install a package
>> into their profiles. This can cause the guix propagated by the package to be
>> used in preference of the real one in $HOME/.config/. It was first noticed on
>> IRC with https://logs.guix.gnu.org/guix/2023-07-22.log#044534, and reproduced
>> with a different package https://logs.guix.gnu.org/guix/2023-07-25.log#054737.
>> ---
>> guix/lint.scm | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>
> I'm not sure this lint warning is helpful as in both cases mentioned in
> the IRC links you provided (guile-imanifest and guix-data-service),
> these packages provide Guile modules that depend on Guix, and therefore
> the guix package is expected in their propagated inputs.
>
> If you remove guix or make it a input instead of a propagated input,
> then this breaks using the package, for example, with this change:
>
> modified gnu/packages/guile-xyz.scm
> @@ -2098,8 +2098,10 @@ (define-public guile-imanifest
> (build-system guile-build-system)
> (native-inputs
> (list guile-3.0))
> + (inputs
> + (list guix))
> (propagated-inputs
> - (list guile-readline guile-colorized guix))
> + (list guile-readline guile-colorized))
> (home-page "https://sr.ht/~brown121407/guile-imanifest")
> (synopsis "Interactive Guix manifests")
> (description "This package provides functions to generate Guix manifests
>
> Using guile-imanifest breaks:
>
> → ./pre-inst-env guix environment --pure --ad-hoc guile guile-imanifest -- guile -c "(use-modules (imanifest))"
> Backtrace:
> ...
>
> ice-9/boot-9.scm:3330:6: In procedure resolve-interface:
> no code for module (guix profiles)
>
>
> I think some other approach is needed to avoid people having problems
> with the guix package appearing in their users profile.
>
> [[End of PGP Signed Part]]

That makes sense. Though, this lint is just a warning, so if nothing
else, it should make people think if guix really needs to be propagated.
If this lint is not the solution, then a way to mark packages that are
not recommended to be installed like this would be nice.
L
L
Ludovic Courtès wrote on 20 Aug 2023 22:58
Re: bug#64861: [PATCH] guix: Add lint check for guix as propagated-input
(name . Christopher Baines)(address . mail@cbaines.net)
87fs4dqvml.fsf_-_@gnu.org
Hi,

Christopher Baines <mail@cbaines.net> skribis:

Toggle quote (9 lines)
> Using guile-imanifest breaks:
>
> → ./pre-inst-env guix environment --pure --ad-hoc guile guile-imanifest -- guile -c "(use-modules (imanifest))"
> Backtrace:
> ...
>
> ice-9/boot-9.scm:3330:6: In procedure resolve-interface:
> no code for module (guix profiles)

Maybe ‘guile-imanifest’ should be made a Guix extension, which Guix
searches for in $GUIX_EXTENSIONS_PATH?

An example of that is ‘guix-modules’.

Ludo’.
S
S
Simon Tournier wrote on 7 Sep 2023 16:22
Re: [bug#64861] [PATCH] guix: Add lint check for guix as propagated-input
87bkeekqqx.fsf@gmail.com
Hi,

On Sun, 20 Aug 2023 at 22:58, Ludovic Courtès <ludo@gnu.org> wrote:

Toggle quote (3 lines)
> Maybe ‘guile-imanifest’ should be made a Guix extension, which Guix
> searches for in $GUIX_EXTENSIONS_PATH?

And probably renamed ’guix-imanifest’?

Back to the submission, I think that the propagation of the package guix
means something is wrong. From my point of view, there is two cases:

1. The package uses the stable library API and thus it makes sense to
rely on the package ’guix’. That’s the case for ’gwl’,
’guix-data-service’ for example.

2. The aim of package is to collaborate with the current Guix and thus
there is no point to have the package ’guix’ as inputs. Instead,
the package must rely on GUIX_EXTENSIONS_PATH. That’s the case for
’guix-modules’ or ’guile-imanifest’ (so that needs a fix ;-))

Therefore, I think this new checker makes sense. WDYT?


About #1, IMHO, this is expected:

Toggle snippet (9 lines)
$ guix shell -C gwl -- guix --version
guix shell: error: guix: command not found

(define-public gwl
(inputs
[...]
(list guix

and this is not expected:

Toggle snippet (14 lines)
$ guix shell -C guix-data-service -- guix --version
guix (GNU Guix) 1.4.0-10.4dfdd82
Copyright (C) 2023 the Guix authors
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

(define-public guix-data-service
[...]
(propagated-inputs
(list guix


Cheers,
simon
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 64861@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 64861
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch