bug#58446: 28.2; file-attribute-device-number returns a cons cell instead of an integer

bug#58446: 28.2; file-attribute-device-number returns a cons cell instead of an integer

[Thierry Volpiatto]

The device-number in file-attributes (nth 11) is a cons cell when called
on remote files (see tramp-get-device).
It would be great to notify this in Emacs documentation and how to
interpret this value e.g. (-1 . 1).
I noticed this in fixing a bug in dired-async.el, the code was using `=`
to compare the two values which is legitimate according to docstring.


In GNU Emacs 28.2 (build 1, x86_64-pc-linux-gnu, Motif Version 2.3.8, cairo version 1.16.0)
 of 2022-09-12 built on IPad-S340
Windowing system distributor 'The X.Org Foundation', version 11.0.12013000
System Description: Linux Mint 20.3

Configured using:
 'configure CFLAGS=-O8 --with-mailutils --with-cairo --without-dbus
 --without-gconf --without-gsettings --with-x-toolkit=motif'

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

Important settings:
  value of $LANG: fr_FR.UTF-8
  locale-coding-system: utf-8-unix

Major mode: ƐĽ

Minor modes in effect:
  bug-reference-prog-mode: t
  global-undo-tree-mode: t
  undo-tree-mode: t
  psession-mode: t
  psession-savehist-mode: t
  global-git-gutter-mode: t
  git-gutter-mode: t
  display-time-mode: t
  winner-mode: t
  helm-epa-mode: t
  helm-descbinds-mode: t
  helm-adaptive-mode: t
  helm-mode: t
  helm-minibuffer-history-mode: t
  helm-ff-icon-mode: t
  shell-dirtrack-mode: t
  helm-popup-tip-mode: t
  async-bytecomp-package-mode: t
  dired-async-mode: t
  minibuffer-depth-indicate-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-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
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(epa-mail face-remap addressbook-bookmark tv-mu4e-config mu4e-contrib
mu4e-patch mu4e mu4e-org mu4e-main mu4e-view gnus-art mm-uu mml2015
mm-view mml-smime smime dig gnus-sum gnus-group gnus-undo gnus-start
gnus-dbus gnus-cloud nnimap nnmail mail-source utf7 netrc nnoo gnus-spec
gnus-int gnus-range gnus-win mu4e-headers mu4e-compose mu4e-draft
mu4e-actions smtpmail mu4e-search mu4e-lists mu4e-bookmarks mu4e-mark
mu4e-message shr kinsoku svg flow-fill hl-line mu4e-contacts mu4e-update
mu4e-folders mu4e-server mu4e-context mu4e-obsolete mu4e-vars
mu4e-helpers mu4e-config ido shadow sort mail-extr helm-ring eieio-opt
speedbar ezimage dframe shortdoc help-fns radix-tree emacsbug sendmail
cl-print etags fileloop generator xref tabify jka-compr helm-command
image-file image-converter tramp-cache epa-file char-fold em-unix
em-term term disp-table ehelp em-script em-prompt em-ls em-hist em-pred
em-glob em-cmpl em-dirs esh-var em-basic em-banner em-alias esh-mode
eshell esh-cmd esh-ext esh-opt esh-proc esh-io esh-arg esh-module
esh-groups esh-util esh-toggle tramp-archive tramp-gvfs dbus
helm-x-files helm-for-files markdown-mode flymake-shellcheck
flymake-proc flymake project warnings sh-script smie executable
bug-reference naquadah-theme view solar cal-dst holidays hol-loaddefs
tv-utils osm dom yaml-mode undo-tree diff queue rainbow-mode color
psession frameset log-view pcvs-util bash-completion cl-indent pcase
ffap thingatpt autocrypt-message message rmc puny rfc822 mml mml-sec
mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils
mailheader autocrypt-gnus gnus nnheader gnus-util rmail rmail-loaddefs
rfc2047 rfc2045 mail-utils mm-util mail-prsvr autocrypt-mu4e autocrypt
ietf-drums config-w3m git-gutter mule-util appt diary-lib diary-loaddefs
gud wdired dired-extension org-config ob-gnuplot org-crypt net-utils
time winner autotest-mode autoconf-mode woman man ediff ediff-merg
ediff-mult ediff-wind ediff-diff ediff-help ediff-init ediff-util
init-helm helm-ls-git vc-git diff-mode vc vc-dispatcher helm-fd epa
derived epg rfc6068 epg-config helm-epa helm-imenu imenu
helm-elisp-package helm-find helm-org org ob ob-tangle ob-ref ob-lob
ob-table ob-exp org-macro org-footnote org-src ob-comint org-pcomplete
org-list org-faces org-entities noutline outline org-version
ob-emacs-lisp ob-core ob-eval org-table oc-basic bibtex ol rx org-keys
oc org-compat advice org-macs org-loaddefs cal-menu calendar
cal-loaddefs helm-external isl helm-descbinds helm-wikipedia
all-the-icons all-the-icons-faces data-material data-weathericons
data-octicons data-fileicons data-faicons data-alltheicons cus-edit
wid-edit wfnames helm-ipython helm-elisp helm-eval edebug backtrace
find-func python tramp-sh popup helm-bookmark helm-net xml helm-info
bookmark pp helm-adaptive helm-mode helm-misc helm-files image-dired
image-mode exif filenotify tramp tramp-loaddefs trampver
tramp-integration files-x tramp-compat shell pcomplete parse-time
iso8601 time-date ls-lisp helm-buffers helm-occur helm-tags helm-locate
helm-grep wgrep-helm wgrep grep compile text-property-search comint ring
helm-regexp format-spec ansi-color helm-utils helm-help helm-types
helm-extensions-autoloads helm-config helm-autoloads helm
helm-global-bindings helm-easymenu helm-core async-bytecomp helm-source
helm-multi-match helm-lib dired-async dired-aux dired dired-loaddefs
async diminish cl-extra help-mode mb-depth server edmacro kmacro avoid
cus-load use-package use-package-ensure use-package-delight
use-package-diminish use-package-bind-key bind-key easy-mmode
use-package-core finder-inf package browse-url url url-proxy url-privacy
url-expand url-methods url-history url-cookie url-domsuf url-util
mailcap url-handlers url-parse auth-source cl-seq eieio eieio-core
cl-macs eieio-loaddefs password-cache json subr-x map url-vars seq
byte-opt gv bytecomp byte-compile cconv cl-loaddefs cl-lib info w3m-load
iso-transl tooltip eldoc paren electric uniquify ediff-hook vc-hooks
lisp-float-type elisp-mode 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 lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock
font-lock syntax 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 emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads inotify
lcms2 dynamic-setting font-render-setting cairo motif x-toolkit x
multi-tty make-network-process emacs)

