[PATCH] Automatically set `geiser-guile-load-path' from .dir-locals

[PATCH] Automatically set `geiser-guile-load-path' from .dir-locals

[Maxim Cournoyer]

Hello Guix!

I've been experimenting with the following modification to the
.dir-locals file shipped with Guix, that allows setting per-buffer
variables within Emacs when visiting a file in the same directory (or in
one of its sub-directories).

This makes life a bit easier, by ensuring that a Geiser REPL started
with C-c C-a in a file of either the main 'guix' checkout, a
'guix-core-updates' worktree or a 'guix-staging' worktree will set up
the GUILE_LOAD_PATH and GUILE_LOAD_COMPILED_PATH correctly (via the
`geiser-guile-load-path' Elisp variable).

The only requirement for it to work reliably is that any Guix checkout
or worktree name should start with "guix".

It doesn't require Geiser to be installed (it will just set the above
variable uselessly if it isn't used -- not a big deal).

What do you think?  Is it useful?  Should we include this as part of the
default .dir-locals of Guix?

Patch to follow!

Thanks,

Maxim

Re: [PATCH] Automatically set `geiser-guile-load-path' from .dir-locals

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 340 bytes --]

Hi Maxim!

I believe this would be _very_ useful!  Thank you!

We already have a .dir-locals.el so I see no objection in adding more
goodies in there :)

Could we drop this script in third-party channels?  I assume we would
need to change this "guix" check you mentioned.

Cheers!

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 511 bytes --]

[PATCH] .dir-locals.el: Automatically set the GEISER-GUILE-LOAD-PATH variable.

[Maxim Cournoyer]

* .dir-locals.el: Set the GUIX-DIRECTORY and GEISER-GUILE-LOAD-PATH Emacs
variables based on the location of the .dir-locals file.
---
 .dir-locals.el | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/.dir-locals.el b/.dir-locals.el
index 19f15b3e1a..df5267ab8b 100644
--- a/.dir-locals.el
+++ b/.dir-locals.el
@@ -8,7 +8,31 @@
      ;; For use with 'bug-reference-prog-mode'.
      (bug-reference-url-format . "http://bugs.gnu.org/%s")
      (bug-reference-bug-regexp
-      . "<https?://\\(debbugs\\|bugs\\)\\.gnu\\.org/\\([0-9]+\\)>")))
+      . "<https?://\\(debbugs\\|bugs\\)\\.gnu\\.org/\\([0-9]+\\)>")
+
+     ;; Emacs-Guix
+     (eval . (setq guix-directory
+                   (locate-dominating-file default-directory ".dir-locals.el")))
+
+     ;; Geiser
+     ;; This allows automatically setting the `geiser-guile-load-path'
+     ;; variable when using various Guix checkouts (e.g., via git worktrees).
+     ;; The checkout root directory name should be prefixed by "guix" for it
+     ;; to work correctly.
+     (eval . (let* ((root-dir (expand-file-name
+                               (locate-dominating-file
+                                default-directory ".dir-locals.el")))
+                    ;; Workaround for bug https://issues.guix.gnu.org/43818.
+                    (root-dir* (if (string-suffix-p "/" root-dir)
+                                   (substring root-dir 0 -1)
+                                 root-dir))
+                    (clean-geiser-guile-load-path
+                     (seq-remove (lambda (x)
+                                   (string-match "/guix" x))
+                                 geiser-guile-load-path)))
+               (setq geiser-guile-load-path
+                     (cons root-dir* clean-geiser-guile-load-path))))))
+
  (c-mode          . ((c-file-style . "gnu")))
  (scheme-mode
   .
-- 
2.28.0

Re: [PATCH] .dir-locals.el: Automatically set the GEISER-GUILE-LOAD-PATH variable.

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 602 bytes --]

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> +                    (clean-geiser-guile-load-path
> +                     (seq-remove (lambda (x)
> +                                   (string-match "/guix" x))
> +                                 geiser-guile-load-path)))

I suggest making "/guix" a let-bound variable and add a comment
explaining that channel maintainers need to change it.

That said, why do we need to clean the load path?  Can't we just leave
the other paths?  What if the user actually wants the other elements?

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 511 bytes --]

Re: [PATCH] .dir-locals.el: Automatically set the GEISER-GUILE-LOAD-PATH variable.

[Maxim Cournoyer]

Hello Pierre,

Pierre Neidhardt <mail@ambrevar.xyz> writes:

> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>
>> +                    (clean-geiser-guile-load-path
>> +                     (seq-remove (lambda (x)
>> +                                   (string-match "/guix" x))
>> +                                 geiser-guile-load-path)))
>
> I suggest making "/guix" a let-bound variable and add a comment
> explaining that channel maintainers need to change it.

I'll try the suggestions that Miguel proposed in another reply in this
thread, that should remove the need for this hard-coded value.

> That said, why do we need to clean the load path?  Can't we just leave
> the other paths?  What if the user actually wants the other elements?

I haven't investigated why exactly, but Guile's would get confused when
multiple Guixes were in geiser-guile-load-path and attempt to recompile
everything.

Maxim

Re: [PATCH] .dir-locals.el: Automatically set the GEISER-GUILE-LOAD-PATH variable.

[Miguel Ángel Arruga Vivas]

[-- Attachment #1: Type: text/plain, Size: 2365 bytes --]

Hi!

I think that geiser should use something (project.el, wink wink) to fill
load-path automatically when that's possible.  Nevertheless, I have some
comments for this.

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
> +
> +     ;; Emacs-Guix
> +     (eval . (setq guix-directory
> +                   (locate-dominating-file default-directory ".dir-locals.el")))
> +
> +     ;; Geiser
> +     ;; This allows automatically setting the `geiser-guile-load-path'
> +     ;; variable when using various Guix checkouts (e.g., via git worktrees).
> +     ;; The checkout root directory name should be prefixed by "guix" for it
> +     ;; to work correctly.
> +     (eval . (let* ((root-dir (expand-file-name
> +                               (locate-dominating-file
> +                                default-directory ".dir-locals.el")))
> +                    ;; Workaround for bug https://issues.guix.gnu.org/43818.
> +                    (root-dir* (if (string-suffix-p "/" root-dir)
> +                                   (substring root-dir 0 -1)
> +                                 root-dir))

This is already implemented by directory-file-name.

> +                    (clean-geiser-guile-load-path
> +                     (seq-remove (lambda (x)
> +                                   (string-match "/guix" x))
> +                                 geiser-guile-load-path)))

This fails if geiser-guile-load-path does not exist (void-variable).
The cleanup removes other guixes, isn't it?  I suggest making the
variable buffer-local and forget about hard-coded names. :-)

> +               (setq geiser-guile-load-path
> +                     (cons root-dir* clean-geiser-guile-load-path))))))

This becomes a push with a local variable.  Like this:
-------------------------------8<-------------------------------------
(eval . (setq guix-directory
          (locate-dominating-file default-directory ".dir-locals.el")))
