bug#23407: .dir-local settings get obliterated on running a major mode function.

bug#23407: .dir-local settings get obliterated on running a major mode function.

[Alan Mackenzie]

Hello, Emacs.

In the master branch (I'm sure it's the same in the emacs-25 branch,
too):
1. start Emacs with emacs -Q
2. Visit any emacs lisp file which is part of Emacs with C-x C-f.
3. Do C-h C-v indent-tabs-mode <RET>.  The *Help* buffer starts off
  with:

    indent-tabs-mode is a variable defined in `C source code'.
    Its value is nil
    Original value was t
    Local in buffer follow.el; global value is t

  .  The local value comes from the .dir-locals in the top level Emacs
  directory.
4. Do M-x emacs-lisp-mode.
5. Again do C-h C-v indent-tabs-mode <RET>.  This time, the *Help*
  buffer starts off with:

    indent-tabs-mode is a variable defined in `C source code'.
    Its value is t

  .  This is different from the first *Help* buffer.
6. This is a bug.  On running a major mode, directory local settings
  should be respected.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#23407: .dir-local settings get obliterated on running a major mode function.

[Dmitry Gutov]

On 04/30/2016 01:27 PM, Alan Mackenzie wrote:

> 4. Do M-x emacs-lisp-mode.
> 5. Again do C-h C-v indent-tabs-mode <RET>.  This time, the *Help*
>   buffer starts off with:
>
>     indent-tabs-mode is a variable defined in `C source code'.
>     Its value is t
>
>   .  This is different from the first *Help* buffer.

I can see this too, in emacs-25.

bug#23407: .dir-local settings get obliterated on running a major mode function.

[Glenn Morris]

It's the same with file-local variables, and it always has been.
I don't think this is a bug.

bug#23407: .dir-local settings get obliterated on running a major mode function.

[Glenn Morris]

PS dupe of http://debbugs.gnu.org/15577

bug#23407: .dir-local settings get obliterated on running a major mode function.

[Alan Mackenzie]

Hello, Glenn.

On Sat, Apr 30, 2016 at 02:53:07PM -0400, Glenn Morris wrote:

> PS dupe of http://debbugs.gnu.org/15577

More fully known as:

Subject: bug#15577: 24.3; dir-local variables not applied when switching
 major-mode
Date: Wed, 9 Oct 2013 16:14:00 -0400
From: yary <not.com@gmail.com>

Thanks for the reference.  I'd actually searched debbugs for
".dir-locals" and found nothing.

Anyhow, I've hacked a patch together.  The idea is to call
`hack-local-variables' from `run-mode-hooks' rather than from
`normal-mode'.  Of course, it's never quite that simple.  The main thing
is that the local variables are now set with the major mode rather than
with the visiting of the file.  So file/directory local variables are
now not lost on setting the major mode.

The following patch is based on the master branch from earlier on today.


diff --git a/lisp/files.el b/lisp/files.el
index 132ebce..940a9ac 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -2322,8 +2322,13 @@ normal-mode
     ;; s-a-m and h-l-v may parse the same regions, looking for "mode:".
     (with-demoted-errors "File mode specification error: %s"
       (set-auto-mode))
-    (with-demoted-errors "File local-variables error: %s"
-      (hack-local-variables)))
+    ;; delay-mode-hooks is set when `byte-compile-file' is the caller.
+    ;; It is essential that we call `hack-local-variables' in order to
+    ;; set up `lexical-binding', since `run-mode-hooks' is prevented
+    ;; from doing its job.
+    (when delay-mode-hooks
+      (with-demoted-errors "File local-variables error: %s"
+        (hack-local-variables 'no-mode))))
   ;; Turn font lock off and on, to make sure it takes account of
   ;; whatever file local variables are relevant to it.
   (when (and font-lock-mode
@@ -3297,11 +3302,15 @@ hack-local-variables-filter
 ;; TODO?  Warn once per file rather than once per session?
 (defvar hack-local-variables--warned-lexical nil)
 
-(defun hack-local-variables (&optional mode-only)
+(defun hack-local-variables (&optional handle-mode)
   "Parse and put into effect this buffer's local variables spec.
 Uses `hack-local-variables-apply' to apply the variables.
 
-If MODE-ONLY is non-nil, all we do is check whether a \"mode:\"
+If HANDLE-MODE is nil, we apply all the specified local
+variables.  If HANDLE-MODE is neither nil nor t, we do the same,
+except that any settings of `mode' are ignored.
+
+If HANDLE-MODE is t, all we do is check whether a \"mode:\"
 is specified, and return the corresponding mode symbol, or nil.
 In this case, we try to ignore minor-modes, and only return a
 major-mode.
@@ -3319,7 +3328,7 @@ hack-local-variables
   (let ((enable-local-variables
 	 (and local-enable-local-variables enable-local-variables))
 	result)
-    (unless mode-only
+    (unless (eq handle-mode t)
       (setq file-local-variables-alist nil)
       (with-demoted-errors "Directory-local variables error: %s"
 	;; Note this is a no-op if enable-local-variables is nil.
@@ -3327,18 +3336,19 @@ hack-local-variables
     ;; This entire function is basically a no-op if enable-local-variables
     ;; is nil.  All it does is set file-local-variables-alist to nil.
     (when enable-local-variables
-      ;; This part used to ignore enable-local-variables when mode-only
-      ;; was non-nil.  That was inappropriate, eg consider the
+      ;; This part used to ignore enable-local-variables when handle-mode
+      ;; was t.  That was inappropriate, eg consider the
       ;; (artificial) example of:
       ;; (setq local-enable-local-variables nil)
       ;; Open a file foo.txt that contains "mode: sh".
       ;; It correctly opens in text-mode.
       ;; M-x set-visited-file name foo.c, and it incorrectly stays in text-mode.
       (unless (or (inhibit-local-variables-p)
-		  ;; If MODE-ONLY is non-nil, and the prop line specifies a
+		  ;; If HANDLE-MODE is t, and the prop line specifies a
 		  ;; mode, then we're done, and have no need to scan further.
-		  (and (setq result (hack-local-variables-prop-line mode-only))
-		       mode-only))
+		  (and (setq result (hack-local-variables-prop-line
+                                     (eq handle-mode t)))
+		       (eq handle-mode t)))
 	;; Look for "Local variables:" line in last page.
 	(save-excursion
 	  (goto-char (point-max))
@@ -3393,7 +3403,7 @@ hack-local-variables
 		  (goto-char (point-min))
 
 		  (while (not (or (eobp)
-                                  (and mode-only result)))
+                                  (and (eq handle-mode t) result)))
 		    ;; Find the variable name;
 		    (unless (looking-at hack-local-variable-regexp)
                       (error "Malformed local variable line: %S"
@@ -3410,7 +3420,7 @@ hack-local-variables
 		      (forward-char 1)
 		      (let ((read-circle nil))
 			(setq val (read (current-buffer))))
-		      (if mode-only
+		      (if (eq handle-mode t)
 			  (and (eq var 'mode)
 			       ;; Specifying minor-modes via mode: is
 			       ;; deprecated, but try to reject them anyway.
@@ -3432,6 +3442,7 @@ hack-local-variables
                                     ;; to use 'thisbuf's name in the
                                     ;; warning message.
                                     (or (buffer-file-name thisbuf) ""))))))
+                              ((and (eq var 'mode) handle-mode))
 			      (t
 			       (ignore-errors
 				 (push (cons (if (eq var 'eval)
@@ -3440,8 +3451,8 @@ hack-local-variables
 					     val) result))))))
 		    (forward-line 1))))))))
       ;; Now we've read all the local variables.
-      ;; If MODE-ONLY is non-nil, return whether the mode was specified.
-      (if mode-only result
+      ;; If HANDLE-MODE is t, return whether the mode was specified.
+      (if (eq handle-mode t) result
 	;; Otherwise, set the variables.
 	(hack-local-variables-filter result nil)
 	(hack-local-variables-apply)))))
diff --git a/lisp/subr.el b/lisp/subr.el
index 5f8d830..162ac21 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -1732,10 +1732,14 @@ after-change-major-mode-hook
 
 (defun run-mode-hooks (&rest hooks)
   "Run mode hooks `delayed-mode-hooks' and HOOKS, or delay HOOKS.
-If the variable `delay-mode-hooks' is non-nil, does not run any hooks,
+Call `hack-local-variables' to set up file local and directory local
+variables.
+
+If the variable `delay-mode-hooks' is non-nil, does not do anything,
 just adds the HOOKS to the list `delayed-mode-hooks'.
 Otherwise, runs hooks in the sequence: `change-major-mode-after-body-hook',
-`delayed-mode-hooks' (in reverse order), HOOKS, and finally
+`delayed-mode-hooks' (in reverse order), HOOKS, then runs
+`hack-local-variables' and finally runs the hook
 `after-change-major-mode-hook'.  Major mode functions should use
 this instead of `run-hooks' when running their FOO-mode-hook."
   (if delay-mode-hooks
@@ -1746,6 +1750,9 @@ run-mode-hooks
     (setq hooks (nconc (nreverse delayed-mode-hooks) hooks))
     (setq delayed-mode-hooks nil)
     (apply 'run-hooks (cons 'change-major-mode-after-body-hook hooks))
+    (if (buffer-file-name)
+        (with-demoted-errors "File local-variables error: %s"
+          (hack-local-variables 'no-mode)))
     (run-hooks 'after-change-major-mode-hook)))
 
 (defmacro delay-mode-hooks (&rest body)


-- 
Alan Mackenzie (Nuremberg, Germany).

bug#23407: .dir-local settings get obliterated on running a major mode function.

[Stefan Monnier]

> Anyhow, I've hacked a patch together.  The idea is to call
> `hack-local-variables' from `run-mode-hooks' rather than from
> `normal-mode'.

Good idea.

> +    ;; delay-mode-hooks is set when `byte-compile-file' is the caller.
> +    ;; It is essential that we call `hack-local-variables' in order to
> +    ;; set up `lexical-binding', since `run-mode-hooks' is prevented
> +    ;; from doing its job.
> +    (when delay-mode-hooks
> +      (with-demoted-errors "File local-variables error: %s"
> +        (hack-local-variables 'no-mode))))

But this seems terribly brittle.  Do we care about delay-mode-hooks (as
the code says) or about byte-compile-file (as the comment says)?
If it's the former, then the comment needs to be fixed, if it's the
latter, than we need to find some other way to tell this code what's
going on.


        Stefan

bug#23407: .dir-local settings get obliterated on running a major mode function.

[Alan Mackenzie]

Hello, Stefan.

Thanks for looking at my patch.

On Mon, May 02, 2016 at 12:02:11AM -0400, Stefan Monnier wrote:
> > Anyhow, I've hacked a patch together.  The idea is to call
> > `hack-local-variables' from `run-mode-hooks' rather than from
> > `normal-mode'.

> Good idea.

> > +    ;; delay-mode-hooks is set when `byte-compile-file' is the caller.
> > +    ;; It is essential that we call `hack-local-variables' in order to
> > +    ;; set up `lexical-binding', since `run-mode-hooks' is prevented
> > +    ;; from doing its job.
> > +    (when delay-mode-hooks
> > +      (with-demoted-errors "File local-variables error: %s"
> > +        (hack-local-variables 'no-mode))))

> But this seems terribly brittle.  Do we care about delay-mode-hooks (as
> the code says) or about byte-compile-file (as the comment says)?

This bit of code was necessitated by:

#########################################################################
commit 3ba6b3a9c1e0565ee5f45f11a9c09702a24f8453
Author: Artur Malabarba <bruce.connor.am@gmail.com>
Date:   Sun Apr 12 03:23:35 2015 +0100

    Speed up byte-compilation and autoload generation by avoiding mode-hooks

    This prevents emacs-lisp-mode-hook from being run everytime an
    autoload file is generated, which can account for a fraction of
    package installation time depending on the hooks the user has
    configured.

    * lisp/emacs-lisp/bytecomp.el (byte-compile-file): Use
    * delay-mode-hooks.

    * lisp/emacs-lisp/autoload.el (autoload-find-file)
    (autoload-find-generated-file): Use delay-mode-hooks.
#########################################################################

if hack-local-variables isn't run, lexical-binding (for example) doesn't
get set up, and make bootstrap fails.

> If it's the former, then the comment needs to be fixed, if it's the
> latter, than we need to find some other way to tell this code what's
> going on.

I don't really understand the question.  Sure, that bit of code is ugly.
But the comment both motivates ("it's `byte-compile-file''s fault") and
explains the problem (which is that `delay-mode-hooks' being set would
prevent `run-mode-hook', and thus `hack-local-variable' from running).
It is possible that other stuff might call `normal-mode' like this.
What sort of changes do you advocate for the comment (or for the code)?

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#23407: .dir-local settings get obliterated on running a major mode function.

[Stefan Monnier]

>> > +    ;; delay-mode-hooks is set when `byte-compile-file' is the caller.
>> > +    ;; It is essential that we call `hack-local-variables' in order to
>> > +    ;; set up `lexical-binding', since `run-mode-hooks' is prevented
>> > +    ;; from doing its job.
>> > +    (when delay-mode-hooks
>> > +      (with-demoted-errors "File local-variables error: %s"
>> > +        (hack-local-variables 'no-mode))))

>> But this seems terribly brittle.  Do we care about delay-mode-hooks (as
>> the code says) or about byte-compile-file (as the comment says)?

> This bit of code was necessitated by:

> #########################################################################
> commit 3ba6b3a9c1e0565ee5f45f11a9c09702a24f8453
> Author: Artur Malabarba <bruce.connor.am@gmail.com>
> Date:   Sun Apr 12 03:23:35 2015 +0100
>
>     Speed up byte-compilation and autoload generation by avoiding mode-hooks
>
>     This prevents emacs-lisp-mode-hook from being run everytime an
>     autoload file is generated, which can account for a fraction of
>     package installation time depending on the hooks the user has
>     configured.
>
>     * lisp/emacs-lisp/bytecomp.el (byte-compile-file): Use
>     * delay-mode-hooks.
>
>     * lisp/emacs-lisp/autoload.el (autoload-find-file)
>     (autoload-find-generated-file): Use delay-mode-hooks.
> #########################################################################

Hmm... so you're working around someone else's hack!

> if hack-local-variables isn't run, lexical-binding (for example) doesn't
> get set up, and make bootstrap fails.

Of course: hack-local-variables should be run unconditionally.

>> If it's the former, then the comment needs to be fixed, if it's the
>> latter, than we need to find some other way to tell this code what's
>> going on.
> I don't really understand the question.  Sure, that bit of code is ugly.
> But the comment both motivates ("it's `byte-compile-file''s fault") and
> explains the problem (which is that `delay-mode-hooks' being set would
> prevent `run-mode-hook', and thus `hack-local-variable' from running).
> It is possible that other stuff might call `normal-mode' like this.
> What sort of changes do you advocate for the comment (or for the code)?

How 'bout doing

    (setq-local hack-local-variables--done t)

in hack-local-variables, and then testing that instead of testing
delay-mode-hooks?


        Stefan

bug#23407: .dir-local settings get obliterated on running a major mode function.

[Alan Mackenzie]

Bug fixed in savannah master branch.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#23407: .dir-local settings get obliterated on running a major mode function.

[Alan Mackenzie]

Hello, Stefan.

On Tue, May 03, 2016 at 02:10:45PM -0400, Stefan Monnier wrote:
> >> > +    ;; delay-mode-hooks is set when `byte-compile-file' is the caller.
> >> > +    ;; It is essential that we call `hack-local-variables' in order to
> >> > +    ;; set up `lexical-binding', since `run-mode-hooks' is prevented
> >> > +    ;; from doing its job.
> >> > +    (when delay-mode-hooks
> >> > +      (with-demoted-errors "File local-variables error: %s"
> >> > +        (hack-local-variables 'no-mode))))

> >> But this seems terribly brittle.  Do we care about delay-mode-hooks (as
> >> the code says) or about byte-compile-file (as the comment says)?

[ .... ]

> Hmm... so you're working around someone else's hack!

Yes, so is life.

> > if hack-local-variables isn't run, lexical-binding (for example) doesn't
> > get set up, and make bootstrap fails.

> Of course: hack-local-variables should be run unconditionally.

> >> If it's the former, then the comment needs to be fixed, if it's the
> >> latter, than we need to find some other way to tell this code what's
> >> going on.
> > I don't really understand the question.  Sure, that bit of code is ugly.
> > But the comment both motivates ("it's `byte-compile-file''s fault") and
> > explains the problem (which is that `delay-mode-hooks' being set would
> > prevent `run-mode-hook', and thus `hack-local-variable' from running).
> > It is possible that other stuff might call `normal-mode' like this.
> > What sort of changes do you advocate for the comment (or for the code)?

> How 'bout doing

>     (setq-local hack-local-variables--done t)

> in hack-local-variables, and then testing that instead of testing
> delay-mode-hooks?

That's even more horrible.  What I've done is tidy up the comment (which
now makes no reference to byte-compile-file) and committed the change.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#23407: .dir-local settings get obliterated on running a major mode function.

[Stefan Monnier]

>> (setq-local hack-local-variables--done t)
>> in hack-local-variables, and then testing that instead of testing
>> delay-mode-hooks?
> That's even more horrible.

Huh?  It makes it clear what's going on: the find-file part of the code
wants to make sure that hack-local-variables is called, so
hack-local-variables sets a var when it's called, and find-file checks
it to know whether that's been done or not.

It's reliable and contrary to the code you had, it doesn't need to care
about packages unrelated to find-file nor to hack-local-variables, nor
about variables which are similarly unrelated to find-file nor to
hack-local-variables.


        Stefan