Memory information:
((conses 16 1834152 77310)
 (symbols 48 43794 4)
 (strings 32 346905 22367)
 (string-bytes 1 12371838)
 (vectors 16 89769)
 (vector-slots 8 1894146 124249)
 (floats 8 3996 656)
 (intervals 56 347767 1990)
 (buffers 992 115))
<#secure method=pgpmime mode=sign>

-- 
Thierry

bug#58446: 28.2; file-attribute-device-number returns a cons cell instead of an integer

[Lars Ingebrigtsen]

Thierry Volpiatto <thievol@posteo.net> writes:

> The device-number in file-attributes (nth 11) is a cons cell when called
> on remote files (see tramp-get-device).
> It would be great to notify this in Emacs documentation and how to
> interpret this value e.g. (-1 . 1).
> I noticed this in fixing a bug in dired-async.el, the code was using `=`
> to compare the two values which is legitimate according to docstring.

Hm...  would it be possible for Tramp to stash that information
somewhere else?  It is (as you say) documented to be a number.

bug#58446: 28.2; file-attribute-device-number returns a cons cell instead of an integer

[Michael Albinus]

Lars Ingebrigtsen <larsi@gnus.org> writes:

Hi Lars,

>> The device-number in file-attributes (nth 11) is a cons cell when called
>> on remote files (see tramp-get-device).
>> It would be great to notify this in Emacs documentation and how to
>> interpret this value e.g. (-1 . 1).
>> I noticed this in fixing a bug in dired-async.el, the code was using `=`
>> to compare the two values which is legitimate according to docstring.
>
> Hm...  would it be possible for Tramp to stash that information
> somewhere else?  It is (as you say) documented to be a number.

