diff mbox series

[bug#45632] guix package: Warn if uses has 'guix' package in profile.

Message ID 20210103183202.11224-1-kuba@kadziolka.net
State Under Review
Headers show
Series [bug#45632] guix package: Warn if uses has 'guix' package in profile. | expand

Checks

Context Check Description
cbaines/submitting builds success
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

Maja Kądziołka Jan. 3, 2021, 6:32 p.m. UTC
* guix/scripts/package.scm (warn-about-guix-in-profile): New procedure.
(process-actions): Call WARN-ABOUT-GUIX-IN-PROFILE before executing
transaction.
---

Unresolved questions:
- Is this the right place to put warn-about-guix-in-profile?
- The warning message seems to be hard-wrapped. Is this the right
  line length?
- Do we want to make this configurable? Some other warnings are, but in
  those cases it is the threshold that gets configured. In this case,
  there is no threshold.

 guix/scripts/package.scm | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Simon Tournier Jan. 7, 2021, 4:39 p.m. UTC | #1
Hi,

On Sun, 3 Jan 2021 at 19:33, Jakub Kądziołka <kuba@kadziolka.net> wrote:
>
> * guix/scripts/package.scm (warn-about-guix-in-profile): New procedure.
> (process-actions): Call WARN-ABOUT-GUIX-IN-PROFILE before executing
> transaction.
> ---
>
> Unresolved questions:
> - Is this the right place to put warn-about-guix-in-profile?
> - The warning message seems to be hard-wrapped. Is this the right
>   line length?
> - Do we want to make this configurable? Some other warnings are, but in
>   those cases it is the threshold that gets configured. In this case,
>   there is no threshold.

What is the use case?  I have missed some context.

For example, I have the package 'guix' in some of my profiles and I
would be annoyed to be warned.  And I have it for good reasons. :-)
Once the story about extension is good enough, maybe the warning would
be a good advice, otherwise it could be confusing.  IMHO.


All the best,
simon
Maja Kądziołka Jan. 7, 2021, 5:11 p.m. UTC | #2
On Thu Jan 7, 2021 at 5:39 PM CET, zimoun wrote:
> Hi,
>
> On Sun, 3 Jan 2021 at 19:33, Jakub Kądziołka <kuba@kadziolka.net>
> wrote:
> >
> > * guix/scripts/package.scm (warn-about-guix-in-profile): New procedure.
> > (process-actions): Call WARN-ABOUT-GUIX-IN-PROFILE before executing
> > transaction.
> > ---
> >
> > Unresolved questions:
> > - Is this the right place to put warn-about-guix-in-profile?
> > - The warning message seems to be hard-wrapped. Is this the right
> >   line length?
> > - Do we want to make this configurable? Some other warnings are, but in
> >   those cases it is the threshold that gets configured. In this case,
> >   there is no threshold.
>
> What is the use case? I have missed some context.

As the actual warning text explains, including the guix package in a
profile loaded by default breaks the 'guix pull' update mechanism, as
the guix package from the profile will take priority.

This is inspired by some tech support on IRC where this turned out to be
a root cause of a problem.

Moreover, if I'm not missing anything, running 'guix package -u' with
such a setup would *downgrade* the guix package each time.

> For example, I have the package 'guix' in some of my profiles and I
> would be annoyed to be warned. And I have it for good reasons. :-)

Okay, I can see now that it is not wise to skimp on configurability for
this. Though, I would like to hear more about your usecase - how do you
keep your guix updated, for example?

> Once the story about extension is good enough,

As in adding your own commands under the 'guix $CMD' namespace, or
something more sophisticated? For the former, git seems to have a quite
good mechanism - we could borrow that.

Regards,
Jakub Kądziołka
Tobias Geerinckx-Rice Jan. 7, 2021, 5:15 p.m. UTC | #3
Jakub, Simon,

How about warning on pull instead?  That should address the ‘halp, 
me Guix be Benjamin Buttonin'’ support calls that I assume 
inspired this.

Simon, would that reduce the noise in your case?

Jakub Kądziołka 写道:
> - The warning message seems to be hard-wrapped. Is this the 
> right
>   line length?

Avoid hard-wrapping output entirely, by ’escaping’ the newlines

    (warning (G_ "like\
 this."))

Kind regards,

