unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* set-file-extended-attributes and backups
@ 2012-12-21 14:53 Eli Zaretskii
  2012-12-21 16:00 ` Paul Eggert
                   ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: Eli Zaretskii @ 2012-12-21 14:53 UTC (permalink / raw)
  To: emacs-devel; +Cc: Fabrice Popineau

Fabrice reported that since the introduction of ACLs into backing up
of files (as part as saving a buffer), he gets errors (on MS-Windows
8).  It turns out his user profile doesn't have the privileges to set
file security descriptors.  This can be fixed by granting the
privileges in the local policy, but the issue I raise is should this
problem be exposed in the context of backing up and saving files?

I think this problem is not Windows-specific.  So I'm asking here:
does it make sense to fail backup-buffer and backup-buffer-copy just
because set-file-extended-attributes fails?  I think we should ignore
such errors in these cases, and propose the changes below.

P.S.  If anyone wants to argue that set-file-acl should ignore this,
then I disagree: it's up to the application to decide whether or not
to ignore such problems.

=== modified file 'lisp/files.el'
--- lisp/files.el	2012-12-17 15:51:49 +0000
+++ lisp/files.el	2012-12-21 12:10:16 +0000
@@ -4022,7 +4022,8 @@ BACKUPNAME is the backup file name, whic
   (and modes
        (set-file-modes to-name (logand modes #o1777)))
   (and extended-attributes
-       (set-file-extended-attributes to-name extended-attributes)))
+       (ignore-errors
+	 (set-file-extended-attributes to-name extended-attributes))))
 
 (defvar file-name-version-regexp
   "\\(?:~\\|\\.~[-[:alnum:]:#@^._]+\\(?:~[[:digit:]]+\\)?~\\)"
@@ -4738,7 +4739,9 @@ Before and after saving the buffer, this
 				    (file-extended-attributes buffer-file-name)
 				    buffer-file-name))
 	       (set-file-modes buffer-file-name (logior (car setmodes) 128))
-	       (set-file-extended-attributes buffer-file-name (nth 1 setmodes)))))
+	       (ignore-errors
+		 (set-file-extended-attributes buffer-file-name
+					       (nth 1 setmodes))))))
 	(let (success)
 	  (unwind-protect
 	      (progn




^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: set-file-extended-attributes and backups
  2012-12-21 14:53 set-file-extended-attributes and backups Eli Zaretskii
@ 2012-12-21 16:00 ` Paul Eggert
  2012-12-21 16:44   ` Eli Zaretskii
  2012-12-21 18:31 ` Romain Francoise
  2012-12-22 16:05 ` Stefan Monnier
  2 siblings, 1 reply; 25+ messages in thread
From: Paul Eggert @ 2012-12-21 16:00 UTC (permalink / raw)
  To: emacs-devel; +Cc: Romain Francoise

On 12/21/12 06:53, Eli Zaretskii wrote:
> I think this problem is not Windows-specific.  So I'm asking here:
> does it make sense to fail backup-buffer and backup-buffer-copy just
> because set-file-extended-attributes fails?  I think we should ignore
> such errors

On systems where ACLs can deny access to files, failing to
copy an ACL can mean that the copy has more permissions
than the original, no?  Wouldn't that be a security problem?

As I understand it, Windows ACLs can deny access, just as
Posix ACLs can, so this issue is relevant on Windows too.

The recently-added ACL code has some security holes in
this area, doesn't it?  It's copying file mode separately
from copying ACLs.  Surely the code should just copy ACLs,
as there's a race condition now, where the file is
temporarily exposed between the times the mode and the
ACLs are copied.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: set-file-extended-attributes and backups
  2012-12-21 16:00 ` Paul Eggert