Yes, Tramp is out of the documented API in this case. And no, it cannot
be kept somewhere else.

Inodes are unique only on the given file system. That's why there are
device numbers, which identify a file system.

A remote file is not related to any (local) file system. That's why
ange-ftp returns -1 as device-number, always. This is good enough for
practical purposes, because it is different to any device number of a
file system visible on the local host. For the inode number, ange-ftp
uses a "virtual" one, which means for every accessed file it increases
its internal counter. By this, all files accessed by ange-ftp are
regarded as different if they differ in the remote file name, even if
they are equal.

Tramp did inherit this approach from ange-ftp, with the difference that
it uses virtual inode numbers only in case it couldn't determine the
real inode number. It has its own counter for virtual inode numbers,
divided from ange-ftp.

This had several problems. The device number of all connected remote
devices, be it via any Tramp method or via ange-ftp, was always the
same. Remote files were regarded as equal if just the inode number was
the same, be it a real inode number or a virtual number.

Therefore, in January 2003 (almost 20 years ago :-) the device number in
Tramp has changed. It is now a cons cell (-1 . REMOTE), with REMOTE
being a virtual device number generated by Tramp for very connection.

By this, the tupel (inode, device) still identifies a file uniquely,
being it local or remote.

As said, this change happend almost 20 years ago. Device numbers are
usually ignored by Emacs and external packages, I don't remember a
problem report for this over the years. Emacs core creates the
buffer-local variable buffer-file-number, which is indeed (INODE
DEVNUM). See basic-save-buffer:

	      (setq buffer-file-number
		    (nthcdr 10 (file-attributes buffer-file-name)))

And this variable is used for checking whether two files are equal, see
find-buffer-visiting:

                            (equal buffer-file-number number)

That's why it works also with Tramp's interpretation of device
numbers. And again, w/o blame over the years.

I tend to agree with Thierry: we shall document the status quo,
i.e. device numbers can be more than just an integer, and should be
compared via equal. The details of that structure don't matter I
believe.

And perhaps we could even add a helper function to extract the
information for a file's uniqueness:

(defsubst file-attribute-file-number (attributes)
  "The inode and device numbers in ATTRIBUTES returned by `file-attributes'.
It can be used to determine whether two files are identical."
  (nthcdr 10 attributes))

WDYT?

Best regards, Michael.

bug#58446: 28.2; file-attribute-device-number returns a cons cell instead of an integer

[Lars Ingebrigtsen]

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

[explanation snipped]

> As said, this change happend almost 20 years ago. Device numbers are
> usually ignored by Emacs and external packages, I don't remember a
> problem report for this over the years. Emacs core creates the
> buffer-local variable buffer-file-number, which is indeed (INODE
> DEVNUM). See basic-save-buffer:
>
> 	      (setq buffer-file-number
> 		    (nthcdr 10 (file-attributes buffer-file-name)))
>
> And this variable is used for checking whether two files are equal, see
> find-buffer-visiting:
>
>                             (equal buffer-file-number number)
>
> That's why it works also with Tramp's interpretation of device
> numbers. And again, w/o blame over the years.