T G-R
Simon Tournier Jan. 7, 2021, 5:46 p.m. UTC | #4
Hi,

On Thu, 7 Jan 2021 at 18:27, Jakub Kądziołka <kuba@kadziolka.net> wrote:

> As the actual warning text explains, including the guix package in a
> profile loaded by default breaks the 'guix pull' update mechanism, as
> the guix package from the profile will take priority.

Hum? I simply have ~/.config/guix/current/bin first in my PATH.  Well,
I have the package 'guix' in my profile since long time because of
'emacs-guix' and never hit an issue.

> This is inspired by some tech support on IRC where this turned out to be
> a root cause of a problem.

Could you indicate the day?

> Moreover, if I'm not missing anything, running 'guix package -u' with
> such a setup would *downgrade* the guix package each time.

I do not know, I never upgrade this way.  Because I only use
manifest.scm files.  Even, from my personal taste  '-u' and 'guix
upgrade' are Guix abomination and should not exist; another story. ;-)

Again if ~/.config/guix/current/bin first in the PATH, then it should
not be an issue, I guess.

> > For example, I have the package 'guix' in some of my profiles and I
> > would be annoyed to be warned. And I have it for good reasons. :-)
>
> Okay, I can see now that it is not wise to skimp on configurability for
> this. Though, I would like to hear more about your usecase - how do you
> keep your guix updated, for example?

Basically, "guix pull" and "guix package -m".  But I am tracking this
file "guix describe -f channels > channels.scm" and I also do "guix
time-machine -C channels.scm -- package -m manifest.scm".
Otherwise, when I want to give a try for a package, simply "guix
environment --ad-hoc"; time to time with "time-machine" too.

> > Once the story about extension is good enough,
>
> As in adding your own commands under the 'guix $CMD' namespace, or
> something more sophisticated? For the former, git seems to have a quite
> good mechanism - we could borrow that.

From my understanding, Git use shell tricks and cannot be borrowed.
The idea is to have "guix <foo>"; for details, see the recent patch
about extensions by Ricardo and discussion on #guix-hpc (because GWL
is an extension).

All the best,
simon
Simon Tournier Jan. 7, 2021, 5:56 p.m. UTC | #5
Hi Tobias,

On Thu, 7 Jan 2021 at 18:23, Tobias Geerinckx-Rice via Guix-patches
via <guix-patches@gnu.org> wrote:

> How about warning on pull instead?  That should address the ‘halp,
> me Guix be Benjamin Buttonin'’ support calls that I assume
> inspired this.

Wait, does someone install packages in the profile
"~/.config/guix/current"?  And does they install the package 'guix' in
this profile?  Nan...


> Simon, would that reduce the noise in your case?

I am not sure to understand what the problem is; I should miss
something.  My config is simply '~/.config/guix/current/bin' first in
my PATH and I am doing it manually with ~/.bash_profile.  And I have a
lot of profiles that are sourced with a for-loop, some of them contain
the package 'guix'.  And I have never hit an issue but I do not use
'upgrade'.  Howver, I am mainly using Guix on Debian.  Is the issue
Guix System specific?

Well, if the conflict is between the package 'guix' and the 'upgrade'
command, then the warning should be there, IMHO.


All the best,
simon
Maja Kądziołka Jan. 7, 2021, 5:58 p.m. UTC | #6
On Thu Jan 7, 2021 at 6:56 PM CET, zimoun wrote:
> Well, if the conflict is between the package 'guix' and the 'upgrade'
> command, then the warning should be there, IMHO.

The upgrade command is not part of the problem here, the same can be
observed if you use 'guix package -m' instead.

You avoid the problem by putting ~/.config/guix/current/bin first in
your path. I just checked, and that is the default on Guix System.
However, on a foreign distro, ~/.guix-profile/bin will be first in $PATH
unless the user intervenes with manual configuration.

In light of this, perhaps a better solution would be to unify the
profile loading in foreign-Guix's /etc/profile/guix.sh with Guix
System's /etc/profile?

The problem with that is, /etc/profile/guix.sh is only created once by
guix-install.sh, and doesn't get updated... perhaps *this* is something
worth fixing, to start with?

> > How about warning on pull instead?  That should address the ‘halp,
> > me Guix be Benjamin Buttonin'’ support calls that I assume
> > inspired this.
>
> Wait, does someone install packages in the profile
> "~/.config/guix/current"? And does they install the package 'guix' in
> this profile? Nan...