@ 2012-12-21 16:44   ` Eli Zaretskii
  2012-12-21 17:48     ` Paul Eggert
  2012-12-23 16:59     ` Romain Francoise
  0 siblings, 2 replies; 25+ messages in thread
From: Eli Zaretskii @ 2012-12-21 16:44 UTC (permalink / raw)
  To: Paul Eggert; +Cc: romain, emacs-devel

> Date: Fri, 21 Dec 2012 08:00:01 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
> Cc: Romain Francoise <romain@orebokech.com>
> 
> On 12/21/12 06:53, Eli Zaretskii wrote:
> > I think this problem is not Windows-specific.  So I'm asking here:
> > does it make sense to fail backup-buffer and backup-buffer-copy just
> > because set-file-extended-attributes fails?  I think we should ignore
> > such errors
> 
> On systems where ACLs can deny access to files, failing to
> copy an ACL can mean that the copy has more permissions
> than the original, no?

Yes, I think so, at least when we are not backing up by renaming.

> Wouldn't that be a security problem?

Maybe.  But Emacs does the same on platforms where ACLs are not
accessible to Emacs, so if there's a security problem, we already have
it, I think.

> As I understand it, Windows ACLs can deny access, just as
> Posix ACLs can, so this issue is relevant on Windows too.

Yes, Windows ACLs can deny access, and yes, it is relevant.

> The recently-added ACL code has some security holes in
> this area, doesn't it?  It's copying file mode separately
> from copying ACLs.  Surely the code should just copy ACLs,
> as there's a race condition now, where the file is
> temporarily exposed between the times the mode and the
> ACLs are copied.

How about if it tried to copy ACLs, and if that failed, attempted to
copy the file modes?  That would DTRT if possible, and fall back on
the pre-ACL method if not.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: set-file-extended-attributes and backups
  2012-12-21 16:44   ` Eli Zaretskii
@ 2012-12-21 17:48     ` Paul Eggert
  2012-12-21 18:08       ` Eli Zaretskii
  2012-12-23 16:59     ` Romain Francoise
  1 sibling, 1 reply; 25+ messages in thread
From: Paul Eggert @ 2012-12-21 17:48 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: romain, emacs-devel

On 12/21/12 08:44, Eli Zaretskii wrote:
> How about if it tried to copy ACLs, and if that failed, attempted to
> copy the file modes?  That would DTRT if possible, and fall back on
> the pre-ACL method if not.

That could lead to security issues if the file modes are more
permissive than the ACLs.

Is there an easy way to test whether a file's ACLs could deny
access when the file's modes would allow it?  If so, Emacs could
follow your suggestion when that test says "no".  If not, Emacs
could fall back on a conservative approximation to that test.
The simplest conservative approximation that I can think of offhand
is to test whether a file has any nontrivial ACLs.

Whatever test Emacs uses, if the test says "yes" Emacs should
be more cautious: create a destination file with a restrictive
mode (e.g., -rw-------), copy the data, then attempt to copy the ACLs,
and if the ACL copy fails then Emacs should not attempt to change
the mode.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: set-file-extended-attributes and backups
  2012-12-21 17:48     ` Paul Eggert
@ 2012-12-21 18:08       ` Eli Zaretskii
  2012-12-21 18:31         ` Paul Eggert
  0 siblings, 1 reply; 25+ messages in thread
From: Eli Zaretskii @ 2012-12-21 18:08 UTC (permalink / raw)
  To: Paul Eggert; +Cc: romain, emacs-devel

> Date: Fri, 21 Dec 2012 09:48:20 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: emacs-devel@gnu.org, romain@orebokech.com
> 
> On 12/21/12 08:44, Eli Zaretskii wrote:
> > How about if it tried to copy ACLs, and if that failed, attempted to
> > copy the file modes?  That would DTRT if possible, and fall back on
> > the pre-ACL method if not.
> 
> That could lead to security issues if the file modes are more
> permissive than the ACLs.

But we did that until a week ago.  If we want Emacs to be more secure,
just because it can now access ACLs, this decision should be left to
the user, i.e. be a user option.  Otherwise, we are forcing users the
level of security they not necessary want.

> Is there an easy way to test whether a file's ACLs could deny
> access when the file's modes would allow it?

There are no modes without ACLs.  Systems that support ACLs always
provide ACLs for files, just the default ones.  So what you ask is
whether the default ACLs will allow some access that a specific ACLs
won't.  And the answer to that is "it depends on the user" whose
access we are interested in.  E.g., if the default ACLs allow some
access to the file's group, the answer depends on whether a user
belongs to that group.

> The simplest conservative approximation that I can think of offhand
> is to test whether a file has any nontrivial ACLs.

That's not good enough, I think: if the nontrivial ACLs specify the
same group as the file's group, the modes and the ACLs are equivalent,
although the ACLs are "nontrivial".

