bug#21699: 24.5; Bug in backup-buffer-copy and/or set-file-extended-attributes etc [set-file-extended-attributes]

[Eli Barzilay]

[I'm working on the other things that you asked now...]


On Mon, Oct 19, 2015 at 3:09 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Mon, 19 Oct 2015 09:50:06 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>> Cc: 21699@debbugs.gnu.org
>>
>> > Date: Mon, 19 Oct 2015 09:38:08 +0300
>> > From: Eli Zaretskii <eliz@gnu.org>
>> > Cc: 21699@debbugs.gnu.org
>> >
>> > So after fixing set-file-extended-attributes as you suggest, does the
>> > problem still happen for you?
>>
>> Actually, your suggested variant also returns nil for me.

Well, it would return nil since the result of `file-extended-attributes'
would include (selinux-context nil nil nil nil) which sould make it try
to use `set-file-selinux-context' and that returns nil.  (That was my
original point #3, addressed below.)


>> I need something like this instead:
>> [...2nd version...]

This one would indeed return t for me, since it stops trying after the
first t result from `set-file-acl'.


> I installed a slightly different variant of this (which always invokes
> the corresponding low-level primitive for each type of extended
> attributes, instead of skipping all those after the first success), to
> keep the original semantics.

Well, you added this in the docstring:

    Value is t if the function succeeds in setting the attributes.

which is a little vague -- I think that it would be better to say

    Value is t if the function succeeds in setting at least one of the
    attributes.

*BUT* I doubt that this is a good idea, since on a system that supports
both acl and selinux-context you probably want a t result to indicate
that all of the extended settings worked.  So I think that my original
suggestion is better if `file-extended-attributes' doesn't produce
values that are not supported.

So ... moving on to that third problem, I fixed
`file-extended-attributes' to do that: it doesn't include an `acl' value
if `file-acl' returned nil, and doesn't include `selinux-context' if
`file-selinux-context' returned (nil nil nil nil).  I used this with my
version of `set-file-extended-attributes' (which returns t only when all
settings returned non-nil), and that resolved my backup problem.  The
code for the two functions, with updated docstrings, is below.

-------------------------------------------------------------------------------

(defun file-extended-attributes (filename)
  "Return an alist of extended attributes of file FILENAME.

Extended attributes are platform-specific metadata about the file,
such as SELinux context, list of ACL entries, etc.  Only supported
metadata is included."
  (let ((attrs '()) x)
    (unless (equal nil (setq x (file-acl filename)))
      (push `(acl . ,x) attrs))
    (unless (equal '(nil nil nil nil)
                   (setq x (file-selinux-context filename)))
      (push `(selinux-context . ,x) attrs))
    attrs))

(defun set-file-extended-attributes (filename attributes)
  "Set extended attributes of file FILENAME to ATTRIBUTES.

ATTRIBUTES must be an alist of file attributes as returned by
`file-extended-attributes'.  Value is t if the function succeeds
in setting all of the given attributes."
  (let ((result t))
    (dolist (elt attributes)
      (let ((attr (car elt))
            (val (cdr elt)))
        (unless (cond ((eq attr 'acl)
                       (set-file-acl filename val))
                      ((eq attr 'selinux-context)
                       (set-file-selinux-context filename val)))
          (setq result nil))))
    result))

-------------------------------------------------------------------------------

-- 
                    ((x=>x(x))(x=>x(x)))                   Eli Barzilay:
                    http://barzilay.org/                   Maze is Life!