From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp2 ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id D9eSK+U2sF+pbAAA0tVLHw (envelope-from ) for ; Sat, 14 Nov 2020 19:58:29 +0000 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp2 with LMTPS id gJgBJ+U2sF97WQAAB5/wlQ (envelope-from ) for ; Sat, 14 Nov 2020 19:58:29 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 5454794051B for ; Sat, 14 Nov 2020 19:58:29 +0000 (UTC) Received: from localhost ([::1]:36650 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ke1gq-00023M-7j for larch@yhetil.org; Sat, 14 Nov 2020 14:58:28 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:59280) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ke1gh-00023F-Vh for guix-devel@gnu.org; Sat, 14 Nov 2020 14:58:19 -0500 Received: from dustycloud.org ([50.116.34.160]:53652) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ke1gf-0005iU-Dn for guix-devel@gnu.org; Sat, 14 Nov 2020 14:58:19 -0500 Received: from twig (localhost [127.0.0.1]) by dustycloud.org (Postfix) with ESMTPS id 41C8526650; Sat, 14 Nov 2020 14:57:35 -0500 (EST) References: <87tuuixjno.fsf@gmail.com> <87zh3w5ytp.fsf@dustycloud.org> <87wnz05u7d.fsf@dustycloud.org> <87tuu46tqm.fsf@gmail.com> User-agent: mu4e 1.4.13; emacs 27.1 From: Christopher Lemmer Webber To: Miguel =?utf-8?Q?=C3=81ngel?= Arruga Vivas Subject: Re: [PATCH] Automatically set `geiser-guile-load-path' from .dir-locals In-reply-to: <87tuu46tqm.fsf@gmail.com> Date: Sat, 14 Nov 2020 14:57:03 -0500 Message-ID: <87lff3iueo.fsf@dustycloud.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Received-SPF: pass client-ip=50.116.34.160; envelope-from=cwebber@dustycloud.org; helo=dustycloud.org X-detected-operating-system: by eggs.gnu.org: First seen = 2020/11/14 14:57:59 X-ACL-Warn: Detected OS = Linux 2.2.x-3.x [generic] [fuzzy] X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: guix-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: guix-devel@gnu.org, Maxim Cournoyer Errors-To: guix-devel-bounces+larch=yhetil.org@gnu.org Sender: "Guix-devel" X-Scanner: ns3122888.ip-94-23-21.eu Authentication-Results: aspmx1.migadu.com; dkim=none; dmarc=none; spf=pass (aspmx1.migadu.com: domain of guix-devel-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-devel-bounces@gnu.org X-Spam-Score: -1.01 X-TUID: L+sebZAk/6rX Miguel =C3=81ngel 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 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 =3D (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-di= rectory ".dir-locals.el"))) (root-dir* (directory-file-name root-dir))) (un= less (boundp 'geiser-guile-load-path) (defvar geiser-guile-load-path 'nil))= (make-local-variable 'geiser-guile-load-path) (require 'cl-lib) (cl-pushne= w root-dir* geiser-guile-load-path :test #'string-equal)) >>> eval((let* ((root-dir (expand-file-name (locate-dominating-file defau= lt-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-p= ushnew root-dir* geiser-guile-load-path :test #'string-equal))) >>> hack-one-local-variable(eval (let* ((root-dir (expand-file-name (loca= te-dominating-file default-directory ".dir-locals.el"))) (root-dir* (direct= ory-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 #'str= ing-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/s= cribble...")) nil nil t) >>> vc-diff(nil t) >>> funcall-interactively(vc-diff nil t) >>> call-interactively(vc-diff nil nil) >>> command-execute(vc-diff) >>>=20=20 >>> - 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 =3D 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(...!!!!)=20 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 worktree= s). - (eval . (let* ((root-dir (expand-file-name - (locate-dominating-file - default-directory ".dir-locals.el"))) - ;; Workaround for bug https://issues.guix.gnu.org/4381= 8. - (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))) +=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20 + (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))))))) =20 (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?