> Whatever test Emacs uses, if the test says "yes" Emacs should
> be more cautious: create a destination file with a restrictive
> mode (e.g., -rw-------), copy the data, then attempt to copy the ACLs,
> and if the ACL copy fails then Emacs should not attempt to change
> the mode.

That assumes that -rw------- is secure.  But that assumption is false,
because ACLs can be more restrictive than that, even on Posix
platforms.  E.g., they could disallow write access to the user who
makes the copy, or disallow attributes changes.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: set-file-extended-attributes and backups
  2012-12-21 14:53 set-file-extended-attributes and backups Eli Zaretskii
  2012-12-21 16:00 ` Paul Eggert
@ 2012-12-21 18:31 ` Romain Francoise
  2012-12-22 23:03   ` Fabrice Popineau
  2012-12-22 16:05 ` Stefan Monnier
  2 siblings, 1 reply; 25+ messages in thread
From: Romain Francoise @ 2012-12-21 18:31 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Fabrice Popineau, emacs-devel

Hi Eli,

Eli Zaretskii <eliz@gnu.org> writes:

> I think this problem is not Windows-specific.  So I'm asking here:
> does it make sense to fail backup-buffer and backup-buffer-copy just
> because set-file-extended-attributes fails?

Probably not. We should follow coreutils and just issue a warning if we
can't copy the acl (which is the default behavior of `mv' and also `cp' if
`--preserve=all' is used).

> I think we should ignore such errors in these cases, and propose the
> changes below.

I think that is too radical, it should display a warning so that the user
can decide what to do. Since Emacs only makes a backup the first time you
save, the warning shouldn't be too annoying.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: set-file-extended-attributes and backups
  2012-12-21 18:08       ` Eli Zaretskii
@ 2012-12-21 18:31         ` Paul Eggert
  0 siblings, 0 replies; 25+ messages in thread
From: Paul Eggert @ 2012-12-21 18:31 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: romain, emacs-devel

On 12/21/12 10:08, Eli Zaretskii wrote:

> But we did that until a week ago.

True, but we're trying to do the right thing with ACLs now,
rather than ignore them and do the wrong thing.

> this decision should be left to the user, i.e. be a user option.

Possibly, but the default should be safe, i.e., it shouldn't
grant access rights that were not already there.

> So what you ask is
> whether the default ACLs will allow some access that a specific ACLs
> won't.  And the answer to that is "it depends ..."

Yes, and the question is whether it's easy to find out
all the dependencies, so that Emacs can tell whether the
default ACLs would allow any access that the correct ACLs
would deny.  My guess is that the answer is "no", unfortunately.

>> The simplest conservative approximation that I can think of offhand
>> is to test whether a file has any nontrivial ACLs.
> 
> That's not good enough, I think: if the nontrivial ACLs specify the
> same group as the file's group, the modes and the ACLs are equivalent,
> although the ACLs are "nontrivial".

Sure, but here it's OK from a security point of view to use a
conservative approximation, i.e., a test that sometimes says
"yes" even when the true answer is "no".  The only downside is
that when the conservative approximation is incorrect, then when
the ACLs cannot be copied the file will end up in mode -rw-------.
That's annoying, but it's safe and I hope it's rare.

> That assumes that -rw------- is secure.  But that assumption is false,
> because ACLs can be more restrictive than that, even on Posix
> platforms.

No, because if an attacker can read and write a file with
permissions -rw-------, then the attacker owns the file
(or is superuser) and can change its ACLs.  ACLs cannot stop
such an attacker.  So long as Emacs doesn't grant permissions
to anybody other than the owner, Emacs is not giving any
secrets away that aren't being given away already.

As a minor nicety, we could use mode ----------, if we prefer being safer
in an advisory sense.  Mode ---------- won't stop an attacker
who is the owner, but it would be more likely to prevent
blundering owners from shooting themselves in the foot.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: set-file-extended-attributes and backups
  2012-12-21 14:53 set-file-extended-attributes and backups Eli Zaretskii
  2012-12-21 16:00 ` Paul Eggert
  2012-12-21 18:31 ` Romain Francoise
@ 2012-12-22 16:05 ` Stefan Monnier
  2012-12-22 17:03   ` Eli Zaretskii
  2 siblings, 1 reply; 25+ messages in thread
From: Stefan Monnier @ 2012-12-22 16:05 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Fabrice Popineau, emacs-devel

> +       (ignore-errors
> +	 (set-file-extended-attributes to-name extended-attributes))))
 
