bug#61901: 30.0.50; [PATCH v3] Add safe-local-variable-directories variable.

[Eli Zaretskii <eliz@gnu.org>]

> Date: Tue, 25 Apr 2023 16:40:07 +0000
> From:  Antero Mejr via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> Updated safe-local-variable-directories patch onto master and added bug
> number to commit message.

Thanks, see some comments below.

> Also should I use git --reroll-count to make v2 patches, v3, etc?

You don't have to.  The version part is removed by "git am" anyway,
and it is not important for patch review here.

> --- a/doc/lispref/variables.texi
> +++ b/doc/lispref/variables.texi
> @@ -1977,6 +1977,13 @@ this can be controlled by using this variable, which is a list of
>  symbols.
>  @end defvar
>  
> +@defvar safe-local-variable-directories
> +This is a list of directories where local variables are always enabled.
> +Directory-local variables loaded from these directories, such as the
> +variables in @file{.dir-locals.el}, will be enabled even if they are
> +risky.
> +@end defvar

This variable should also be documented in the Emacs user manual, not
only in the ELisp Reference manual -- it's a user option, and a very
important one at that.

> ++++
> +** New variable 'safe-local-variable-directories'.
> +This variable is used to to permanently trust directories containing
> +risky directory-local variables.

I would rephrase:

  This variable names directories in which Emacs will treat all
  directory-local variables as safe.

>  ALL-VARS is the list of all variables to be set up.
> @@ -3734,7 +3744,9 @@ 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.)
>  i  -- to ignore the local variables list, and permanently mark these
> -      values (*) as ignored\n\n")
> +      values (*) as ignored
> ++  -- to apply the local variables list, and permanently trust all
> +      directory-local variables in this directory\n\n")

I would remove the "permanently" part, it would just confuse here.

> @@ -3908,6 +3924,7 @@ DIR-NAME is the name of the associated directory.  Otherwise it is nil."
>  		  (null unsafe-vars)
>  		  (null risky-vars))
>  	     (memq enable-local-variables '(:all :safe))
> +             (member dir-name safe-local-variable-directories)

If you use 'member' for this test, then (a) the documentation of
safe-local-variable-directories should explicitly say that the
directories in the list must be in full absolute form, and (b) we
should consider the various issues with file names that are not
'equal' as strings, but still name the same directory, such as
letter-case differences on case-insensitive filesystems.  And what
about equality of "foo/" "and "foo"?

Also, is 'dir-name' above guaranteed to be a fully-expanded absolute
file name?

> +(ert-deftest files-tests-safe-local-variable-directories ()
> +  ;; safe-local-variable-directories should be risky,
> +  ;; so use it as an arbitrary risky variable.
> +  (let ((test-alist '((safe-local-variable-directories . "some_val")))
> +        (fakedir "test1/test2")
> +        (enable-local-eval t))
> +    (with-temp-buffer
> +      (setq safe-local-variable-directories (list fakedir))

The test should use absolute directory names for directories you put
into safe-local-variable-directories.