(eval . (when (boundp 'geiser-guile-load-path)
          (make-local-variable 'geiser-guile-load-path)
          (push (directory-file-name
                  (expand-file-name
                    (locate-dominating-file default-directory
                                            ".dir-locals.el")))
                geiser-guile-load-path))
------------------------------->8-------------------------------------

Happy hacking!
Miguel

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 658 bytes --]

Re: [PATCH] .dir-locals.el: Automatically set the GEISER-GUILE-LOAD-PATH variable.

[Maxim Cournoyer]

Hello Miguel!

Miguel Ángel Arruga Vivas <rosen644835@gmail.com> writes:

> Hi!
>
> I think that geiser should use something (project.el, wink wink) to fill
> load-path automatically when that's possible.  Nevertheless, I have some
> comments for this.
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>> +
>> +     ;; Emacs-Guix
>> +     (eval . (setq guix-directory
>> +                   (locate-dominating-file default-directory ".dir-locals.el")))
>> +
>> +     ;; Geiser
>> +     ;; This allows automatically setting the `geiser-guile-load-path'
>> +     ;; variable when using various Guix checkouts (e.g., via git worktrees).
>> +     ;; The checkout root directory name should be prefixed by "guix" for it
>> +     ;; to work correctly.
>> +     (eval . (let* ((root-dir (expand-file-name
>> +                               (locate-dominating-file
>> +                                default-directory ".dir-locals.el")))
>> +                    ;; Workaround for bug https://issues.guix.gnu.org/43818.
>> +                    (root-dir* (if (string-suffix-p "/" root-dir)
>> +                                   (substring root-dir 0 -1)
>> +                                 root-dir))
>
> This is already implemented by directory-file-name.

Neat!

>> +                    (clean-geiser-guile-load-path
>> +                     (seq-remove (lambda (x)
>> +                                   (string-match "/guix" x))
>> +                                 geiser-guile-load-path)))
>
> This fails if geiser-guile-load-path does not exist (void-variable).
> The cleanup removes other guixes, isn't it?  I suggest making the
> variable buffer-local and forget about hard-coded names. :-)

That's a good suggestion!  I toyed with it and it's a bit tricky but I
think the v2 patch I'll send as a follow-up does the trick.  My concern
was also supporting when a user has previously set
geiser-guile-load-path in their .emacs init file, e.g.:

--8<---------------cut here---------------start------------->8---
(setq geiser-guile-load-path (list (expand-file-name "~/src/guix")
                                   (expand-file-name "~/src/shepherd")))
--8<---------------cut here---------------end--------------->8---

That would mean their entries don't get cleaned up (it seems this
doesn't matter in my latest tests with the v2 patch though!).

>> + (setq geiser-guile-load-path + (cons root-dir*
>> clean-geiser-guile-load-path))))))
>
> This becomes a push with a local variable.  Like this:
>
> (eval . (setq guix-directory
>           (locate-dominating-file default-directory ".dir-locals.el")))
> (eval . (when (boundp 'geiser-guile-load-path)

This check makes it so that if geiser-guile-load-path is not already
defined, nothing happens.  It is likely that this is the case, as when
relying on just Geiser's autoloads, it is not loaded.  The user would
have to either set explicitly before hand or call (require
'geiser-guile), which we can't rely on.  But we can drop this check.

>           (make-local-variable 'geiser-guile-load-path) (push
>           (directory-file-name (expand-file-name
>           (locate-dominating-file default-directory
>           ".dir-locals.el"))) geiser-guile-load-path))

I ended up using `cl-pushnew' here instead of push, as otherwise
repeated entries were accumulated.

One thing that worried me was the %load-compiled-path not appearing in
the order defined from guile-geiser-load-path, but in my latest tests as
mentioned above it didn't matter.  Below, the %load-path and
%load-compiled-path variables with this patch, when
geiser-guile-load-path is predefined with '("/home/maxim/src/shepherd"
"/home/maxim/src/guix") from my .emacs file:

;; scheme@(guile-user)> %load-path
;; $2 = ("/home/maxim/src/guix-core-updates"
         "/gnu/store/...-emacs-geiser-0.12/share/geiser/guile/"
         "/home/maxim/src/guix"
         "/home/maxim/src/shepherd" 
         [...])
;; scheme@(guile-user)> %load-compiled-path
;; $3 = ("/home/maxim/src/shepherd"
         "/home/maxim/src/guix"
         "/home/maxim/src/guix-core-updates"
         [...])

Patch v2 incoming.

Thank you,

Maxim

[PATCH v2] .dir-locals.el: Automatically set the GEISER-GUILE-LOAD-PATH variable.

[Maxim Cournoyer]

* .dir-locals.el: Set the GUIX-DIRECTORY and GEISER-GUILE-LOAD-PATH Emacs
variables based on the location of the .dir-locals file.
---
 .dir-locals.el | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/.dir-locals.el b/.dir-locals.el
index 19f15b3e1a..68df95a6d5 100644
--- a/.dir-locals.el
+++ b/.dir-locals.el
@@ -8,7 +8,26 @@
      ;; For use with 'bug-reference-prog-mode'.
      (bug-reference-url-format . "http://bugs.gnu.org/%s")
      (bug-reference-bug-regexp
-      . "<https?://\\(debbugs\\|bugs\\)\\.gnu\\.org/\\([0-9]+\\)>")))
+      . "<https?://\\(debbugs\\|bugs\\)\\.gnu\\.org/\\([0-9]+\\)>")
+
+     ;; Emacs-Guix
+     (eval . (setq guix-directory
+                   (locate-dominating-file default-directory ".dir-locals.el")))
+
+     ;; Geiser
+     ;; This allows automatically setting the `geiser-guile-load-path'
+     ;; variable when using various Guix checkouts (e.g., via git worktrees).
+     (eval . (let* ((root-dir (expand-file-name
+                               (locate-dominating-file
+                                default-directory ".dir-locals.el")))
+                    ;; Workaround for bug https://issues.guix.gnu.org/43818.
+                    (root-dir* (directory-file-name root-dir)))
+               (unless (fboundp 'geiser-guile-load-path)
+                 (defvar geiser-guile-load-path '()))
+               (make-local-variable 'geiser-guile-load-path)
+               (cl-pushnew root-dir* geiser-guile-load-path
+                           :test #'string-equal)))))
+
  (c-mode          . ((c-file-style . "gnu")))
  (scheme-mode
   .
-- 
2.28.0

Re: [PATCH v2] .dir-locals.el: Automatically set the GEISER-GUILE-LOAD-PATH variable.

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 102 bytes --]

I haven't tested but looking good to me otherwise :)

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 511 bytes --]

Re: [PATCH v2] .dir-locals.el: Automatically set the GEISER-GUILE-LOAD-PATH variable.

[Miguel Ángel Arruga Vivas]

[-- Attachment #1: Type: text/plain, Size: 3249 bytes --]

Hello, Maxim!

Thanks for your effort in this, some comments with the quotes for
context.

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>> This fails if geiser-guile-load-path does not exist (void-variable).
>> The cleanup removes other guixes, isn't it?  I suggest making the
>> variable buffer-local and forget about hard-coded names. :-)
>
> That's a good suggestion!  I toyed with it and it's a bit tricky but I
> think the v2 patch I'll send as a follow-up does the trick.  My concern
> was also supporting when a user has previously set
> geiser-guile-load-path in their .emacs init file, e.g.:
>
> (setq geiser-guile-load-path (list (expand-file-name "~/src/guix")
>                                    (expand-file-name "~/src/shepherd")))
>
> That would mean their entries don't get cleaned up (it seems this
> doesn't matter in my latest tests with the v2 patch though!).

That cleanup seems to me responsibility of that .emacs maintainer
instead of something to take into account in .dir-locals. ;-)

>> (eval . (setq guix-directory
>>           (locate-dominating-file default-directory ".dir-locals.el")))
>> (eval . (when (boundp 'geiser-guile-load-path)
>
> This check makes it so that if geiser-guile-load-path is not already
> defined, nothing happens.  It is likely that this is the case, as when
> relying on just Geiser's autoloads, it is not loaded.  The user would
> have to either set explicitly before hand or call (require
> 'geiser-guile), which we can't rely on.  But we can drop this check.

You're right, as you can only bind the keys and enable it when used, not
at file load as I do, great catch. :-)

> One thing that worried me was the %load-compiled-path not appearing in
> the order defined from guile-geiser-load-path, but in my latest tests as
> mentioned above it didn't matter.

With the right directories (meaning no-conflicts between modules) it
shouldn't matter, but it's weird.  Looking into geiser-guile.el the load
path is provided to guile through -L parameters in
geiser-guile--parameters, and the extra path for Geiser code is added
with geiser-guile--set-geiser-load-path (that's why it is not added to
%load-compiled-path)...  I'd have to check Guile to be sure why are the
differences, but they seem harmless.

> +               (unless (fboundp 'geiser-guile-load-path)
> +                 (defvar geiser-guile-load-path '()))

This checks the function definition, not the variable, as Emacs Lisp has
two separate namespaces.

> I ended up using `cl-pushnew' here instead of push, as otherwise
> repeated entries were accumulated.

I wanted to avoid exactly that check, as the variable should end up with
duplicated entries only when you call it twice from the same buffer, how
could that happen?

> +               (make-local-variable 'geiser-guile-load-path)
> +               (cl-pushnew root-dir* geiser-guile-load-path
> +                           :test #'string-equal)))))

If there is some way this may happen, then this call is OK, but I'd try
to stay with a cheaper push unless it's really needed, as O(1) < O(n),
for almost every n. :-)

Again, thank you very much for taking care of this, as it would make
life easier for everybody of us who uses Geiser.

Happy hacking!
Miguel

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 658 bytes --]

Re: [PATCH v2] .dir-locals.el: Automatically set the GEISER-GUILE-LOAD-PATH variable.

[Maxim Cournoyer]

Hello Miguel!

Miguel Ángel Arruga Vivas <rosen644835@gmail.com> writes:

> Hello, Maxim!
>
> Thanks for your effort in this, some comments with the quotes for
> context.

Thanks for your comments, they've already made this proposed change much
better!

> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>>> This fails if geiser-guile-load-path does not exist (void-variable).
>>> The cleanup removes other guixes, isn't it?  I suggest making the
>>> variable buffer-local and forget about hard-coded names. :-)
>>
>> That's a good suggestion!  I toyed with it and it's a bit tricky but I
>> think the v2 patch I'll send as a follow-up does the trick.  My concern
>> was also supporting when a user has previously set
>> geiser-guile-load-path in their .emacs init file, e.g.:
>>
>> (setq geiser-guile-load-path (list (expand-file-name "~/src/guix")
>>                                    (expand-file-name "~/src/shepherd")))
>>
>> That would mean their entries don't get cleaned up (it seems this
>> doesn't matter in my latest tests with the v2 patch though!).
>
> That cleanup seems to me responsibility of that .emacs maintainer
> instead of something to take into account in .dir-locals. ;-)

Indeed, it could be seen that way!  The good news is that it doesn't
seem to cause any problems anyway, should they forget an entry for Guix
there.