I'd use something like with-demoted-errors instead.


        Stefan



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: set-file-extended-attributes and backups
  2012-12-22 16:05 ` Stefan Monnier
@ 2012-12-22 17:03   ` Eli Zaretskii
  2012-12-23 13:37     ` Stefan Monnier
  0 siblings, 1 reply; 25+ messages in thread
From: Eli Zaretskii @ 2012-12-22 17:03 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: fabrice.popineau, emacs-devel

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org,  Fabrice Popineau <fabrice.popineau@supelec.fr>
> Date: Sat, 22 Dec 2012 11:05:21 -0500
> 
> > +       (ignore-errors
> > +	 (set-file-extended-attributes to-name extended-attributes))))
>  
> I'd use something like with-demoted-errors instead.

What about the race condition pointed out by Paul?  Should we try
set-file-extended-attributes first, and fall back on set-file-modes if
that fails (after displaying a message)?



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: set-file-extended-attributes and backups
  2012-12-21 18:31 ` Romain Francoise
@ 2012-12-22 23:03   ` Fabrice Popineau
  2012-12-23  3:54     ` Eli Zaretskii
  0 siblings, 1 reply; 25+ messages in thread
From: Fabrice Popineau @ 2012-12-22 23:03 UTC (permalink / raw)
  To: Romain Francoise; +Cc: Eli Zaretskii, emacs-devel

[-- Attachment #1: Type: text/plain, Size: 774 bytes --]

>
> I think that is too radical, it should display a warning so that the user
> can decide what to do. Since Emacs only makes a backup the first time you
> save, the warning shouldn't be too annoying.
>

The warning would happen to every single file you edit with emacs.
And I'm running Windows with an account with elevated privileges (so-called
admin account) with default settings. I guess lots of people are in this
situation.
If I run emacs "as administrator", then there is no problem. But this is
far too
dangerous in my opinion.

Eli as offered an interesting option : to avoidt trying to set the acl if
it hasn't changed.
Which should be the case all the time (or most of the time). Combining the
warning
and this other fix could be the best solution.

-- 
Fabrice

[-- Attachment #2: Type: text/html, Size: 1345 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: set-file-extended-attributes and backups
  2012-12-22 23:03   ` Fabrice Popineau
@ 2012-12-23  3:54     ` Eli Zaretskii
  2012-12-23 17:17       ` Eli Zaretskii
  0 siblings, 1 reply; 25+ messages in thread
From: Eli Zaretskii @ 2012-12-23  3:54 UTC (permalink / raw)
  To: Fabrice Popineau; +Cc: romain, emacs-devel

> From: Fabrice Popineau <fabrice.popineau@supelec.fr>
> Date: Sun, 23 Dec 2012 00:03:54 +0100
> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
> 
> Eli as offered an interesting option : to avoid trying to set the acl if
> it hasn't changed.

I will do this as part of the Windows implementation of the ACL
interfaces.  So if this kind of no-op ACL setting can barf only on
Windows, I think a warning in set-file-extended-attributes should be
enough.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: set-file-extended-attributes and backups
  2012-12-22 17:03   ` Eli Zaretskii
@ 2012-12-23 13:37     ` Stefan Monnier
  2012-12-29 17:20       ` Eli Zaretskii
  0 siblings, 1 reply; 25+ messages in thread
From: Stefan Monnier @ 2012-12-23 13:37 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: fabrice.popineau, emacs-devel

>> > +       (ignore-errors
>> > +	 (set-file-extended-attributes to-name extended-attributes))))
>> I'd use something like with-demoted-errors instead.
> What about the race condition pointed out by Paul?  Should we try
> set-file-extended-attributes first, and fall back on set-file-modes if
> that fails (after displaying a message)?

That sounds fine, yes.


        Stefan



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: set-file-extended-attributes and backups
  2012-12-21 16:44   ` Eli Zaretskii
  2012-12-21 17:48     ` Paul Eggert
@ 2012-12-23 16:59     ` Romain Francoise
  2012-12-23 17:35       ` Eli Zaretskii
  1 sibling, 1 reply; 25+ messages in thread
From: Romain Francoise @ 2012-12-23 16:59 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Paul Eggert, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> How about if it tried to copy ACLs, and if that failed, attempted to
> copy the file modes?  That would DTRT if possible, and fall back on
> the pre-ACL method if not.

