diff mbox series

[bug#58583,v3] scripts: package: Forbid installation of the guix package.

Message ID 20221017165057.15648-1-paren@disroot.org
State New
Headers show
Series [bug#58583,v3] scripts: package: Forbid installation of the guix package. | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git-branch success View Git branch
cbaines/applying patch success View Laminar job
cbaines/issue success View issue

Commit Message

\( Oct. 17, 2022, 4:50 p.m. UTC
* guix/scripts/package.scm (package->manifest-entry*): Fail if the
  package to be installed is guix from the default channel.
---
 guix/scripts/package.scm | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Simon Tournier Oct. 17, 2022, 6:14 p.m. UTC | #1
Hi,

Cool!  Nice initiative.

On lun., 17 oct. 2022 at 17:50, "\( via Guix-patches" via <guix-patches@gnu.org> wrote:

> +    (report-error (G_ "the 'guix' package should not be installed~%"))

Instead of an error, I would prefer a warning.  Because, sometimes it is
useful to have this Guix library. :-)


Cheers,
simon
Maxim Cournoyer Oct. 27, 2022, 8:04 p.m. UTC | #2
Hi,

zimoun <zimon.toutoune@gmail.com> writes:

> Hi,
>
> Cool!  Nice initiative.
>
> On lun., 17 oct. 2022 at 17:50, "\( via Guix-patches" via <guix-patches@gnu.org> wrote:
>
>> +    (report-error (G_ "the 'guix' package should not be installed~%"))
>
> Instead of an error, I would prefer a warning.  Because, sometimes it is
> useful to have this Guix library. :-)

The Guix API would be available without having to 'guix install guix' in
the first place, no?
Maxim Cournoyer Oct. 27, 2022, 8:11 p.m. UTC | #3
Hello,

"(" <paren@disroot.org> writes:

> * guix/scripts/package.scm (package->manifest-entry*): Fail if the
>   package to be installed is guix from the default channel.
> ---
>  guix/scripts/package.scm | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
> index 7ba2661bbb..9f023ea7b5 100644
> --- a/guix/scripts/package.scm
> +++ b/guix/scripts/package.scm
> @@ -12,6 +12,7 @@
>  ;;; Copyright © 2018 Steve Sprang <scs@stevesprang.com>
>  ;;; Copyright © 2022 Josselin Poiret <dev@jpoiret.xyz>
>  ;;; Copyright © 2022 Antero Mejr <antero@mailbox.org>
> +;;; Copyright © 2022 ( <paren@disroot.org>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -699,7 +700,14 @@ (define (store-item->manifest-entry item)
>  
>  (define (package->manifest-entry* package output)
>    "Like 'package->manifest-entry', but attach PACKAGE provenance meta-data to
> -the resulting manifest entry."
> +the resulting manifest entry, and report an error if PACKAGE is the 'guix'
> +package from the default channel."
> +  (when (and (string=? (package-name package) "guix")
> +             (string-prefix? "gnu/" (location-file
> +                                     (package-location package))))
> +    (report-error (G_ "the 'guix' package should not be installed~%"))
> +    (display-hint (G_ "use 'guix pull' to fetch the latest Guix revision"))
> +    (exit 1))
>    (manifest-entry-with-provenance
>     (package->manifest-entry package output)))

Instead of exiting directly here, would it make sense to use
raise-exception with a compounded message and &fix-hint condition?  When
working with the Guix API, I prefer to encounter exceptions rather than
errors + exit.  For a recent example I used, see:
https://issues.guix.gnu.org/58812#5-lineno360.
Simon Tournier Oct. 28, 2022, 7:44 a.m. UTC | #4
Hi Maxim,

On Thu, 27 Oct 2022 at 16:04, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:

>>> +    (report-error (G_ "the 'guix' package should not be installed~%"))
>>
>> Instead of an error, I would prefer a warning.  Because, sometimes it is
>> useful to have this Guix library. :-)
>
> The Guix API would be available without having to 'guix install guix' in
> the first place, no?

Not necessary or I am missing something.  For instance, you want to
build some Guile application relying on the Guix library; similarly as
you have, say, guile-commonmark or any other Guile packages.

Yes, it is possible to do without installing the package guix but it is
not handy.  Aside, it is not always clear which revision of the API is
available when the package guix fixes it.

For instance, if an user packs their Guile application using some other
Guile libraries including the Guix library, then for developing, it
becomes not possible to just locally create a profile.

Well, I do not have a strong opinion on that, for what it is worth, I
would prefer a strong warning instead of an hard error.  But maybe I am
missing something.


Cheers,
simon
\( Oct. 28, 2022, 2:31 p.m. UTC | #5
On Fri Oct 28, 2022 at 8:44 AM BST, zimoun wrote:
> Not necessary or I am missing something.  For instance, you want to
> build some Guile application relying on the Guix library; similarly as
> you have, say, guile-commonmark or any other Guile packages.

What about just this?

  guix shell guix

That's still possible.

    -- (
Maxim Cournoyer Oct. 28, 2022, 3:47 p.m. UTC | #6
Hi Simon,

zimoun <zimon.toutoune@gmail.com> writes:

> Hi Maxim,
>
> On Thu, 27 Oct 2022 at 16:04, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
>>>> +    (report-error (G_ "the 'guix' package should not be installed~%"))
>>>
>>> Instead of an error, I would prefer a warning.  Because, sometimes it is
>>> useful to have this Guix library. :-)
>>
>> The Guix API would be available without having to 'guix install guix' in
>> the first place, no?
>
> Not necessary or I am missing something.  For instance, you want to
> build some Guile application relying on the Guix library; similarly as
> you have, say, guile-commonmark or any other Guile packages.
>
> Yes, it is possible to do without installing the package guix but it is
> not handy.  Aside, it is not always clear which revision of the API is
> available when the package guix fixes it.
>
> For instance, if an user packs their Guile application using some other
> Guile libraries including the Guix library, then for developing, it
> becomes not possible to just locally create a profile.
>
> Well, I do not have a strong opinion on that, for what it is worth, I
> would prefer a strong warning instead of an hard error.  But maybe I am
> missing something.

Does the benefit of fixing the Guix API used via a user profile
installed Guix package outweigh the cons of downgrading the version of
guix used as the user's package manager?  I don't think so.  By
installing the inner 'guix' into your user profile, you are basically
downgrading its version compared to the one you used to install it.
That's a pretty confusing thing to happen for most users.

As paren suggested, I think it's best to enter a dedicated 'guix shell'
profile with Guix when developing with the Guix API, else use the 'guix
repl' ability to run scripts.

It may be slightly inconvenient, but it's better than allowing Guix to
be silently downgraded upon unsuspecting users, in my opinion.
Tobias Geerinckx-Rice Oct. 28, 2022, 4:20 p.m. UTC | #7
Heyo,

zimoun 写道:
> Not necessary or I am missing something.  For instance, you want 
> to
> build some Guile application relying on the Guix library; 
> similarly as
> you have, say, guile-commonmark or any other Guile packages.

Sure.

> Yes, it is possible to do without installing the package guix 
> but it is
> not handy.  Aside, it is not always clear which revision of the 
> API is
> available when the package guix fixes it.

OK.  But could you explain more clearly how

  $ guix install guix

is involved, and how it is ‘handy’?

How does one continue to use guix *as a package manager*, having 
now silently broken ‘guix pull’?

> For instance, if an user packs their Guile application using 
> some other
> Guile libraries including the Guix library, then for developing, 
> it
> becomes not possible to just locally create a profile.

Why not?

> But maybe I am missing something.

Or we are!

Kind regards,

T G-R
Tobias Geerinckx-Rice Oct. 28, 2022, 5:01 p.m. UTC | #8
Tobias Geerinckx-Rice via Guix-patches via 写道:
>> For instance, if an user packs their Guile application using 
>> some
>> other
>> Guile libraries including the Guix library, then for 
>> developing, it
>> becomes not possible to just locally create a profile.
>
> Why not?

Would this be address by refusing only to ‘guix install guix’ 
without an explicit --profile argument?  This would eliminate 99% 
of unintentional footguns.  We could still warn.

Kind regards,

T G-R
Simon Tournier Nov. 2, 2022, 11:47 a.m. UTC | #9
Hi (, Maxim and Tobias,

Well, as I said, I do not have a strong opinion.  If 3 of you think an
error is better than a warning, then I rally to the proposal.

Minor comments about yours. :-)


On ven., 28 oct. 2022 at 15:31, "\( via Guix-patches" via <guix-patches@gnu.org> wrote:

> What about just this?
>
>   guix shell guix
>
> That's still possible.

To be precise, the correct would be:

    guix time-machine -C channels.scm -- shell guix

which is… equivalent to define a profile. ;-)  i.e.,

    guix package -i guix -p my/dev


On ven., 28 oct. 2022 at 11:47, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:

> Does the benefit of fixing the Guix API used via a user profile
> installed Guix package outweigh the cons of downgrading the version of
> guix used as the user's package manager?  I don't think so.  By
> installing the inner 'guix' into your user profile, you are basically
> downgrading its version compared to the one you used to install it.
> That's a pretty confusing thing to happen for most users.

I agree.  However, to me, it is a warning (or a hint) – «hey you are
probably doing something wrong» – and not an error – «we provide you
something but no, not this way».

Therefore, why do we provide the ’guix’ package in the first place?


(BTW, I think the correct way to use Guix as a library is to use it via
GUIX_EXTENSIONS_PATH as pioneered by gwl and followed by
guix-modules. :-))


On ven., 28 oct. 2022 at 18:20, Tobias Geerinckx-Rice via Guix-patches via <guix-patches@gnu.org> wrote:

> How does one continue to use guix *as a package manager*, having 
> now silently broken ‘guix pull’?

There is a confusion here, maybe?  Guix is also a Guile library and that
library is designed around package management.

Well, maybe instead the package ’guix’, it should be renamed
’guile-guix’ or ’guile-libguix’.



On ven., 28 oct. 2022 at 19:01, Tobias Geerinckx-Rice via Guix-patches via <guix-patches@gnu.org> wrote:

> Would this be address by refusing only to ‘guix install guix’ 
> without an explicit --profile argument?  This would eliminate 99% 
> of unintentional footguns.  We could still warn.

Personally, I do not consider ~/.guix-profile more special.  But maybe,
it would help to address the newcomer’s confusion.



Again, I think a strong warning is better than a hard error but I do not
have a strong opinion.


Cheers,
simon
Tobias Geerinckx-Rice Nov. 2, 2022, 1:19 p.m. UTC | #10
Heyo,

Thanks for the clarifications!  I hope you don't feel like you 
were dragged into a discussion against your will.  If so, I really 
do apologise.

I think all intentions here were the opposite: to make sure that 
even a ‘weak’ opinion was properly considered.  It might turn out 
to be more robust than the ‘strong’ ones ;-)  That's one of Guix's 
strengths IMO.