>>> (eval . (setq guix-directory
>>>           (locate-dominating-file default-directory ".dir-locals.el")))
>>> (eval . (when (boundp 'geiser-guile-load-path)
>>
>> This check makes it so that if geiser-guile-load-path is not already
>> defined, nothing happens.  It is likely that this is the case, as when
>> relying on just Geiser's autoloads, it is not loaded.  The user would
>> have to either set explicitly before hand or call (require
>> 'geiser-guile), which we can't rely on.  But we can drop this check.
>
> You're right, as you can only bind the keys and enable it when used, not
> at file load as I do, great catch. :-)
>
>> One thing that worried me was the %load-compiled-path not appearing in
>> the order defined from guile-geiser-load-path, but in my latest tests as
>> mentioned above it didn't matter.
>
> With the right directories (meaning no-conflicts between modules) it
> shouldn't matter, but it's weird.  Looking into geiser-guile.el the load
> path is provided to guile through -L parameters in
> geiser-guile--parameters, and the extra path for Geiser code is added
> with geiser-guile--set-geiser-load-path (that's why it is not added to
> %load-compiled-path)...  I'd have to check Guile to be sure why are the
> differences, but they seem harmless.

OK, thank you for investigating!

>> +               (unless (fboundp 'geiser-guile-load-path)
>> +                 (defvar geiser-guile-load-path '()))
>
> This checks the function definition, not the variable, as Emacs Lisp has
> two separate namespaces.

Oops, good catch!

>> I ended up using `cl-pushnew' here instead of push, as otherwise
>> repeated entries were accumulated.
>
> I wanted to avoid exactly that check, as the variable should end up with
> duplicated entries only when you call it twice from the same buffer, how
> could that happen?
>
>> +               (make-local-variable 'geiser-guile-load-path)
>> +               (cl-pushnew root-dir* geiser-guile-load-path
>> +                           :test #'string-equal)))))
>
> If there is some way this may happen, then this call is OK, but I'd try
> to stay with a cheaper push unless it's really needed, as O(1) < O(n),
> for almost every n. :-)

The way I could easily trigger this was to open a dired buffer, and
hitting 'g' to refresh its contents.

> Again, thank you very much for taking care of this, as it would make
> life easier for everybody of us who uses Geiser.

I'm glad to read it! :-)

I'll be sending a v3 with the fboundp woopsie above fixed.

Maxim

Re: [PATCH v2] .dir-locals.el: Automatically set the GEISER-GUILE-LOAD-PATH variable.

[Miguel Ángel Arruga Vivas]

[-- Attachment #1: Type: text/plain, Size: 1451 bytes --]

Hi Maxim!

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
> Thanks for your comments, they've already made this proposed change much
> better!

You did that, they only may have pointed somewhere, but the effort is
yours, so thank you again.

>> That cleanup seems to me responsibility of that .emacs maintainer
>> instead of something to take into account in .dir-locals. ;-)
>
> Indeed, it could be seen that way!  The good news is that it doesn't
> seem to cause any problems anyway, should they forget an entry for Guix
> there.

Cool, one thing less to worry then.

>> If there is some way this may happen, then this call is OK, but I'd try
>> to stay with a cheaper push unless it's really needed, as O(1) < O(n),
>> for almost every n. :-)
>
> The way I could easily trigger this was to open a dired buffer, and
> hitting 'g' to refresh its contents.

That usually is bind to revert-buffer, and they're being loaded again,
so you're right.  Was the definition for other modes intended?  I didn't
noticed because I copied directly the code onto scheme-mode sexp without
looking at the context, what a reviewer... ;-P

> I'll be sending a v3 with the fboundp woopsie above fixed.

I've taken a look to v3 and LGTM.  Even the answer to the question is
no, I wouldn't send another patch only for that hypothetical change.
Anybody can speak up if they have any objection; if they don't, I think
you should push the patch.

Happy hacking!
Miguel

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 658 bytes --]

[PATCH v3] .dir-locals.el: Automatically set the GEISER-GUILE-LOAD-PATH variable.

[Maxim Cournoyer]

* .dir-locals.el: Set the GUIX-DIRECTORY and GEISER-GUILE-LOAD-PATH Emacs
variables based on the location of the .dir-locals file.
---
 .dir-locals.el | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/.dir-locals.el b/.dir-locals.el
index 19f15b3e1a..0496e41ca2 100644
--- a/.dir-locals.el
+++ b/.dir-locals.el
@@ -8,7 +8,26 @@
      ;; For use with 'bug-reference-prog-mode'.
      (bug-reference-url-format . "http://bugs.gnu.org/%s")
      (bug-reference-bug-regexp
-      . "<https?://\\(debbugs\\|bugs\\)\\.gnu\\.org/\\([0-9]+\\)>")))
+      . "<https?://\\(debbugs\\|bugs\\)\\.gnu\\.org/\\([0-9]+\\)>")
+
+     ;; Emacs-Guix
+     (eval . (setq guix-directory
+                   (locate-dominating-file default-directory ".dir-locals.el")))
+
+     ;; Geiser
+     ;; This allows automatically setting the `geiser-guile-load-path'
+     ;; variable when using various Guix checkouts (e.g., via git worktrees).
+     (eval . (let* ((root-dir (expand-file-name
+                               (locate-dominating-file
+                                default-directory ".dir-locals.el")))
+                    ;; Workaround for bug https://issues.guix.gnu.org/43818.
+                    (root-dir* (directory-file-name root-dir)))
+               (unless (boundp 'geiser-guile-load-path)
+                 (defvar geiser-guile-load-path '()))
+               (make-local-variable 'geiser-guile-load-path)
+               (cl-pushnew root-dir* geiser-guile-load-path
+                           :test #'string-equal)))))
+
  (c-mode          . ((c-file-style . "gnu")))
  (scheme-mode
   .
-- 
2.28.0

Re: [PATCH v3] .dir-locals.el: Automatically set the GEISER-GUILE-LOAD-PATH variable.

[Maxim Cournoyer]

Hello,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> * .dir-locals.el: Set the GUIX-DIRECTORY and GEISER-GUILE-LOAD-PATH Emacs
> variables based on the location of the .dir-locals file.
> ---
>  .dir-locals.el | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/.dir-locals.el b/.dir-locals.el
> index 19f15b3e1a..0496e41ca2 100644
> --- a/.dir-locals.el
> +++ b/.dir-locals.el
> @@ -8,7 +8,26 @@
>       ;; For use with 'bug-reference-prog-mode'.
>       (bug-reference-url-format . "http://bugs.gnu.org/%s")
>       (bug-reference-bug-regexp
> -      . "<https?://\\(debbugs\\|bugs\\)\\.gnu\\.org/\\([0-9]+\\)>")))
> +      . "<https?://\\(debbugs\\|bugs\\)\\.gnu\\.org/\\([0-9]+\\)>")
> +
> +     ;; Emacs-Guix
> +     (eval . (setq guix-directory
> +                   (locate-dominating-file default-directory ".dir-locals.el")))
> +
> +     ;; Geiser
> +     ;; This allows automatically setting the `geiser-guile-load-path'
> +     ;; variable when using various Guix checkouts (e.g., via git worktrees).
> +     (eval . (let* ((root-dir (expand-file-name
> +                               (locate-dominating-file
> +                                default-directory ".dir-locals.el")))
> +                    ;; Workaround for bug https://issues.guix.gnu.org/43818.
> +                    (root-dir* (directory-file-name root-dir)))
> +               (unless (boundp 'geiser-guile-load-path)
> +                 (defvar geiser-guile-load-path '()))
> +               (make-local-variable 'geiser-guile-load-path)
> +               (cl-pushnew root-dir* geiser-guile-load-path
> +                           :test #'string-equal)))))
> +
>   (c-mode          . ((c-file-style . "gnu")))
>   (scheme-mode
>    .

Pushed to master as 0e1b0958bd.

Thank you!

Maxim

Re: [PATCH v3] .dir-locals.el: Automatically set the GEISER-GUILE-LOAD-PATH variable.

[Miguel Ángel Arruga Vivas]

[-- Attachment #1: Type: text/plain, Size: 368 bytes --]

Hi,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
> Pushed to master as 0e1b0958bd.

Sorry, this broke etc/indent-code.el and I didn't noticed, my bad.
cl-lib is not loaded there and people noticed in the IRC channel.  I've
pushed 96d0f0b13819a68480e204716c1af6605cfdcb4c to solve this adding
(require 'cl-lib) before the cl-pushnew call.

Best regards,
Miguel

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 658 bytes --]

Re: [PATCH] Automatically set `geiser-guile-load-path' from .dir-locals

[zimoun]

Hi Maxim,

Thank you for working on this!

On Sun, 25 Oct 2020 at 14:08, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:

> The only requirement for it to work reliably is that any Guix checkout
> or worktree name should start with "guix".

I have read all the thread and the comments, neither v2.  What do you
mean by “any Guix checkout or worktree name should start with "guix"”?

Does that mean that the term “guix” should be in the absolute path name,
i.e., “guix/my-worktree-name”?  Or does it mean that the term “guix”
should start the worktree, i.e., “guix-my-worktree-name”?


All the best,
simon

Re: [PATCH] Automatically set `geiser-guile-load-path' from .dir-locals

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 369 bytes --]

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

> Does that mean that the term “guix” should be in the absolute path name,
> i.e., “guix/my-worktree-name”?  Or does it mean that the term “guix”
> should start the worktree, i.e., “guix-my-worktree-name”?

The latter, but this is no longer needed in v2.

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 511 bytes --]

Re: [PATCH] Automatically set `geiser-guile-load-path' from .dir-locals

[Christopher Lemmer Webber]

Maxim Cournoyer writes:

> Hello Guix!
>
> I've been experimenting with the following modification to the
> .dir-locals file shipped with Guix, that allows setting per-buffer
> variables within Emacs when visiting a file in the same directory (or in
> one of its sub-directories).
>
> This makes life a bit easier, by ensuring that a Geiser REPL started
> with C-c C-a in a file of either the main 'guix' checkout, a
> 'guix-core-updates' worktree or a 'guix-staging' worktree will set up
> the GUILE_LOAD_PATH and GUILE_LOAD_COMPILED_PATH correctly (via the
> `geiser-guile-load-path' Elisp variable).
>
> The only requirement for it to work reliably is that any Guix checkout
> or worktree name should start with "guix".
>
> It doesn't require Geiser to be installed (it will just set the above
> variable uselessly if it isn't used -- not a big deal).
>
> What do you think?  Is it useful?  Should we include this as part of the
> default .dir-locals of Guix?
>
> Patch to follow!
>
> Thanks,
>
> Maxim

I'm a bit unsure what the implications fully are with this change, but
here was my user experience:

 - Did a pull using magit to guix
 - Suddenly every file I open in Guix is prompting me if I want to
   enable these dir-locals, I notice some have "eval" and I don't know
   what it's doing so I say no
 - But it's asking me every file
 - And oh no, it's asking me about ten times for every magit operation!
   (Presumably due to the reload operation.)  Fine okay fine, YES, okay
   cool it's quiet now... I hope that was safe.

Later...

 - I'm hacking on another file in another repo
 - C-x v = (check to see a diff of the work-in-progress changes of the
   file I'm working on)
 - Error in the minibuffer!  "Wrong type argument: stringp, nil"
 - wtf, okay M-x toggle-debug-on-error

Debugger entered--Lisp error: (wrong-type-argument stringp nil)
  expand-file-name(nil)
  (let* ((root-dir (expand-file-name (locate-dominating-file default-directory ".dir-locals.el"))) (root-dir* (directory-file-name root-dir))) (unless (boundp 'geiser-guile-load-path) (defvar geiser-guile-load-path 'nil)) (make-local-variable 'geiser-guile-load-path) (require 'cl-lib) (cl-pushnew root-dir* geiser-guile-load-path :test #'string-equal))
  eval((let* ((root-dir (expand-file-name (locate-dominating-file default-directory ".dir-locals.el"))) (root-dir* (directory-file-name root-dir))) (unless (boundp 'geiser-guile-load-path) (defvar geiser-guile-load-path 'nil)) (make-local-variable 'geiser-guile-load-path) (require 'cl-lib) (cl-pushnew root-dir* geiser-guile-load-path :test #'string-equal)))
  hack-one-local-variable(eval (let* ((root-dir (expand-file-name (locate-dominating-file default-directory ".dir-locals.el"))) (root-dir* (directory-file-name root-dir))) (unless (boundp 'geiser-guile-load-path) (defvar geiser-guile-load-path 'nil)) (make-local-variable 'geiser-guile-load-path) (require 'cl-lib) (cl-pushnew root-dir* geiser-guile-load-path :test #'string-equal)))
  hack-local-variables-apply()
  hack-dir-local-variables-non-file-buffer()
  diff-mode()
  vc-diff-internal(t (Git ("/home/cwebber/devel/scribble/scribble-lib/scribble...")) nil nil t)
  vc-diff(nil t)
  funcall-interactively(vc-diff nil t)
  call-interactively(vc-diff nil nil)
  command-execute(vc-diff)
 
 - Oh... it's whatever thing I enabled earlier in the guix repo.  Well
   now my vc-diff is broken outside of there... :\

I'm presuming that if I understood whatever this is doing, it's probably
something that gives me a better user experience if I accept it while
working on Guix.  But a) for whatever reason Emacs' dir-locals stuff is
written in such a way that it antagonizes me for not accepting it and I
didn't know what it eval was (maybe this is a lack of understanding in
how to "say no and get it to listen to me"... I didn't resist for too
long) and b) it seems like this change isn't scoped to Guix's checkout
itself somehow...

Re: [PATCH] Automatically set `geiser-guile-load-path' from .dir-locals

[Christopher Lemmer Webber]

Also, I hope this email isn't interpreted as being dismissive or negative
about what looks like it's probably a real usability improvement for
hacking on Guix!  I just thought my experience indicated maybe there are
additional considerations to address.

Christopher Lemmer Webber writes:

> Maxim Cournoyer writes:
>
>> Hello Guix!
>>
>> I've been experimenting with the following modification to the
>> .dir-locals file shipped with Guix, that allows setting per-buffer
>> variables within Emacs when visiting a file in the same directory (or in
>> one of its sub-directories).
>>
>> This makes life a bit easier, by ensuring that a Geiser REPL started
>> with C-c C-a in a file of either the main 'guix' checkout, a
>> 'guix-core-updates' worktree or a 'guix-staging' worktree will set up
>> the GUILE_LOAD_PATH and GUILE_LOAD_COMPILED_PATH correctly (via the
>> `geiser-guile-load-path' Elisp variable).
>>
>> The only requirement for it to work reliably is that any Guix checkout
>> or worktree name should start with "guix".
>>
>> It doesn't require Geiser to be installed (it will just set the above
>> variable uselessly if it isn't used -- not a big deal).
>>
>> What do you think?  Is it useful?  Should we include this as part of the
>> default .dir-locals of Guix?
>>
>> Patch to follow!
>>
>> Thanks,
>>
>> Maxim
>
> I'm a bit unsure what the implications fully are with this change, but
> here was my user experience:
>
>  - Did a pull using magit to guix
>  - Suddenly every file I open in Guix is prompting me if I want to
>    enable these dir-locals, I notice some have "eval" and I don't know
>    what it's doing so I say no
>  - But it's asking me every file
>  - And oh no, it's asking me about ten times for every magit operation!
>    (Presumably due to the reload operation.)  Fine okay fine, YES, okay
>    cool it's quiet now... I hope that was safe.
>
> Later...
>
>  - I'm hacking on another file in another repo
>  - C-x v = (check to see a diff of the work-in-progress changes of the
>    file I'm working on)
>  - Error in the minibuffer!  "Wrong type argument: stringp, nil"
>  - wtf, okay M-x toggle-debug-on-error
>
> Debugger entered--Lisp error: (wrong-type-argument stringp nil)
>   expand-file-name(nil)
>   (let* ((root-dir (expand-file-name (locate-dominating-file default-directory ".dir-locals.el"))) (root-dir* (directory-file-name root-dir))) (unless (boundp 'geiser-guile-load-path) (defvar geiser-guile-load-path 'nil)) (make-local-variable 'geiser-guile-load-path) (require 'cl-lib) (cl-pushnew root-dir* geiser-guile-load-path :test #'string-equal))
>   eval((let* ((root-dir (expand-file-name (locate-dominating-file default-directory ".dir-locals.el"))) (root-dir* (directory-file-name root-dir))) (unless (boundp 'geiser-guile-load-path) (defvar geiser-guile-load-path 'nil)) (make-local-variable 'geiser-guile-load-path) (require 'cl-lib) (cl-pushnew root-dir* geiser-guile-load-path :test #'string-equal)))
>   hack-one-local-variable(eval (let* ((root-dir (expand-file-name (locate-dominating-file default-directory ".dir-locals.el"))) (root-dir* (directory-file-name root-dir))) (unless (boundp 'geiser-guile-load-path) (defvar geiser-guile-load-path 'nil)) (make-local-variable 'geiser-guile-load-path) (require 'cl-lib) (cl-pushnew root-dir* geiser-guile-load-path :test #'string-equal)))
>   hack-local-variables-apply()
>   hack-dir-local-variables-non-file-buffer()
>   diff-mode()
>   vc-diff-internal(t (Git ("/home/cwebber/devel/scribble/scribble-lib/scribble...")) nil nil t)
>   vc-diff(nil t)
>   funcall-interactively(vc-diff nil t)
>   call-interactively(vc-diff nil nil)
>   command-execute(vc-diff)
>  
>  - Oh... it's whatever thing I enabled earlier in the guix repo.  Well
>    now my vc-diff is broken outside of there... :\
>
> I'm presuming that if I understood whatever this is doing, it's probably
> something that gives me a better user experience if I accept it while
> working on Guix.  But a) for whatever reason Emacs' dir-locals stuff is
> written in such a way that it antagonizes me for not accepting it and I
> didn't know what it eval was (maybe this is a lack of understanding in
> how to "say no and get it to listen to me"... I didn't resist for too
> long) and b) it seems like this change isn't scoped to Guix's checkout
> itself somehow...

Re: [PATCH] Automatically set `geiser-guile-load-path' from .dir-locals

[Miguel Ángel Arruga Vivas]

[-- Attachment #1: Type: text/plain, Size: 5153 bytes --]

Hi Christopher,

First of all I want to say sorry: I've tested this and reviewed the
patch, and this is the second issue that already has caused, so yes, my
tests weren't enough at all.

Christopher Lemmer Webber <cwebber@dustycloud.org> writes:

> Also, I hope this email isn't interpreted as being dismissive or negative
> about what looks like it's probably a real usability improvement for
> hacking on Guix!  I just thought my experience indicated maybe there are
> additional considerations to address.

At least from my side I see your report as something positive, I cannot
see how could it be negative at all, and I'd like to thank you for it.

> Christopher Lemmer Webber writes:
>> I'm a bit unsure what the implications fully are with this change, but
>> here was my user experience:
>>
>>  - Did a pull using magit to guix

I use magit too, so I guess this isn't the source of the error.

>>  - Suddenly every file I open in Guix is prompting me if I want to
>>    enable these dir-locals, I notice some have "eval" and I don't know
>>    what it's doing so I say no

Saying no here shouldn't be a problem, as the variables should always be
optional.

>>  - But it's asking me every file

If every file means "every file on guix project" that should be the
normal behavior of Emacs for .dir-locals.el since this file was
introduced long before the patch: you can mark the 'eval' lines to be
accepted, or to be rejected always, but they're loaded with each file.
A problem could be that the UI shows the ones you have already accepted
too, and simply marks with * the new ones.

If it means "every other file too", I'm unable to reproduce that with a
fresh Emacs session.

>>  - And oh no, it's asking me about ten times for every magit operation!
>>    (Presumably due to the reload operation.)  Fine okay fine, YES, okay
>>    cool it's quiet now... I hope that was safe.

The only effects of the new code should be:

  * First eval: Set guix-directory for emacs-guix to the folder where
    .dir-locals.el is located.  This affects the whole emacs-guix, but
    AFAIU this isn't your issue.

  * Second eval:
    - Make geiser-guile-load-path buffer-local, and optionally define it
      as empty if it was void.
    - Add the directory where .dir-locals.el is located to
      geiser-guile-load-path.

>> Later...
>>
>>  - I'm hacking on another file in another repo
>>  - C-x v = (check to see a diff of the work-in-progress changes of the
>>    file I'm working on)

Thanks, with that I reproduce the problem, but I cannot debug it right
now.  I'll be available in some hours, as soon as I have anything I'll
update about this.

>>  - Error in the minibuffer!  "Wrong type argument: stringp, nil"
>>  - wtf, okay M-x toggle-debug-on-error
>>
>> Debugger entered--Lisp error: (wrong-type-argument stringp nil)
>>   expand-file-name(nil)
>>   (let* ((root-dir (expand-file-name (locate-dominating-file default-directory ".dir-locals.el"))) (root-dir* (directory-file-name root-dir))) (unless (boundp 'geiser-guile-load-path) (defvar geiser-guile-load-path 'nil)) (make-local-variable 'geiser-guile-load-path) (require 'cl-lib) (cl-pushnew root-dir* geiser-guile-load-path :test #'string-equal))
>>   eval((let* ((root-dir (expand-file-name (locate-dominating-file default-directory ".dir-locals.el"))) (root-dir* (directory-file-name root-dir))) (unless (boundp 'geiser-guile-load-path) (defvar geiser-guile-load-path 'nil)) (make-local-variable 'geiser-guile-load-path) (require 'cl-lib) (cl-pushnew root-dir* geiser-guile-load-path :test #'string-equal)))
>>   hack-one-local-variable(eval (let* ((root-dir (expand-file-name (locate-dominating-file default-directory ".dir-locals.el"))) (root-dir* (directory-file-name root-dir))) (unless (boundp 'geiser-guile-load-path) (defvar geiser-guile-load-path 'nil)) (make-local-variable 'geiser-guile-load-path) (require 'cl-lib) (cl-pushnew root-dir* geiser-guile-load-path :test #'string-equal)))
>>   hack-local-variables-apply()
>>   hack-dir-local-variables-non-file-buffer()
>>   diff-mode()
>>   vc-diff-internal(t (Git ("/home/cwebber/devel/scribble/scribble-lib/scribble...")) nil nil t)
>>   vc-diff(nil t)
>>   funcall-interactively(vc-diff nil t)
>>   call-interactively(vc-diff nil nil)
>>   command-execute(vc-diff)
>>  
>>  - Oh... it's whatever thing I enabled earlier in the guix repo.  Well
>>    now my vc-diff is broken outside of there... :\
>>
>> I'm presuming that if I understood whatever this is doing, it's probably
>> something that gives me a better user experience if I accept it while
>> working on Guix.  But a) for whatever reason Emacs' dir-locals stuff is
>> written in such a way that it antagonizes me for not accepting it and I
>> didn't know what it eval was (maybe this is a lack of understanding in
>> how to "say no and get it to listen to me"... I didn't resist for too
>> long) and b) it seems like this change isn't scoped to Guix's checkout
>> itself somehow...

Sorry again, as soon as I have a bit of time I'll update.

Happy hacking!
Miguel

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 658 bytes --]

Re: [PATCH] Automatically set `geiser-guile-load-path' from .dir-locals

[Christopher Lemmer Webber]

Miguel Ángel Arruga Vivas writes:

> Hi Christopher,
>
> First of all I want to say sorry: I've tested this and reviewed the
> patch, and this is the second issue that already has caused, so yes, my
> tests weren't enough at all.
>
> Christopher Lemmer Webber <cwebber@dustycloud.org> writes:
>
>> Also, I hope this email isn't interpreted as being dismissive or negative
>> about what looks like it's probably a real usability improvement for
>> hacking on Guix!  I just thought my experience indicated maybe there are
>> additional considerations to address.
>
> At least from my side I see your report as something positive, I cannot
> see how could it be negative at all, and I'd like to thank you for it.

Thanks.  I don't have time to fully respond inline to your comments
right now, but I appreciate the emails and work of both you and Maxim.
Thank you both, and glad we haven't had a miscommunication where we
mistakenly thought we were at odds. :)

Re: [PATCH] Automatically set `geiser-guile-load-path' from .dir-locals

[Christopher Lemmer Webber]

Miguel Ángel Arruga Vivas writes:

> Hi Christopher,
>
> First of all I want to say sorry: I've tested this and reviewed the
> patch, and this is the second issue that already has caused, so yes, my
> tests weren't enough at all.
>
> Christopher Lemmer Webber <cwebber@dustycloud.org> writes:
>
>> Also, I hope this email isn't interpreted as being dismissive or negative
>> about what looks like it's probably a real usability improvement for
>> hacking on Guix!  I just thought my experience indicated maybe there are
>> additional considerations to address.
>
> At least from my side I see your report as something positive, I cannot
> see how could it be negative at all, and I'd like to thank you for it.
>
>> Christopher Lemmer Webber writes:
>>> I'm a bit unsure what the implications fully are with this change, but
>>> here was my user experience:
>>>
>>>  - Did a pull using magit to guix
>
> I use magit too, so I guess this isn't the source of the error.
>
>>>  - Suddenly every file I open in Guix is prompting me if I want to
>>>    enable these dir-locals, I notice some have "eval" and I don't know
>>>    what it's doing so I say no
>
> Saying no here shouldn't be a problem, as the variables should always be
> optional.
>
>>>  - But it's asking me every file
>
> If every file means "every file on guix project" that should be the
> normal behavior of Emacs for .dir-locals.el since this file was
> introduced long before the patch: you can mark the 'eval' lines to be
> accepted, or to be rejected always, but they're loaded with each file.
> A problem could be that the UI shows the ones you have already accepted
> too, and simply marks with * the new ones.
>
> If it means "every other file too", I'm unable to reproduce that with a
> fresh Emacs session.
>
>>>  - And oh no, it's asking me about ten times for every magit operation!
>>>    (Presumably due to the reload operation.)  Fine okay fine, YES, okay
>>>    cool it's quiet now... I hope that was safe.
>
> The only effects of the new code should be:
>
>   * First eval: Set guix-directory for emacs-guix to the folder where
>     .dir-locals.el is located.  This affects the whole emacs-guix, but
>     AFAIU this isn't your issue.
>
>   * Second eval:
>     - Make geiser-guile-load-path buffer-local, and optionally define it
>       as empty if it was void.
>     - Add the directory where .dir-locals.el is located to
>       geiser-guile-load-path.
>
>>> Later...
>>>
>>>  - I'm hacking on another file in another repo
>>>  - C-x v = (check to see a diff of the work-in-progress changes of the
>>>    file I'm working on)
>
> Thanks, with that I reproduce the problem, but I cannot debug it right
> now.  I'll be available in some hours, as soon as I have anything I'll
> update about this.
>
>>>  - Error in the minibuffer!  "Wrong type argument: stringp, nil"
>>>  - wtf, okay M-x toggle-debug-on-error
>>>
>>> Debugger entered--Lisp error: (wrong-type-argument stringp nil)
>>>   expand-file-name(nil)
>>>   (let* ((root-dir (expand-file-name (locate-dominating-file default-directory ".dir-locals.el"))) (root-dir* (directory-file-name root-dir))) (unless (boundp 'geiser-guile-load-path) (defvar geiser-guile-load-path 'nil)) (make-local-variable 'geiser-guile-load-path) (require 'cl-lib) (cl-pushnew root-dir* geiser-guile-load-path :test #'string-equal))
>>>   eval((let* ((root-dir (expand-file-name (locate-dominating-file default-directory ".dir-locals.el"))) (root-dir* (directory-file-name root-dir))) (unless (boundp 'geiser-guile-load-path) (defvar geiser-guile-load-path 'nil)) (make-local-variable 'geiser-guile-load-path) (require 'cl-lib) (cl-pushnew root-dir* geiser-guile-load-path :test #'string-equal)))
>>>   hack-one-local-variable(eval (let* ((root-dir (expand-file-name (locate-dominating-file default-directory ".dir-locals.el"))) (root-dir* (directory-file-name root-dir))) (unless (boundp 'geiser-guile-load-path) (defvar geiser-guile-load-path 'nil)) (make-local-variable 'geiser-guile-load-path) (require 'cl-lib) (cl-pushnew root-dir* geiser-guile-load-path :test #'string-equal)))
>>>   hack-local-variables-apply()
>>>   hack-dir-local-variables-non-file-buffer()
>>>   diff-mode()
>>>   vc-diff-internal(t (Git ("/home/cwebber/devel/scribble/scribble-lib/scribble...")) nil nil t)
>>>   vc-diff(nil t)
>>>   funcall-interactively(vc-diff nil t)
>>>   call-interactively(vc-diff nil nil)
>>>   command-execute(vc-diff)
>>>  
>>>  - Oh... it's whatever thing I enabled earlier in the guix repo.  Well
>>>    now my vc-diff is broken outside of there... :\
>>>
>>> I'm presuming that if I understood whatever this is doing, it's probably
>>> something that gives me a better user experience if I accept it while
>>> working on Guix.  But a) for whatever reason Emacs' dir-locals stuff is
>>> written in such a way that it antagonizes me for not accepting it and I
>>> didn't know what it eval was (maybe this is a lack of understanding in
>>> how to "say no and get it to listen to me"... I didn't resist for too
>>> long) and b) it seems like this change isn't scoped to Guix's checkout
>>> itself somehow...
>
> Sorry again, as soon as I have a bit of time I'll update.
>
> Happy hacking!
> Miguel

I figured out what was happening!  The bug is *technically* in vc-mode.
However, nontheless it manifested here...

Here's what happened.  vc-mode has some various hacks, as you can see
above with "hack-local-variables-apply"... which traverses the dirlocals
stuff.  (Not sure what the purpose is, didn't look too long.)

However for whatever reason, vc-mode also seems to be reusing buffers
such as `*vc-diff*'... and somehow still is left in the directory
context it *first* was used in.

Thus if I C-x v = in a guix-oriented buffer first, and then switch to
another completely different project and do the same, it's loading the
dirlocals from Guix(...!!!!) 

This is clearly a bug in vc-mode, I'll try to report it as such.

In the meanwhile, I used this hacky "fix".  Maybe worth applying for the
moment... what do you think of it?

#+BEGIN_SRC diff
diff --git a/.dir-locals.el b/.dir-locals.el
index 8e5d3902e3..2aa446a4f6 100644
--- a/.dir-locals.el
+++ b/.dir-locals.el
@@ -17,17 +17,19 @@
      ;; Geiser
      ;; This allows automatically setting the `geiser-guile-load-path'
      ;; variable when using various Guix checkouts (e.g., via git worktrees).
-     (eval . (let* ((root-dir (expand-file-name
-                               (locate-dominating-file
-                                default-directory ".dir-locals.el")))
-                    ;; Workaround for bug https://issues.guix.gnu.org/43818.
-                    (root-dir* (directory-file-name root-dir)))
-               (unless (boundp 'geiser-guile-load-path)
-                 (defvar geiser-guile-load-path '()))
-               (make-local-variable 'geiser-guile-load-path)
-               (require 'cl-lib)
-               (cl-pushnew root-dir* geiser-guile-load-path
-                           :test #'string-equal)))))
+     (eval . (let ((root-dir-unexpanded (locate-dominating-file
+                                         default-directory ".dir-locals.el")))
+               (when root-dir-unexpanded
+                 (let* ((root-dir (expand-file-name root-dir-unexpanded))
+                        ;; Workaround for bug https://issues.guix.gnu.org/43818.
+                        (root-dir* (directory-file-name root-dir)))
+                   
+                   (unless (boundp 'geiser-guile-load-path)
+                     (defvar geiser-guile-load-path '()))
+                   (make-local-variable 'geiser-guile-load-path)
+                   (require 'cl-lib)
+                   (cl-pushnew root-dir* geiser-guile-load-path
+                               :test #'string-equal)))))))
 
  (c-mode          . ((c-file-style . "gnu")))
  (scheme-mode
#+END_SRC

Btw, I'm not very familiar with dirlocals.  I see that setq is used in
the previous definition.  Woudl setq-local be better... or does it do
that by default?

Re: [PATCH] Automatically set `geiser-guile-load-path' from .dir-locals

[Maxim Cournoyer]

Hello Christopher,

CC'ing Ludovic since they had that problem as well, so might be
interested in testing your solution.

Christopher Lemmer Webber <cwebber@dustycloud.org> writes:

[...]

> I figured out what was happening!  The bug is *technically* in vc-mode.
> However, nontheless it manifested here...
>
> Here's what happened.  vc-mode has some various hacks, as you can see
> above with "hack-local-variables-apply"... which traverses the dirlocals
> stuff.  (Not sure what the purpose is, didn't look too long.)
>
> However for whatever reason, vc-mode also seems to be reusing buffers
> such as `*vc-diff*'... and somehow still is left in the directory
> context it *first* was used in.
>
> Thus if I C-x v = in a guix-oriented buffer first, and then switch to
> another completely different project and do the same, it's loading the
> dirlocals from Guix(...!!!!)
>
> This is clearly a bug in vc-mode, I'll try to report it as such.

Thank you for the investigation.  I'd be really happy if you could
report the problem upstream (M-x report-emacs-bug) and link to it here!
Be sure to include a very detailed description of how to reproduce; I
for one still am unable to reproduce with the above instructions.  I've
tried many ways, using a pure environment, not loading my Emacs init
file, etc. based on what Miguel had written in IRC at the time of the
initial report, but to no avail.

> In the meanwhile, I used this hacky "fix".  Maybe worth applying for the
> moment... what do you think of it?

I'd like to have the upstream bug linked in that fix rather than the
Guix one; that way it'll be possible to track upstream resolution and
know when the workaround can be removed.

> #+BEGIN_SRC diff
> diff --git a/.dir-locals.el b/.dir-locals.el
> index 8e5d3902e3..2aa446a4f6 100644
> --- a/.dir-locals.el
> +++ b/.dir-locals.el
> @@ -17,17 +17,19 @@
>       ;; Geiser
>       ;; This allows automatically setting the `geiser-guile-load-path'
>       ;; variable when using various Guix checkouts (e.g., via git worktrees).
> -     (eval . (let* ((root-dir (expand-file-name
> -                               (locate-dominating-file
> -                                default-directory ".dir-locals.el")))
> -                    ;; Workaround for bug https://issues.guix.gnu.org/43818.
> -                    (root-dir* (directory-file-name root-dir)))
> -               (unless (boundp 'geiser-guile-load-path)
> -                 (defvar geiser-guile-load-path '()))
> -               (make-local-variable 'geiser-guile-load-path)
> -               (require 'cl-lib)
> -               (cl-pushnew root-dir* geiser-guile-load-path
> -                           :test #'string-equal)))))
> +     (eval . (let ((root-dir-unexpanded (locate-dominating-file
> +                                         default-directory ".dir-locals.el")))
> +               (when root-dir-unexpanded
> +                 (let* ((root-dir (expand-file-name root-dir-unexpanded))
> +                        ;; Workaround for bug https://issues.guix.gnu.org/43818.
> +                        (root-dir* (directory-file-name root-dir)))
> +
> +                   (unless (boundp 'geiser-guile-load-path)
> +                     (defvar geiser-guile-load-path '()))
> +                   (make-local-variable 'geiser-guile-load-path)
> +                   (require 'cl-lib)
> +                   (cl-pushnew root-dir* geiser-guile-load-path
> +                               :test #'string-equal)))))))
>
>   (c-mode          . ((c-file-style . "gnu")))
>   (scheme-mode
> #+END_SRC
>
> Btw, I'm not very familiar with dirlocals.  I see that setq is used in
> the previous definition.  Woudl setq-local be better... or does it do
> that by default?

Good observation.  The `guix-directory' is set globally.  It could be
neater if we made it also a buffer-local variable like the
`geiser-guile-load-path' below.

Thank you for having persevered in understanding the root cause of the
issue you had, and sharing the details here.

Maxim

Re: [PATCH] Automatically set `geiser-guile-load-path' from .dir-locals

[Miguel Ángel Arruga Vivas]

[-- Attachment #1: Type: text/plain, Size: 2742 bytes --]

Hi Maxim and Christopher,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>[...]
> Christopher Lemmer Webber <cwebber@dustycloud.org> writes:
> [...]
>> I figured out what was happening!  The bug is *technically* in vc-mode.
>> However, nontheless it manifested here...
>>
>> Here's what happened.  vc-mode has some various hacks, as you can see
>> above with "hack-local-variables-apply"... which traverses the dirlocals
>> stuff.  (Not sure what the purpose is, didn't look too long.)

The file where these functions are located is lisp/files.el, right on
the Emacs core.  Some modes add hooks there, like flyspell or cc, but
not vc, so I don't really think the problem is exclusive from that mode
after some debugging.

This code ends up in file-local-variables-alist, even though
dir-local-variables-alist contains the correct values for some reason I
still don't really understand.

>> However for whatever reason, vc-mode also seems to be reusing buffers
>> such as `*vc-diff*'... and somehow still is left in the directory
>> context it *first* was used in.

This may be the culprit but I think it isn't the issue, as
file-local-variables-alist accumulates every eval marked as nil, not
only the first/last one... when it fails, as we've seen.

>> Thus if I C-x v = in a guix-oriented buffer first, and then switch to
>> another completely different project and do the same, it's loading the
>> dirlocals from Guix(...!!!!)
>>
>> This is clearly a bug in vc-mode, I'll try to report it as such.

> Thank you for the investigation.  I'd be really happy if you could
> report the problem upstream (M-x report-emacs-bug) aznd link to it here!

I haven't reported it yet, but as you can see I have a reproducer script
attached.  I haven't seen anything in vc-code that points in that
direction, surely though Emacs people will have a better understanding.

Christopher, would you mind to CC me if you open the bug?  I can do it
too if you tell me to, but I don't want to create a duplicate entry if
we do it roughly at the same time.

> [..] Miguel had written in IRC at the time of the initial report, but
> to no avail.

Maxim, could you test the script to check if we can narrow the cases?
It shows the README in the emacs it opens, so it should be
straight-forward.

>> In the meanwhile, I used this hacky "fix".  Maybe worth applying for the
>> moment... what do you think of it?
>
> I'd like to have the upstream bug linked in that fix rather than the
> Guix one; that way it'll be possible to track upstream resolution and
> know when the workaround can be removed.

Apart from the tracking reference, I agree that it's worth applying it.
And also, thank you both for making easier to work on guix. :-)

Happy hacking!
Miguel


[-- Attachment #2: reproducer --]
[-- Type: application/x-sh, Size: 801 bytes --]

Re: [PATCH] Automatically set `geiser-guile-load-path' from .dir-locals

[Christopher Lemmer Webber]

Miguel Ángel Arruga Vivas writes:

> Hi Maxim and Christopher,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>>[...]
>> Christopher Lemmer Webber <cwebber@dustycloud.org> writes:
>> [...]
>>> I figured out what was happening!  The bug is *technically* in vc-mode.
>>> However, nontheless it manifested here...
>>>
>>> Here's what happened.  vc-mode has some various hacks, as you can see
>>> above with "hack-local-variables-apply"... which traverses the dirlocals
>>> stuff.  (Not sure what the purpose is, didn't look too long.)
>
> The file where these functions are located is lisp/files.el, right on
> the Emacs core.  Some modes add hooks there, like flyspell or cc, but
> not vc, so I don't really think the problem is exclusive from that mode
> after some debugging.
>
> This code ends up in file-local-variables-alist, even though
> dir-local-variables-alist contains the correct values for some reason I
> still don't really understand.
>
>>> However for whatever reason, vc-mode also seems to be reusing buffers
>>> such as `*vc-diff*'... and somehow still is left in the directory
>>> context it *first* was used in.
>
> This may be the culprit but I think it isn't the issue, as
> file-local-variables-alist accumulates every eval marked as nil, not
> only the first/last one... when it fails, as we've seen.
>
>>> Thus if I C-x v = in a guix-oriented buffer first, and then switch to
>>> another completely different project and do the same, it's loading the
>>> dirlocals from Guix(...!!!!)
>>>
>>> This is clearly a bug in vc-mode, I'll try to report it as such.
>
>> Thank you for the investigation.  I'd be really happy if you could
>> report the problem upstream (M-x report-emacs-bug) aznd link to it here!
>
> I haven't reported it yet, but as you can see I have a reproducer script
> attached.  I haven't seen anything in vc-code that points in that
> direction, surely though Emacs people will have a better understanding.
>
> Christopher, would you mind to CC me if you open the bug?  I can do it
> too if you tell me to, but I don't want to create a duplicate entry if
> we do it roughly at the same time.

It sounds like you're already putting together the work to do it.  If
you don't mind doing it that would save me a lot of time right
now... I'm quite swamped!  I'd be very grateful!

>> [..] Miguel had written in IRC at the time of the initial report, but
>> to no avail.
>
> Maxim, could you test the script to check if we can narrow the cases?
> It shows the README in the emacs it opens, so it should be
> straight-forward.
>
>>> In the meanwhile, I used this hacky "fix".  Maybe worth applying for the
>>> moment... what do you think of it?
>>
>> I'd like to have the upstream bug linked in that fix rather than the
>> Guix one; that way it'll be possible to track upstream resolution and
>> know when the workaround can be removed.
>
> Apart from the tracking reference, I agree that it's worth applying it.
> And also, thank you both for making easier to work on guix. :-)

Okay cool; since you two have already basically reviewed this code I'll
just make the suggested change and push it to the master branch.  Thank
you!

Re: [PATCH] Automatically set `geiser-guile-load-path' from .dir-locals

[Christopher Lemmer Webber]

Christopher Lemmer Webber writes:

> Miguel Ángel Arruga Vivas writes:
>
>> I haven't reported it yet, but as you can see I have a reproducer script
>> attached.  I haven't seen anything in vc-code that points in that
>> direction, surely though Emacs people will have a better understanding.
>>
>> Christopher, would you mind to CC me if you open the bug?  I can do it
>> too if you tell me to, but I don't want to create a duplicate entry if
>> we do it roughly at the same time.
>
> It sounds like you're already putting together the work to do it.  If
> you don't mind doing it that would save me a lot of time right
> now... I'm quite swamped!  I'd be very grateful!
>
>> Apart from the tracking reference, I agree that it's worth applying it.
>> And also, thank you both for making easier to work on guix. :-)
>
> Okay cool; since you two have already basically reviewed this code I'll
> just make the suggested change and push it to the master branch.  Thank
> you!

I've pushed the fix to master.  I also did the setq-local thing as
another commit.

However, since Miguel is the one submitting the upstream report, I left
a TODO item to link to that once done.  Miguel, do you mind committing
that once done?

Re: [PATCH] Automatically set `geiser-guile-load-path' from .dir-locals

[Miguel Ángel Arruga Vivas]

[-- Attachment #1: Type: text/plain, Size: 775 bytes --]

Hi!

Christopher Lemmer Webber <cwebber@dustycloud.org> writes:
[...]
> It sounds like you're already putting together the work to do it.  If
> you don't mind doing it that would save me a lot of time right
> now... I'm quite swamped!  I'd be very grateful!

No problem, I just didn't want to end up with two reports for the same
issue.  It's already open as <https://bugs.gnu.org/44698>.

>>[...]
> I've pushed the fix to master.  I also did the setq-local thing as
> another commit.

Thanks.

> However, since Miguel is the one submitting the upstream report, I left
> a TODO item to link to that once done.  Miguel, do you mind committing
> that once done?

Pushed the update for the TODO line to master as
3428c66c5a4d5f1a5fd2f1ad4cd3105993ae8e6d.

Happy hacking!
Miguel

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 658 bytes --]

Re: [PATCH] Automatically set `geiser-guile-load-path' from .dir-locals

[Christopher Lemmer Webber]

Miguel Ángel Arruga Vivas writes:

> Hi!
>
> Christopher Lemmer Webber <cwebber@dustycloud.org> writes:
> [...]
>> It sounds like you're already putting together the work to do it.  If
>> you don't mind doing it that would save me a lot of time right
>> now... I'm quite swamped!  I'd be very grateful!
>
> No problem, I just didn't want to end up with two reports for the same
> issue.  It's already open as <https://bugs.gnu.org/44698>.
>
>>>[...]
>> I've pushed the fix to master.  I also did the setq-local thing as
>> another commit.
>
> Thanks.
>
>> However, since Miguel is the one submitting the upstream report, I left
>> a TODO item to link to that once done.  Miguel, do you mind committing
>> that once done?
>
> Pushed the update for the TODO line to master as
> 3428c66c5a4d5f1a5fd2f1ad4cd3105993ae8e6d.

Great, thank you! :)

Re: [PATCH] Automatically set `geiser-guile-load-path' from .dir-locals

[Maxim Cournoyer]

Hello Miguel,

Miguel Ángel Arruga Vivas <rosen644835@gmail.com> writes:

> Hi!
>
> Christopher Lemmer Webber <cwebber@dustycloud.org> writes:
> [...]
>> It sounds like you're already putting together the work to do it.  If
>> you don't mind doing it that would save me a lot of time right
>> now... I'm quite swamped!  I'd be very grateful!
>
> No problem, I just didn't want to end up with two reports for the same
> issue.  It's already open as <https://bugs.gnu.org/44698>.

Woo!  That was quick!

I've tested the repro script and could reproduce for the first time (the
steps must be followed scrupulously otherwise just changing buffer can
cause the bad behavior to disappear)!

Thank you for the efficiency with which you handled the issue, it's much
appreciated.

Maxim

Re: [PATCH] Automatically set `geiser-guile-load-path' from .dir-locals

[Miguel Ángel Arruga Vivas]

Hello Maxim,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Hello Miguel,
>
> Miguel Ángel Arruga Vivas <rosen644835@gmail.com> writes:
>
>> Hi!
>>
>> Christopher Lemmer Webber <cwebber@dustycloud.org> writes:
>> [...]
>>> It sounds like you're already putting together the work to do it.  If
>>> you don't mind doing it that would save me a lot of time right
>>> now... I'm quite swamped!  I'd be very grateful!
>>
>> No problem, I just didn't want to end up with two reports for the same
>> issue.  It's already open as <https://bugs.gnu.org/44698>.
>
> Woo!  That was quick!

Well, I wouldn't call it quick, as I wanted to pin-point at least the
issue, but it took me a long time to even trim the reproducer.
Christopher mail really was a heads up to at least report the bug to
Emacs people.

> I've tested the repro script and could reproduce for the first time (the
> steps must be followed scrupulously otherwise just changing buffer can
> cause the bad behavior to disappear)!

Thank you for the confirmation.

> Thank you for the efficiency with which you handled the issue, it's much
> appreciated.

You're welcome.  Nonetheless, I wish I already had a solution... :-(

Happy hacking!
Miguel

Re: [PATCH] Automatically set `geiser-guile-load-path' from .dir-locals

[Maxim Cournoyer]

Hi Christopher,

Christopher Lemmer Webber <cwebber@dustycloud.org> writes:

[...]

> I'm a bit unsure what the implications fully are with this change, but
> here was my user experience:
>
>  - Did a pull using magit to guix
>  - Suddenly every file I open in Guix is prompting me if I want to
>    enable these dir-locals, I notice some have "eval" and I don't know
>    what it's doing so I say no
>  - But it's asking me every file
>  - And oh no, it's asking me about ten times for every magit operation!
>    (Presumably due to the reload operation.)  Fine okay fine, YES, okay
>    cool it's quiet now... I hope that was safe.

Thanks for the feedback (even though it's not a great experience you've
had!).  If I understand correctly when prompted like this following the
change to .dir-locals.el:

--8<---------------cut here---------------start------------->8---
The local variables list in /home/maxim/src/guix/
contains values that may not be safe (*).

Do you want to apply it?  You can type
y  -- to apply the local variables list.
n  -- to ignore the local variables list.
!  -- to apply the local variables list, and permanently mark these
      values (*) as safe (in the future, they will be set automatically.)

    fill-column : 78
    tab-width : 8
    sentence-end-double-space : t
    bug-reference-url-format : "http://bugs.gnu.org/%s"
    bug-reference-bug-regexp : "<https?://\\(debbugs\\|bugs\\)\\.gnu\\.org/\\([0-9]+\\)>"
  * eval : (setq guix-directory (locate-dominating-file default-directory ".dir-locals.el"))
  * eval : (let* ((root-dir (expand-file-name (locate-dominating-file default-directory ".dir-locals.el"))) (root-dir* (directory-file-name root-dir))) (unless (boundp 'geiser-guile-load-path) (defvar geiser-guile-load-path 'nil)) (make-local-variable 'geiser-guile-load-path) (require 'cl-lib) (cl-pushnew root-dir* geiser-guile-load-path :test #'string-equal))
--8<---------------cut here---------------end--------------->8---

You've been trying to decline with 'n', but Emacs doesn't remember your
choice and keeps bugging you with it.  To be honest, that seems a design
shortcoming in Emacs; there's a '!' that means 'yes and remember it' but
there doesn't seem to be an equivalent option for 'no and remember it'.
So it keeps forgetting.

Also note that this isn't the first use of 'eval' in the Guix
.dir-locals file; so if you started hacking on the Guix sources in Emacs
for the first time, you'd have been greeted with this (!):

--8<---------------cut here---------------start------------->8---
The local variables list in /home/maxim/src/guix/
contains values that may not be safe (*).

Do you want to apply it?  You can type
y  -- to apply the local variables list.
n  -- to ignore the local variables list.
!  -- to apply the local variables list, and permanently mark these
      values (*) as safe (in the future, they will be set automatically.)

    fill-column : 78
    tab-width : 8
    sentence-end-double-space : t
    bug-reference-url-format : "http://bugs.gnu.org/%s"
    bug-reference-bug-regexp : "<https?://\\(debbugs\\|bugs\\)\\.gnu\\.org/\\([0-9]+\\)>"
    indent-tabs-mode : nil
    eval : (put 'eval-when 'scheme-indent-function 1)
    eval : (put 'call-with-prompt 'scheme-indent-function 1)
    eval : (put 'test-assert 'scheme-indent-function 1)
    eval : (put 'test-assertm 'scheme-indent-function 1)
    eval : (put 'test-equalm 'scheme-indent-function 1)
    eval : (put 'test-equal 'scheme-indent-function 1)
    eval : (put 'test-eq 'scheme-indent-function 1)
    eval : (put 'call-with-input-string 'scheme-indent-function 1)
    eval : (put 'guard 'scheme-indent-function 1)
    eval : (put 'lambda* 'scheme-indent-function 1)
    eval : (put 'substitute* 'scheme-indent-function 1)
    eval : (put 'match-record 'scheme-indent-function 2)
    eval : (put 'modify-phases 'scheme-indent-function 1)
    eval : (put 'replace 'scheme-indent-function 1)
    eval : (put 'add-before 'scheme-indent-function 2)
    eval : (put 'add-after 'scheme-indent-function 2)
    eval : (put 'modify-services 'scheme-indent-function 1)
    eval : (put 'with-directory-excursion 'scheme-indent-function 1)
    eval : (put 'with-file-lock 'scheme-indent-function 1)
    eval : (put 'with-file-lock/no-wait 'scheme-indent-function 1)
    eval : (put 'with-profile-lock 'scheme-indent-function 1)
    eval : (put 'with-writable-file 'scheme-indent-function 2)
    eval : (put 'package 'scheme-indent-function 0)
    eval : (put 'package/inherit 'scheme-indent-function 1)
    eval : (put 'origin 'scheme-indent-function 0)
    eval : (put 'build-system 'scheme-indent-function 0)
    eval : (put 'bag 'scheme-indent-function 0)
    eval : (put 'graft 'scheme-indent-function 0)
    eval : (put 'operating-system 'scheme-indent-function 0)
    eval : (put 'file-system 'scheme-indent-function 0)
    eval : (put 'manifest-entry 'scheme-indent-function 0)
    eval : (put 'manifest-pattern 'scheme-indent-function 0)
    eval : (put 'substitute-keyword-arguments 'scheme-indent-function 1)
    eval : (put 'with-store 'scheme-indent-function 1)
    eval : (put 'with-external-store 'scheme-indent-function 1)
    eval : (put 'with-error-handling 'scheme-indent-function 0)
    eval : (put 'with-mutex 'scheme-indent-function 1)
    eval : (put 'with-atomic-file-output 'scheme-indent-function 1)
    eval : (put 'call-with-compressed-output-port 'scheme-indent-function 2)
    eval : (put 'call-with-decompressed-port 'scheme-indent-function 2)
    eval : (put 'call-with-gzip-input-port 'scheme-indent-function 1)
    eval : (put 'call-with-gzip-output-port 'scheme-indent-function 1)
    eval : (put 'call-with-lzip-input-port 'scheme-indent-function 1)
    eval : (put 'call-with-lzip-output-port 'scheme-indent-function 1)
    eval : (put 'signature-case 'scheme-indent-function 1)
    eval : (put 'emacs-batch-eval 'scheme-indent-function 0)
    eval : (put 'emacs-batch-edit-file 'scheme-indent-function 1)
    eval : (put 'emacs-substitute-sexps 'scheme-indent-function 1)
    eval : (put 'emacs-substitute-variables 'scheme-indent-function 1)
    eval : (put 'with-derivation-narinfo 'scheme-indent-function 1)
    eval : (put 'with-derivation-substitute 'scheme-indent-function 2)
    eval : (put 'with-status-report 'scheme-indent-function 1)
    eval : (put 'with-status-verbosity 'scheme-indent-function 1)
    eval : (put 'with-build-handler 'scheme-indent-function 1)
    eval : (put 'mlambda 'scheme-indent-function 1)
    eval : (put 'mlambdaq 'scheme-indent-function 1)
    eval : (put 'syntax-parameterize 'scheme-indent-function 1)
    eval : (put 'with-monad 'scheme-indent-function 1)
    eval : (put 'mbegin 'scheme-indent-function 1)
    eval : (put 'mwhen 'scheme-indent-function 1)
    eval : (put 'munless 'scheme-indent-function 1)
    eval : (put 'mlet* 'scheme-indent-function 2)
    eval : (put 'mlet 'scheme-indent-function 2)
    eval : (put 'run-with-store 'scheme-indent-function 1)
    eval : (put 'run-with-state 'scheme-indent-function 1)
    eval : (put 'wrap-program 'scheme-indent-function 1)
    eval : (put 'with-imported-modules 'scheme-indent-function 1)
    eval : (put 'with-extensions 'scheme-indent-function 1)
    eval : (put 'with-parameters 'scheme-indent-function 1)
    eval : (put 'let-system 'scheme-indent-function 1)
    eval : (put 'with-database 'scheme-indent-function 2)
    eval : (put 'call-with-transaction 'scheme-indent-function 1)
    eval : (put 'with-statement 'scheme-indent-function 3)
    eval : (put 'call-with-retrying-transaction 'scheme-indent-function 1)
    eval : (put 'call-with-savepoint 'scheme-indent-function 1)
    eval : (put 'call-with-retrying-savepoint 'scheme-indent-function 1)
    eval : (put 'call-with-container 'scheme-indent-function 1)
    eval : (put 'container-excursion 'scheme-indent-function 1)
    eval : (put 'eventually 'scheme-indent-function 1)
    eval : (put 'call-with-progress-reporter 'scheme-indent-function 1)
    eval : (put 'with-repository 'scheme-indent-function 2)
    eval : (put 'with-temporary-git-repository 'scheme-indent-function 2)
    eval : (put 'with-temporary-git-worktree 'scheme-indent-function 2)
    eval : (put 'with-environment-variables 'scheme-indent-function 1)
    eval : (put 'with-fresh-gnupg-setup 'scheme-indent-function 1)
    eval : (put 'with-paginated-output-port 'scheme-indent-function 1)
  * eval : (modify-syntax-entry 126 "'")
  * eval : (modify-syntax-entry 36 "'")
  * eval : (modify-syntax-entry 43 "'")
--8<---------------cut here---------------end--------------->8---

So I don't see how the new behavior is much different.  The experience
is bad unless you trust (and tell Emacs to remember your choice) the
.dir-locals with '!', and sadly Emacs doesn't seem to provide an
alternative.

> Later...
>
>  - I'm hacking on another file in another repo
>  - C-x v = (check to see a diff of the work-in-progress changes of the
>    file I'm working on)
>  - Error in the minibuffer!  "Wrong type argument: stringp, nil"
>  - wtf, okay M-x toggle-debug-on-error
>
> Debugger entered--Lisp error: (wrong-type-argument stringp nil)
>   expand-file-name(nil)
>   (let* ((root-dir (expand-file-name (locate-dominating-file default-directory ".dir-locals.el"))) (root-dir* (directory-file-name root-dir))) (unless (boundp 'geiser-guile-load-path) (defvar geiser-guile-load-path 'nil)) (make-local-variable 'geiser-guile-load-path) (require 'cl-lib) (cl-pushnew root-dir* geiser-guile-load-path :test #'string-equal))
>   eval((let* ((root-dir (expand-file-name (locate-dominating-file default-directory ".dir-locals.el"))) (root-dir* (directory-file-name root-dir))) (unless (boundp 'geiser-guile-load-path) (defvar geiser-guile-load-path 'nil)) (make-local-variable 'geiser-guile-load-path) (require 'cl-lib) (cl-pushnew root-dir* geiser-guile-load-path :test #'string-equal)))
>   hack-one-local-variable(eval (let* ((root-dir (expand-file-name (locate-dominating-file default-directory ".dir-locals.el"))) (root-dir* (directory-file-name root-dir))) (unless (boundp 'geiser-guile-load-path) (defvar geiser-guile-load-path 'nil)) (make-local-variable 'geiser-guile-load-path) (require 'cl-lib) (cl-pushnew root-dir* geiser-guile-load-path :test #'string-equal)))
>   hack-local-variables-apply()
>   hack-dir-local-variables-non-file-buffer()
>   diff-mode()
>   vc-diff-internal(t (Git ("/home/cwebber/devel/scribble/scribble-lib/scribble...")) nil nil t)
>   vc-diff(nil t)
>   funcall-interactively(vc-diff nil t)
>   call-interactively(vc-diff nil nil)
>   command-execute(vc-diff)
>
>  - Oh... it's whatever thing I enabled earlier in the guix repo.  Well
>    now my vc-diff is broken outside of there... :\

This is quite unexpected, given the new Elisp snippet should run only
for buffers attached to the Guix source tree.

> I'm presuming that if I understood whatever this is doing, it's probably
> something that gives me a better user experience if I accept it while
> working on Guix.  But a) for whatever reason Emacs' dir-locals stuff is
> written in such a way that it antagonizes me for not accepting it and I
> didn't know what it eval was (maybe this is a lack of understanding in
> how to "say no and get it to listen to me"... I didn't resist for too
> long)

I think my explanation above covers a); basically this is a shortcoming
in Emacs and was already the case with the previous version of
.dir-locals.

b) it seems like this change isn't scoped to Guix's checkout
 itself somehow...

I'll try to reproduce, and report back.

Thanks for the feedback!

Maxim