If we do that we need to change `set-file-acl' to treat ENOTSUP as an
error otherwise modes won't be copied on filesystems without ACL support.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: set-file-extended-attributes and backups
  2012-12-23  3:54     ` Eli Zaretskii
@ 2012-12-23 17:17       ` Eli Zaretskii
  0 siblings, 0 replies; 25+ messages in thread
From: Eli Zaretskii @ 2012-12-23 17:17 UTC (permalink / raw)
  To: fabrice.popineau; +Cc: romain, emacs-devel

> Date: Sun, 23 Dec 2012 05:54:17 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: romain@orebokech.com, emacs-devel@gnu.org
> 
> > From: Fabrice Popineau <fabrice.popineau@supelec.fr>
> > Date: Sun, 23 Dec 2012 00:03:54 +0100
> > Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
> > 
> > Eli as offered an interesting option : to avoid trying to set the acl if
> > it hasn't changed.
> 
> I will do this as part of the Windows implementation of the ACL
> interfaces.

Done.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: set-file-extended-attributes and backups
  2012-12-23 16:59     ` Romain Francoise
@ 2012-12-23 17:35       ` Eli Zaretskii
  2012-12-24  0:59         ` Stefan Monnier
  0 siblings, 1 reply; 25+ messages in thread
From: Eli Zaretskii @ 2012-12-23 17:35 UTC (permalink / raw)
  To: Romain Francoise; +Cc: eggert, emacs-devel

> From: Romain Francoise <romain@orebokech.com>
> Cc: Paul Eggert <eggert@cs.ucla.edu>,  emacs-devel@gnu.org
> Date: Sun, 23 Dec 2012 17:59:52 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > How about if it tried to copy ACLs, and if that failed, attempted to
> > copy the file modes?  That would DTRT if possible, and fall back on
> > the pre-ACL method if not.
> 
> If we do that we need to change `set-file-acl' to treat ENOTSUP as an
> error otherwise modes won't be copied on filesystems without ACL support.

How about changing its return value instead?  Say, return t if ACLs
set, nil otherwise.  It always bothers me to have an API that silently
fails without any tangible indication.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: set-file-extended-attributes and backups
  2012-12-23 17:35       ` Eli Zaretskii
@ 2012-12-24  0:59         ` Stefan Monnier
  2012-12-24  3:44           ` Eli Zaretskii
  0 siblings, 1 reply; 25+ messages in thread
From: Stefan Monnier @ 2012-12-24  0:59 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Romain Francoise, emacs-devel, eggert

> How about changing its return value instead?  Say, return t if ACLs
> set, nil otherwise.  It always bothers me to have an API that silently
> fails without any tangible indication.

I think it should fail (i.e. signal an error) for any case where the
file's ACLs were not set as requested.

The error signaled should be different for "ACLs not supported" and "you
do not have the right to set ACL on this file".


        Stefan



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: set-file-extended-attributes and backups
  2012-12-24  0:59         ` Stefan Monnier
@ 2012-12-24  3:44           ` Eli Zaretskii
  2012-12-24  5:18             ` Stefan Monnier
  0 siblings, 1 reply; 25+ messages in thread
From: Eli Zaretskii @ 2012-12-24  3:44 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: romain, emacs-devel, eggert

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Romain Francoise <romain@orebokech.com>,  eggert@cs.ucla.edu,  emacs-devel@gnu.org
> Date: Sun, 23 Dec 2012 19:59:35 -0500
> 
> > How about changing its return value instead?  Say, return t if ACLs
> > set, nil otherwise.  It always bothers me to have an API that silently
> > fails without any tangible indication.
> 
> I think it should fail (i.e. signal an error) for any case where the
> file's ACLs were not set as requested.

ENOTSUP usually means the filesystem doesn't support ACLs.  Having an
error that every single use of it will want to ignore doesn't seem to
be useful.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: set-file-extended-attributes and backups
  2012-12-24  3:44           ` Eli Zaretskii
