GNU bug report logs - #29225
Tramp backup-by-copying in a Converting ACL Invalid argument error on Windows 7

Previous Next

Package: emacs;

Reported by: Shuguang Sun <shuguang <at> gmail.com>

Date: Thu, 9 Nov 2017 08:25:01 UTC

Severity: normal

Fixed in version 26.1

Done: Michael Albinus <michael.albinus <at> gmx.de>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 29225 in the body.
You can then email your comments to 29225 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#29225; Package emacs. (Thu, 09 Nov 2017 08:25:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Shuguang Sun <shuguang <at> gmail.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Thu, 09 Nov 2017 08:25:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Shuguang Sun <shuguang <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Tramp backup-by-copying in a Converting ACL Invalid argument error on
 Windows 7
Date: Thu, 9 Nov 2017 16:24:39 +0800
[Message part 1 (text/plain, inline)]
Hi,

Windows 7
GNU Emacs 26.0.50 (build 1, x86_64-w64-mingw32) of 2017-08-27
(backup-by-copying t)

When I tramp to a host (linux) and edit a file, and save it, I get an error
about the backup as below. The ACL is in different format on Linus and on
Windows. It is "user::..." on linux, however, it looks like "O:S-1-5-21-"
on windows.

Could we allow the user to set whether to copy ACl for certain type of
files?

