bug#36830: 26.2; find-file-visit-truename is not honored as file local variable

bug#36830: 26.2; find-file-visit-truename is not honored as file local variable

[Gustavo Barros]

Hi all,

`find-file-visit-truename` is defined in "files.el" and is included 
there as a safe-local-variable (as long as boolean).  However, its use 
as a file local variable does not seem to be honored.

Steps to reproduce.

Consider a scenario with the following directory tree:

#+begin_example
~/
~/FolderA/
~/FolderA/TargetFile.txt
~/FolderB/
~/FolderB/LinkToTargetFile.txt -> ~/FolderA/TargetFile.txt
#+end_example

"~/FolderA/TargetFile.txt" has `find-file-visit-truename` set to true as 
a file local variable.

Start ~emacs -Q~ and visit "~/FolderB/LinkToTargetFile.txt".

Eval ~(buffer-file-name)~ and the result is: 
"~/FolderB/LinkToTargetFile.txt" (substituted home folder). Whereas 
=(file-truename (buffer-file-name))= reports "~/FolderA/TargetFile.txt", 
which would be the expected visited file, given the local file variable.


Best regards,
Gustavo Barros.

PS: I’ve asked about this elsewhere 
(https://emacs.stackexchange.com/q/51495/18951). But, since then, 
investigating this further, and considering this works for 
`vc-follow-symlinks` (which is being discussed at 
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=33264), I came to think 
this indeed might be unexpected behavior. Thus this report.




In GNU Emacs 26.2 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.30)
 of 2019-04-19 built on gusbrs-laptop
Windowing system distributor 'The X.Org Foundation', version 
11.0.11906000
System Description:	Linux Mint 19.1 Tessa

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Making completion list...
"/home/gustavo/FolderB/LinkToTargetFile.txt"
Making completion list...
"/home/gustavo/FolderA/TargetFile.txt"
Configured using:
 'configure --with-mailutils --with-xwidgets --with-modules'

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS GLIB
NOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES THREADS XWIDGETS
LIBSYSTEMD LCMS2

Important settings:
  value of $LC_MONETARY: pt_BR.UTF-8
  value of $LC_NUMERIC: pt_BR.UTF-8
  value of $LANG: en_US.UTF-8
  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 seq byte-opt gv
bytecomp byte-compile cconv cl-loaddefs cl-lib dired dired-loaddefs
format-spec rfc822 mml easymenu mml-sec password-cache epa derived epg
epg-config gnus-util rmail rmail-loaddefs mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils elec-pair time-date
mule-util 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 menu-bar
rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core term/tty-colors frame 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 minibuffer
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 lcms2 dynamic-setting system-font-setting font-render-setting
xwidget-internal move-toolbar gtk x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 16 96167 6183)
 (symbols 48 20414 1)
 (miscs 40 54 104)
 (strings 32 28445 1315)
 (string-bytes 1 748145)
 (vectors 16 14078)
 (vector-slots 8 502466 7254)
 (floats 8 51 146)
 (intervals 56 523 0)
 (buffers 992 13))

bug#36830: 26.2; find-file-visit-truename is not honored as file local variable

[Lars Ingebrigtsen]

Gustavo Barros <gusbrs.2016@gmail.com> writes:

> `find-file-visit-truename` is defined in "files.el" and is included 
> there as a safe-local-variable (as long as boolean).  However, its use 
> as a file local variable does not seem to be honored.
>
> Steps to reproduce.
>
> Consider a scenario with the following directory tree:
>
> #+begin_example
> ~/
> ~/FolderA/
> ~/FolderA/TargetFile.txt
> ~/FolderB/
> ~/FolderB/LinkToTargetFile.txt -> ~/FolderA/TargetFile.txt
> #+end_example
>
> "~/FolderA/TargetFile.txt" has `find-file-visit-truename` set to true as 
> a file local variable.

Could you include the contents of this file?

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

bug#36830: 26.2; find-file-visit-truename is not honored as file local variable

[Gustavo Barros]

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


Hi Lars,

thank you for looking into this.

On Fri, Aug 23 2019, Lars Ingebrigtsen wrote:

>
> Could you include the contents of this file?

Sure. The file from which I reported, and which I include here now, was 
just a dummy file with the variable of interest set to true with 
`add-file-local-variable`:

#+name: TargetFile.txt
#+begin_example

;; Local Variables:
;; find-file-visit-truename: t
;; End:
#+end_example

(Also in annex)

