Subject: Re: bug#29225: Tramp backup-by-copying in a Converting ACL Invalid argument error on Windows 7

From: Eli Zaretskii <[email-masked]>
Date: Fri, 10 Nov 2017 17:13:06 +0200

> From: Michael Albinus <[email-masked]>
> Cc: [email-masked],  [email-masked]
> 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.