Debugger entered--Lisp error: (file-error "Converting ACL" "Invalid
argument"
"c:/Users/user/HOME/.emacs.d/autosave/Rfiles/!drive_c!plink!!user <at> host
!!!path!user!Ava!_mortality.R.~1~")

set-file-acl("c:/Users/user/HOME/.emacs.d/autosave/Rfiles/!drive_c!plink!!user <at> host!!!path!user!Ava!_mortality.R.~1~"
"user::rw-\ngroup::rw-\nother::r--\n")

set-file-extended-attributes("c:/Users/user/HOME/.emacs.d/autosave/Rfiles/!drive_c!plink!!user <at> host!!!path!user!Ava!_mortality.R.~1~"
((acl . "user::rw-\ngroup::rw-\nother::r--\n") (selinux-context nil nil nil
nil)))
  backup-buffer-copy("/plink:user <at> host:/path/user/Ava/_mortality.R"
"c:/Users/user/HOME/.emacs.d/autosave/Rfiles/!drive_c!plink!!user <at> host!!!path!user!Ava!_mortality.R.~1~"
436 ((acl . "user::rw-\ngroup::rw-\nother::r--\n") (selinux-context nil nil
nil nil)))
  backup-buffer()
  basic-save-buffer-2()
  basic-save-buffer-1()
  basic-save-buffer(t)
  save-buffer(1)
  funcall-interactively(save-buffer 1)
  call-interactively(save-buffer nil nil)
  command-execute(save-buffer)

Best Regards,
Shuguang
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29225; Package emacs. (Thu, 09 Nov 2017 17:19:02 GMT) Full text and rfc822 format available.

Message #8 received at 29225 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Shuguang Sun <shuguang <at> gmail.com>, Michael Albinus <michael.albinus <at> gmx.de>
Cc: 29225 <at> debbugs.gnu.org
Subject: Re: bug#29225: Tramp backup-by-copying in a Converting ACL Invalid
 argument error on Windows 7
Date: Thu, 09 Nov 2017 19:18:46 +0200
> From: Shuguang Sun <shuguang <at> gmail.com>
> Date: Thu, 9 Nov 2017 16:24:39 +0800
> 
> Could we allow the user to set whether to copy ACl for certain type of files?

That should work automatically.

> Debugger entered--Lisp error: (file-error "Converting ACL" "Invalid argument"
> "c:/Users/user/HOME/.emacs.d/autosave/Rfiles/!drive_c!plink!!user <at> host!!!path!user!Ava!_mortality.R.~1~")
> 
>   set-file-acl
> ("c:/Users/user/HOME/.emacs.d/autosave/Rfiles/!drive_c!plink!!user <at> host!!!path!user!Ava!_mortality.R.~1~"
> "user::rw-\ngroup::rw-\nother::r--\n")
>   set-file-extended-attributes
> ("c:/Users/user/HOME/.emacs.d/autosave/Rfiles/!drive_c!plink!!user <at> host!!!path!user!Ava!_mortality.R.~1~"
> ((acl . "user::rw-\ngroup::rw-\nother::r--\n") (selinux-context nil nil nil nil)))
>   backup-buffer-copy("/plink:user <at> host:/path/user/Ava/_mortality.R"
> "c:/Users/user/HOME/.emacs.d/autosave/Rfiles/!drive_c!plink!!user <at> host!!!path!user!Ava!_mortality.R.~1~"
> 436 ((acl . "user::rw-\ngroup::rw-\nother::r--\n") (selinux-context nil nil nil nil)))

This seems to indicate that Unix-style ACLs are passed to native
Windows ACL functions, which won't work.  Michael, could you please
look into this?  I'd expect ACLs derived from remote files never to be
used on local files.

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29225; Package emacs. (Fri, 10 Nov 2017 12:42:02 GMT) Full text and rfc822 format available.

Message #11 received at 29225 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 29225 <at> debbugs.gnu.org, Shuguang Sun <shuguang <at> gmail.com>
Subject: Re: bug#29225: Tramp backup-by-copying in a Converting ACL Invalid
 argument error on Windows 7
Date: Fri, 10 Nov 2017 13:41:04 +0100
Eli Zaretskii <eliz <at> gnu.org> writes:

Hi Eli,

>> Debugger entered--Lisp error: (file-error "Converting ACL" "Invalid argument"
>> "c:/Users/user/HOME/.emacs.d/autosave/Rfiles/!drive_c!plink!!user <at> host!!!path!user!Ava!_mortality.R.~1~")
>> 
>>   set-file-acl
>> ("c:/Users/user/HOME/.emacs.d/autosave/Rfiles/!drive_c!plink!!user <at> host!!!path!user!Ava!_mortality.R.~1~"
>> "user::rw-\ngroup::rw-\nother::r--\n")
>>   set-file-extended-attributes
>> ("c:/Users/user/HOME/.emacs.d/autosave/Rfiles/!drive_c!plink!!user <at> host!!!path!user!Ava!_mortality.R.~1~"
>> ((acl . "user::rw-\ngroup::rw-\nother::r--\n") (selinux-context nil nil nil nil)))
>>   backup-buffer-copy("/plink:user <at> host:/path/user/Ava/_mortality.R"
>> "c:/Users/user/HOME/.emacs.d/autosave/Rfiles/!drive_c!plink!!user <at> host!!!path!user!Ava!_mortality.R.~1~"
>> 436 ((acl . "user::rw-\ngroup::rw-\nother::r--\n") (selinux-context nil nil nil nil)))
>
> This seems to indicate that Unix-style ACLs are passed to native
> Windows ACL functions, which won't work.  Michael, could you please
> look into this?  I'd expect ACLs derived from remote files never to be
> used on local files.

And vice versa.

However, Tramp has no chance to do something here. It offers own
implementations of `file-acl' and `set-file-acl'; both functions don't
know where the returned ACL shall be used (´file-acl'), or where the ACL
comes from (`set-file-acl'). Therefore, the docstring of `set-file-acl'
says

"Value is t if setting of ACL was successful, nil otherwise."

The error message "Converting ACL" "Invalid argument" comes from
Ffile_acl of fileio.c. It *raises* an error instead of silently
returning Qnil, as advertised by the docstring. A similar wrong
behaviour I've found in `tramp-smb-handle-set-file-acl'.

I would convert both functions to return nil instead of raising an
error, in the emacs-26 branch. Any objection?

> Thanks.

Best regards, Michael.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29225; Package emacs. (Fri, 10 Nov 2017 13:08:01 GMT) Full text and rfc822 format available.

Message #14 received at 29225 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 29225 <at> debbugs.gnu.org, shuguang <at> gmail.com
Subject: Re: bug#29225: Tramp backup-by-copying in a Converting ACL Invalid
 argument error on Windows 7
Date: Fri, 10 Nov 2017 15:06:49 +0200
> From: Michael Albinus <michael.albinus <at> gmx.de>
> Cc: Shuguang Sun <shuguang <at> gmail.com>,  29225 <at> debbugs.gnu.org
> Date: Fri, 10 Nov 2017 13:41:04 +0100
> 
> "Value is t if setting of ACL was successful, nil otherwise."
> 
> The error message "Converting ACL" "Invalid argument" comes from
> Ffile_acl of fileio.c. It *raises* an error instead of silently
> returning Qnil, as advertised by the docstring. A similar wrong
> behaviour I've found in `tramp-smb-handle-set-file-acl'.
> 
> I would convert both functions to return nil instead of raising an
> error, in the emacs-26 branch. Any objection?

I think it should return nil when acl_errno_valid returns false, and
otherwise signal an error.  It currently calls acl_errno_valid in one
of the two places where error could happen, but not in the other.  And
the doc string should be amended to say that.  WDYT?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29225; Package emacs. (Fri, 10 Nov 2017 14:40:01 GMT) Full text and rfc822 format available.

Message #17 received at 29225 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 29225 <at> debbugs.gnu.org, shuguang <at> gmail.com
Subject: Re: bug#29225: Tramp backup-by-copying in a Converting ACL Invalid
 argument error on Windows 7
Date: Fri, 10 Nov 2017 15:39:39 +0100
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:

Hi Eli,

> I think it should return nil when acl_errno_valid returns false, and
> otherwise signal an error.  It currently calls acl_errno_valid in one
> of the two places where error could happen, but not in the other.  And
> the doc string should be amended to say that.  WDYT?

Seems to work, yes. acl_from_text returns EINVAL for wrong ACLs, so the
error would be suppressed, because acl_errno_valid returns false this case.

I've tested successfully the appended patch on GNU/Linux.

(set-file-acl "/tmp/123" "dummy")

returns nil with the applied patch, and (file-error "Converting ACL"
"Invalid argument" "/tmp/123") w/o the patch.

I cannot test on MS Windows.

I don't know whether we must extend the docstring. Summarizing probable
errors is not the default in Emacs docstrings. And the docstring does
not promise, that no error at all will happen.

If you don't object, I'll commit the patch to emacs-26. Plus minor
changes in Tramp. Maybe I'll find also the time next days, to add the
missing tests for file-acl and set-file-acl in tramp-tests.el. See the
TODO list at the end of that file.

Best regards, Michael.

[Message part 2 (text/plain, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29225; Package emacs. (Fri, 10 Nov 2017 15:14:02 GMT) Full text and rfc822 format available.

Message #20 received at 29225 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 29225 <at> debbugs.gnu.org, shuguang <at> gmail.com
Subject: Re: bug#29225: Tramp backup-by-copying in a Converting ACL Invalid
 argument error on Windows 7
Date: Fri, 10 Nov 2017 17:13:06 +0200
> From: Michael Albinus <michael.albinus <at> gmx.de>
> Cc: shuguang <at> gmail.com,  29225 <at> debbugs.gnu.org
> Date: Fri, 10 Nov 2017 15:39:39 +0100
> 
> > I think it should return nil when acl_errno_valid returns false, and
> > otherwise signal an error.  It currently calls acl_errno_valid in one
> > of the two places where error could happen, but not in the other.  And
> > the doc string should be amended to say that.  WDYT?
> 
> Seems to work, yes. acl_from_text returns EINVAL for wrong ACLs, so the
> error would be suppressed, because acl_errno_valid returns false this case.
> 
> I've tested successfully the appended patch on GNU/Linux.
> 
> (set-file-acl "/tmp/123" "dummy")
> 
> returns nil with the applied patch, and (file-error "Converting ACL"
> "Invalid argument" "/tmp/123") w/o the patch.
> 
> I cannot test on MS Windows.

I don't see why it wouldn't work on Windows as well.

> I don't know whether we must extend the docstring. Summarizing probable
> errors is not the default in Emacs docstrings. And the docstring does
> not promise, that no error at all will happen.

That's okay, I can always amend the doc string later.

> If you don't object, I'll commit the patch to emacs-26. Plus minor
> changes in Tramp.

No objection, but please only condition the call to 'error' on
acl_errno_valid.  I see no reason to proceed with calling acl_set_file
if acl_from_text fails.

> Maybe I'll find also the time next days, to add the missing tests
> for file-acl and set-file-acl in tramp-tests.el.

Sounds good, thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#29225; Package emacs. (Sun, 12 Nov 2017 13:22:02 GMT) Full text and rfc822 format available.

Message #23 received at 29225 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 29225 <at> debbugs.gnu.org, shuguang <at> gmail.com
Subject: Re: bug#29225: Tramp backup-by-copying in a Converting ACL Invalid
 argument error on Windows 7
Date: Sun, 12 Nov 2017 14:21:41 +0100
Eli Zaretskii <eliz <at> gnu.org> writes:

> No objection, but please only condition the call to 'error' on
> acl_errno_valid.  I see no reason to proceed with calling acl_set_file
> if acl_from_text fails.

Done.

>> Maybe I'll find also the time next days, to add the missing tests
>> for file-acl and set-file-acl in tramp-tests.el.
>
> Sounds good, thanks.

Also done.

I've committed the patch to the emacs-26 branch. Shuguang, do you have a
chance to check this?

Best regards, Michael.




Reply sent to Michael Albinus <michael.albinus <at> gmx.de>:
You have taken responsibility. (Wed, 17 Jan 2018 10:06:02 GMT) Full text and rfc822 format available.

Notification sent to Shuguang Sun <shuguang <at> gmail.com>:
bug acknowledged by developer. (Wed, 17 Jan 2018 10:06:02 GMT) Full text and rfc822 format available.

Message #28 received at 29225-done <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 29225-done <at> debbugs.gnu.org, shuguang <at> gmail.com
Subject: Re: bug#29225: Tramp backup-by-copying in a Converting ACL Invalid
 argument error on Windows 7
Date: Wed, 17 Jan 2018 11:05:12 +0100
Version: 26.1

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

> I've committed the patch to the emacs-26 branch. Shuguang, do you have a
> chance to check this?

No News Are Good News. I'm closing the bug.

Best regards, Michael.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Wed, 14 Feb 2018 12:24:05 GMT) Full text and rfc822 format available.

This bug report was last modified 6 years and 44 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.