bug#64858: ~guix shell -C -f guix.scm …~ should not always need ~--rebuild-cache~ option to build the expected environment.

bug#64858: ~guix shell -C -f guix.scm …~ should not always need ~--rebuild-cache~ option to build the expected environment.

[Pierre-Henry Fröhring]

[-- Attachment #1.1: Type: text/plain, Size: 4778 bytes --]

Hello Guix!

As discussed the [[
https://logs.guix.gnu.org/guix/2023-07-21.log#142414][other day]], I'm
providing a more detailed description
(see below) of the unexpected behaviour and an archive containing
enough material to reproduce the bug.

* Experiment
** pkgex-1 -> /gnu/store/0yk3xz85…

The Guix package ~pkgex-1~ is built then its path (~/gnu/store/0yk3xz85…~)
is
shown from within a container (~guix shell -C -f guix.scm ripgrep fd
coreutils
emacs~).

#+begin_example
$ make build # equivalent to: guix build -f guix.scm
…
$ guix shell -C -f guix.scm ripgrep fd coreutils emacs
[env]$ ls -al $EMACSLOADPATH/
total 32
dr-xr-xr-x 2 65534 overflow 4096 Jan  1  1970 .
dr-xr-xr-x 3 65534 overflow 4096 Jan  1  1970 ..
lrwxrwxrwx 1 65534 overflow   90 Jan  1  1970 guix-emacs.el ->
/gnu/store/0ibk105zcyg27i9gifbf3lhgm6n78z66-emacs-28.2/share/emacs/site-lisp/guix-emacs.el
lrwxrwxrwx 1 65534 overflow   91 Jan  1  1970 guix-emacs.elc ->
/gnu/store/0ibk105zcyg27i9gifbf3lhgm6n78z66-emacs-28.2/share/emacs/site-lisp/guix-emacs.elc
lrwxrwxrwx 1 65534 overflow   81 Jan  1  1970 pkgex-1 ->
/gnu/store/0yk3xz85gamig58iska1py6rvn9924ss-pkgex-1/share/emacs/site-lisp/pkgex-1
lrwxrwxrwx 1 65534 overflow   90 Jan  1  1970 site-start.el ->
/gnu/store/0ibk105zcyg27i9gifbf3lhgm6n78z66-emacs-28.2/share/emacs/site-lisp/site-start.el
lrwxrwxrwx 1 65534 overflow   91 Jan  1  1970 site-start.elc ->
/gnu/store/0ibk105zcyg27i9gifbf3lhgm6n78z66-emacs-28.2/share/emacs/site-lisp/site-start.elc
lrwxrwxrwx 1 65534 overflow   90 Jan  1  1970 subdirs.el ->
/gnu/store/75z28mg9fd0v3mjcg3jmrah9ihnziqcb-emacs-subdirs/share/emacs/site-lisp/subdirs.el
#+end_example

** /gnu/store/8k18bghzcijbps8kix3wqp34x4smfc5l-pkgex-1

This very file (~pkgex.el.org~) is updated with this content then the
package is
built again.

#+begin_example
$ make build # equivalent to: guix build -f guix.scm
…
/gnu/store/8k18bghzcijbps8kix3wqp34x4smfc5l-pkgex-1
#+end_example

** pkgex-1 -> /gnu/store/0yk3xz85…

Unexpectedly, the package linked from within the container using the same
command as above is not updated, we observe:

- ~pkgex-1 -> /gnu/store/0yk3xz85…~

instead of:

- ~pkgex-1 -> /gnu/store/8k18bghz…~

#+begin_example
$ guix shell -C -f guix.scm ripgrep fd coreutils emacs
[env]$ ls -al $EMACSLOADPATH/
total 32
dr-xr-xr-x 2 65534 overflow 4096 Jan  1  1970 .
dr-xr-xr-x 3 65534 overflow 4096 Jan  1  1970 ..
lrwxrwxrwx 1 65534 overflow   90 Jan  1  1970 guix-emacs.el ->
/gnu/store/0ibk105zcyg27i9gifbf3lhgm6n78z66-emacs-28.2/share/emacs/site-lisp/guix-emacs.el
lrwxrwxrwx 1 65534 overflow   91 Jan  1  1970 guix-emacs.elc ->
/gnu/store/0ibk105zcyg27i9gifbf3lhgm6n78z66-emacs-28.2/share/emacs/site-lisp/guix-emacs.elc
lrwxrwxrwx 1 65534 overflow   81 Jan  1  1970 pkgex-1 ->
/gnu/store/0yk3xz85gamig58iska1py6rvn9924ss-pkgex-1/share/emacs/site-lisp/pkgex-1
lrwxrwxrwx 1 65534 overflow   90 Jan  1  1970 site-start.el ->
/gnu/store/0ibk105zcyg27i9gifbf3lhgm6n78z66-emacs-28.2/share/emacs/site-lisp/site-start.el
lrwxrwxrwx 1 65534 overflow   91 Jan  1  1970 site-start.elc ->
/gnu/store/0ibk105zcyg27i9gifbf3lhgm6n78z66-emacs-28.2/share/emacs/site-lisp/site-start.elc
lrwxrwxrwx 1 65534 overflow   90 Jan  1  1970 subdirs.el ->
/gnu/store/75z28mg9fd0v3mjcg3jmrah9ihnziqcb-emacs-subdirs/share/emacs/site-lisp/subdirs.el
#+end_example

** pkgex-1 -> /gnu/store/8k18bghz…

Nevertheless, if we build a new environment (because we added the
~tree~ package), then, the newly built package
(~/gnu/store/8k18bghz…~) is taken into account.

#+begin_example
$ guix shell -C -f guix.scm ripgrep fd coreutils emacs tree
…
[env]$ ls -al $EMACSLOADPATH/
total 32
dr-xr-xr-x 2 65534 overflow 4096 Jan  1  1970 .
dr-xr-xr-x 3 65534 overflow 4096 Jan  1  1970 ..
lrwxrwxrwx 1 65534 overflow   90 Jan  1  1970 guix-emacs.el ->
/gnu/store/0ibk105zcyg27i9gifbf3lhgm6n78z66-emacs-28.2/share/emacs/site-lisp/guix-emacs.el
lrwxrwxrwx 1 65534 overflow   91 Jan  1  1970 guix-emacs.elc ->
/gnu/store/0ibk105zcyg27i9gifbf3lhgm6n78z66-emacs-28.2/share/emacs/site-lisp/guix-emacs.elc
lrwxrwxrwx 1 65534 overflow   81 Jan  1  1970 pkgex-1 ->
/gnu/store/8k18bghzcijbps8kix3wqp34x4smfc5l-pkgex-1/share/emacs/site-lisp/pkgex-1
lrwxrwxrwx 1 65534 overflow   90 Jan  1  1970 site-start.el ->
/gnu/store/0ibk105zcyg27i9gifbf3lhgm6n78z66-emacs-28.2/share/emacs/site-lisp/site-start.el
lrwxrwxrwx 1 65534 overflow   91 Jan  1  1970 site-start.elc ->
/gnu/store/0ibk105zcyg27i9gifbf3lhgm6n78z66-emacs-28.2/share/emacs/site-lisp/site-start.elc
lrwxrwxrwx 1 65534 overflow   90 Jan  1  1970 subdirs.el ->
/gnu/store/n7yizf59v4gvjlr66swh3q3kkz3v1vag-emacs-subdirs/share/emacs/site-lisp/subdirs.el
#+end_example

[-- Attachment #1.2: Type: text/html, Size: 5213 bytes --]

[-- Attachment #2: 1-bug.tar.gz --]
[-- Type: application/gzip, Size: 22803 bytes --]

bug#64858: ~guix shell -C -f guix.scm …~ should not always need ~--rebuild-cache~ option to build the expected environment.

[Ludovic Courtès]

Hi Pierre-Henry,

Pierre-Henry Fröhring <phfrohring@deeplinks.com> skribis:

> $ guix shell -C -f guix.scm ripgrep fd coreutils emacs

[...]

> This very file (~pkgex.el.org~) is updated with this content then the
> package is
> built again.
>
> #+begin_example
> $ make build # equivalent to: guix build -f guix.scm
> …
> /gnu/store/8k18bghzcijbps8kix3wqp34x4smfc5l-pkgex-1
> #+end_example
>
>
> ** pkgex-1 -> /gnu/store/0yk3xz85…
>
> Unexpectedly, the package linked from within the container using the same
> command as above is not updated, we observe:

I don’t fully understand the setup, but I can at least explain what you
can expect.

When using ‘-f guix.scm’, ‘guix shell’ caches based on the mtime of
‘guix.scm’: if ‘guix.scm’ is modified, then the cache is invalidated,
otherwise the cache is considered up-to-date and used.

IIUC, you’re modifying a different file, ‘pkgex.el.org’.  ‘guix shell’
does not know about it and thus goes ahead and reuses the previous
value.

I guess the current behavior is good when you’re doing:

  guix shell -D -f guix.scm

which is the primary use case that comes to mind, but it’s inappropriate
when doing:

  guix shell -f guix.scm

in cases where ‘guix.scm’ defines a package with $PWD as its source.

I guess we could maybe try to special-case that in
‘profile-cached-gc-root’.

Ludo’.

bug#64858: [PATCH] In 'guix shell', discard ad-hoc profile when 'guix.scm' file is newer. (Closes: #64858)

[Felix Lechner via Bug reports for GNU Guix]

Fixes a bug that prevents rebuilds for folks who use 'guix shell' repeatedly
to refine a package declaration located in a 'guix.scm' file.

The mtime of 'guix.scm' is never evaluated because 'file' is #f here and the
code path taken always returns 0 as the purported mtime.

  https://github.com/guix-mirror/guix/blob/71b92466430acb8c91841522dc0eb7d766af4388/guix/scripts/shell.scm#L390

In the consuming code, the 'timestamp' is always less than the mtime of the
profile in the multi-line AND-ed conditional here:

  https://github.com/guix-mirror/guix/blob/71b92466430acb8c91841522dc0eb7d766af4388/guix/scripts/shell.scm#L250

As a result, outdated ad-hoc profiles are always reused unless they are
expressly deleted via:

  rm ~/.cache/guix/profiles/*

The bug was potentially introduced in commit c42b7baf when 'package was
transformed to 'ad-hoc-package without also changing the line in this commit.

A minimal reproducer can be found here:

  https://codeberg.org/lechner/shell-bug
---

Hi Ludo'

> ‘guix shell’ caches based on the mtime of ‘guix.scm’

That's how is should work, but it doesn't as explained above. Here is
a patch.

Please adjust the commit message to your liking. Thanks!

Kind regards
Felix

 guix/scripts/shell.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/guix/scripts/shell.scm b/guix/scripts/shell.scm
index 10ea110fee..d29367c8ee 100644
--- a/guix/scripts/shell.scm
+++ b/guix/scripts/shell.scm
@@ -398,7 +398,7 @@ (define (key->file key)
            (values #f #f)))
       ((('nesting? . #t) . rest)
        (loop rest system file (append specs '("nested guix"))))
-      ((('load . ('package candidate)) . rest)
+      ((('load . ('ad-hoc-package candidate)) . rest)
        (if (and (not file) (null? specs))
            (loop rest system candidate specs)
            (values #f #f)))

base-commit: 71b92466430acb8c91841522dc0eb7d766af4388
-- 
2.41.0

bug#64858: ~guix shell -C -f guix.scm …~ should not always need ~--rebuild-cache~ option to build the expected environment.

[Ludovic Courtès]

Hi Felix,

Felix Lechner <felix.lechner@lease-up.com> skribis:

> --- a/guix/scripts/shell.scm
> +++ b/guix/scripts/shell.scm
> @@ -398,7 +398,7 @@ (define (key->file key)
>             (values #f #f)))
>        ((('nesting? . #t) . rest)
>         (loop rest system file (append specs '("nested guix"))))
> -      ((('load . ('package candidate)) . rest)
> +      ((('load . ('ad-hoc-package candidate)) . rest)
>         (if (and (not file) (null? specs))
>             (loop rest system candidate specs)
>             (values #f #f)))

Oooh.  So there were really two bugs:

  1. The one you describe Felix, where ‘guix shell -f guix.scm’ would
     cache things in a nonsensical way (as if you had just run ‘guix
     shell’ with no arguments and no ‘guix.scm’ or ‘manifest.scm’ files
     in $PWD).

  2. The use case issue that I understood from Pierre-Henry’s report,
     which is that ‘guix shell -f guix.scm’ shouldn’t have any caching
     in the first place.

I fixed it with these two commits (the first one is almost what you
proposed, Felix):

  5283d24062 shell: Disable caching for ‘guix shell -f guix.scm’.
  762be40098 shell: Correct cache key for ‘guix shell -f guix.scm’.

It seems to do the right thing now.  Let me know what you think!

Thanks,
Ludo’.

bug#64858: ~guix shell -C -f guix.scm …~ should not always need ~--rebuild-cache~ option to build the expected environment.

[Felix Lechner via Bug reports for GNU Guix]

Hi Ludo'

On Wed, Nov 22 2023, Ludovic Courtès wrote:

> Oooh.  So there were really two bugs:

Sorry I commingled them! I read through many bug report in hope of being
economical.

My hijacking worked out fine, I hope, since Pierre-Henry's report was
resolved at the same time.

I understand Pierre-Henry's use case now.

> It seems to do the right thing now.  Let me know what you think!

Thank you! The fixes look good. I am sure they will help many people.

Personally, I won't be able to test for a while. Updating my system
takes thirty hours because it needs a fixed eudev. The patch is tiny [1]
but not universally accepted as the best solution.

Also, the HEAD in Git doesn't always build. I update every two months.

Thank you for the quick fix!

Kind regards
Felix

[1] https://issues.guix.gnu.org/63508#24

bug#64858: ~guix shell -C -f guix.scm …~ should not always need ~--rebuild-cache~ option to build the expected environment.

[Felix Lechner via Bug reports for GNU Guix]

Hi Ludo'

On Wed, Nov 22 2023, Felix Lechner wrote:

> Personally, I won't be able to test for a while.

I cherry-picked your commits. They work as intended.

My part of this bug was solved. Thank you!

Kind regards
Felix