Nah, the scenario is much less convoluted:

1. $PATH contains ~/.guix-profile/bin:~/.config/guix/current/bin, in
that order, as that is currently the default on a foreign distro.

2. User adds the guix package to their ~/.guix-profile. This doesn't
need to happen with 'guix package -i', in fact the IRC conversation that
inspired this [1] (though I seem to recall earlier occurrences of the
same problem) the user was making use of a manifest file. It might even
be easier to encounter this with a manifest file due to its clean-slate
nature.

3. ~/.guix-profile/bin/guix now takes priority. This means
 - guix describe doesn't work
 - guix pull has no effect
 - guix package (-u or -m) will use an older 'guix' package than is
   currently installed, as a packaged guix version doesn't contain the
   package definition of itself, but the one that was previously
   packaged.

I hope this helps resolve your confusion.

Regards,
Jakub Kądziołka

[1]: http://logs.guix.gnu.org/guix/2021-01-01.log#163217
Simon Tournier Jan. 7, 2021, 7:25 p.m. UTC | #7
Hi,

On Thu, 7 Jan 2021 at 19:24, Jakub Kądziołka <kuba@kadziolka.net> wrote:

[...]

> The problem with that is, /etc/profile/guix.sh is only created once by
> guix-install.sh, and doesn't get updated... perhaps *this* is something
> worth fixing, to start with?

[...]

> I hope this helps resolve your confusion.

Thanks for the explanations.  Yes, maybe guix-install.sh and
/etc/profile/guix.sh should be fixed before adding a warning.  Because
somehow, the problem is not to have the package 'guix' but the order
in PATH.

All the best,
simon
Ricardo Wurmus Jan. 7, 2021, 8:31 p.m. UTC | #8
zimoun <zimon.toutoune@gmail.com> writes:

>> > Once the story about extension is good enough,
>>
>> As in adding your own commands under the 'guix $CMD' namespace, or
>> something more sophisticated? For the former, git seems to have a quite
>> good mechanism - we could borrow that.
>
> From my understanding, Git use shell tricks and cannot be borrowed.
> The idea is to have "guix <foo>"; for details, see the recent patch
> about extensions by Ricardo and discussion on #guix-hpc (because GWL
> is an extension).

This is a solved problem as far as I’m concerned.  (It still needs to be
documented in the manual and maybe in the cookbook.)

You need to set GUIX_EXTENSIONS_PATH to a location that contains a Guile
module “(guix extensions my-command)” that defines “my-command” with
“define-command” (so it needs to import (guix scripts)).  The category
should be “extension” (it’s a new category that has been added
recently).

As long as that’s the case Guix will be able to list it in “guix --help”
and execute it.

Here is the entry point of the Guix Workflow Language, which uses this
very mechanism:

    https://git.savannah.gnu.org/cgit/gwl.git/tree/guix/extensions/workflow.scm.in
diff mbox series

Patch

diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index 6faf2adb7a..bc42bd7d84 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -9,6 +9,7 @@ 
 ;;; Copyright © 2019 Tobias Geerinckx-Rice <me@tobias.gr>
 ;;; Copyright © 2020 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2020 Simon Tournier <zimon.toutoune@gmail.com>
+;;; Copyright © 2021 Jakub Kądziołka <kuba@kadziolka.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -863,6 +864,15 @@  processed, #f otherwise."
     (switch-generation . ,switch-generation-action)
     (delete-generations . ,delete-generations-action)))
 
+(define (warn-about-guix-in-profile manifest)
+  "Emit a warning if MANIFEST contains a 'guix' package."
+  (when (manifest-installed? manifest
+          (manifest-pattern
+            (name "guix")))
+    (warning (G_ "Installing the 'guix' package in a user profile is not recommended,
+as it will conflict with the installation managed by 'guix pull'. Consider running
+'guix package -r guix'.\n"))))
+
 (define (process-actions store opts)
   "Process any install/remove/upgrade action from OPTS."
 
@@ -928,6 +938,7 @@  processed, #f otherwise."
                                (manifest-entries manifest)))))
 
       (warn-about-old-distro)
+      (warn-about-guix-in-profile new)
 
       (unless (manifest-transaction-null? trans)
         ;; When '--manifest' is used, display information about TRANS as if we