I'll not ask further questions below.

zimoun 写道:
> Therefore, why do we provide the ’guix’ package in the first 
> place?

That ‘guix install guix’ is an error does *not* imply that the 
mere existence of the ‘guix’ package is an error.  I think we can 
keep those separate.

>> How does one continue to use guix *as a package manager*, 
>> having 
>> now silently broken ‘guix pull’?
>
> There is a confusion here, maybe?  Guix is also a Guile library 
> and that
> library is designed around package management.

Right.  My problem is: I don't understand what's confusing about 
that fact, so it's hard to communicate effectively about what I 
don't see…

> Well, maybe instead the package ’guix’, it should be renamed
> ’guile-guix’ or ’guile-libguix’.

That would be going against the spirit of our own naming rules, 
unless you mean that it should be a ‘library-only’ variant that 
lacks /bin/guix.

Now *that* I do find mildly confusing—but only because it's 
starting to get complex :-)  Do we then put /bin/guix in 
‘guix-libguix:bin’?  Or a second package?  Etc.

So I'd rather keep ‘guix’ available as ‘guix’.

> Personally, I do not consider ~/.guix-profile more special.

Nor do I.  I agree that ‘-p ~/.guix-profile’ shouldn't be magical, 
or I would have suggested an approach different from ('s from the 
start.

Kind regards,

T G-R
Simon Tournier Nov. 2, 2022, 3:48 p.m. UTC | #11
Hi Tobias,

On mer., 02 nov. 2022 at 14:19, Tobias Geerinckx-Rice via Guix-patches via <guix-patches@gnu.org> wrote:

> Thanks for the clarifications!  I hope you don't feel like you 
> were dragged into a discussion against your will.  If so, I really 
> do apologise.
>
> I think all intentions here were the opposite: to make sure that 
> even a ‘weak’ opinion was properly considered.  It might turn out 
> to be more robust than the ‘strong’ ones ;-)  That's one of Guix's 
> strengths IMO.

