bug#32090: 26.1; connection-local-variables do not match as described

bug#32090: 26.1; connection-local-variables do not match as described

[Christopher Cooper]

According to the docstring for 'connection-local-criteria-alist',
"CRITERIA is a plist identifying a connection and the application
using this connection. ... All properties are optional; if
CRITERIA is nil, it always applies."

This does not appear to be true. In fact, this is plainly evident by
simple visual inspection of 'connection-local-get-profiles'. Only
:application seems to be optional.

Reproduction is quite simple. Run emacs -Q and eval the following elisp:

(defvar test--var nil)
(setf enable-connection-local-variables t)
(connection-local-set-profile-variables 'test-profile '((test--var . t)))
(connection-local-set-profiles nil 'test-profile)

Now open any TRAMP connection. Switch to the TRAMP buffer (where
connection-local-variables should have taken effect) and do
M-: test--var. The result is nil.

If you conduct the same test, but specify the :protocol, :user, and
:machine as part of the criteria in the 'connection-local-set-profiles'
call, test--var will evaluate to t, which is correct.

At best this is a case of misleading documentation, and at worst it is a
feature that never got implemented. Connection-local variables would be
very useful to me if they worked as described. I am glad to provide more
info or help in any capacity.


In GNU Emacs 26.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.30)
 of 2018-07-05 built on juergen
Windowing system distributor 'The X.Org Foundation', version 11.0.12000000
System -e Description:    Arch Linux

Configured using:
 'configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
 --localstatedir=/var --with-x-toolkit=gtk3 --with-xft --with-modules
 'CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong
 -fno-plt' CPPFLAGS=-D_FORTIFY_SOURCE=2
 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now'

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

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Magit

Minor modes in effect:
  erc-truncate-mode: t
  erc-spelling-mode: t
  erc-notify-mode: t
  erc-list-mode: t
  erc-menu-mode: t
  erc-autojoin-mode: t
  erc-ring-mode: t
  erc-networks-mode: t
  erc-pcomplete-mode: t
  erc-track-mode: t
  erc-track-minor-mode: t
  erc-match-mode: t
  erc-button-mode: t
  erc-fill-mode: t
  erc-stamp-mode: t
  erc-netsplit-mode: t
  erc-irccontrols-mode: t
  erc-noncommands-mode: t
  erc-move-to-prompt-mode: t
  erc-readonly-mode: t
  global-magit-file-mode: t
  magit-auto-revert-mode: t
  global-git-commit-mode: t
  async-bytecomp-package-mode: t
  ido-vertical-mode: t
  smartparens-global-mode: t
  editorconfig-mode: t
  global-auto-complete-mode: t
  counsel-mode: t
  ivy-mode: t
  show-paren-mode: t
  global-anzu-mode: t
  anzu-mode: t
  volatile-highlights-mode: t
  global-linum-mode: t
  shell-dirtrack-mode: t
  recentf-mode: t
  diff-auto-refine-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
/home/cooperc/.emacs.d/elpa/git-commit-2.13.0/git-commit hides
/home/cooperc/.emacs.d/elpa/magit-2.13.0/git-commit