Yes, I agree -- we should probably just document this.  But I was
wondering whether it would be useful to make the `file-attribute'
structure extendable?  We could have a twelfth (optional) element that
could be an plist of properties, and then we'd have a place to stash
stuff.

(That might break stuff, though.)

But if we haven't needed that until now, we're probably not going to
need it going forward, and we should just document the current status
quo. 

> And perhaps we could even add a helper function to extract the
> information for a file's uniqueness:
>
> (defsubst file-attribute-file-number (attributes)
>   "The inode and device numbers in ATTRIBUTES returned by `file-attributes'.
> It can be used to determine whether two files are identical."
>   (nthcdr 10 attributes))
>
> WDYT?

Makes sense to me.

bug#58446: 28.2; file-attribute-device-number returns a cons cell instead of an integer

[Eli Zaretskii]

> Cc: Thierry Volpiatto <thievol@posteo.net>, 58446@debbugs.gnu.org
> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Thu, 13 Oct 2022 08:31:06 +0200
> 
> Michael Albinus <michael.albinus@gmx.de> writes:
> 
> But if we haven't needed that until now, we're probably not going to
> need it going forward, and we should just document the current status
> quo. 
> 
> > And perhaps we could even add a helper function to extract the
> > information for a file's uniqueness:
> >
> > (defsubst file-attribute-file-number (attributes)
> >   "The inode and device numbers in ATTRIBUTES returned by `file-attributes'.
> > It can be used to determine whether two files are identical."
> >   (nthcdr 10 attributes))
> >
> > WDYT?
> 
> Makes sense to me.

Agreed.

bug#58446: 28.2; file-attribute-device-number returns a cons cell instead of an integer

[Michael Albinus]

Version: 29.1

Eli Zaretskii <eliz@gnu.org> writes:

>> But if we haven't needed that until now, we're probably not going to
>> need it going forward, and we should just document the current status
>> quo.
>>
>> > And perhaps we could even add a helper function to extract the
>> > information for a file's uniqueness:
>> >
>> > (defsubst file-attribute-file-number (attributes)
>> >   "The inode and device numbers in ATTRIBUTES returned by `file-attributes'.
>> > It can be used to determine whether two files are identical."
>> >   (nthcdr 10 attributes))
>> >
>> > WDYT?
>>
>> Makes sense to me.
>
> Agreed.

Pushed to master. Closing the bug.

Best regards, Michael.

bug#58446: 28.2; file-attribute-device-number returns a cons cell instead of an integer

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> (defsubst file-attribute-file-number (attributes)
>   "The inode and device numbers in ATTRIBUTES returned by `file-attributes'.
> It can be used to determine whether two files are identical."
>   (nthcdr 10 attributes))
>
> WDYT?

Sounds good, except:

- The name `file-attribute-file-number` doesn't sound right
  because it doesn't return a number.

- I wouldn't use `defsubst` (so it can more easily be modified in the
  future, e.g. in case we add more fields to the attributes or use some
  other representation for attributes).

- I would document it more abstractly, mentioning inode and device
  number only as *examples* of things it might contain.


        Stefan

bug#58446: 28.2; file-attribute-device-number returns a cons cell instead of an integer

[Michael Albinus]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

Hi Stefan,

>> (defsubst file-attribute-file-number (attributes)
>>   "The inode and device numbers in ATTRIBUTES returned by `file-attributes'.
>> It can be used to determine whether two files are identical."
>>   (nthcdr 10 attributes))
>>
>> WDYT?
>
> Sounds good, except:
>
> - The name `file-attribute-file-number` doesn't sound right
>   because it doesn't return a number.

The name is a reminiscence of the existing variable buffer-file-number,
which serves exactly the same purpose.

> - I wouldn't use `defsubst` (so it can more easily be modified in the
>   future, e.g. in case we add more fields to the attributes or use some
>   other representation for attributes).

All other accessor functions for file-attributes are defsubsts.

> - I would document it more abstractly, mentioning inode and device
>   number only as *examples* of things it might contain.

There is no intention to use it for anything else. It shall return
(nthcdr 10 attributes) like all the other file-attributes accessor
functions return for the respective slots.

>         Stefan

Best regards, Michael.