@ 2012-12-24  5:18             ` Stefan Monnier
  2012-12-24  8:25               ` Michael Albinus
  2012-12-24 16:24               ` Eli Zaretskii
  0 siblings, 2 replies; 25+ messages in thread
From: Stefan Monnier @ 2012-12-24  5:18 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: romain, emacs-devel, eggert

> ENOTSUP usually means the filesystem doesn't support ACLs.  Having an
> error that every single use of it will want to ignore doesn't seem to
> be useful.

But if the FS doesn't support ACL, then file-acl should have returned
nil, so we don't need to call set-file-acl at all.  IOW, those errors
should not be frequent.


        Stefan



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: set-file-extended-attributes and backups
  2012-12-24  5:18             ` Stefan Monnier
@ 2012-12-24  8:25               ` Michael Albinus
  2012-12-24 16:24               ` Eli Zaretskii
  1 sibling, 0 replies; 25+ messages in thread
From: Michael Albinus @ 2012-12-24  8:25 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: eggert, Eli Zaretskii, romain, emacs-devel

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

>> ENOTSUP usually means the filesystem doesn't support ACLs.  Having an
>> error that every single use of it will want to ignore doesn't seem to
>> be useful.
>
> But if the FS doesn't support ACL, then file-acl should have returned
> nil, so we don't need to call set-file-acl at all.  IOW, those errors
> should not be frequent.

And Tramp will suppress both errors.

>         Stefan

Best regards, Michael.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: set-file-extended-attributes and backups
  2012-12-24  5:18             ` Stefan Monnier
  2012-12-24  8:25               ` Michael Albinus
@ 2012-12-24 16:24               ` Eli Zaretskii
  1 sibling, 0 replies; 25+ messages in thread
From: Eli Zaretskii @ 2012-12-24 16:24 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: romain, emacs-devel, eggert

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: romain@orebokech.com,  eggert@cs.ucla.edu,  emacs-devel@gnu.org
> Date: Mon, 24 Dec 2012 00:18:59 -0500
> 
> > ENOTSUP usually means the filesystem doesn't support ACLs.  Having an
> > error that every single use of it will want to ignore doesn't seem to
> > be useful.
> 
> But if the FS doesn't support ACL, then file-acl should have returned
> nil, so we don't need to call set-file-acl at all.

What about copying files from one FS to another?  Like from the DoK to
your local disk or vice versa?

> IOW, those errors should not be frequent.

Frequency is immaterial here, IMO.  If, when the error happens, we
will always want to ignore it, then raising it is not useful.
Especially that we would need to invent yet another kind of error to
distinguish a not-supported error from something-else-wrong error.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: set-file-extended-attributes and backups
  2012-12-23 13:37     ` Stefan Monnier
@ 2012-12-29 17:20       ` Eli Zaretskii
  2012-12-29 17:50         ` Eli Zaretskii
  0 siblings, 1 reply; 25+ messages in thread
From: Eli Zaretskii @ 2012-12-29 17:20 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: fabrice.popineau, emacs-devel

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: fabrice.popineau@supelec.fr,  emacs-devel@gnu.org
> Date: Sun, 23 Dec 2012 08:37:52 -0500
> 
> >> > +       (ignore-errors
> >> > +	 (set-file-extended-attributes to-name extended-attributes))))
> >> I'd use something like with-demoted-errors instead.
> > What about the race condition pointed out by Paul?  Should we try
> > set-file-extended-attributes first, and fall back on set-file-modes if
> > that fails (after displaying a message)?
> 
> That sounds fine, yes.

Now done on the trunk.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: set-file-extended-attributes and backups
  2012-12-29 17:20       ` Eli Zaretskii
@ 2012-12-29 17:50         ` Eli Zaretskii
  2012-12-29 19:12           ` Michael Albinus
  0 siblings, 1 reply; 25+ messages in thread
From: Eli Zaretskii @ 2012-12-29 17:50 UTC (permalink / raw)
  To: Michael Albinus; +Cc: monnier, emacs-devel

> Date: Sat, 29 Dec 2012 19:20:30 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: fabrice.popineau@supelec.fr, emacs-devel@gnu.org
> 
> > From: Stefan Monnier <monnier@iro.umontreal.ca>
> > Cc: fabrice.popineau@supelec.fr,  emacs-devel@gnu.org
> > Date: Sun, 23 Dec 2012 08:37:52 -0500
> > 
> > >> > +       (ignore-errors
> > >> > +	 (set-file-extended-attributes to-name extended-attributes))))
> > >> I'd use something like with-demoted-errors instead.
> > > What about the race condition pointed out by Paul?  Should we try
> > > set-file-extended-attributes first, and fall back on set-file-modes if
> > > that fails (after displaying a message)?
> > 
> > That sounds fine, yes.
> 
> Now done on the trunk.

One side effect of these changes is that set-file-acl and
set-file-selinux-context now return t when they succeeded to set the
ACLs resp. SELinux context, and nil otherwise.  Micheal, could you
please adapt the Tramp handlers to this paradigm?  Thanks.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: set-file-extended-attributes and backups
  2012-12-29 17:50         ` Eli Zaretskii