For sure. :-)


Well, we agree that many people are confused by

 1. which version of Guix they are running,
 2. the package named ’guix’ which time to time is installed with a
    wrong understanding about what it is.

And we agree that the patch is a way to address that.  We also agree
that raising a message when running “guix install guix” (whatever the
profile) is an appropriate mean to address the issue.

Where we disagree is only if the message must be an error stopping any
other actions or if the message must be a warning – letting people shoot
in their foot if they really want to, fully being aware that they could
be burnt.

Yours arguments are not convincing me that an error is adequate because
I am raising corner cases (e.g., guix as a Guile library).  And you are
not convinced by my arguments, pointing that are not worth the
exception.

Well, let agree that we disagree and move forward. :-)  I rally to the
proposal about put an error.  At worse, there is many workarounds for
people really wanting the package named ’guix’ in some profile. :-)


Just other minor comments – because now I am dragged into a discussion
against my will ;-) – so let keep my opinion as clear as I am able to.


> zimoun 写道:
>> Therefore, why do we provide the ’guix’ package in the first 
>> place?
>
> That ‘guix install guix’ is an error does *not* imply that the 
> mere existence of the ‘guix’ package is an error.  I think we can 
> keep those separate.

We agree that “guix install guix” is most of the time an error and an
user’s misunderstanding.  We want address the confusion and one part of
the confusion is from the package named ’guix’.  Therefore, it appears
to me a question: why do we provide the package named ’guix’ in the
first place?