bug#58446: 28.2; file-attribute-device-number returns a cons cell instead of an integer

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> - The name `file-attribute-file-number` doesn't sound right
>>   because it doesn't return a number.
> The name is a reminiscence of the existing variable buffer-file-number,
> which serves exactly the same purpose.

One error doesn't justify another.
Maybe a better name would be "file identifier"?

>> - I wouldn't use `defsubst` (so it can more easily be modified in the
>>   future, e.g. in case we add more fields to the attributes or use some
>>   other representation for attributes).
> All other accessor functions for file-attributes are defsubsts.

Every `defsubst` should be judged on its own individual value.
This one doesn't seem to be justified.

>> - I would document it more abstractly, mentioning inode and device
>>   number only as *examples* of things it might contain.
> There is no intention to use it for anything else.  It shall return
> (nthcdr 10 attributes) like all the other file-attributes accessor
> functions return for the respective slots.

I did not suggest changing its implementation.  Only its documentation.
The doc should describe the intended semantics of the return value
without documenting how it's implemented.


        Stefan

bug#58446: 28.2; file-attribute-device-number returns a cons cell instead of an integer

[Michael Albinus]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

Hi Stefan,

>>> - The name `file-attribute-file-number` doesn't sound right
>>>   because it doesn't return a number.
>> The name is a reminiscence of the existing variable buffer-file-number,
>> which serves exactly the same purpose.
>
> One error doesn't justify another.
> Maybe a better name would be "file identifier"?

True, but I'd let decide the maintainers.

>>> - I wouldn't use `defsubst` (so it can more easily be modified in the
>>>   future, e.g. in case we add more fields to the attributes or use some
>>>   other representation for attributes).
>> All other accessor functions for file-attributes are defsubsts.
>
> Every `defsubst` should be judged on its own individual value.
> This one doesn't seem to be justified.

As I said, it should be an "accessor function" for the result of
file-attributes. Just a stupid one, which returns inode and
device. That's the intention, and not something more sophisticated about
identifying a file.

>>> - I would document it more abstractly, mentioning inode and device
>>>   number only as *examples* of things it might contain.
>> There is no intention to use it for anything else.  It shall return
>> (nthcdr 10 attributes) like all the other file-attributes accessor
>> functions return for the respective slots.
>
> I did not suggest changing its implementation.  Only its documentation.
> The doc should describe the intended semantics of the return value
> without documenting how it's implemented.

The intended semantics is what's described. I understand that you see
something more for the future. I don't see it, sorry. No other use case
I could think of.

We have file-equal-p for more sophisticated checks, and this has even
file name handler support.

>         Stefan

Best regards, Michael.

bug#58446: 28.2; file-attribute-device-number returns a cons cell instead of an integer

[Eli Zaretskii]

> Cc: Thierry Volpiatto <thievol@posteo.net>, Lars Ingebrigtsen <larsi@gnus.org>,
>  58446@debbugs.gnu.org
> From: Michael Albinus <michael.albinus@gmx.de>
> Date: Fri, 14 Oct 2022 21:03:15 +0200
> 
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> 
> Hi Stefan,
> 
> >>> - The name `file-attribute-file-number` doesn't sound right
> >>>   because it doesn't return a number.
> >> The name is a reminiscence of the existing variable buffer-file-number,
> >> which serves exactly the same purpose.
> >
> > One error doesn't justify another.
> > Maybe a better name would be "file identifier"?
> 
> True, but I'd let decide the maintainers.

It would be better, yes.  I already changed "device number" to eithe
just "device" or "device identifier", in the documentation you touched
today, since talking about "device number" when it can be a cons of 2
numbers doesn't sound like a good idea.

bug#58446: 28.2; file-attribute-device-number returns a cons cell instead of an integer

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

Hi Eli,

> It would be better, yes.  I already changed "device number" to eithe
> just "device" or "device identifier", in the documentation you touched
> today, since talking about "device number" when it can be a cons of 2
> numbers doesn't sound like a good idea.

I've changed the name to file-attribute-file-identifier. Pushed to master.

Best regards, Michael.