[Bug-tar] Re: [PATCH] Extended attribute support for tar

[James Antill]

On Wed, 2006-08-02 at 10:24 +0300, Sergey Poznyakoff wrote:
> James Antill <address@hidden> wrote:
> 
> >  Hi, I've created support for creating/extracting SELinux/user/root
> > xattrs in GNUtar. If you have some free time, I'd appreciate anyone
> > looking over the patch and letting me know if anything needs to change
> > for inclusion.
> 
> Thank you. After a quick glance it turns out the patch does need something
> done before accepting for inclusion:

 Thanks for the reply.

> 1. All xattrs-specific stuff should be concentrated in one source module
>    instead of being scattered around 7 source files;

 I can see why you'd want this for some of the code (xheader_xattr_* for
example) ... but, for instance, I'd assume that the code in
start_header() and options processing in tar.c should stay where it is?

> 2. Proper autoconf machinery should be added to detect the presence of
>    the special calls it uses (fgetfilecon, flistxattr and a lot of
>    others) and disable it if any of them is not available;

 Yeh, that's on my TODO list.

> 3. I don't see any reason to create a second (prefix) table in
>    xheader.c, I'd prefer to expand the functionality of the main table
>    instead;

 I can combine the tables with a "is_prefix" type member, I'm not sure
this is better though. As the code should probably still run through the
table twice in lookup (one for a direct match and once for prefix
matches), and ignore the prefix entries in the other functions.
 But I'm happy to do that, if you want.

> 4. The patch is based on 1.15.90 version, current CVS version has moved
>    ahead since then (in particular, xheader.c was changed), so it would
>    be advisable to base it on the CVS head.

 Yeh, I assume that.

> >  Known "issues":
> [...] 

> > 7. Currently tar defaults to not doing anything when creating an
> > archive, so you need to pass --xattrs or --selinux to get all xattrs or
> > just SELinux context information in the archive.
> 
> That's OK as well.

 Would it be ok to have one or both of these on by default, at some
point in the future?

> > 8. AIUI star only includes the headers when using it's special exustar
> > format, the patch I've done for GNUtar just includes it on the else
> > clause of:
> > 
> >   if (archive_format == V7_FORMAT || numeric_owner_option)
> > 
> > ...which probably needs to be just the former test, or maybe limited to
> > certain formats?
> 
> It should be limited to POSIX.1-2001 format only (POSIX_FORMAT). 

 This would mean the default would have to change when you enable that
option[1], is there a reason to not allow it with the gnu format? Also
to be compatible[2] with star we need to at least extract from ustar
typed archives (star's exustar is treated by GNUtar as ustar ... AIUI).

 What are the downsides to changing the default to be posix -- which is
basically what would be happening as xattr usage is leaking into more
and more applications. The documentation says that this is planned when
full support for posix archives is done, is this close to happening?


[1] Or have the horribly ugly UI that star has where you need to specify
--xattrs and --format=posix or it'd fail, which seems to be much worse
for no real gain.

[2] I can't think of a reason we'd want to not allow people to extract
star archives they've already made.

-- 
James Antill <address@hidden>

signature.asc
Description: This is a digitally signed message part