As I send you this now, I realize I should perhaps have chosen some 
other file format for the report, as text-mode does not have a 
predefined comment syntax. I don’t know if this is relevant to the local 
variables machinery. I suppose it isn’t, as my initial issue with this 
arose in Org files, for which the comment syntax is clear. But if some 
adjustments on the report for a more proper setting are somehow 
desired/relevant, let me know.

Best regards,
Gustavo.



[-- Attachment #2: TargetFile.txt --]
[-- Type: text/plain, Size: 60 bytes --]


;; Local Variables:
;; find-file-visit-truename: t
;; End:

bug#36830: 26.2; find-file-visit-truename is not honored as file local variable

[Lars Ingebrigtsen]

Gustavo Barros <gusbrs.2016@gmail.com> writes:

> Sure. The file from which I reported, and which I include here now, was 
> just a dummy file with the variable of interest set to true with 
> `add-file-local-variable`:

Thanks; I'm able to reproduce the bug in Emacs 27, too.

I'm not sure what the fix is, though.  Here's how it's set:

(defun find-file-noselect-1 (buf filename nowarn rawfile truename number)

[...]

      (if find-file-visit-truename
	  (setq buffer-file-name (expand-file-name buffer-file-truename)))

[...]

	(after-find-file error (not nowarn)))

`after-find-file' is the function that interprets the file local
variables, so we're setting the buffer file name before we've set that
variable locally.

One option would be to re-check the variable after `after-find-file',
but that seems a bit hacky.

Any opinions?

diff --git a/lisp/files.el b/lisp/files.el
index f76635017d..bde8a466d0 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -2413,7 +2413,11 @@ find-file-noselect-1
 	    (setq buffer-file-coding-system 'no-conversion)
 	    (set-buffer-major-mode buf)
 	    (setq-local find-file-literally t))
-	(after-find-file error (not nowarn)))
+	(after-find-file error (not nowarn))
+        ;; In case `find-file-visit-truename' is set as a file-local
+        ;; variable, recompute the buffer file name.
+        (when find-file-visit-truename
+	  (setq buffer-file-name (expand-file-name buffer-file-truename))))
       (current-buffer))))
 \f
 (defun insert-file-contents-literally (filename &optional visit beg end replace)


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

bug#36830: 26.2; find-file-visit-truename is not honored as file local variable

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Fri, 23 Aug 2019 20:59:55 +0200
> Cc: 36830@debbugs.gnu.org
> 
>       (if find-file-visit-truename
> 	  (setq buffer-file-name (expand-file-name buffer-file-truename)))
> 
> [...]
> 
> 	(after-find-file error (not nowarn)))
> 
> `after-find-file' is the function that interprets the file local
> variables, so we're setting the buffer file name before we've set that
> variable locally.
> 
> One option would be to re-check the variable after `after-find-file',
> but that seems a bit hacky.
> 
> Any opinions?

I don't think this variable was designed to be set from file-local
variables block.  Visiting a file and naming its buffer are two racy
actions, and where there's a race there will be chicken-and-egg type
of problems.

> --- a/lisp/files.el
> +++ b/lisp/files.el
> @@ -2413,7 +2413,11 @@ find-file-noselect-1
>  	    (setq buffer-file-coding-system 'no-conversion)
>  	    (set-buffer-major-mode buf)
>  	    (setq-local find-file-literally t))
> -	(after-find-file error (not nowarn)))
> +	(after-find-file error (not nowarn))
> +        ;; In case `find-file-visit-truename' is set as a file-local
> +        ;; variable, recompute the buffer file name.
> +        (when find-file-visit-truename
> +	  (setq buffer-file-name (expand-file-name buffer-file-truename))))

I expect such renaming to cause future bugs, FWIW.  Or maybe not, but
this is beyond hacky, IMO.

Maybe we should just document that this variable cannot be file-local.

bug#36830: 26.2; find-file-visit-truename is not honored as file local variable

[Gustavo Barros]

Hi Eli, Hi Lars,

On Fri, Aug 23 2019, Eli Zaretskii wrote:

>> From: Lars Ingebrigtsen <larsi@gnus.org>
>> Date: Fri, 23 Aug 2019 20:59:55 +0200
>> Cc: 36830@debbugs.gnu.org
>> 
>>       (if find-file-visit-truename
>> 	  (setq buffer-file-name (expand-file-name 
>> buffer-file-truename)))
>> 
>> [...]
>> 
>> 	(after-find-file error (not nowarn)))
>> 
>> `after-find-file' is the function that interprets the file local
>> variables, so we're setting the buffer file name before we've set 
>> that
>> variable locally.
>> 
>> One option would be to re-check the variable after `after-find-file',
>> but that seems a bit hacky.
>> 
>> Any opinions?
>
> I don't think this variable was designed to be set from file-local
> variables block.  Visiting a file and naming its buffer are two racy
> actions, and where there's a race there will be chicken-and-egg type
> of problems.
>
>> --- a/lisp/files.el
>> +++ b/lisp/files.el
>> @@ -2413,7 +2413,11 @@ find-file-noselect-1
>>  	    (setq buffer-file-coding-system 'no-conversion)
>>  	    (set-buffer-major-mode buf)
>>  	    (setq-local find-file-literally t))
>> -	(after-find-file error (not nowarn)))
>> +	(after-find-file error (not nowarn))
>> +        ;; In case `find-file-visit-truename' is set as a file-local
>> +        ;; variable, recompute the buffer file name.
>> +        (when find-file-visit-truename
>> +	  (setq buffer-file-name (expand-file-name 
>> buffer-file-truename))))
>
> I expect such renaming to cause future bugs, FWIW.  Or maybe not, but
> this is beyond hacky, IMO.
>
> Maybe we should just document that this variable cannot be file-local.

I admittedly reported this from a user perspective, and do not fully 
grasp what is going on in this case.  But I think I can add something 
here if I point that `vc-follow-symlinks` does work as a file local 
variable.  Maybe I’m wrong in comparing these two variables, but if not, 
this could both provide some approach that works for this purpose, and 
could perhaps reduce the concerns raised by Eli.

I may also add that I would hardly set `find-file-visit-truename` 
globally, but I would find it useful locally (I don’t claim to be any 
reference in this respect though).  Furthermore, if the race mentioned 
by Eli does prove to be insurmountable, and if there is any difference 
between setting this as "file-local" or as "dir-local", the latter could 
prove useful even if the former could not be maintained.  (I don’t know 
if dir-local variables are set before the file is actually visited, but 
that is what I have in mind in mentioning this.  If that is the case, 
the race might be thus circumvented.)

Best,
Gustavo.

bug#36830: 26.2; find-file-visit-truename is not honored as file local variable

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> I don't think this variable was designed to be set from file-local
> variables block.  Visiting a file and naming its buffer are two racy
> actions, and where there's a race there will be chicken-and-egg type
> of problems.

[...]

> I expect such renaming to cause future bugs, FWIW.  Or maybe not, but
> this is beyond hacky, IMO.

Yes, I think so, too, but:

> Maybe we should just document that this variable cannot be file-local.

files.el has this:

(put 'find-file-visit-truename 'safe-local-variable 'booleanp)

It was changed to booleanp in 2007 (from the presumably invalid
`boolean'), so it didn't work before 2007 for that reason, and it hasn't
worked after 2007 because it's checked too late.

So perhaps the fix here is to just remove that `put'?

On the other hand, it would be nice if it worked, because it seems like
a pretty useful thing to be able to customise on a per-file basis.
Perhaps.

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

bug#36830: 26.2; find-file-visit-truename is not honored as file local variable

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: gusbrs.2016@gmail.com,  36830@debbugs.gnu.org
> Date: Sun, 25 Aug 2019 07:39:29 +0200
> 
> > Maybe we should just document that this variable cannot be file-local.
> 
> files.el has this:
> 
> (put 'find-file-visit-truename 'safe-local-variable 'booleanp)
> 
> It was changed to booleanp in 2007 (from the presumably invalid
> `boolean'), so it didn't work before 2007 for that reason, and it hasn't
> worked after 2007 because it's checked too late.
> 
> So perhaps the fix here is to just remove that `put'?

Fine with me.

> On the other hand, it would be nice if it worked, because it seems like
> a pretty useful thing to be able to customise on a per-file basis.

I agree.  If someone can come up with a way to resolve the race, I'm
all ears.

We have similar problems in startup.el, with variables that depend on
potentially customizable other variables, and the solutions are... not
pretty and quite fragile.  In particular, that kind of problems was
the main reason why we introduced the early-init file.

bug#36830: 26.2; find-file-visit-truename is not honored as file local variable

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> So perhaps the fix here is to just remove that `put'?
>
> Fine with me.

OK; done, and while there were other general issues about ordering of
file-local variables discussed here, I think we're not going to make
further progress on that in this context, so I'm closing this bug
report.

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