@ 2012-12-29 19:12           ` Michael Albinus
  2012-12-30 10:59             ` Michael Albinus
  0 siblings, 1 reply; 25+ messages in thread
From: Michael Albinus @ 2012-12-29 19:12 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> One side effect of these changes is that set-file-acl and
> set-file-selinux-context now return t when they succeeded to set the
> ACLs resp. SELinux context, and nil otherwise.  Micheal, could you
> please adapt the Tramp handlers to this paradigm?  Thanks.

Done for ACL. For SELinux, I'll check later.

Best regards, Michael.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: set-file-extended-attributes and backups
  2012-12-29 19:12           ` Michael Albinus
@ 2012-12-30 10:59             ` Michael Albinus
  2012-12-30 17:21               ` Eli Zaretskii
  0 siblings, 1 reply; 25+ messages in thread
From: Michael Albinus @ 2012-12-30 10:59 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, emacs-devel

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

> Eli Zaretskii <eliz@gnu.org> writes:
>
>> One side effect of these changes is that set-file-acl and
>> set-file-selinux-context now return t when they succeeded to set the
>> ACLs resp. SELinux context, and nil otherwise.  Micheal, could you
>> please adapt the Tramp handlers to this paradigm?  Thanks.
>
> Done for ACL. For SELinux, I'll check later.

SELinux case is also fixed now.

Best regards, Michael.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: set-file-extended-attributes and backups
  2012-12-30 10:59             ` Michael Albinus
@ 2012-12-30 17:21               ` Eli Zaretskii
  0 siblings, 0 replies; 25+ messages in thread
From: Eli Zaretskii @ 2012-12-30 17:21 UTC (permalink / raw)
  To: Michael Albinus; +Cc: monnier, emacs-devel

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: monnier@iro.umontreal.ca,  emacs-devel@gnu.org
> Date: Sun, 30 Dec 2012 11:59:54 +0100
> 
> Michael Albinus <michael.albinus@gmx.de> writes:
> 
> > Eli Zaretskii <eliz@gnu.org> writes:
> >
> >> One side effect of these changes is that set-file-acl and
> >> set-file-selinux-context now return t when they succeeded to set the
> >> ACLs resp. SELinux context, and nil otherwise.  Micheal, could you
> >> please adapt the Tramp handlers to this paradigm?  Thanks.
> >
> > Done for ACL. For SELinux, I'll check later.
> 
> SELinux case is also fixed now.

Thank you!



^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2012-12-30 17:21 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-21 14:53 set-file-extended-attributes and backups Eli Zaretskii
2012-12-21 16:00 ` Paul Eggert
2012-12-21 16:44   ` Eli Zaretskii
2012-12-21 17:48     ` Paul Eggert
2012-12-21 18:08       ` Eli Zaretskii
2012-12-21 18:31         ` Paul Eggert
2012-12-23 16:59     ` Romain Francoise
2012-12-23 17:35       ` Eli Zaretskii
2012-12-24  0:59         ` Stefan Monnier
2012-12-24  3:44           ` Eli Zaretskii
2012-12-24  5:18             ` Stefan Monnier
2012-12-24  8:25               ` Michael Albinus
2012-12-24 16:24               ` Eli Zaretskii
2012-12-21 18:31 ` Romain Francoise
2012-12-22 23:03   ` Fabrice Popineau
2012-12-23  3:54     ` Eli Zaretskii
2012-12-23 17:17       ` Eli Zaretskii
2012-12-22 16:05 ` Stefan Monnier
2012-12-22 17:03   ` Eli Zaretskii
2012-12-23 13:37     ` Stefan Monnier
2012-12-29 17:20       ` Eli Zaretskii
2012-12-29 17:50         ` Eli Zaretskii
2012-12-29 19:12           ` Michael Albinus
2012-12-30 10:59             ` Michael Albinus
2012-12-30 17:21               ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).