bug#44066: 27.1; Values ignored when using .dir-locals.el and .dir-locals-2.el together

bug#44066: 27.1; Values ignored when using .dir-locals.el and .dir-locals-2.el together

[Jacob First]

Hello,

I've experienced a case where some directory local variable values are
ignored when .dir-locals.el and .dir-locals-2.el are both present in a
directory. As Emacs normally sets all the variable values when only one
of the files is present, and I can find nothing in the manual to suggest
that the behavior should be different when both are present, I believe
this behavior is erroneous.

To reproduce, first create a directory with 3 files with the filenames
and the contents listed below:

= file.txt =
test file

= .dir-locals.el =
((nil . ((eval . (setq var1 'foo)) (eval . (setq var2 'bar)))))

= .dir-locals-2.el =
((nil . ((eval . (setq var3 'baz)) (eval . (setq var4 'qux)))))

Then run:

    emacs -Q file.txt

Emacs should display a prompt with a message beginning "The local
variables list in /path/to/the/dir/ contains values that may not be safe
(*)" with a list of potentially unsafe values at the bottom.

I expect to see the following values listed in some order:

  * eval : (setq var1 'foo)
  * eval : (setq var2 'bar)
  * eval : (setq var3 'baz)
  * eval : (setq var4 'qux)

Instead, only the following values are listed:

  * eval : (setq var4 'qux)
  * eval : (setq var2 'bar)

If I then type ! to accept the values, evaluating each variable via M-:
will confirm that only var2 and var4 have been set and that var1 and
var3 are unbound.

If I quit Emacs, delete .dir-locals-2.el, and follow the above steps
again, the prompt now lists all the values from .dir-locals.el as
expected:

  * eval : (setq var1 'foo)
  * eval : (setq var2 'bar)

Similarly if I delete .dir-locals.el but keep .dir-locals-2.el intact,
the prompt lists all the values from .dir-locals-2.el as expected:

  * eval : (setq var3 'baz)
  * eval : (setq var4 'qux)

Autogenerated system info follows. Thank you.

***

In GNU Emacs 27.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.20, cairo version 1.16.0)
Windowing system distributor 'The X.Org Foundation', version 11.0.12009000
Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
End of buffer [3 times]
Please type y, n, or !, or C-v/M-v to scroll: !
Setting ‘safe-local-variable-values’ temporarily since "emacs -q" would overwrite customizations

Configured using:
 'configure
 CONFIG_SHELL=/gnu/store/pwcp239kjf7lnj5i4lkdzcfcxwcfyk72-bash-minimal-5.0.16/bin/bash
 SHELL=/gnu/store/pwcp239kjf7lnj5i4lkdzcfcxwcfyk72-bash-minimal-5.0.16/bin/bash
 --prefix=/gnu/store/zcrrjwkzzlx96szsxmacflrl3bybm413-emacs-27.1
 --enable-fast-install --with-modules --with-cairo
 --disable-build-details'

Configured features:
XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND DBUS GSETTINGS GLIB NOTIFY
INOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF
ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES THREADS JSON PDUMPER
GMP

Important settings:
  value of $EMACSLOADPATH: /home/jfirst/.local/share/guix/extra-profiles/gui/gui/share/emacs/27.1/lisp:/home/jfirst/.local/share/guix/extra-profiles/gui/gui/share/emacs/site-lisp
  value of $LANG: en_US.utf8
  locale-coding-system: utf-8-unix

Major mode: Text

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
format-spec rfc822 mml mml-sec password-cache epa derived epg epg-config
gnus-util rmail rmail-loaddefs text-property-search time-date subr-x
mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils
mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr
mail-utils cus-edit easymenu cus-start cus-load wid-edit cl-loaddefs
cl-lib map seq byte-opt gv bytecomp byte-compile cconv tooltip eldoc
electric uniquify ediff-hook vc-hooks lisp-float-type mwheel term/x-win
x-win term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame minibuffer cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite charscript charprop case-table epa-hook
jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice loaddefs
button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote threads dbusbind inotify dynamic-setting system-font-setting
font-render-setting cairo move-toolbar gtk x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 16 66357 9862)
 (symbols 48 7546 1)
 (strings 32 20201 1623)
 (string-bytes 1 628515)
 (vectors 16 10568)
 (vector-slots 8 138494 9480)
 (floats 8 29 23)
 (intervals 56 194 0)
 (buffers 1000 13))

bug#44066: 27.1; Values ignored when using .dir-locals.el and .dir-locals-2.el together

[Lars Ingebrigtsen]

Jacob First <jacob.first@member.fsf.org> writes:

> To reproduce, first create a directory with 3 files with the filenames
> and the contents listed below:
>
> = file.txt =
> test file
>
> = .dir-locals.el =
> ((nil . ((eval . (setq var1 'foo)) (eval . (setq var2 'bar)))))
>
> = .dir-locals-2.el =
> ((nil . ((eval . (setq var3 'baz)) (eval . (setq var4 'qux)))))

This was fun to debug, because there's so much caching going on that...
well.

Anyway, here's what's happening if you have two .dir-locals files:

(setq variables '((nil (eval setq var1 'foo) (eval setq var2 'bar))))
(setq newvars '((nil (eval setq var3 'baz) (eval setq var4 'qux))))

Then this is run:

(map-merge-with 'list (lambda (a b) (map-merge 'list a b))
                                    variables
                                    newvars)
=> ((nil (eval setq var4 'qux) (eval setq var2 'bar)))

If there's only a single .dir-locals file, nothing is merged and the
form is returned as is.

This certainly seems like a bug, and it's present in Emacs 26, too
(which is the earliest one I have here).

But...  is this syntax even allowed?  Does it work in the
single-dir-locals-file case by accident?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#44066: 27.1; Values ignored when using .dir-locals.el and .dir-locals-2.el together

[Jacob First]

Hi,

> Anyway, here's what's happening if you have two .dir-locals files:
>
> (setq variables '((nil (eval setq var1 'foo) (eval setq var2 'bar))))
> (setq newvars '((nil (eval setq var3 'baz) (eval setq var4 'qux))))
> 
> Then this is run:
> 
> (map-merge-with 'list (lambda (a b) (map-merge 'list a b))
>                                     variables
>                                     newvars)
> => ((nil (eval setq var4 'qux) (eval setq var2 'bar)))

Thanks!

> But...  is this syntax even allowed?  Does it work in the
> single-dir-locals-file case by accident?

Are you asking if multiple `eval' are allowed per file? I'm pretty sure
they are; at least, I've seen them used regularly and can't find a
restriction in the manual. In any case, there's still a problem when
each list contains only a single `eval':

(setq variables '((nil (eval setq var1 'foo))))
(setq newvars '((nil (eval setq var3 'baz))))

(map-merge-with 'list (lambda (a b)
                        (map-merge 'list a b))
                variables
                newvars)

=> ((nil (eval setq var3 'baz)))

Presumably (setq var1 'foo) should also be evaluated, right?

Another thing that might imply support for multiple `eval' is intended:
if in .dir-locals.el I have

((nil . ((eval . (setq var1 'foo))))
 (scheme-mode . ((eval . (setq var2 'bar)))))

then when I open a file in Scheme mode, both var1 and var2 are set
rather than the Scheme setting overriding, even though for normal
variables, the more specific setting is supposed to take precedence.
This is getting a bit speculative though.

bug#44066: 27.1; Values ignored when using .dir-locals.el and .dir-locals-2.el together

[Lars Ingebrigtsen]

Jacob First <jacob.first@member.fsf.org> writes:

> Are you asking if multiple `eval' are allowed per file? I'm pretty sure
> they are; at least, I've seen them used regularly and can't find a
> restriction in the manual. In any case, there's still a problem when
> each list contains only a single `eval':
>
> (setq variables '((nil (eval setq var1 'foo))))
> (setq newvars '((nil (eval setq var3 'baz))))
>
> (map-merge-with 'list (lambda (a b)
>                         (map-merge 'list a b))
>                 variables
>                 newvars)
>
> => ((nil (eval setq var3 'baz)))
>
> Presumably (setq var1 'foo) should also be evaluated, right?

Yup.  I was a bit unsure what the point of the merge was at all, but
it's for pruning repeated variables, like so:

(setq variables '((nil (foo . bar1) (zot . bar2))))
(setq newvars '((nil (qux . bar1) (zot . bar3))))

(map-merge-with 'list (lambda (a b)
                        (map-merge 'list a b))
                variables
                newvars)
=> ((nil (qux . bar1) (foo . bar1) (zot . bar3)))                

So I'm guessing the `eval' case just wasn't considered when the merge
bit was written.

I think, in general, all the `eval' bits have to be preserved, and
nothing should be pruned?  So the fix would be to handle `eval's
separately. 

I've now done this on the trunk, and it makes the test case work for me.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#44066: 27.1; Values ignored when using .dir-locals.el and .dir-locals-2.el together

[Jacob First]

> I think, in general, all the `eval' bits have to be preserved, and
> nothing should be pruned?  So the fix would be to handle `eval's
> separately.
> 
> I've now done this on the trunk, and it makes the test case work for me.

Thanks for the fix. I tested as of commit 39271ed and it works for me
too.