Features:
(sort mail-extr emacsbug sendmail shr svg xml dom cus-edit pcase avy
browse-url jka-compr erc-truncate erc-autoaway erc-spelling flyspell
ispell erc-log eieio-opt speedbar sb-image ezimage dframe help-fns
cl-print cus-start cus-load erc-notify apropos two-column iso-transl
erc-list erc-menu erc-join erc-ring erc-networks erc-pcomplete erc-track
erc-match erc-button erc-fill erc-stamp erc-netsplit erc-goodies erc
erc-backend erc-compat mm-archive network-stream starttls url-cache smex
magit-bookmark bookmark vc vc-dispatcher windmove misearch multi-isearch
bug-reference git-rebase editorconfig-core editorconfig-core-handle
editorconfig-fnmatch magit-version magit-obsolete magit-blame
magit-stash magit-bisect magit-remote magit-commit magit-sequence
magit-notes magit-worktree magit-tag magit-merge magit-branch
magit-reset magit-collab ghub let-alist magit-files magit-refs
magit-status magit magit-repos magit-apply magit-wip magit-log
which-func imenu magit-diff smerge-mode magit-core magit-autorevert
autorevert filenotify magit-process magit-margin magit-mode git-commit
magit-git magit-section magit-utils crm magit-popup log-edit message
rfc822 mml mml-sec epa derived epg gnus-util rmail rmail-loaddefs
mm-decode mm-bodies mm-encode mailabbrev mail-utils gmm-utils mailheader
pcvs-util add-log with-editor async-bytecomp async colir color
material-theme server paredit hl-todo highlight-symbol time dot-tests
buttercup ert pp ewoc debug buttercup-compat shadow kotct-loaddefs
user-hub machine-local user-config-system cg505-hub ido-vertical-mode
system-hub darwin code-hub language-hub tex go web-c sh java rust ruby
fish fish-mode elixir smartparens-elixir elixir-mode pkg-info url-http
tls gnutls url url-proxy url-privacy url-expand url-methods url-history
mailcap url-auth mail-parse rfc2231 rfc2047 rfc2045 mm-util ietf-drums
mail-prsvr url-cookie url-domsuf url-util url-gw nsm rmc puny epl
elixir-smie smie elisp csharp c smart-tabs-mode code-navigation
indentation smartparens-c smartparens-config smartparens-text
smartparens magit-c editorconfig-c editorconfig behavior-hub text
window-management pane-management completion-c flycheck json map
find-func rx subr-x dash flymake-proc flymake warnings
auto-complete-config auto-complete edmacro kmacro popup ido-grid-mode
ido counsel dired dired-loaddefs compile esh-util swiper ivy delsel
ivy-overlay ffap visual-hub theme text-visuals paren anzu thingatpt
etags xref project volatile-highlights frame-c startup-c linum-c
linum-off linum file-hub file-content tramp-c recentf-c tramp-cache
tramp-sh tramp tramp-compat tramp-loaddefs trampver ucs-normalize shell
pcomplete comint ansi-color ring parse-time format-spec advice recentf
tree-widget wid-edit backup package-hub cl-extra help-mode verification
cl repositories dependencies vc-git diff-mode easy-mmode time-date
elec-pair autoload radix-tree lisp-mnt finder-inf info package easymenu
epg-config url-handlers url-parse auth-source cl-seq eieio eieio-core
cl-macs eieio-loaddefs password-cache url-vars seq byte-opt gv bytecomp
byte-compile cconv cl-loaddefs cl-lib 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 dbusbind inotify lcms2 dynamic-setting system-font-setting
font-render-setting move-toolbar gtk x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 16 1489394 166969)
 (symbols 48 46723 1)
 (miscs 40 4164 3342)
 (strings 32 243899 19322)
 (string-bytes 1 6360239)
 (vectors 16 100112)
 (vector-slots 8 2176130 110892)
 (floats 8 409 1000)
 (intervals 56 139320 3213)
 (buffers 992 46))

bug#32090: 26.1; connection-local-variables do not match as described

[Michael Albinus]

Christopher Cooper <christopher.c.cooper@gmail.com> writes:

Hi Christopher,