This is a honest question.  Maybe this patch is not addressing at the
correct level the source of the confusion.  And maybe the fix should be
at another level.

Aside some corner cases as described elsewhere (guix as a Guile
library), why do we need to provide a package named ’guix’?  In order to
allow,

   guix shell -D guix

for feeding a development environment for Guix.  Something else?

Somehow, my point is not to imply that the package named ’guix’ is an
error but instead to think if we really need this package named ’guix’.


>> Well, maybe instead the package ’guix’, it should be renamed
>> ’guile-guix’ or ’guile-libguix’.
>
> That would be going against the spirit of our own naming rules, 
> unless you mean that it should be a ‘library-only’ variant that 
> lacks /bin/guix.

Euh, why is it going against the spirit of the naming rules?  All Guile
packages are prefixed by ’guile-’, as Haskell by ’ghc-’, as R by ’r-’,
etc.

And for instance, the package ’python-nose’ provides ’bin/nosetests’.
Idem for ’python-pylint’ and ’bin/pylint’; for the two I quickly found.


> Now *that* I do find mildly confusing—but only because it's 
> starting to get complex :-)  Do we then put /bin/guix in 
> ‘guix-libguix:bin’?  Or a second package?  Etc.

Here, I am confused. :-) Aside that ’guile-guix’ would be a perfectly
fine name, I miss the logic: on one hand a willing to error because
’bin/guix’ and on the other hand trying to define various outputs to
keep such ’bin/guix’.

Or I miss some humour behind.


Cheers,
simon
Maxim Cournoyer Nov. 3, 2022, 3:03 p.m. UTC | #12
Hi Simon,

zimoun <zimon.toutoune@gmail.com> writes:

[...]

> Euh, why is it going against the spirit of the naming rules?  All Guile
> packages are prefixed by ’guile-’, as Haskell by ’ghc-’, as R by ’r-’,
> etc.
>
> And for instance, the package ’python-nose’ provides ’bin/nosetests’.
> Idem for ’python-pylint’ and ’bin/pylint’; for the two I quickly found.

Referring to info '(guix) Package Naming', that doesn't seem to be a
written rule; my rule of thumb here would be: if something exists to be
used exclusively as a command, drop the language-specific prefix.  If it
is a library or both a library and a command, keep the prefix.  In
doubt, keep the prefix.

So supposing 'git' was implemented in Python, it'd still be called
'git', not 'python-git'.  pylint could/should probably be named
"pylint", but perhaps it's also usable as a Python library, I haven't
checked.
Simon Tournier Nov. 3, 2022, 6:32 p.m. UTC | #13
Hi Maxim,

On jeu., 03 nov. 2022 at 11:03, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:

> Referring to info '(guix) Package Naming', that doesn't seem to be a
> written rule; my rule of thumb here would be: if something exists to be
> used exclusively as a command, drop the language-specific prefix.  If it
> is a library or both a library and a command, keep the prefix.  In
> doubt, keep the prefix.

Yes, for sure.  And I agree that we should follow as close as possible
this rule of thumb because it provides a good consistency.

My point is: the line is not always clear and for some packages we have
a grey area.

Another example, we have pandoc and ghc-pandoc.  IIRC, these two
packages had been created for a similar reason as we are discussing
here; from my understanding.

All in all, I am convinced that raising an error when installing the
package named ’guix’ is not an appropriated patch for fixing the
confusion generated by the package named ’guix’.

Well, I have said my opinion and this patch is not a breaking change
revolutionizing the world neither. ;-) Feel free to push a patch
containing what you find appropriate.


Cheers,
simon
diff mbox series

Patch

diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index 7ba2661bbb..9f023ea7b5 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -12,6 +12,7 @@ 
 ;;; Copyright © 2018 Steve Sprang <scs@stevesprang.com>
 ;;; Copyright © 2022 Josselin Poiret <dev@jpoiret.xyz>
 ;;; Copyright © 2022 Antero Mejr <antero@mailbox.org>
+;;; Copyright © 2022 ( <paren@disroot.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -699,7 +700,14 @@  (define (store-item->manifest-entry item)
 
 (define (package->manifest-entry* package output)
   "Like 'package->manifest-entry', but attach PACKAGE provenance meta-data to
-the resulting manifest entry."
+the resulting manifest entry, and report an error if PACKAGE is the 'guix'
+package from the default channel."
+  (when (and (string=? (package-name package) "guix")
+             (string-prefix? "gnu/" (location-file
+                                     (package-location package))))
+    (report-error (G_ "the 'guix' package should not be installed~%"))
+    (display-hint (G_ "use 'guix pull' to fetch the latest Guix revision"))
+    (exit 1))
   (manifest-entry-with-provenance
    (package->manifest-entry package output)))