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 |
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 |
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
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?
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.
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
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. -- (
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.
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 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
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
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
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
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.
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 --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)))