> According to the docstring for 'connection-local-criteria-alist',
> "CRITERIA is a plist identifying a connection and the application
> using this connection. ... All properties are optional; if
> CRITERIA is nil, it always applies."
>
> This does not appear to be true. In fact, this is plainly evident by
> simple visual inspection of 'connection-local-get-profiles'. Only
> :application seems to be optional.
>
> Reproduction is quite simple. Run emacs -Q and eval the following elisp:
>
> (defvar test--var nil)
> (setf enable-connection-local-variables t)
> (connection-local-set-profile-variables 'test-profile '((test--var . t)))
> (connection-local-set-profiles nil 'test-profile)
>
> Now open any TRAMP connection. Switch to the TRAMP buffer (where
> connection-local-variables should have taken effect) and do
> M-: test--var. The result is nil.
>
> If you conduct the same test, but specify the :protocol, :user, and
> :machine as part of the criteria in the 'connection-local-set-profiles'
> call, test--var will evaluate to t, which is correct.
>
> At best this is a case of misleading documentation, and at worst it is a
> feature that never got implemented. Connection-local variables would be
> very useful to me if they worked as described. I am glad to provide more
> info or help in any capacity.

It is not misleading documentation. It is the Tramp implementation, how
connection-local variables are set. In
tramp-set-connection-local-variables, you'll see

--8<---------------cut here---------------start------------->8---
(tramp-compat-funcall
 'hack-connection-local-variables-apply
 `(:application tramp
   :protocol    ,(tramp-file-name-method vec)
   :user        ,(tramp-file-name-user-domain vec)
   :machine     ,(tramp-file-name-host-port vec)))))
--8<---------------cut here---------------end--------------->8---

That means, the Tramp implementation requires :application to be 'tramp
or nil, *and* it requires setting of :protocol, :user and :machine in
your criteria, which match the respective remote file name.

Connection-lcal variables in general work as expected. Eval in the
*scratch* buffer:

--8<---------------cut here---------------start------------->8---
(defvar test--var nil)
(setf enable-connection-local-variables t)
(connection-local-set-profile-variables 'test-profile '((test--var . t)))
(connection-local-set-profiles nil 'test-profile)

;; Check initial value.
(describe-variable 'test--var)

;; Set connection local variables for a random criteria.
(hack-connection-local-variables-apply '(:application foo))

;; Check the value, again.
(describe-variable 'test--var)
--8<---------------cut here---------------end--------------->8---

In fact, properties in criteria are optional only in case the
application allows this in its `hack-connection-local-variables-apply'
call, except for :application. This must either match, or be nil.

The Tramp manual gives you an example how to use connection-local
variables fo Tramp itself, see (info "(tramp) Remote processes") . I
agree, it should be more verbose to explain, how a criteria for Tramp
must look like. Will add it.

Best regards, Michael.

bug#32090: 26.1; connection-local-variables do not match as described

[Christopher Cooper]

Thank you for the speedy response!

On Sun, Jul 8, 2018 at 5:46 AM, Michael Albinus <michael.albinus@gmx.de> wrote:
> It is not misleading documentation. It is the Tramp implementation, how
> connection-local variables are set. In
> tramp-set-connection-local-variables, you'll see
>
> --8<---------------cut here---------------start------------->8---
> (tramp-compat-funcall
>  'hack-connection-local-variables-apply
>  `(:application tramp
>    :protocol    ,(tramp-file-name-method vec)
>    :user        ,(tramp-file-name-user-domain vec)
>    :machine     ,(tramp-file-name-host-port vec)))))
> --8<---------------cut here---------------end--------------->8---
>
> That means, the Tramp implementation requires :application to be 'tramp
> or nil, *and* it requires setting of :protocol, :user and :machine in
> your criteria, which match the respective remote file name.

This makes sense, except that I'm still confused on why :application
can be nil but :protocol, :user, and :machine cannot be.

> Connection-lcal variables in general work as expected. Eval in the
> *scratch* buffer:
>
> --8<---------------cut here---------------start------------->8---
> (defvar test--var nil)
> (setf enable-connection-local-variables t)
> (connection-local-set-profile-variables 'test-profile '((test--var . t)))
> (connection-local-set-profiles nil 'test-profile)
>
> ;; Check initial value.
> (describe-variable 'test--var)
>
> ;; Set connection local variables for a random criteria.
> (hack-connection-local-variables-apply '(:application foo))
>
> ;; Check the value, again.
> (describe-variable 'test--var)
> --8<---------------cut here---------------end--------------->8---

This works, but only because the criteria passed to
'hack-connection-local-variables-apply' does not supply a :machine,
:user, or :host.

> In fact, properties in criteria are optional only in case the
> application allows this in its `hack-connection-local-variables-apply'
> call, except for :application. This must either match, or be nil.

I think I am beginning to understand... it is essentially up to the
application to make the other properties optional. Obviously it
wouldn't make sense for the application to make :application optional,
so that is the only one that is optional everywhere.

> The Tramp manual gives you an example how to use connection-local
> variables fo Tramp itself, see (info "(tramp) Remote processes") . I
> agree, it should be more verbose to explain, how a criteria for Tramp
> must look like. Will add it.

The Tramp info page seems pretty fine to me. The real issue is in two places:

(info "(elisp) Connection Local Variables"):
> -- Function: connection-local-set-profiles criteria &rest profiles
>     ...
>     All properties are optional; if
>     CRITERIA is ‘nil’, it always applies.

But as determined above, if CRITERIA is 'nil', it does not always
apply. It is up to the application to provide this feature, and Tramp
does not.
In fact, there is even an example showing this feature, which appears
be plain wrong:

>     If CRITERIA is ‘nil’, it applies for all remote connections.
>     Therefore, the example above would be equivalent to
>
>          (connection-local-set-profiles
>            '(:application 'tramp :protocol "ssh" :machine "localhost")
>            'remote-bash)
>
>          (connection-local-set-profiles
>            '(:application 'tramp :protocol "sudo"
>              :user "root" :machine "localhost")
>            'remote-ksh)
>
>          (connection-local-set-profiles
>            nil 'remote-null-device)

At least with current Tramp, the 'remote-null-device profile will
never apply, since it does not provide the properties needed by Tramp.
This also means that this example is not equivalent to the above
example as stated.

The other place of confusion was the docstring for
'connection-local-criteria-alist' that I mentioned initially.

I think ultimately, it comes down to a confusion over what "optional"
means. I took it to mean: "It this property is provided, it will be
checked when matching. If not, we don't care about the value of this
property." And, as I stated earlier, I don't understand how the
sentence "If CRITERIA is 'nil', it always applies." is try in any
sense of those words. If the feature is working as intended, those are
the worst offenders as far as confusing documentation.

Once again, I appreciate this feature and your explanation here. I
hope this is helpful in clarifying the documentation.

Christopher

bug#32090: Fwd: bug#32090: 26.1; connection-local-variables do not match as described

[Michael Albinus]

-------------------- Start of forwarded message --------------------
From: Michael Albinus <michael.albinus@gmx.de>
Subject: Re: bug#32090: 26.1; connection-local-variables do not match as described
Date: Sun, 08 Jul 2018 17:16:42 +0200

Christopher Cooper <christopher.c.cooper@gmail.com> writes:

Hi Christoph,

>>> (tramp-compat-funcall
>>>  'hack-connection-local-variables-apply
>>>  `(:application tramp
>>>    :protocol    ,(tramp-file-name-method vec)
>>>    :user        ,(tramp-file-name-user-domain vec)
>>>    :machine     ,(tramp-file-name-host-port vec)))))
>>> --8<---------------cut here---------------end--------------->8---
>>>
>>> That means, the Tramp implementation requires :application to be 'tramp
>>> or nil, *and* it requires setting of :protocol, :user and :machine in
>>> your criteria, which match the respective remote file name.
>>
>> This makes sense, except that I'm still confused on why :application
>> can be nil but :protocol, :user, and :machine cannot be.

Well, being also the Tramp maintainer, I have designed connection-local
variables with Tramp in mind :-)

Other packages have shown interest to use connection-local variables as
well (nnimap, gnutls, if I'm not mistaken), but they haven't implemented
it yet. And so there are no further feature requests.

>> The Tramp info page seems pretty fine to me. The real issue is in two places:
>>
>> (info "(elisp) Connection Local Variables"):
>>> -- Function: connection-local-set-profiles criteria &rest profiles
>>>     ...
>>>     All properties are optional; if
>>>     CRITERIA is ‘nil’, it always applies.
>>
>> But as determined above, if CRITERIA is 'nil', it does not always
>> apply. It is up to the application to provide this feature, and Tramp
>> does not.
>> In fact, there is even an example showing this feature, which appears
>> be plain wrong:
>>
>>>     If CRITERIA is ‘nil’, it applies for all remote connections.
>>>     Therefore, the example above would be equivalent to
>>>
>>>          (connection-local-set-profiles
>>>            '(:application 'tramp :protocol "ssh" :machine "localhost")
>>>            'remote-bash)
>>>
>>>          (connection-local-set-profiles
>>>            '(:application 'tramp :protocol "sudo"
>>>              :user "root" :machine "localhost")
>>>            'remote-ksh)
>>>
>>>          (connection-local-set-profiles
>>>            nil 'remote-null-device)
>>
>> At least with current Tramp, the 'remote-null-device profile will
>> never apply, since it does not provide the properties needed by Tramp.
>> This also means that this example is not equivalent to the above
>> example as stated.

Yes, that's an error. Will fix the documentation.

>> The other place of confusion was the docstring for
>> 'connection-local-criteria-alist' that I mentioned initially.
>>
>> I think ultimately, it comes down to a confusion over what "optional"
>> means. I took it to mean: "It this property is provided, it will be
>> checked when matching. If not, we don't care about the value of this
>> property." And, as I stated earlier, I don't understand how the
>> sentence "If CRITERIA is 'nil', it always applies." is try in any
>> sense of those words. If the feature is working as intended, those are
>> the worst offenders as far as confusing documentation.
>>
>> Once again, I appreciate this feature and your explanation here. I
>> hope this is helpful in clarifying the documentation.

Well, I'm also open to extend connection-local variables to other
application's need but Tramp. Proposals welcome!

The downside is, that such changes must go to the master branch (which
will become Emacs 27.1). The emacs-26 branch is open for bug fixing only.

>> Christopher

Best regards, Michael.
-------------------- End of forwarded message --------------------

bug#32090: 26.1; connection-local-variables do not match as described

[Michael Albinus]

Christopher Cooper <christopher.c.cooper@gmail.com> writes:

Hi Christopher,

>> That means, the Tramp implementation requires :application to be 'tramp
>> or nil, *and* it requires setting of :protocol, :user and :machine in
>> your criteria, which match the respective remote file name.
>
> This makes sense, except that I'm still confused on why :application
> can be nil but :protocol, :user, and :machine cannot be.
>
> I think ultimately, it comes down to a confusion over what "optional"
> means. I took it to mean: "It this property is provided, it will be
> checked when matching. If not, we don't care about the value of this
> property." And, as I stated earlier, I don't understand how the
> sentence "If CRITERIA is 'nil', it always applies." is try in any
> sense of those words. If the feature is working as intended, those are
> the worst offenders as far as confusing documentation.

Well, I've slept over this last night, and I've convinced myself that
you are right. Every property shall be optional, not only :application.
As documented.

So I've committed a patch to the emacs-26 branch. Maybe you have a
chance to check this out; otherwise the patch is accessible here:
<http://git.savannah.gnu.org/cgit/emacs.git/patch/?id=917158f8c91121572f38d641096e171540d0bac2>

> Once again, I appreciate this feature and your explanation here. I
> hope this is helpful in clarifying the documentation.

If you want see some more examples for intended use, see file
test/lisp/files-x-tests.el. Note, that I have adapted it as well for the
patch.

> Christopher

Best regards, Michael.

bug#32090: 26.1; connection-local-variables do not match as described

[Michael Albinus]

Michael Albinus <michael.albinus@gmx.de> writes:

Hi Christopher,

>>> That means, the Tramp implementation requires :application to be 'tramp
>>> or nil, *and* it requires setting of :protocol, :user and :machine in
>>> your criteria, which match the respective remote file name.
>>
>> This makes sense, except that I'm still confused on why :application
>> can be nil but :protocol, :user, and :machine cannot be.
>>
>> I think ultimately, it comes down to a confusion over what "optional"
>> means. I took it to mean: "It this property is provided, it will be
>> checked when matching. If not, we don't care about the value of this
>> property." And, as I stated earlier, I don't understand how the
>> sentence "If CRITERIA is 'nil', it always applies." is try in any
>> sense of those words. If the feature is working as intended, those are
>> the worst offenders as far as confusing documentation.
>
> Well, I've slept over this last night, and I've convinced myself that
> you are right. Every property shall be optional, not only :application.
> As documented.
>
> So I've committed a patch to the emacs-26 branch. Maybe you have a
> chance to check this out; otherwise the patch is accessible here:
> <http://git.savannah.gnu.org/cgit/emacs.git/patch/?id=917158f8c91121572f38d641096e171540d0bac2>
>
>> Once again, I appreciate this feature and your explanation here. I
>> hope this is helpful in clarifying the documentation.
>
> If you want see some more examples for intended use, see file
> test/lisp/files-x-tests.el. Note, that I have adapted it as well for the
> patch.

Are there still problems wfor you with this? Otherwise, I believe the
bug could be closed.

>> Christopher

Best regards, Michael.

bug#32090: 26.1; connection-local-variables do not match as described

[Glenn Morris]

Michael Albinus wrote:

> Are there still problems wfor you with this? Otherwise, I believe the
> bug could be closed.

FWIW, I strongly suggest adopting the reverse philosophy: when you think
a bug is fixed, close it, and invite the submitter to comment if it
seems unfixed. There are just too many reports to do it the other way.
A closed bug can still be corresponded with. (It gets archived after a
month with no comments, which seems quite appropriate to me.)

bug#32090: 26.1; connection-local-variables do not match as described

[Michael Albinus]

Glenn Morris <rgm@gnu.org> writes:

Hi Glenn,

> FWIW, I strongly suggest adopting the reverse philosophy: when you think
> a bug is fixed, close it, and invite the submitter to comment if it
> seems unfixed. There are just too many reports to do it the other way.
> A closed bug can still be corresponded with. (It gets archived after a
> month with no comments, which seems quite appropriate to me.)

I understand your concern. However, due to my daily job I'm socialized
to hear to testers carefully ...

I'm using the ELPA debbugs package for my Emacs workflow. Every bug I
care will be tagged locally. Showing the local tagged bugs (via
debbugs-gnu), I have all bugs of my todo list, just some few dozen. No
closed bug; when I bug is losed I remove the local tag.

When I fix a bug in the repository, I add the "fixed" marker, and ask
the OP for confirmation. If there is no answer for a while, a reminder
is sent. If there's still no confirmation, I close the bug.

Yes, fixed bugs need longer to be closed. But they are marked as fixed,
and they don't become forgotten  because they are still tagged
locally. I belief this is robust enough.

Best regards, Michael.

bug#32090: 26.1; connection-local-variables do not match as described

[Michael Albinus]

Version: 26.2

Michael Albinus <michael.albinus@gmx.de> writes:

Hi Christopher,

> Are there still problems wfor you with this? Otherwise, I believe the
> bug could be closed.

No news are good news. I'm closing the bug.

>>> Christopher

Best regards, Michael.