Re: extracting read-only files with --xattrs throws an error: Cannot open: Permission denied

[Pavel Raiskup]

Hello, this has been fixed in tar v1.35, commits 0b74885 and 35d9845 and
e7987b7, https://github.com/praiskup/tar/pull/2

Pavel

On pondělí 28. srpna 2023 10:38:31 CEST Piotr Łobacz wrote:
> BTW. I have found a bug for that https://savannah.gnu.org/bugs/?59184
> 
> W dniu 28.08.2023 o 09:47, Piotr Łobacz pisze:
> >
> > Hi All,
> >
> > It occurs exactly as in the subject of this mail, that if we are 
> > trying to extract an archive with files that are read only and with 
> > --xattrs attribute an error occurs e.g.
> >
> > pokybuild@debian11-ty-1:~/yocto-worker/wic/build/build/tmp/work/x86_64-linux/cdrtools-native/3.01/temp$
> >  
> > cat log.do_populate_sysroot_setscene
> > DEBUG: Executing python function do_populate_sysroot_setscene
> > DEBUG: Executing shell function sstate_unpack_package
> > recipe-sysroot-native/
> > recipe-sysroot-native/sysroot-providers/
> > recipe-sysroot-native/sysroot-providers/cdrtools-native
> > recipe-sysroot-native/usr/
> > recipe-sysroot-native/usr/bin/
> > recipe-sysroot-native/usr/bin/cdda2ogg
> > recipe-sysroot-native/usr/bin/cdda2wav
> > recipe-sysroot-native/usr/bin/mkhybrid
> > recipe-sysroot-native/usr/bin/devdump
> > recipe-sysroot-native/usr/bin/isodebug
> > recipe-sysroot-native/usr/bin/scgskeleton
> > recipe-sysroot-native/usr/bin/btcflash
> > recipe-sysroot-native/usr/bin/cdrecord
> > recipe-sysroot-native/usr/bin/readcd
> > recipe-sysroot-native/usr/bin/isoinfo
> > recipe-sysroot-native/usr/bin/isodump
> > recipe-sysroot-native/usr/bin/cdda2mp3
> > recipe-sysroot-native/usr/bin/scgcheck
> > recipe-sysroot-native/usr/bin/mkisofs
> > recipe-sysroot-native/usr/bin/isovfy
> > recipe-sysroot-native/usr/sbin/
> > recipe-sysroot-native/usr/sbin/rscsi
> > recipe-sysroot-native/usr/share/
> > recipe-sysroot-native/usr/lib/
> > recipe-sysroot-native/usr/lib/libfind.a
> > recipe-sysroot-native/usr/lib/libschily.a
> > recipe-sysroot-native/usr/lib/libedc_ecc_dec.a
> > recipe-sysroot-native/usr/lib/libhfs.a
> > recipe-sysroot-native/usr/lib/libcdrdeflt.a
> > recipe-sysroot-native/usr/lib/libscg.a
> > recipe-sysroot-native/usr/lib/libfile.a
> > recipe-sysroot-native/usr/lib/siconv/
> > recipe-sysroot-native/usr/lib/siconv/cp10006
> > recipe-sysroot-native/usr/lib/siconv/iso8859-16
> > recipe-sysroot-native/usr/lib/siconv/iso8859-9
> > recipe-sysroot-native/usr/lib/siconv/cp10007
> > recipe-sysroot-native/usr/lib/siconv/cp10029
> > recipe-sysroot-native/usr/lib/siconv/iso8859-6
> > recipe-sysroot-native/usr/lib/siconv/iso8859-4
> > recipe-sysroot-native/usr/lib/siconv/cp1257
> > recipe-sysroot-native/usr/lib/siconv/iso8859-13
> > tar: recipe-sysroot-native/usr/lib/libfind.a: Cannot open: Permission 
> > denied
> > tar: recipe-sysroot-native/usr/lib/libschily.a: Cannot open: 
> > Permission denied
> > tar: recipe-sysroot-native/usr/lib/libedc_ecc_dec.a: Cannot open: 
> > Permission denied
> > tar: recipe-sysroot-native/usr/lib/libhfs.a: Cannot open: Permission 
> > denied
> > tar: recipe-sysroot-native/usr/lib/libcdrdeflt.a: Cannot open: 
> > Permission denied
> > tar: recipe-sysroot-native/usr/lib/libscg.a: Cannot open: Permission 
> > denied
> > tar: recipe-sysroot-native/usr/lib/libfile.a: Cannot open: Permission 
> > denied
> > tar: recipe-sysroot-native/usr/lib/siconv/cp10006: Cannot open: 
> > Permission denied
> > tar: recipe-sysroot-native/usr/lib/siconv/iso8859-16: Cannot open: 
> > Permission denied
> > tar: recipe-sysroot-native/usr/lib/siconv/iso8859-9: Cannot open: 
> > Permission denied
> > tar: recipe-sysroot-native/usr/lib/siconv/cp10007: Cannot open: 
> > Permission denied
> > tar: recipe-sysroot-native/usr/lib/siconv/cp10029: Cannot open: 
> > Permission denied
> > tar: recipe-sysroot-native/usr/lib/siconv/iso8859-6: Cannot open: 
> > Permission denied
> > tar: recipe-sysroot-native/usr/lib/siconv/iso8859-4: Cannot open: 
> > Permission denied
> > tar: recipe-sysroot-native/usr/lib/siconv/cp1257: Cannot open: 
> > Permission denied
> > tar: recipe-sysroot-native/usr/lib/siconv/iso8859-13: Cannot open: 
> > Permission denied
> > recipe-sysroot-native/usr/lib/siconv/iso8859-2
> > recipe-sysroot-native/usr/lib/siconv/cp437
> > tar: recipe-sysroot-native/usr/lib/siconv/iso8859-2: Cannot open: 
> > Permission denied
> > tar: recipe-sysroot-native/usr/lib/siconv/cp437: Cannot open: 
> > Permission denied
> > recipe-sysroot-native/usr/lib/siconv/cp775
> > recipe-sysroot-native/usr/lib/siconv/iso8859-15
> > tar: recipe-sysroot-native/usr/lib/siconv/cp775: Cannot open: 
> > Permission denied
> > tar: recipe-sysroot-native/usr/lib/siconv/iso8859-15: Cannot open: 
> > Permission denied
> > recipe-sysroot-native/usr/lib/siconv/cp857
> > tar: recipe-sysroot-native/usr/lib/siconv/cp857: Cannot open: 
> > Permission denied
> > recipe-sysroot-native/usr/lib/siconv/cp1258
> > recipe-sysroot-native/usr/lib/siconv/cp855
> > tar: recipe-sysroot-native/usr/lib/siconv/cp1258: Cannot open: 
> > Permission denied
> > tar: recipe-sysroot-native/usr/lib/siconv/cp855: Cannot open: 
> 

Re: tar 1.35: --delete creates tarfiles with duplicate file paths

[Pavel Raiskup]

On pátek 11. srpna 2023 16:49:31 CEST Pavel Raiskup wrote:
> On pátek 11. srpna 2023 14:32:18 CEST Ed Santiago wrote:
> > On Fri, Aug 11, 2023 at 08:33:03AM +0200, Sergey Poznyakoff wrote:
> > > Hi Ed,
> > > 
> > > > The --delete flag in 1.35 can generate a tarball that is
> > > > unreadable by podman and possibly other tools using the
> > > > Go tar library:
> > > 
> > > Unfortunately I cannot reproduce it,
> > > 
> > > > $ podman create --name a quay.io/libpod/testimage:20221018
> > > > $ podman export a >/tmp/a.tar
> > > > $ tar --delete -f /tmp/a.tar home/podman/pause
> > > 
> > > Can you send me the a.tar archive before and after delete.
> > > or put them somewhere so I can download them?
> > 
> > Curious! It's 100% reproducible on my end. I've attached ante
> > and post (before/after, but ASCII sortable) to:
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=2230127
> > 
> > ...and added a comment describing the dups found with
> > 
> > tar tf a.post.tar | sort | uniq -cd
> 
> Is this related to some of the Fedora downstream patches?  FTR, here are
> "vanilla" GNU tar RPM builds:
> https://copr.fedorainfracloud.org/coprs/praiskup/tar-upstream-head/
> 
> The CI is has not been building for a few days now, needs upstream fix:
> https://www.mail-archive.com/bug-tar@gnu.org/msg06277.html

I can confirm I can reproduce the behavior even with vanilla tar (in
Fedora Rawhide container, on Fedora 38 host).

Pavel


> Adding maintainer to CC.
> Pavel
> 
> > Hope this helps,
> > Ed
> > 
> 
> 

Re: tar 1.35: --delete creates tarfiles with duplicate file paths

[Pavel Raiskup]

On pátek 11. srpna 2023 14:32:18 CEST Ed Santiago wrote:
> On Fri, Aug 11, 2023 at 08:33:03AM +0200, Sergey Poznyakoff wrote:
> > Hi Ed,
> > 
> > > The --delete flag in 1.35 can generate a tarball that is
> > > unreadable by podman and possibly other tools using the
> > > Go tar library:
> > 
> > Unfortunately I cannot reproduce it,
> > 
> > > $ podman create --name a quay.io/libpod/testimage:20221018
> > > $ podman export a >/tmp/a.tar
> > > $ tar --delete -f /tmp/a.tar home/podman/pause
> > 
> > Can you send me the a.tar archive before and after delete.
> > or put them somewhere so I can download them?
> 
> Curious! It's 100% reproducible on my end. I've attached ante
> and post (before/after, but ASCII sortable) to:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=2230127
> 
> ...and added a comment describing the dups found with
> 
> tar tf a.post.tar | sort | uniq -cd

Is this related to some of the Fedora downstream patches?  FTR, here are
"vanilla" GNU tar RPM builds:
https://copr.fedorainfracloud.org/coprs/praiskup/tar-upstream-head/

The CI is has not been building for a few days now, needs upstream fix:
https://www.mail-archive.com/bug-tar@gnu.org/msg06277.html

Adding maintainer to CC.
Pavel

> Hope this helps,
> Ed
> 

Re: Fix 'make dist' for the pre-release HEAD

[Pavel Raiskup]

On úterý 1. srpna 2023 17:09:16 CEST Pavel Raiskup wrote:
> Hi, I am attaching the "version bump" commit which is needed to fix
> 'make dist' command, per Makefile.in (instantiated from Automake):
> 
> distdir-am: $(DISTFILES)
> @case `sed 15q $(srcdir)/NEWS` in \
> *"$(VERSION)"*) : ;; \
> *) \
>   echo "NEWS not updated; not releasing" 1>&2; \
>   exit 1;; \
> esac
> 
> It fails with:
> 
> NEWS not updated; not releasing
> 
> This breaks our RPM packaging CI:
> 
> https://github.com/praiskup/tar/commits/ci

Note it actually helps here:
https://github.com/praiskup/tar/pull/4

Pavel

> 
> Pavel
> 

Fix 'make dist' for the pre-release HEAD

[Pavel Raiskup]

Hi, I am attaching the "version bump" commit which is needed to fix
'make dist' command, per Makefile.in (instantiated from Automake):

distdir-am: $(DISTFILES)
@case `sed 15q $(srcdir)/NEWS` in \
*"$(VERSION)"*) : ;; \
*) \
  echo "NEWS not updated; not releasing" 1>&2; \
  exit 1;; \
esac

It fails with:

NEWS not updated; not releasing

This breaks our RPM packaging CI:

https://github.com/praiskup/tar/commits/ci

Pavel
>From 51f2d6f3fee492a4be9a772c6720f90ec00ed34b Mon Sep 17 00:00:00 2001
From: Pavel Raiskup 
Date: Tue, 1 Aug 2023 15:32:24 +0200
Subject: [PATCH] Bump to a pre-release versions

---
 NEWS | 2 +-
 configure.ac | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index 42a34513..d57c86d3 100644
--- a/NEWS
+++ b/NEWS
@@ -1,7 +1,7 @@
 GNU tar NEWS - User visible changes. 2023-08-01
 Please send GNU tar bug reports to 
 
-version TBD
+version 1.35.90
 
 * New manual section "Reproducibility", for reproducible tarballs.
 
diff --git a/configure.ac b/configure.ac
index 338e1834..4367d880 100644
--- a/configure.ac
+++ b/configure.ac
@@ -17,7 +17,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-AC_INIT([GNU tar], [1.35], [bug-tar@gnu.org])
+AC_INIT([GNU tar], [1.35.90], [bug-tar@gnu.org])
 AC_CONFIG_SRCDIR([src/tar.c])
 AC_CONFIG_AUX_DIR([build-aux])
 AC_CONFIG_HEADERS([config.h])
-- 
2.41.0

Re: exclude17.at and exclude18.at tests are missing in the tar-1.35 tarball

[Pavel Raiskup]

On čtvrtek 20. července 2023 22:26:31 CEST Paul Eggert wrote:
> On 2023-07-20 12:28, Lukas Javorsky wrote:
> > It does fail as the testsuite doesn't find those two tests.
> 
> Odd, as it works for me from a fresh tarball straight from gnu.org. See 
> attached transcript, which I ran on Ubuntu 23.04 x86-64. Perhaps the 
> timestamps are messed up on your copy?

I think the problems happen because we do do `autoreconf -v` for the
GNU tar package in Fedora (we have several patches applied on top of the
upstream tarball).  Some distributions (not Fedora) do this by default
per their policies, so I would eventually view this as a (low-priority?)
tarball issue.

Pavel

Re: [PATCH] Don't pre-create read-only files with --extract --xattrs

[Pavel Raiskup]

Thank you for the fix in 35d9845d5d41023 and 0b74885e81b90d6ab4.  The
patches simplify the code a lot.  I just think two in-code doc fixes are
worth it, patch attached.  Per https://github.com/praiskup/tar/pull/2

Pavel

On středa 24. listopadu 2021 15:56:05 CEST Pavel Raiskup wrote:
> Gently pinging on this again.  This causes invalid extraction failures.
> I'm ready for the review cycles (patch attached :-)!
> 
> Pavel
> 
> On Monday, February 15, 2021 10:00:20 AM CET Pavel Raiskup wrote:
> > Ping, I can see there's 1.34 release now, thank you!
> > 
> > But the git isn't updated yet so I'm not sure this has been fixed or not.
> > 
> > Pavel
> > 
> > On Tuesday, February 9, 2021 5:06:31 PM CET Pavel Raiskup wrote:
> > > Gently ping on this.  This is easily reproducible, so I am re-attaching
> > > the patch including a new test-case that fails with the current code:
> > > 
> > > $ tar --xattrs -xf tar
> > > tar: setxattrat: Cannot set 'user.attr' extended attribute for file 
> > > 'file': Permission denied
> > > tar: file: Cannot open: Permission denied
> > >     tar: Exiting with failure status due to previous errors
> > > 
> > > Pavel
> > > 
> > > On Friday, October 9, 2020 1:39:11 PM CET Pavel Raiskup wrote:
> > > > We used to respect the target file mode when pre-creating files in
> > > > set_xattr, so we also pre-created read-only files that we were not able
> > > > to open later for writing.  This is now fixed, and we always create the
> > > > file with S_IWUSR.
> > > > 
> > > > Fixes the original bug report https://bugzilla.redhat.com/1886540
> > > > 
> > > > * src/extract.c (set_xattr): Blindly add S_IWUSR flag to pre-created
> > > > files, to avoid openat failures later.
> > > > ---
> > > >  src/extract.c | 8 +++-
> > > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/src/extract.c b/src/extract.c
> > > > index b73a591..2e650ad 100644
> > > > --- a/src/extract.c
> > > > +++ b/src/extract.c
> > > > @@ -865,7 +865,13 @@ set_xattr (char const *file_name, struct 
> > > > tar_stat_info const *st,
> > > >  
> > > >for (;;)
> > > >  {
> > > > -  if (!mknodat (chdir_fd, file_name, mode ^ 
> > > > invert_permissions, 0))
> > > > +  /* We'll open the file with O_WRONLY later by 
> > > > open_output_file,
> > > > + therefore we need to give us the S_IWUSR bit.  If the 
> > > > file was
> > > > + meant to be user-read-only, the permissions will be 
> > > > corrected by
> > > > + the set_stat call. */
> > > > +  mode_t initial_mode = mode ^ invert_permissions | S_IWUSR;
> > > > +
> > > > +  if (!mknodat (chdir_fd, file_name, initial_mode, 0))
> > > >  {
> > > >/* Successfully created file */
> > > >xattrs_xattrs_set (st, file_name, typeflag, 0);
> > > > 
> > > 
> > > 
> > 
> > 
> > 
> > 
> > 
> 
> 

>From 4fc376ec43c8a235306ef719db3e157a67969598 Mon Sep 17 00:00:00 2001
From: Pavel Raiskup 
Date: Thu, 18 May 2023 14:30:08 +0200
Subject: [PATCH] Comment a bit on the xattr extraction logic

* src/extract.c (extract_file): Document why we pre-create with S_IWUSR.
(set_xattr): Drop the INVERT_PERMISSIONS doc leftover.
---
 src/extract.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/extract.c b/src/extract.c
index aec5de69..7adc7aff 100644
--- a/src/extract.c
+++ b/src/extract.c
@@ -902,11 +902,10 @@ maybe_recoverable (char *file_name, bool regular, bool *interdir_made)
(e.g. on Lustre distributed parallel filesystem - setting info about how many
servers is this file striped over, stripe size, mirror copies, etc.
in advance dramatically improves the following  performance of reading and
-   writing a file).  If not restoring permissions, invert the INVERT_PERMISSIONS
-   bits from the file's current permissions.  TYPEFLAG specifies the type of the
-   file.  Return a negative number (setting errno) on failure, zero if
-   successful but FILE_NAME was not created (e.g., xattrs not
-   available), and a positive number if FILE_NAME was created.  */
+   writing a file).  TYPEFLAG specifies the type of the file.  Return a negative
+   number (setting errno) on failure, zero if successful but FILE_NAME was not
+   creat

Re: [PATCH] Fix --xattrs-include='*' documentation

[Pavel Raiskup]

Ping on this doc-only patch, re-attaching.

Pavel

On pondělí 29. listopadu 2021 9:52:14 CEST Pavel Raiskup wrote:
> One more update after the review in [1].  Patch is updated.
> 
> [1] https://github.com/praiskup/tar/pull/3
> 
> Thanks, Pavel
> 
> On Wednesday, November 24, 2021 4:06:38 PM CET Pavel Raiskup wrote:
> > Updated patch is attached (more appropriate place for the second chunk).
> > 
> > Pavel
> > 
> > On Wednesday, November 24, 2021 3:32:12 PM CET Pavel Raiskup wrote:
> > > * doc/tar.texi (Extended File Attributes): The default extraction
> > > pattern consists of just 'user.*' namespace only.  While on it, try
> > > to explain the reasons for this default behavior.
> > > ---
> > >  doc/tar.texi | 15 ---
> > >  1 file changed, 12 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/doc/tar.texi b/doc/tar.texi
> > > index 389a3448..e03ce3d4 100644
> > > --- a/doc/tar.texi
> > > +++ b/doc/tar.texi
> > > @@ -5647,9 +5647,14 @@ Disable extended attributes support.  This is the 
> > > default.
> > >  Attribute names are strings prefixed by a @dfn{namespace} name and a dot.
> > >  Currently, four namespaces exist: @samp{user}, @samp{trusted},
> > >  @samp{security} and @samp{system}.  By default, when @option{--xattr}
> > > -is used, all names are stored in the archive (or extracted, if using
> > > -@option{--extract}).  This can be controlled using the following
> > > -options:
> > > +is used, all names are stored in the archive (with @option{--create}),
> > > +but only @samp{user} namespace is extracted (if using 
> > > @option{--extract}).
> > > +The reason for this behavior is that any other, system defined attributes
> > > +don't provide us sufficient compatibility promise.  Storing all 
> > > attributes
> > > +is safe operation for the archiving purposes.  Though extracting those
> > > +(often security related) attributes on a different system than originally
> > > +archived can lead to extraction failures, or even misinterpretations.
> > > +This behavior can be controlled using the following options:
> > >  
> > >  @table @option
> > >  @item --xattrs-exclude=@var{pattern}
> > > @@ -5659,6 +5664,10 @@ Specify exclude pattern for extended attributes.
> > >  Specify include pattern for extended attributes.
> > >  @end table
> > >  
> > > +Users shall manually check the attributes are binary compatible with the
> > > +target system first, before any other namespace is extracted with an
> > > +explicit @option{--xattr-include} option.
> > > +
> > >  Here, the @var{pattern} is a globbing pattern.  For example, the
> > >  following command:
> > >  
> > > 
> > 
> > 
> 
> 

>From b48a2560096645a0927c8d35c64723a4139eaa62 Mon Sep 17 00:00:00 2001
From: Pavel Raiskup 
Date: Wed, 24 Nov 2021 14:42:07 +0100
Subject: [PATCH] Fix --xattr-include='*' documentation

* doc/tar.texi (Extended File Attributes): The default extraction
pattern consists of just 'user.*' namespace only.  While on it, try
to explain the reasons for this default behavior.
---
 doc/tar.texi | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/doc/tar.texi b/doc/tar.texi
index 389a3448..8de34728 100644
--- a/doc/tar.texi
+++ b/doc/tar.texi
@@ -5646,10 +5646,15 @@ Disable extended attributes support.  This is the default.
 
 Attribute names are strings prefixed by a @dfn{namespace} name and a dot.
 Currently, four namespaces exist: @samp{user}, @samp{trusted},
-@samp{security} and @samp{system}.  By default, when @option{--xattr}
-is used, all names are stored in the archive (or extracted, if using
-@option{--extract}).  This can be controlled using the following
-options:
+@samp{security} and @samp{system}.  By default, when @option{--xattrs}
+is used, all names are stored in the archive (with @option{--create}),
+but only @samp{user} namespace is extracted (if using @option{--extract}).
+The reason for this behavior is that any other, system defined attributes
+don't provide us sufficient compatibility promise.  Storing all attributes
+is safe operation for the archiving purposes.  Though extracting those
+(often security related) attributes on a different system than originally
+archived can lead to extraction failures, or even misinterpretations.
+This behavior can be controlled using the following options:
 
 @table @option
 @item --xattrs-exclude=@var{pattern}
@@ -5669,6 +5674,10 @@ $ @kbd{tar --xattrs --xattrs-exclude='user.*' -c a.tar .}
 will include in the archive @file{a.tar} all attributes, except those
 from the @samp{user} namespace.
 
+Users shall check the attributes are binary compatible with the target system
+before any other namespace is extracted with an explicit
+@option{--xattrs-include} option.
+
 Any number of these options can be given, thereby creating lists of
 include and exclude patterns.
 
-- 
2.33.1

Re: Build failure from git HEAD

[Pavel Raiskup]

On Tuesday, June 14, 2022 1:51:18 AM CEST Paul Eggert wrote:
> Thanks for reporting that. I installed the attached to fix it.

Indeed helped.  Thank you, Paul.
Pavel

Build failure from git HEAD

[Pavel Raiskup]

The build on Fedora suddenly started to fail:
https://github.com/praiskup/tar/commits/ci

Seems like since the "build: update gnulib and paxutils submodules to latest"
commit.

Build failure:
make  distdir-am
make[3]: Entering directory '/workdir/tar/gnu'
make[3]: Leaving directory '/workdir/tar/gnu'
make[2]: Leaving directory '/workdir/tar/gnu'
 (cd lib && make  top_distdir=../tar-1.34.90 distdir=../tar-1.34.90/lib \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[2]: Entering directory '/workdir/tar/lib'
  GEN  rmt-command.h
make  distdir-am
make[3]: Entering directory '/workdir/tar/lib'
make[3]: Leaving directory '/workdir/tar/lib'
make[2]: Leaving directory '/workdir/tar/lib'
 (cd rmt && make  top_distdir=../tar-1.34.90 distdir=../tar-1.34.90/rmt \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[2]: Entering directory '/workdir/tar/rmt'
make  distdir-am
make[3]: Entering directory '/workdir/tar/rmt'
make[3]: Leaving directory '/workdir/tar/rmt'
make[3]: *** No rule to make target 'DISTFILES', needed by 'distdir-am'.  Stop.
make[2]: Leaving directory '/workdir/tar/rmt'
make[2]: *** [Makefile:1605: distdir] Error 2
make[1]: Leaving directory '/workdir/tar'
make[1]: *** [Makefile:1682: distdir-am] Error 1
make: *** [Makefile:1675: distdir] Error 2

Full log file (will be removed soon):
https://download.copr.fedorainfracloud.org/results/praiskup/tar-upstream-head/srpm-builds/04512935/builder-live.log.gz

Pavel

Re: Failed to 'make dist' from git (on Fedora)

[Pavel Raiskup]

On Monday, December 13, 2021 3:19:37 PM CET Sergey Poznyakoff wrote:
> Thanks for reporting.  Commit ac119c80 should fix that.  Please, pull.

Thanks for the fix!  It helped:
https://copr.fedorainfracloud.org/coprs/praiskup/tar-upstream-head/build/3039903/

Pavel

Failed to 'make dist' from git (on Fedora)

[Pavel Raiskup]

Hello, I've been playing with automatic rebuilds of GNU tar against Fedora/EPEL
a bit, and the recent commits in 'master' branch seem to break the build
process from git [2]:

+ make dist-xz
make  distdir-am
make[1]: Entering directory '/workdir/tar'
if test -d .git; then  \
  cmd=./build-aux/gitlog-to-changelog; \
  if test -n "2009-03-06"; then \
cmd="$cmd --since=\"2009-03-06\""; \
  fi;  \
  if test -n "ChangeLog.amend"; then   \
cmd="$cmd --amend=ChangeLog.amend";   \
  fi;  \
  $cmd --format='%s%n%n%b%n' |\
sed '/$/d' | fmt -s > ./cl-t; \
  if test -n "ChangeLog.CVS" && test -f "ChangeLog.CVS"; \
  then \
echo "" >> ./cl-t;  \
cat "ChangeLog.CVS" | \
  sed '/^Local Variables:/,/^End:/d' >> ./cl-t; \
  fi;  \
  echo "Local Variables:" >> ./cl-t;\
  echo "mode: change-log" >> ./cl-t;\
  echo "version-control: never"  >> ./cl-t; \
  echo "buffer-read-only: t" >> ./cl-t; \
  echo "End:" >> ./cl-t;\
  rm -f ./ChangeLog;\
  mv ./cl-t ./ChangeLog; \
fi
Can't locate object method "new" via package "Safe" (perhaps you forgot to 
load "Safe"?) at ./build-aux/gitlog-to-changelog line 326,  line 545.
NEWS not updated; not releasing
make[1]: *** [Makefile:1660: distdir-am] Error 1
make[1]: Leaving directory '/workdir/tar'
make: *** [Makefile:1657: distdir] Error 2

[1] https://github.com/praiskup/tar/commits/ci
[2] 
https://download.copr.fedorainfracloud.org/results/praiskup/tar-upstream-head/srpm-builds/03032192/builder-live.log.gz

Pavel

Re: [PATCH] Fix --xattrs-include='*' documentation

[Pavel Raiskup]

One more update after the review in [1].  Patch is updated.

[1] https://github.com/praiskup/tar/pull/3

Thanks, Pavel

On Wednesday, November 24, 2021 4:06:38 PM CET Pavel Raiskup wrote:
> Updated patch is attached (more appropriate place for the second chunk).
> 
> Pavel
> 
> On Wednesday, November 24, 2021 3:32:12 PM CET Pavel Raiskup wrote:
> > * doc/tar.texi (Extended File Attributes): The default extraction
> > pattern consists of just 'user.*' namespace only.  While on it, try
> > to explain the reasons for this default behavior.
> > ---
> >  doc/tar.texi | 15 ---
> >  1 file changed, 12 insertions(+), 3 deletions(-)
> > 
> > diff --git a/doc/tar.texi b/doc/tar.texi
> > index 389a3448..e03ce3d4 100644
> > --- a/doc/tar.texi
> > +++ b/doc/tar.texi
> > @@ -5647,9 +5647,14 @@ Disable extended attributes support.  This is the 
> > default.
> >  Attribute names are strings prefixed by a @dfn{namespace} name and a dot.
> >  Currently, four namespaces exist: @samp{user}, @samp{trusted},
> >  @samp{security} and @samp{system}.  By default, when @option{--xattr}
> > -is used, all names are stored in the archive (or extracted, if using
> > -@option{--extract}).  This can be controlled using the following
> > -options:
> > +is used, all names are stored in the archive (with @option{--create}),
> > +but only @samp{user} namespace is extracted (if using @option{--extract}).
> > +The reason for this behavior is that any other, system defined attributes
> > +don't provide us sufficient compatibility promise.  Storing all attributes
> > +is safe operation for the archiving purposes.  Though extracting those
> > +(often security related) attributes on a different system than originally
> > +archived can lead to extraction failures, or even misinterpretations.
> > +This behavior can be controlled using the following options:
> >  
> >  @table @option
> >  @item --xattrs-exclude=@var{pattern}
> > @@ -5659,6 +5664,10 @@ Specify exclude pattern for extended attributes.
> >  Specify include pattern for extended attributes.
> >  @end table
> >  
> > +Users shall manually check the attributes are binary compatible with the
> > +target system first, before any other namespace is extracted with an
> > +explicit @option{--xattr-include} option.
> > +
> >  Here, the @var{pattern} is a globbing pattern.  For example, the
> >  following command:
> >  
> > 
> 
> 

>From b48a2560096645a0927c8d35c64723a4139eaa62 Mon Sep 17 00:00:00 2001
From: Pavel Raiskup 
Date: Wed, 24 Nov 2021 14:42:07 +0100
Subject: [PATCH] Fix --xattr-include='*' documentation

* doc/tar.texi (Extended File Attributes): The default extraction
pattern consists of just 'user.*' namespace only.  While on it, try
to explain the reasons for this default behavior.
---
 doc/tar.texi | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/doc/tar.texi b/doc/tar.texi
index 389a3448..8de34728 100644
--- a/doc/tar.texi
+++ b/doc/tar.texi
@@ -5646,10 +5646,15 @@ Disable extended attributes support.  This is the default.
 
 Attribute names are strings prefixed by a @dfn{namespace} name and a dot.
 Currently, four namespaces exist: @samp{user}, @samp{trusted},
-@samp{security} and @samp{system}.  By default, when @option{--xattr}
-is used, all names are stored in the archive (or extracted, if using
-@option{--extract}).  This can be controlled using the following
-options:
+@samp{security} and @samp{system}.  By default, when @option{--xattrs}
+is used, all names are stored in the archive (with @option{--create}),
+but only @samp{user} namespace is extracted (if using @option{--extract}).
+The reason for this behavior is that any other, system defined attributes
+don't provide us sufficient compatibility promise.  Storing all attributes
+is safe operation for the archiving purposes.  Though extracting those
+(often security related) attributes on a different system than originally
+archived can lead to extraction failures, or even misinterpretations.
+This behavior can be controlled using the following options:
 
 @table @option
 @item --xattrs-exclude=@var{pattern}
@@ -5669,6 +5674,10 @@ $ @kbd{tar --xattrs --xattrs-exclude='user.*' -c a.tar .}
 will include in the archive @file{a.tar} all attributes, except those
 from the @samp{user} namespace.
 
+Users shall check the attributes are binary compatible with the target system
+before any other namespace is extracted with an explicit
+@option{--xattrs-include} option.
+
 Any number of these options can be given, thereby creating lists of
 include and exclude patterns.
 
-- 
2.33.1

Re: [PATCH] Fix --xattr-include='*' documentation

[Pavel Raiskup]

Updated patch is attached (more appropriate place for the second chunk).

Pavel

On Wednesday, November 24, 2021 3:32:12 PM CET Pavel Raiskup wrote:
> * doc/tar.texi (Extended File Attributes): The default extraction
> pattern consists of just 'user.*' namespace only.  While on it, try
> to explain the reasons for this default behavior.
> ---
>  doc/tar.texi | 15 ---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/doc/tar.texi b/doc/tar.texi
> index 389a3448..e03ce3d4 100644
> --- a/doc/tar.texi
> +++ b/doc/tar.texi
> @@ -5647,9 +5647,14 @@ Disable extended attributes support.  This is the 
> default.
>  Attribute names are strings prefixed by a @dfn{namespace} name and a dot.
>  Currently, four namespaces exist: @samp{user}, @samp{trusted},
>  @samp{security} and @samp{system}.  By default, when @option{--xattr}
> -is used, all names are stored in the archive (or extracted, if using
> -@option{--extract}).  This can be controlled using the following
> -options:
> +is used, all names are stored in the archive (with @option{--create}),
> +but only @samp{user} namespace is extracted (if using @option{--extract}).
> +The reason for this behavior is that any other, system defined attributes
> +don't provide us sufficient compatibility promise.  Storing all attributes
> +is safe operation for the archiving purposes.  Though extracting those
> +(often security related) attributes on a different system than originally
> +archived can lead to extraction failures, or even misinterpretations.
> +This behavior can be controlled using the following options:
>  
>  @table @option
>  @item --xattrs-exclude=@var{pattern}
> @@ -5659,6 +5664,10 @@ Specify exclude pattern for extended attributes.
>  Specify include pattern for extended attributes.
>  @end table
>  
> +Users shall manually check the attributes are binary compatible with the
> +target system first, before any other namespace is extracted with an
> +explicit @option{--xattr-include} option.
> +
>  Here, the @var{pattern} is a globbing pattern.  For example, the
>  following command:
>  
> 

>From 82457322dd91eb887963a9537baa705176442d08 Mon Sep 17 00:00:00 2001
From: Pavel Raiskup 
Date: Wed, 24 Nov 2021 14:42:07 +0100
Subject: [PATCH] Fix --xattr-include='*' documentation

* doc/tar.texi (Extended File Attributes): The default extraction
pattern consists of just 'user.*' namespace only.  While on it, try
to explain the reasons for this default behavior.
---
 doc/tar.texi | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/doc/tar.texi b/doc/tar.texi
index 389a3448..952bb875 100644
--- a/doc/tar.texi
+++ b/doc/tar.texi
@@ -5647,9 +5647,14 @@ Disable extended attributes support.  This is the default.
 Attribute names are strings prefixed by a @dfn{namespace} name and a dot.
 Currently, four namespaces exist: @samp{user}, @samp{trusted},
 @samp{security} and @samp{system}.  By default, when @option{--xattr}
-is used, all names are stored in the archive (or extracted, if using
-@option{--extract}).  This can be controlled using the following
-options:
+is used, all names are stored in the archive (with @option{--create}),
+but only @samp{user} namespace is extracted (if using @option{--extract}).
+The reason for this behavior is that any other, system defined attributes
+don't provide us sufficient compatibility promise.  Storing all attributes
+is safe operation for the archiving purposes.  Though extracting those
+(often security related) attributes on a different system than originally
+archived can lead to extraction failures, or even misinterpretations.
+This behavior can be controlled using the following options:
 
 @table @option
 @item --xattrs-exclude=@var{pattern}
@@ -5669,6 +5674,10 @@ $ @kbd{tar --xattrs --xattrs-exclude='user.*' -c a.tar .}
 will include in the archive @file{a.tar} all attributes, except those
 from the @samp{user} namespace.
 
+Users shall check the attributes are binary compatible with the target system
+before any other namespace is extracted with an explicit
+@option{--xattr-include} option.
+
 Any number of these options can be given, thereby creating lists of
 include and exclude patterns.
 
-- 
2.33.1

Re: [PATCH] Don't pre-create read-only files with --extract --xattrs

[Pavel Raiskup]

Gently pinging on this again.  This causes invalid extraction failures.
I'm ready for the review cycles (patch attached :-)!

Pavel

On Monday, February 15, 2021 10:00:20 AM CET Pavel Raiskup wrote:
> Ping, I can see there's 1.34 release now, thank you!
> 
> But the git isn't updated yet so I'm not sure this has been fixed or not.
> 
> Pavel
> 
> On Tuesday, February 9, 2021 5:06:31 PM CET Pavel Raiskup wrote:
> > Gently ping on this.  This is easily reproducible, so I am re-attaching
> > the patch including a new test-case that fails with the current code:
> > 
> > $ tar --xattrs -xf tar
> > tar: setxattrat: Cannot set 'user.attr' extended attribute for file 
> > 'file': Permission denied
> > tar: file: Cannot open: Permission denied
> > tar: Exiting with failure status due to previous errors
> > 
> > Pavel
> > 
> > On Friday, October 9, 2020 1:39:11 PM CET Pavel Raiskup wrote:
> > > We used to respect the target file mode when pre-creating files in
> > > set_xattr, so we also pre-created read-only files that we were not able
> > > to open later for writing.  This is now fixed, and we always create the
> > > file with S_IWUSR.
> > > 
> > > Fixes the original bug report https://bugzilla.redhat.com/1886540
> > > 
> > > * src/extract.c (set_xattr): Blindly add S_IWUSR flag to pre-created
> > > files, to avoid openat failures later.
> > > ---
> > >  src/extract.c | 8 +++-
> > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/src/extract.c b/src/extract.c
> > > index b73a591..2e650ad 100644
> > > --- a/src/extract.c
> > > +++ b/src/extract.c
> > > @@ -865,7 +865,13 @@ set_xattr (char const *file_name, struct 
> > > tar_stat_info const *st,
> > >  
> > >for (;;)
> > >  {
> > > -  if (!mknodat (chdir_fd, file_name, mode ^ invert_permissions, 
> > > 0))
> > > +  /* We'll open the file with O_WRONLY later by open_output_file,
> > > + therefore we need to give us the S_IWUSR bit.  If the file 
> > > was
> > > + meant to be user-read-only, the permissions will be 
> > > corrected by
> > > + the set_stat call. */
> > > +  mode_t initial_mode = mode ^ invert_permissions | S_IWUSR;
> > > +
> > > +  if (!mknodat (chdir_fd, file_name, initial_mode, 0))
> > >  {
> > >/* Successfully created file */
> > >xattrs_xattrs_set (st, file_name, typeflag, 0);
> > > 
> > 
> > 
> 
> 
> 
> 
> 

>From 5f9283cd00273115097920da7ece8e97ccf1902b Mon Sep 17 00:00:00 2001
From: Pavel Raiskup 
Date: Fri, 9 Oct 2020 12:03:22 +0200
Subject: [PATCH] Don't pre-create read-only files with --extract --xattrs

We used to respect the target file mode when pre-creating files in
set_xattr, so we also pre-created read-only files that we were not able
to open later for writing.  This is now fixed, and we always create the
file with S_IWUSR.

Fixes the original bug report https://bugzilla.redhat.com/1886540

* src/extract.c (set_xattr): Blindly add S_IWUSR flag to pre-created
files, to avoid openat failures later.
* tests/xattr08.at: New test-case file.
---
 src/extract.c|  8 +++-
 tests/xattr08.at | 45 +
 2 files changed, 52 insertions(+), 1 deletion(-)
 create mode 100644 tests/xattr08.at

diff --git a/src/extract.c b/src/extract.c
index 850a08a2..1bfe36a6 100644
--- a/src/extract.c
+++ b/src/extract.c
@@ -860,7 +860,13 @@ set_xattr (char const *file_name, struct tar_stat_info const *st,
 
   for (;;)
 {
-  if (!mknodat (chdir_fd, file_name, mode ^ invert_permissions, 0))
+  /* We'll open the file with O_WRONLY later by open_output_file,
+ therefore we need to give us the S_IWUSR bit.  If the file was
+ meant to be user-read-only, the permissions will be corrected by
+ the set_stat call. */
+  mode_t initial_mode = mode ^ invert_permissions | S_IWUSR;
+
+  if (!mknodat (chdir_fd, file_name, initial_mode, 0))
 {
   /* Successfully created file */
   xattrs_xattrs_set (st, file_name, typeflag, 0);
diff --git a/tests/xattr08.at b/tests/xattr08.at
new file mode 100644
index ..0dd3ed34
--- /dev/null
+++ b/tests/xattr08.at
@@ -0,0 +1,45 @@
+# Process this file with autom4te to create testsuite. -*- Autotest -*-
+#
+# Test suite for GNU tar.
+# Copyright 2021 Free Software Foundation, Inc.
+
+# This file is part of GNU tar.
+
+# GNU tar is free software; you can redis

[PATCH] Fix --xattr-include='*' documentation

[Pavel Raiskup]

* doc/tar.texi (Extended File Attributes): The default extraction
pattern consists of just 'user.*' namespace only.  While on it, try
to explain the reasons for this default behavior.
---
 doc/tar.texi | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/doc/tar.texi b/doc/tar.texi
index 389a3448..e03ce3d4 100644
--- a/doc/tar.texi
+++ b/doc/tar.texi
@@ -5647,9 +5647,14 @@ Disable extended attributes support.  This is the 
default.
 Attribute names are strings prefixed by a @dfn{namespace} name and a dot.
 Currently, four namespaces exist: @samp{user}, @samp{trusted},
 @samp{security} and @samp{system}.  By default, when @option{--xattr}
-is used, all names are stored in the archive (or extracted, if using
-@option{--extract}).  This can be controlled using the following
-options:
+is used, all names are stored in the archive (with @option{--create}),
+but only @samp{user} namespace is extracted (if using @option{--extract}).
+The reason for this behavior is that any other, system defined attributes
+don't provide us sufficient compatibility promise.  Storing all attributes
+is safe operation for the archiving purposes.  Though extracting those
+(often security related) attributes on a different system than originally
+archived can lead to extraction failures, or even misinterpretations.
+This behavior can be controlled using the following options:
 
 @table @option
 @item --xattrs-exclude=@var{pattern}
@@ -5659,6 +5664,10 @@ Specify exclude pattern for extended attributes.
 Specify include pattern for extended attributes.
 @end table
 
+Users shall manually check the attributes are binary compatible with the
+target system first, before any other namespace is extracted with an
+explicit @option{--xattr-include} option.
+
 Here, the @var{pattern} is a globbing pattern.  For example, the
 following command:
 
-- 
2.33.1

Re: [PATCH] xattrs: fix capabilities root test

[Pavel Raiskup]

Unfortunately, the old variant of this patch was applied.  So the test-case is
not yet fixed.  I'm attaching a correction patch.

Thank you!
Pavel


On Monday, January 25, 2021 10:08:39 AM CET Pavel Raiskup wrote:
> I'm attaching updated patch.  The getcap output actually changed like this:
> 
> - dir/file = cap_chown+ei
> + dir/file cap_chown=ei
> 
> Pavel
> 
> On Tuesday, January 19, 2021 4:47:44 PM CET Pavel Raiskup wrote:
> > Related discussion in the Fedora pull-request:
> > https://src.fedoraproject.org/rpms/tar/pull-request/8
> > 
> > * tests/capabs_raw01.at: Newer systems (currently e.g. Fedora 34)
> > print getcap output in format CAP=VAL, not CAP+VAL.
> > ---
> >  tests/capabs_raw01.at | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/capabs_raw01.at b/tests/capabs_raw01.at
> > index f9b97736..a1d94116 100644
> > --- a/tests/capabs_raw01.at
> > +++ b/tests/capabs_raw01.at
> > @@ -44,10 +44,11 @@ rm -rf dir
> >  # restore _all_ xattrs (not just the user.* domain)
> >  tar --xattrs --xattrs-include='*' -xf archive.tar
> >  
> > -getcap dir/file
> > +# Newer systems print = instead of + here
> > +getcap dir/file | sed 's/+/=/'
> >  ],
> >  [0],
> > -[dir/file = cap_chown+ei
> > +[dir/file = cap_chown=ei
> >  ])
> >  
> >  AT_CLEANUP
> > 
> 
> 

>From a27aa0fce1bb3ee60d63ab5577845cac004755e2 Mon Sep 17 00:00:00 2001
From: Pavel Raiskup 
Date: Mon, 1 Mar 2021 14:36:05 +0100
Subject: [PATCH] Fix tests/capabs_raw01.at once more

The output format of getcap changed once more.

* tests/capabs_raw01.at: Accept all three formats, with '+', '=' and
also without any symbol.
---
 tests/capabs_raw01.at | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tests/capabs_raw01.at b/tests/capabs_raw01.at
index a1d94116..6c167cb6 100644
--- a/tests/capabs_raw01.at
+++ b/tests/capabs_raw01.at
@@ -44,11 +44,15 @@ rm -rf dir
 # restore _all_ xattrs (not just the user.* domain)
 tar --xattrs --xattrs-include='*' -xf archive.tar
 
-# Newer systems print = instead of + here
-getcap dir/file | sed 's/+/=/'
+# The output changed a few times, first the '+' sign was replaced with '=' and
+# then the '=' symbol disappeared:
+# 1. dir/file + cap_chown+ei
+# 2. dir/file = cap_chown+ei
+# 3. dir/file cap_chown=ei
+getcap dir/file | sed -e 's/+/=/' -e 's|dir/file = |dir/file |'
 ],
 [0],
-[dir/file = cap_chown=ei
+[dir/file cap_chown=ei
 ])
 
 AT_CLEANUP
-- 
2.30.1

Re: [PATCH] Don't pre-create read-only files with --extract --xattrs

[Pavel Raiskup]

Ping, I can see there's 1.34 release now, thank you!

But the git isn't updated yet so I'm not sure this has been fixed or not.

Pavel

On Tuesday, February 9, 2021 5:06:31 PM CET Pavel Raiskup wrote:
> Gently ping on this.  This is easily reproducible, so I am re-attaching
> the patch including a new test-case that fails with the current code:
> 
> $ tar --xattrs -xf tar
> tar: setxattrat: Cannot set 'user.attr' extended attribute for file 
> 'file': Permission denied
> tar: file: Cannot open: Permission denied
> tar: Exiting with failure status due to previous errors
> 
> Pavel
> 
> On Friday, October 9, 2020 1:39:11 PM CET Pavel Raiskup wrote:
> > We used to respect the target file mode when pre-creating files in
> > set_xattr, so we also pre-created read-only files that we were not able
> > to open later for writing.  This is now fixed, and we always create the
> > file with S_IWUSR.
> > 
> > Fixes the original bug report https://bugzilla.redhat.com/1886540
> > 
> > * src/extract.c (set_xattr): Blindly add S_IWUSR flag to pre-created
> > files, to avoid openat failures later.
> > ---
> >  src/extract.c | 8 +++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/extract.c b/src/extract.c
> > index b73a591..2e650ad 100644
> > --- a/src/extract.c
> > +++ b/src/extract.c
> > @@ -865,7 +865,13 @@ set_xattr (char const *file_name, struct tar_stat_info 
> > const *st,
> >  
> >for (;;)
> >  {
> > -  if (!mknodat (chdir_fd, file_name, mode ^ invert_permissions, 0))
> > +  /* We'll open the file with O_WRONLY later by open_output_file,
> > + therefore we need to give us the S_IWUSR bit.  If the file was
> > + meant to be user-read-only, the permissions will be corrected 
> > by
> > + the set_stat call. */
> > +  mode_t initial_mode = mode ^ invert_permissions | S_IWUSR;
> > +
> > +  if (!mknodat (chdir_fd, file_name, initial_mode, 0))
> >  {
> >/* Successfully created file */
> >xattrs_xattrs_set (st, file_name, typeflag, 0);
> > 
> 
> 

Re: [PATCH] Don't pre-create read-only files with --extract --xattrs

[Pavel Raiskup]

Gently ping on this.  This is easily reproducible, so I am re-attaching
the patch including a new test-case that fails with the current code:

$ tar --xattrs -xf tar
tar: setxattrat: Cannot set 'user.attr' extended attribute for file 'file': 
Permission denied
tar: file: Cannot open: Permission denied
tar: Exiting with failure status due to previous errors

Pavel

On Friday, October 9, 2020 1:39:11 PM CET Pavel Raiskup wrote:
> We used to respect the target file mode when pre-creating files in
> set_xattr, so we also pre-created read-only files that we were not able
> to open later for writing.  This is now fixed, and we always create the
> file with S_IWUSR.
> 
> Fixes the original bug report https://bugzilla.redhat.com/1886540
> 
> * src/extract.c (set_xattr): Blindly add S_IWUSR flag to pre-created
> files, to avoid openat failures later.
> ---
>  src/extract.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/src/extract.c b/src/extract.c
> index b73a591..2e650ad 100644
> --- a/src/extract.c
> +++ b/src/extract.c
> @@ -865,7 +865,13 @@ set_xattr (char const *file_name, struct tar_stat_info 
> const *st,
>  
>for (;;)
>  {
> -  if (!mknodat (chdir_fd, file_name, mode ^ invert_permissions, 0))
> +  /* We'll open the file with O_WRONLY later by open_output_file,
> + therefore we need to give us the S_IWUSR bit.  If the file was
> + meant to be user-read-only, the permissions will be corrected by
> + the set_stat call. */
> +  mode_t initial_mode = mode ^ invert_permissions | S_IWUSR;
> +
> +  if (!mknodat (chdir_fd, file_name, initial_mode, 0))
>  {
>/* Successfully created file */
>xattrs_xattrs_set (st, file_name, typeflag, 0);
> 

>From 42201a0ce560846f770cd5305d04287b13e4bd2b Mon Sep 17 00:00:00 2001
From: Pavel Raiskup 
Date: Fri, 9 Oct 2020 12:03:22 +0200
Subject: [PATCH] Don't pre-create read-only files with --extract --xattrs

We used to respect the target file mode when pre-creating files in
set_xattr, so we also pre-created read-only files that we were not able
to open later for writing.  This is now fixed, and we always create the
file with S_IWUSR.

Fixes the original bug report https://bugzilla.redhat.com/1886540

* src/extract.c (set_xattr): Blindly add S_IWUSR flag to pre-created
files, to avoid openat failures later.
* tests/xattr08.at: New test-case file.
---
 src/extract.c|  8 +++-
 tests/xattr08.at | 45 +
 2 files changed, 52 insertions(+), 1 deletion(-)
 create mode 100644 tests/xattr08.at

diff --git a/src/extract.c b/src/extract.c
index 80009a54..358603fe 100644
--- a/src/extract.c
+++ b/src/extract.c
@@ -865,7 +865,13 @@ set_xattr (char const *file_name, struct tar_stat_info const *st,
 
   for (;;)
 {
-  if (!mknodat (chdir_fd, file_name, mode ^ invert_permissions, 0))
+  /* We'll open the file with O_WRONLY later by open_output_file,
+ therefore we need to give us the S_IWUSR bit.  If the file was
+ meant to be user-read-only, the permissions will be corrected by
+ the set_stat call. */
+  mode_t initial_mode = mode ^ invert_permissions | S_IWUSR;
+
+  if (!mknodat (chdir_fd, file_name, initial_mode, 0))
 {
   /* Successfully created file */
   xattrs_xattrs_set (st, file_name, typeflag, 0);
diff --git a/tests/xattr08.at b/tests/xattr08.at
new file mode 100644
index ..0dd3ed34
--- /dev/null
+++ b/tests/xattr08.at
@@ -0,0 +1,45 @@
+# Process this file with autom4te to create testsuite. -*- Autotest -*-
+#
+# Test suite for GNU tar.
+# Copyright 2021 Free Software Foundation, Inc.
+
+# This file is part of GNU tar.
+
+# GNU tar is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+
+# GNU tar is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+# Test description: Test for extracting read-only files with xattrs.
+#
+# Relevant mailing list thread:
+# https://lists.gnu.org/archive/html/bug-tar/2020-10/msg1.html
+
+AT_SETUP([xattrs: extract read-only files])
+AT_KEYWORDS([xattrs xattr07])
+
+AT_TAR_CHECK([
+AT_XATTRS_PREREQ
+
+touch file
+setfattr -n 'user.attr' -v value file
+chmod 444 file
+tar --xattrs -cf tar file

Re: [PATCH] xattrs: fix capabilities root test

[Pavel Raiskup]

I'm attaching updated patch.  The getcap output actually changed like this:

- dir/file = cap_chown+ei
+ dir/file cap_chown=ei

Pavel

On Tuesday, January 19, 2021 4:47:44 PM CET Pavel Raiskup wrote:
> Related discussion in the Fedora pull-request:
> https://src.fedoraproject.org/rpms/tar/pull-request/8
> 
> * tests/capabs_raw01.at: Newer systems (currently e.g. Fedora 34)
> print getcap output in format CAP=VAL, not CAP+VAL.
> ---
>  tests/capabs_raw01.at | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/capabs_raw01.at b/tests/capabs_raw01.at
> index f9b97736..a1d94116 100644
> --- a/tests/capabs_raw01.at
> +++ b/tests/capabs_raw01.at
> @@ -44,10 +44,11 @@ rm -rf dir
>  # restore _all_ xattrs (not just the user.* domain)
>  tar --xattrs --xattrs-include='*' -xf archive.tar
>  
> -getcap dir/file
> +# Newer systems print = instead of + here
> +getcap dir/file | sed 's/+/=/'
>  ],
>  [0],
> -[dir/file = cap_chown+ei
> +[dir/file = cap_chown=ei
>  ])
>  
>  AT_CLEANUP
> 

>From 8b32042ed8e06ac13643945d0a6b5bacaa8cbc9f Mon Sep 17 00:00:00 2001
From: Pavel Raiskup 
Date: Tue, 19 Jan 2021 16:45:23 +0100
Subject: [PATCH] xattrs: fix capabilities root test

Related discussion in the Fedora pull-request:
https://src.fedoraproject.org/rpms/tar/pull-request/8

* tests/capabs_raw01.at: Newer systems (currently e.g. Fedora 34)
print getcap output in format CAP=VAL, not CAP+VAL.
---
 tests/capabs_raw01.at | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tests/capabs_raw01.at b/tests/capabs_raw01.at
index f9b97736..988251cf 100644
--- a/tests/capabs_raw01.at
+++ b/tests/capabs_raw01.at
@@ -44,10 +44,13 @@ rm -rf dir
 # restore _all_ xattrs (not just the user.* domain)
 tar --xattrs --xattrs-include='*' -xf archive.tar
 
-getcap dir/file
+# Newer systems switched to different format:
+# - dir/file = cap_chown+ei
+# + dir/file cap_chown=ei
+getcap dir/file | sed -e 's/+/=/' -e 's|dir/file = |dir/file |'
 ],
 [0],
-[dir/file = cap_chown+ei
+[dir/file cap_chown=ei
 ])
 
 AT_CLEANUP
-- 
2.29.2

[PATCH] xattrs: fix capabilities root test

[Pavel Raiskup]

Related discussion in the Fedora pull-request:
https://src.fedoraproject.org/rpms/tar/pull-request/8

* tests/capabs_raw01.at: Newer systems (currently e.g. Fedora 34)
print getcap output in format CAP=VAL, not CAP+VAL.
---
 tests/capabs_raw01.at | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tests/capabs_raw01.at b/tests/capabs_raw01.at
index f9b97736..a1d94116 100644
--- a/tests/capabs_raw01.at
+++ b/tests/capabs_raw01.at
@@ -44,10 +44,11 @@ rm -rf dir
 # restore _all_ xattrs (not just the user.* domain)
 tar --xattrs --xattrs-include='*' -xf archive.tar
 
-getcap dir/file
+# Newer systems print = instead of + here
+getcap dir/file | sed 's/+/=/'
 ],
 [0],
-[dir/file = cap_chown+ei
+[dir/file = cap_chown=ei
 ])
 
 AT_CLEANUP
-- 
2.29.2

[PATCH] Don't pre-create read-only files with --extract --xattrs

[Pavel Raiskup]

We used to respect the target file mode when pre-creating files in
set_xattr, so we also pre-created read-only files that we were not able
to open later for writing.  This is now fixed, and we always create the
file with S_IWUSR.

Fixes the original bug report https://bugzilla.redhat.com/1886540

* src/extract.c (set_xattr): Blindly add S_IWUSR flag to pre-created
files, to avoid openat failures later.
---
 src/extract.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/extract.c b/src/extract.c
index b73a591..2e650ad 100644
--- a/src/extract.c
+++ b/src/extract.c
@@ -865,7 +865,13 @@ set_xattr (char const *file_name, struct tar_stat_info 
const *st,
 
   for (;;)
 {
-  if (!mknodat (chdir_fd, file_name, mode ^ invert_permissions, 0))
+  /* We'll open the file with O_WRONLY later by open_output_file,
+ therefore we need to give us the S_IWUSR bit.  If the file was
+ meant to be user-read-only, the permissions will be corrected by
+ the set_stat call. */
+  mode_t initial_mode = mode ^ invert_permissions | S_IWUSR;
+
+  if (!mknodat (chdir_fd, file_name, initial_mode, 0))
 {
   /* Successfully created file */
   xattrs_xattrs_set (st, file_name, typeflag, 0);
-- 
2.28.0

Bug in --sparse --diff mode

[Pavel Raiskup]

Hi, I'm attaching patch for bug in sparse diff mode, randomly detected by
testsuite on i686 Fedora:
https://bugzilla.redhat.com/show_bug.cgi?id=1853469

Pavel
>From f7d4cda53aebcab50fb1aa91027f2da9da2251bc Mon Sep 17 00:00:00 2001
From: Pavel Raiskup 
Date: Thu, 9 Jul 2020 21:22:10 +0200
Subject: [PATCH] Bugfix --sparse --diff mode

Originally reported in:
https://bugzilla.redhat.com/show_bug.cgi?id=1853469

* src/sparse.c (check_data_region): Only compare the part of buffer
really fed by safe_read(), not whole rdsize.
---
 src/sparse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/sparse.c b/src/sparse.c
index e60b16a..cc3c515 100644
--- a/src/sparse.c
+++ b/src/sparse.c
@@ -684,7 +684,7 @@ check_data_region (struct tar_sparse_file *file, size_t i)
 	}
   size_left -= bytes_read;
   mv_size_left (file->stat_info->archive_file_size - file->dumped_size);
-  if (memcmp (blk->buffer, diff_buffer, rdsize))
+  if (memcmp (blk->buffer, diff_buffer, bytes_read))
 	{
 	  report_difference (file->stat_info, _("Contents differ"));
 	  return false;
-- 
2.26.2

Don't print `Ignoring unknown extended header` message for each file

[Pavel Raiskup]

Per report [1], do you think that we could hash the already reported
messages somewhere, and skip printing them repeatedly on stderr (for each
file), and _by default_?

  $ tar tf some.star.gz
  ...
  /usr/bin/tar: Ignoring unknown extended header keyword 'SCHILY.dev'
  /usr/bin/tar: Ignoring unknown extended header keyword 'SCHILY.ino'
  ...
  /usr/bin/tar: Ignoring unknown extended header keyword 'SCHILY.dev'
  /usr/bin/tar: Ignoring unknown extended header keyword 'SCHILY.ino'
  ...

Perhaps we could print
  ...
  /usr/bin/tar: Ignoring unknown extended header keyword 'SCHILY.dev'
  ...
  /usr/bin/tar: The --no-repeated-errors filter: Ignoring unknown extended 
header keyword 'SCHILY.dev'.
  ...

Of course we could add support for those headers...  but that is different
thing now.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1823866

Pavel

Re: [Bug-tar] Suprising behavior of extract with --xattrs

[Pavel Raiskup]

On Monday, June 3, 2019 10:27:12 PM CEST Brian Murray wrote:
> When enabling extended attribute support in a tar file (--xattrs) all
> extended attributes are stored in the archive, however when the same archive
> is extracted only the user.* extended attributes are extracted. To have all
> the extended attributes read and applied on extraction one must also use the
> '--xattrs-include=*' option.

This decision was done because only user.* attributes are 100% safe to
extract.  The security.* (especially capabilities) can have some binary
format specific to the box creating the archive but incompatible with host
extracting the archive.

For security.selinux there is special option --selinux (so xattr
security.selinux can be ignored), and for security.capability there should
be implemented some new option (probably using libcap.so?).

> I find this behavior surprising and especially so given that the documentation
> indicates that "By default, when `--xattr' is used, all names are stored
> in the archive (or extracted, if using `--extract')"[1].

Probably the docs need an update then.

Pavel

Re: [Bug-tar] gtar's ACL support is still unusable

[Pavel Raiskup]

Thanks for the report! +cc acl-devel

On Thursday, March 14, 2019 2:51:10 PM CET Joerg Schilling wrote:
> Trying to unpack the reference archives for the POSIX ACL proposal from
> 1993 that was withdrawn in 1997 results in something like:
> 
> /tmp/tar-1.31/src/tar --acls -xpf acl-test3.tar.gz 
> /tmp/tar-1.31/src/tar: default/dir2: Warnung: Funktion acl_from_text 
> fehlgeschlagen
> /tmp/tar-1.31/src/tar: default/dir3: Warnung: Funktion acl_from_text 
> fehlgeschlagen
> /tmp/tar-1.31/src/tar: default: Warnung: Funktion acl_from_text fehlgeschlagen
> /tmp/tar-1.31/src/tar: default: Warnung: Funktion acl_from_text fehlgeschlagen

This is because we use acl_from_text() without pre-filtering, which
doesn't accept the fourth UID/GID number value in e.g.
ACL record 'user:joe:rwx:503' (stored in the archive):

   $ tar -t -vv --acls -f acl-test5.tar
   ...
   drwxrwxr-x+ gruenbacher/assis 0 2001-11-04 04:43 default/dir2/
 a: user::rwx,user:joe:rwx:503,group::r-x,mask::rwx,other::r-x
   ...
   $ tar -xf --acls -f acl-test5.tar
   ...
   tar: default/dir2: Warning: Cannot acl_from_text: Invalid argument
   ...

I did not notice this so far, since we don't add the fourth numeric
argument to the SCHILY.acl.access header;  neither star does that (on
Linux at least, despite the claim in manual page).

I'm curious whether we should think about fixing this in acl_from_text()
function directly, or whether this should be handled solely in archivers.

Pavel

Re: [Bug-tar] Please fix the build with gcc 8.1

[Pavel Raiskup]

On Friday, November 16, 2018 4:09:12 PM CET Pavel Raiskup wrote:
> On Friday, August 3, 2018 12:48:07 PM CET Paul Eggert wrote:
> > Sergey Poznyakoff wrote:
> > > The ioctl code needs a good rewrite indeed. However, I'm more concerned
> > > about that:
> > > 
> > >> /usr/include/bits/socket.h: In function ‘__cmsg_nxthdr’:
> > >> /usr/include/bits/socket.h:315:12: error: cast increases required 
> > >> alignment of target type [-Werror=cast-align]
> > >> __cmsg = (struct cmsghdr *) ((unsigned char *) __cmsg
> > > The minimal reproducible file is a one-liner:
> > > 
> > >#include 
> > > 
> > > Running
> > >   
> > >gcc-8 -Wsystem-headers -Wcast-align=strict -Werror -O2 -c 1.c
> > > 
> > > produces the error above.
> > 
> > Although that's annoying, it's a false alarm, since the code properly 
> > aligns the 
> > pointer before using it by using CMSG_ALIGN. The warning in rtapelib.c, 
> > though, 
> > is not so obviously a false alarm.
> 
> Ping on this.

Could we please fix the build?  Or perhaps disable the -Werror by default, at
least.  I guess this is pretty deterrent failure after ./bootstrap &&
./configure && make failure.

Pavel

[Bug-tar] src/misc.c ignores getcwd() errors

[Pavel Raiskup]

I've got a report [1] that we should handle NULL return value from
xgetcwd() call.  Let me know if you prefer me to write the patch in this
case.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1680469

Pavel

Re: [Bug-tar] [PATCH] tests: fix race in dirrem01 and dirrem02

[Pavel Raiskup]

On Sunday, March 18, 2018 7:51:36 AM CET Sergey Poznyakoff wrote:
> Thanks for the patch and apologies for the delay. I pushed it.

The problem regressed back, per Fedora builds [1, 2] (see build.log links)
where ppc64le/s390x/aarch64 fail randomly again.  It seems like a reversed
race problem which is described in 64b43fdf70d82c39eb2ca900cd4f8e49 (it
might happen that genfile is not fast enough to unlink in time).

[1] https://koji.fedoraproject.org/koji/taskinfo?taskID=31934510
[2] https://koji.fedoraproject.org/koji/taskinfo?taskID=31934005

Pavel

Re: [Bug-tar] Please fix the build with gcc 8.1

[Pavel Raiskup]

On Friday, August 3, 2018 12:48:07 PM CET Paul Eggert wrote:
> Sergey Poznyakoff wrote:
> > The ioctl code needs a good rewrite indeed. However, I'm more concerned
> > about that:
> > 
> >> /usr/include/bits/socket.h: In function ‘__cmsg_nxthdr’:
> >> /usr/include/bits/socket.h:315:12: error: cast increases required 
> >> alignment of target type [-Werror=cast-align]
> >> __cmsg = (struct cmsghdr *) ((unsigned char *) __cmsg
> > The minimal reproducible file is a one-liner:
> > 
> >#include 
> > 
> > Running
> >   
> >gcc-8 -Wsystem-headers -Wcast-align=strict -Werror -O2 -c 1.c
> > 
> > produces the error above.
> 
> Although that's annoying, it's a false alarm, since the code properly aligns 
> the 
> pointer before using it by using CMSG_ALIGN. The warning in rtapelib.c, 
> though, 
> is not so obviously a false alarm.

Ping on this.

Pavel

Re: [Bug-tar] Encrypting tar contents

[Pavel Raiskup]

Hi Kirk,

On Monday, October 1, 2018 5:39:10 PM CEST Kirk Liberty wrote:
> Has there been any consideration/investigation/work towards adding native
> encryption to tar?

It's possible through --use-compress-program/-I, see the manual:
http://www.gnu.org/software/tar/manual/html_node/gzip.html , cit.:

|The '--use-compress-program' option, in particular, lets you
| implement your own filters, not necessarily dealing with
| compression/decompression.  For example, suppose you wish to implement
| PGP encryption on top of compression, using 'gpg' (*note gpg:
| (gpg)Top.).  The following script does that:

Pavel

Re: [Bug-tar] error "Not found in archive"

[Pavel Raiskup]

On Tuesday, September 11, 2018 10:30:28 PM CEST Sergey Poznyakoff wrote:
> Since the second copy failed to extract (being absent from the archive),
> tar reported the fact with the 'Not found in archive' diagnostic
> message.

Is there a way to make the diagnostic message more obvious?  Something like
'another copy of  is not found in the archive'?

Pavel

Re: [Bug-tar] [PATCH] wordsplit: avoid leak if WRDSF_NOVAR is not enabled

[Pavel Raiskup]

On Tuesday, July 31, 2018 11:00:52 AM CEST Sergey Poznyakoff wrote:
> I have installed the most recent version. Thank you.

Thanks, since -Werror is the default in git, here are some new warnings:

wordsplit.c: In function ‘wordsplit_dump_nodes’:
wordsplit.c:560:38: error: format ‘%x’ expects argument of type ‘unsigned 
int’, but argument 5 has type ‘int’ [-Werror=format=]
  wsp->ws_debug ("(%02d) %4d: %p: %#04x (%s):%s;",
  ^
  %#04x
wordsplit.c:562:16:
  n, p, p->flags, wsnode_flagstr (p->flags), p->v.word);
   
wordsplit.c:564:38: error: format ‘%x’ expects argument of type ‘unsigned 
int’, but argument 5 has type ‘int’ [-Werror=format=]
  wsp->ws_debug ("(%02d) %4d: %p: %#04x (%s):%.*s;",
  ^
  %#04x
wordsplit.c:566:16:
  n, p, p->flags, wsnode_flagstr (p->flags),
   
wordsplit.c: In function ‘expvar’:
wordsplit.c:1340:23: error: this statement may fall through 
[-Werror=implicit-fallthrough=]
   wsp->ws_usererr = value;
   ^~~
wordsplit.c:1342:5: note: here
 default:
 ^~~

Pavel

Re: [Bug-tar] [PATCH] avoid some resource leaks

[Pavel Raiskup]

On Tuesday, July 31, 2018 11:09:13 AM CEST Sergey Poznyakoff wrote:
> Applied. Thank you.

Argh, sorry.  There's now double free if iconv() fails, because assign_string
blindly frees already freed pointer;  I haven't noticed before. Patch attached.

Pavel
>From 1900925af783ad413cb072cf7b8f46c2779571b8 Mon Sep 17 00:00:00 2001
From: Pavel Raiskup 
Date: Tue, 31 Jul 2018 12:57:59 +0200
Subject: [PATCH] Fix decode_string() double-free

Bug added by commit 577dc345653947a31.

* src/utf8.c (utf8_convert): Don't touch '*output' string pointer
if we return 'false', to avoid unexpected side-effects.
---
 src/utf8.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/utf8.c b/src/utf8.c
index 168d636..abf26bc 100644
--- a/src/utf8.c
+++ b/src/utf8.c
@@ -65,7 +65,7 @@ bool
 utf8_convert (bool to_utf, char const *input, char **output)
 {
   char ICONV_CONST *ib;
-  char *ob;
+  char *ob, *ret;
   size_t inlen;
   size_t outlen;
   iconv_t cd = utf8_init (to_utf);
@@ -80,14 +80,15 @@ utf8_convert (bool to_utf, char const *input, char **output)
 
   inlen = strlen (input) + 1;
   outlen = inlen * MB_LEN_MAX + 1;
-  ob = *output = xmalloc (outlen);
+  ob = ret = xmalloc (outlen);
   ib = (char ICONV_CONST *) input;
   if (iconv (cd, , , , ) == -1)
 {
-  free (*output);
+  free (ret);
   return false;
 }
   *ob = 0;
+  *output = ret;
   return true;
 }
 
-- 
2.17.1

Re: [Bug-tar] --exclude-vcs-ignores does not read .gitignore files from parent directories

[Pavel Raiskup]

On Tuesday, July 31, 2018 11:23:27 AM CEST Sergey Poznyakoff wrote:
> Hi Pavel,
> 
> > I don't like that idea; IMO GNU tar should rely on /bin/git to interpret
> > gitignore ('git check-ignore').
> 
> Allow me to disagree. Calling external program when building a file list
> does not seem to be the best idea. First, it would considerably slow
> down the process. Secondly, it would entail additional memory overhead
> (for constructing the command line etc.)

We'd have to use optimization (e.g. check-ignore accepts --stdin, and then it
seem to accept line-by-line input).  And yes, ideally there would be a library
wrapping this all around...

I agree that it would cost something, but it should be optimal (it would be hard
to believe that we could implement .gitignore equivalently && more effective).

Pavel

[Bug-tar] [PATCH] Report race on systems without O_DIRECTORY

[Pavel Raiskup]

* src/names.c (collect_and_sort_names): Report ENOTDIR after
successful fstat() but !S_ISDIR.
---
 src/names.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/names.c b/src/names.c
index f6ad9fe..f4dc978 100644
--- a/src/names.c
+++ b/src/names.c
@@ -1767,6 +1767,11 @@ collect_and_sort_names (void)
  name->found_count++;
  add_hierarchy_to_namelist (, name);
}
+ else
+   {
+ errno = ENOTDIR;
+ open_diag (name->name);
+   }
}
}
 
-- 
2.17.1

Re: [Bug-tar] --exclude-vcs-ignores does not read .gitignore files from parent directories

[Pavel Raiskup]

On Monday, July 30, 2018 3:04:06 PM CEST Michal Novotny wrote:
> Can I at least make tar (by passing some additional arguments) take the
> parent .gitignores into consideration?

I don't like that idea; IMO GNU tar should rely on /bin/git to interpret
gitignore ('git check-ignore').

> Is there a summary, how tar's support for .gitignore files differs from
> git's native implementation?

I don't think there's such documentation, see also:
https://lists.gnu.org/archive/html/bug-tar/2016-12/msg1.html

Pavel

Re: [Bug-tar] [PATCH] tests: fix race in dirrem01 and dirrem02

[Pavel Raiskup]

On Tuesday, March 13, 2018 9:42:41 PM CEST Pavel Raiskup wrote:
> ping, still valid issue

ping

> On Thursday, January 4, 2018 7:17:31 PM CET Pavel Raiskup wrote:
> > Previously the '--checkpoint-action=echo' was triggered after
> > '--checkpoint-action=sleep=1' - so the order of events *usually*
> > was (for --format='gnu'):
> > 
> >   ...
> >   1. checkpoint handler before write of 'dir/sub' member
> >   2. one-second delay
> >   3. stderr write: 'tar: Write checkpoint 3'
> >   4. write the member 'dir/sub' into the archive
> >   5. check that the member's ctime has not been changed
> >   6. genfile's detecting 'Write checkpoint', doing unlink
> >   ...
> > 
> > But sometimes, the genfile was fast enough to win the race and
> > unlinked the directory before the member was written into the
> > archive (IOW, the order was 1-2-3-6-4-5).  This led to the
> > occasional warning 'tar: dir/sub: file changed as we read it'.
> > 
> > Swap the order of 'sleep=1' and 'echo' actions so the genfile
> > utility has (hopefully) enough time to do the unlink before
> > writing the file into the archive (enforce 1-2-3-6-4-5 order).
> > 
> > * tests/dirrem01.at: Swap 'sleep=1' and 'echo' actions.
> > * tests/dirrem02.at: Likewise.
> > ---
> >  tests/dirrem01.at | 5 +++--
> >  tests/dirrem02.at | 7 ---
> >  2 files changed, 7 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tests/dirrem01.at b/tests/dirrem01.at
> > index 40344dc..dabc206 100644
> > --- a/tests/dirrem01.at
> > +++ b/tests/dirrem01.at
> > @@ -47,14 +47,15 @@ gnu)   CPT=3;;
> >  esac
> >  
> >  genfile --run --checkpoint=$CPT --unlink dir/sub/file2 --unlink dir/sub -- 
> > \
> > -   tar --blocking-factor=1 --checkpoint=1 
> > --checkpoint-action='sleep=1' \
> > -   --checkpoint-action='echo' -c -f archive.tar \
> > +   tar --blocking-factor=1 --checkpoint=1 --checkpoint-action='echo' \
> > +   --checkpoint-action='sleep=1' -c -f archive.tar \
> > --listed-incremental db -v dir >/dev/null
> >  ],
> >  [1],
> >  [ignore],
> >  [tar: dir: Directory is new
> >  tar: dir/sub: Directory is new
> > +tar: dir/sub: file changed as we read it
> >  tar: dir/sub: File removed before we read it
> >  ],[],[],[gnu,posix])
> >  
> > diff --git a/tests/dirrem02.at b/tests/dirrem02.at
> > index e1cf9ef..924454f 100644
> > --- a/tests/dirrem02.at
> > +++ b/tests/dirrem02.at
> > @@ -20,7 +20,7 @@
> >  
> >  # Description:
> >  #
> > -# When an explicitley named directory disappears during creation
> > +# When an explicitly named directory disappears during creation
> >  # of incremental dump, tar should still exit with TAREXIT_FAILURE (2).
> >  #
> >  # For further details see dirrem01.at
> > @@ -44,14 +44,15 @@ gnu)   CPT=3;;
> >  esac
> >  
> >  genfile --run --checkpoint=$CPT --unlink dir/sub/file2 --unlink dir/sub -- 
> > \
> > -   tar --blocking-factor=1 --checkpoint=1 
> > --checkpoint-action='sleep=1' \
> > -   --checkpoint-action='echo' -c -f archive.tar \
> > +   tar --blocking-factor=1 --checkpoint=1 --checkpoint-action='echo' \
> > +   --checkpoint-action='sleep=1' -c -f archive.tar \
> > --listed-incremental db -v dir dir/sub >/dev/null
> >  ],
> >  [2],
> >  [ignore],
> >  [tar: dir: Directory is new
> >  tar: dir/sub: Directory is new
> > +tar: dir/sub: file changed as we read it
> >  tar: dir/sub: Cannot open: No such file or directory
> >  tar: Exiting with failure status due to previous errors
> >  ],[],[],[gnu,posix])
> > 
> 
> 
> 
> 
> 
> 

[Bug-tar] Please fix the build with gcc 8.1

[Pavel Raiskup]

With current gnulib submodule hash - build system throws:

  CC   stdopen.o
cc1: error: -Wabi won't warn about anything [-Werror=abi]
cc1: error: -Wabi won't warn about anything [-Werror=abi]
cc1: note: -Wabi warns about differences from the most up-to-date ABI, 
which is also used by default
cc1: note: -Wabi warns about differences from the most up-to-date ABI, 
which is also used by default
cc1: note: use e.g. -Wabi=11 to warn about changes from GCC 7
cc1: note: use e.g. -Wabi=11 to warn about changes from GCC 7
cc1: error: -Wabi won't warn about anything [-Werror=abi]
cc1: note: -Wabi warns about differences from the most up-to-date ABI, 
which is also used by default
cc1: note: use e.g. -Wabi=11 to warn about changes from GCC 7
cc1: error: -Wabi won't warn about anything [-Werror=abi]
cc1: note: -Wabi warns about differences from the most up-to-date ABI, 
which is also used by default
cc1: note: use e.g. -Wabi=11 to warn about changes from GCC 7
cc1: error: -Wabi won't warn about anything [-Werror=abi]
cc1: note: -Wabi warns about differences from the most up-to-date ABI, 
which is also used by default
cc1: note: use e.g. -Wabi=11 to warn about changes from GCC 7
cc1: all warnings being treated as errors

With updated gnulib to the latest git version - the build system shows:

  CC   rtapelib.o rtapelib.c: In function ‘rmt_ioctl__’:
rtapelib.c:675:18: error: cast increases required alignment of target 
type [-Werror=cast-align]
  uintmax_t u = (((struct mtop *) argument)->mt_count < 0
  ^
rtapelib.c:676:27: error: cast increases required alignment of target 
type [-Werror=cast-align]
  ? - (uintmax_t) ((struct mtop *) argument)->mt_count
   ^
rtapelib.c:677:27: error: cast increases required alignment of target 
type [-Werror=cast-align]
  :   (uintmax_t) ((struct mtop *) argument)->mt_count);
   ^
rtapelib.c:684:7: error: cast increases required alignment of target 
type [-Werror=cast-align]
  if (((struct mtop *) argument)->mt_count < 0)
   ^
rtapelib.c:690:5: error: cast increases required alignment of target 
type [-Werror=cast-align]
((struct mtop *) argument)->mt_op, p);
 ^
rtapelib.c:735:7: error: cast increases required alignment of target 
type [-Werror=cast-align]
  if (((struct mtget *) argument)->MTIO_CHECK_FIELD < 256)

Pavel

[Bug-tar] [PATCH] avoid some resource leaks

[Pavel Raiskup]

* src/incremen.c (store_rename): Free temp_name, leaked before for
each renamed directory with --listed-incremental.
* src/transform.c (add_literal_segment): Tighten arguments by
const.
(parse_transform_expr): Free 'str', leaked storage for each
--transform option before.
* src/utf8.c (utf8_convert): Deallocate buffer for failed iconv()
call so callers don't have to.
---
 src/incremen.c  | 1 +
 src/transform.c | 3 ++-
 src/utf8.c  | 9 ++---
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/incremen.c b/src/incremen.c
index ca611f6..7c26eb8 100644
--- a/src/incremen.c
+++ b/src/incremen.c
@@ -915,6 +915,7 @@ store_rename (struct directory *dir, struct obstack *stk)
obstack_code_rename (stk, p->orig->name, p->name);
 
  obstack_code_rename (stk, "", prev->name);
+ free (temp_name);
}
 }
 }
diff --git a/src/transform.c b/src/transform.c
index 3fae3c0..6ef0da6 100644
--- a/src/transform.c
+++ b/src/transform.c
@@ -101,7 +101,7 @@ add_segment (struct transform *tf)
 }
 
 static void
-add_literal_segment (struct transform *tf, char *str, char *end)
+add_literal_segment (struct transform *tf, const char *str, const char *end)
 {
   size_t len = end - str;
   if (len)
@@ -403,6 +403,7 @@ parse_transform_expr (const char *expr)
cur++;
 }
   add_literal_segment (tf, beg, cur);
+  free(str);
 
   return p;
 }
diff --git a/src/utf8.c b/src/utf8.c
index a018ce0..6dfda87 100644
--- a/src/utf8.c
+++ b/src/utf8.c
@@ -68,7 +68,6 @@ utf8_convert (bool to_utf, char const *input, char **output)
   char *ob;
   size_t inlen;
   size_t outlen;
-  size_t rc;
   iconv_t cd = utf8_init (to_utf);
 
   if (cd == 0)
@@ -83,9 +82,13 @@ utf8_convert (bool to_utf, char const *input, char **output)
   outlen = inlen * MB_LEN_MAX + 1;
   ob = *output = xmalloc (outlen);
   ib = (char ICONV_CONST *) input;
-  rc = iconv (cd, , , , );
+  if (-1 == iconv (cd, , , , ))
+{
+  free (*output);
+  return false;
+}
   *ob = 0;
-  return rc != -1;
+  return true;
 }
 
 
-- 
2.17.1

[Bug-tar] paxutils: rmt.c missing va_end()

[Pavel Raiskup]

Hi,

please consider fixing rmt_write function:

  rmt_write (const char *fmt, ...)
  {
va_list ap;
va_start (ap, fmt);
vfprintf (stdout, fmt, ap);
// missing va_end
fflush (stdout);
VDEBUG (10, "S: ", fmt, ap);
// potential re-use of 'ap' (requires new va_start)
  }

the first issue is not a problem in GNU/Linux GCC/Glibc ecosystem, but
still it seems to be good candidate for fix for compatibility.

Pavel

[Bug-tar] [PATCH] wordsplit: avoid leak if WRDSF_NOVAR is not enabled

[Pavel Raiskup]

Sorry, I meant WRDSF_NOVAR.  Updated patch attached.  It's not very important
issue for GNU tar, but wordsplit.c is meant to be library code - so it's worth
fixing IMO.

Pavel

On Friday, July 27, 2018 5:24:46 PM CEST Pavel Raiskup wrote:
> * lib/wordsplit.c (expvar): Don't copy string returned from
> wordsplit_find_env by strdup(), it's not needed and the value is
> never freed.
> ---
>  lib/wordsplit.c | 10 ++
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/wordsplit.c b/lib/wordsplit.c
> index f2ecada..a186f2e 100644
> --- a/lib/wordsplit.c
> +++ b/lib/wordsplit.c
> @@ -707,7 +707,6 @@ expvar (struct wordsplit *wsp, const char *str, size_t 
> len,
>size_t i = 0;
>const char *defstr = NULL;
>const char *value;
> -  const char *vptr;
>struct wordsplit_node *newnode;
>const char *start = str - 1;
>  
> @@ -770,13 +769,8 @@ expvar (struct wordsplit *wsp, const char *str, size_t 
> len,
>   i   - its length
>   defstr - default replacement str */
>  
> -  vptr = wordsplit_find_env (wsp, str, i);
> -  if (vptr)
> -{
> -  value = strdup (vptr);
> -  if (!value)
> - return _wsplt_nomem (wsp);
> -}
> +  if ((value = wordsplit_find_env (wsp, str, i)))
> +; /* returns pointer into wsp->ws_env */
>else if (wsp->ws_flags & WRDSF_GETVAR)
>  value = wsp->ws_getvar (str, i, wsp->ws_closure);
>    else if (wsp->ws_flags & WRDSF_UNDEF)
> 

>From 364aeb8cdff0f966b8eab7b8abb7036155f4cdb1 Mon Sep 17 00:00:00 2001
From: Pavel Raiskup 
Date: Fri, 27 Jul 2018 17:21:41 +0200
Subject: [PATCH] wordsplit: avoid leak if WRDSF_NOVAR is not enabled

* lib/wordsplit.c (expvar): Don't copy string returned from
wordsplit_find_env by strdup (), it's not needed and the value is
never freed.  The only affected wordsplit () call in tar is in
src/system.c.
---
 lib/wordsplit.c | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/lib/wordsplit.c b/lib/wordsplit.c
index f2ecada..a186f2e 100644
--- a/lib/wordsplit.c
+++ b/lib/wordsplit.c
@@ -707,7 +707,6 @@ expvar (struct wordsplit *wsp, const char *str, size_t len,
   size_t i = 0;
   const char *defstr = NULL;
   const char *value;
-  const char *vptr;
   struct wordsplit_node *newnode;
   const char *start = str - 1;
 
@@ -770,13 +769,8 @@ expvar (struct wordsplit *wsp, const char *str, size_t len,
  i   - its length
  defstr - default replacement str */
 
-  vptr = wordsplit_find_env (wsp, str, i);
-  if (vptr)
-{
-  value = strdup (vptr);
-  if (!value)
-	return _wsplt_nomem (wsp);
-}
+  if ((value = wordsplit_find_env (wsp, str, i)))
+; /* returns pointer into wsp->ws_env */
   else if (wsp->ws_flags & WRDSF_GETVAR)
 value = wsp->ws_getvar (str, i, wsp->ws_closure);
   else if (wsp->ws_flags & WRDSF_UNDEF)
-- 
2.17.1

[Bug-tar] [PATCH] wordsplit: avoid leak if WRDSF_ENV

[Pavel Raiskup]

* lib/wordsplit.c (expvar): Don't copy string returned from
wordsplit_find_env by strdup(), it's not needed and the value is
never freed.
---
 lib/wordsplit.c | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/lib/wordsplit.c b/lib/wordsplit.c
index f2ecada..a186f2e 100644
--- a/lib/wordsplit.c
+++ b/lib/wordsplit.c
@@ -707,7 +707,6 @@ expvar (struct wordsplit *wsp, const char *str, size_t len,
   size_t i = 0;
   const char *defstr = NULL;
   const char *value;
-  const char *vptr;
   struct wordsplit_node *newnode;
   const char *start = str - 1;
 
@@ -770,13 +769,8 @@ expvar (struct wordsplit *wsp, const char *str, size_t len,
  i   - its length
  defstr - default replacement str */
 
-  vptr = wordsplit_find_env (wsp, str, i);
-  if (vptr)
-{
-  value = strdup (vptr);
-  if (!value)
-   return _wsplt_nomem (wsp);
-}
+  if ((value = wordsplit_find_env (wsp, str, i)))
+; /* returns pointer into wsp->ws_env */
   else if (wsp->ws_flags & WRDSF_GETVAR)
 value = wsp->ws_getvar (str, i, wsp->ws_closure);
   else if (wsp->ws_flags & WRDSF_UNDEF)
-- 
2.17.1

Re: [Bug-tar] tar HEAD: difflink.at portability issue

[Pavel Raiskup]

On Thursday, April 26, 2018 7:01:05 PM CEST Christian Weisgerber wrote:
> In the tar 1.30 release there was a problem with the difflink.at
> test; this has been fixed in HEAD.  However, the test still fails
> on *BSD and there is an underlying portability issue.
> 
> The preparatory part of the test runs this:
> 
> mkdir a
> genfile -f a/x
> ln -s x a/y
> ln a/y a/z
> 
> What is the expected result?

It is regression test for 1bf590ab fix, which changed tar's reporting like:

  - a/z: Not linked to a/z
  + a/z: Not linked to a/y

So one could add ./configure check to detect that 'ln -P' works, and then
invent some test magic in difflink.at..  But IMO it would be fine enough
to just fix the test so it is working with regular-file hardlinks.

Pavel

Re: [Bug-tar] infinite loop in sparse_dump_region()

[Pavel Raiskup]

On Tuesday, December 19, 2017 9:30:49 AM CET Pavel Raiskup wrote:
> Hi, the proposed patch [1] can still be applied, and the reproducer below is
> still valid.  Please let me know I could fix something in the patch.
> 
> [1] https://www.mail-archive.com/bug-tar@gnu.org/msg04443.html

gently ping

Pavel

> On Sunday, March 16, 2014 10:55:38 PM CET Pavel Raiskup wrote:
> > Hello François,
> > 
> > On Thursday, March 13, 2014 14:43:55 François Ouellet wrote:
> > > When dumping a file which is being actively updated by an application
> > > (in our case it was an outlook pst file on our samba server), safe_read()
> > > can sometimes return 0.
> > > 
> > > When it happens, sparse_dump_region() goes into an infinite loop.
> > > 
> > > I don't know what the proper fix would be.  I just fixed it on our server
> > > with this:
> > 
> > thanks for clear report!  You can try the attached patch if you wanted (but 
> > it
> > has the same effects as yours one, just the message is little bit 
> > different).
> > Reproducer:
> > 
> >   $ truncate -s 10M file
> >   $ tar cSf archive file
> >   $ truncate -s 5M file
> >   $ tar df archive
> > 
> > Could we please apply at least something like the patch attached?
> > Looking at the code, I don't like duplications.  I was thinking
> > about safe_read/blocking_read wrapper with sth. like 'bool may_eof'
> > argument (and others) to make it possible to deal with errors at one
> > place (stopped once I realized that it will be somehow bigger code
> > change).  Would you be interested in such patch?
> > 
> > Pavel
> 
> 
> 

Re: [Bug-tar] [PATCH] tests: fix race in dirrem01 and dirrem02

[Pavel Raiskup]

ping, still valid issue

On Thursday, January 4, 2018 7:17:31 PM CET Pavel Raiskup wrote:
> Previously the '--checkpoint-action=echo' was triggered after
> '--checkpoint-action=sleep=1' - so the order of events *usually*
> was (for --format='gnu'):
> 
>   ...
>   1. checkpoint handler before write of 'dir/sub' member
>   2. one-second delay
>   3. stderr write: 'tar: Write checkpoint 3'
>   4. write the member 'dir/sub' into the archive
>   5. check that the member's ctime has not been changed
>   6. genfile's detecting 'Write checkpoint', doing unlink
>   ...
> 
> But sometimes, the genfile was fast enough to win the race and
> unlinked the directory before the member was written into the
> archive (IOW, the order was 1-2-3-6-4-5).  This led to the
> occasional warning 'tar: dir/sub: file changed as we read it'.
> 
> Swap the order of 'sleep=1' and 'echo' actions so the genfile
> utility has (hopefully) enough time to do the unlink before
> writing the file into the archive (enforce 1-2-3-6-4-5 order).
> 
> * tests/dirrem01.at: Swap 'sleep=1' and 'echo' actions.
> * tests/dirrem02.at: Likewise.
> ---
>  tests/dirrem01.at | 5 +++--
>  tests/dirrem02.at | 7 ---
>  2 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/dirrem01.at b/tests/dirrem01.at
> index 40344dc..dabc206 100644
> --- a/tests/dirrem01.at
> +++ b/tests/dirrem01.at
> @@ -47,14 +47,15 @@ gnu)   CPT=3;;
>  esac
>  
>  genfile --run --checkpoint=$CPT --unlink dir/sub/file2 --unlink dir/sub -- \
> -   tar --blocking-factor=1 --checkpoint=1 --checkpoint-action='sleep=1' \
> -   --checkpoint-action='echo' -c -f archive.tar \
> +   tar --blocking-factor=1 --checkpoint=1 --checkpoint-action='echo' \
> +   --checkpoint-action='sleep=1' -c -f archive.tar \
> --listed-incremental db -v dir >/dev/null
>  ],
>  [1],
>  [ignore],
>  [tar: dir: Directory is new
>  tar: dir/sub: Directory is new
> +tar: dir/sub: file changed as we read it
>  tar: dir/sub: File removed before we read it
>  ],[],[],[gnu,posix])
>  
> diff --git a/tests/dirrem02.at b/tests/dirrem02.at
> index e1cf9ef..924454f 100644
> --- a/tests/dirrem02.at
> +++ b/tests/dirrem02.at
> @@ -20,7 +20,7 @@
>  
>  # Description:
>  #
> -# When an explicitley named directory disappears during creation
> +# When an explicitly named directory disappears during creation
>  # of incremental dump, tar should still exit with TAREXIT_FAILURE (2).
>  #
>  # For further details see dirrem01.at
> @@ -44,14 +44,15 @@ gnu)   CPT=3;;
>  esac
>  
>  genfile --run --checkpoint=$CPT --unlink dir/sub/file2 --unlink dir/sub -- \
> -   tar --blocking-factor=1 --checkpoint=1 --checkpoint-action='sleep=1' \
> -   --checkpoint-action='echo' -c -f archive.tar \
> +   tar --blocking-factor=1 --checkpoint=1 --checkpoint-action='echo' \
> +   --checkpoint-action='sleep=1' -c -f archive.tar \
> --listed-incremental db -v dir dir/sub >/dev/null
>  ],
>  [2],
>  [ignore],
>  [tar: dir: Directory is new
>  tar: dir/sub: Directory is new
> +tar: dir/sub: file changed as we read it
>  tar: dir/sub: Cannot open: No such file or directory
>  tar: Exiting with failure status due to previous errors
>  ],[],[],[gnu,posix])
> 

Re: [Bug-tar] [PATCH] Re: Detection of sparse files is broken on btrfs

[Pavel Raiskup]

On Tuesday, January 9, 2018 11:15:56 AM CET Joerg Schilling wrote:
> Pavel Raiskup <prais...@redhat.com> wrote:
> 
> > On Tuesday, January 9, 2018 8:59:06 AM CET Paul Eggert wrote:
> > > Pavel Raiskup wrote:
> > > > So what about special casing that filesystem, where we can lseek() for
> > > > holes anyway?
> > > 
> > > If we can lseek for holes, then why not just do that?
> >
> > Checking whether lseek() actually works costs some additional syscalls _per
> > sparse_ file;  checking for ST_NBLOCKS() is without this penalty.
> 
> Well, star does this since a long time and the penalty is a few microseconds.

It would be interesting to see how network filesystems are affected.

Pavel

Re: [Bug-tar] [PATCH] Remove nonportable check for files containing only zeroes

[Pavel Raiskup]

On Wednesday, January 10, 2018 3:42:52 AM CET Mark H Weaver wrote:
> From da922703282b0d3b8837a99a9c7fdd32f1d20d49 Mon Sep 17 00:00:00 2001
> From: Mark H Weaver 
> Date: Tue, 9 Jan 2018 20:16:14 -0500
> Subject: [PATCH] Remove nonportable check for files containing only zeroes.
> 
> This check benefitted only one unlikely case (large files containing
> only zeroes, on systems that do not support SEEK_HOLE)

It drops the optimization even for situations when SEEK_HOLE is not
available, which is not 100% necessary.  I'm not proposing doing otherwise
(I actually proposed this in [1]), but I'm rather CCing Andreas once more,
as that's the original requester, the use-cases with lustre were
definitely not unlikely and the question is whether SEEK_HOLE covers them
nowadays.

[1] https://lists.gnu.org/archive/html/bug-tar/2016-07/msg00017.html

Pavel

> and was based on an assumption about file system behavior that is not
> mandated by POSIX and no longer holds in practice, namely that for
> sufficiently large files, (st_blocks == 0) implies that the file
> contains only zeroes.  Examples of file systems that violate this
> assumption include Linux's /proc file system and Btrfs.
>
> * src/sparse.c (sparse_scan_file_wholesparse): Remove this function.
> (sparse_scan_file_seek): Remove the initial check for files containing
> only zeroes.
> ---
>  src/sparse.c | 24 
>  1 file changed, 24 deletions(-)
> 
> diff --git a/src/sparse.c b/src/sparse.c
> index d41c0ea..3de6560 100644
> --- a/src/sparse.c
> +++ b/src/sparse.c
> @@ -261,26 +261,6 @@ sparse_scan_file_raw (struct tar_sparse_file *file)
>return tar_sparse_scan (file, scan_end, NULL);
>  }
>  
> -static bool
> -sparse_scan_file_wholesparse (struct tar_sparse_file *file)
> -{
> -  struct tar_stat_info *st = file->stat_info;
> -  struct sp_array sp = {0, 0};
> -
> -  /* Note that this function is called only for truly sparse files of size 
> >= 1
> - block size (checked via ST_IS_SPARSE before).  See the thread
> - http://www.mail-archive.com/bug-tar@gnu.org/msg04209.html for more info 
> */
> -  if (ST_NBLOCKS (st->stat) == 0)
> -{
> -  st->archive_file_size = 0;
> -  sp.offset = st->stat.st_size;
> -  sparse_add_map (st, );
> -  return true;
> -}
> -
> -  return false;
> -}
> -
>  #ifdef SEEK_HOLE
>  /* Try to engage SEEK_HOLE/SEEK_DATA feature. */
>  static bool
> @@ -343,10 +323,6 @@ sparse_scan_file_seek (struct tar_sparse_file *file)
>  static bool
>  sparse_scan_file (struct tar_sparse_file *file)
>  {
> -  /* always check for completely sparse files */
> -  if (sparse_scan_file_wholesparse (file))
> -return true;
> -
>switch (hole_detection)
>  {
>  case HOLE_DETECTION_DEFAULT:
> 

Re: [Bug-tar] [PATCH] Re: Detection of sparse files is broken on btrfs

[Pavel Raiskup]

On Tuesday, January 9, 2018 8:59:06 AM CET Paul Eggert wrote:
> Pavel Raiskup wrote:
> > So what about special casing that filesystem, where we can lseek() for
> > holes anyway?
> 
> If we can lseek for holes, then why not just do that?

Checking whether lseek() actually works costs some additional syscalls _per
sparse_ file;  checking for ST_NBLOCKS() is without this penalty.

> We shouldn't need special-case code for btrfs per se. Any filesystem
> where we can lseek for holes should take advantage of that optimization.

It is done so actually, the 'wholesparse' is another optimization on top
of that (but usable also in cases where SEEK_HOLE isn't defined at all).

Pavel

[Bug-tar] [PATCH] tests: fix race in dirrem01 and dirrem02

[Pavel Raiskup]

Previously the '--checkpoint-action=echo' was triggered after
'--checkpoint-action=sleep=1' - so the order of events *usually*
was (for --format='gnu'):

  ...
  1. checkpoint handler before write of 'dir/sub' member
  2. one-second delay
  3. stderr write: 'tar: Write checkpoint 3'
  4. write the member 'dir/sub' into the archive
  5. check that the member's ctime has not been changed
  6. genfile's detecting 'Write checkpoint', doing unlink
  ...

But sometimes, the genfile was fast enough to win the race and
unlinked the directory before the member was written into the
archive (IOW, the order was 1-2-3-6-4-5).  This led to the
occasional warning 'tar: dir/sub: file changed as we read it'.

Swap the order of 'sleep=1' and 'echo' actions so the genfile
utility has (hopefully) enough time to do the unlink before
writing the file into the archive (enforce 1-2-3-6-4-5 order).

* tests/dirrem01.at: Swap 'sleep=1' and 'echo' actions.
* tests/dirrem02.at: Likewise.
---
 tests/dirrem01.at | 5 +++--
 tests/dirrem02.at | 7 ---
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/tests/dirrem01.at b/tests/dirrem01.at
index 40344dc..dabc206 100644
--- a/tests/dirrem01.at
+++ b/tests/dirrem01.at
@@ -47,14 +47,15 @@ gnu)   CPT=3;;
 esac
 
 genfile --run --checkpoint=$CPT --unlink dir/sub/file2 --unlink dir/sub -- \
-   tar --blocking-factor=1 --checkpoint=1 --checkpoint-action='sleep=1' \
-   --checkpoint-action='echo' -c -f archive.tar \
+   tar --blocking-factor=1 --checkpoint=1 --checkpoint-action='echo' \
+   --checkpoint-action='sleep=1' -c -f archive.tar \
--listed-incremental db -v dir >/dev/null
 ],
 [1],
 [ignore],
 [tar: dir: Directory is new
 tar: dir/sub: Directory is new
+tar: dir/sub: file changed as we read it
 tar: dir/sub: File removed before we read it
 ],[],[],[gnu,posix])
 
diff --git a/tests/dirrem02.at b/tests/dirrem02.at
index e1cf9ef..924454f 100644
--- a/tests/dirrem02.at
+++ b/tests/dirrem02.at
@@ -20,7 +20,7 @@
 
 # Description:
 #
-# When an explicitley named directory disappears during creation
+# When an explicitly named directory disappears during creation
 # of incremental dump, tar should still exit with TAREXIT_FAILURE (2).
 #
 # For further details see dirrem01.at
@@ -44,14 +44,15 @@ gnu)   CPT=3;;
 esac
 
 genfile --run --checkpoint=$CPT --unlink dir/sub/file2 --unlink dir/sub -- \
-   tar --blocking-factor=1 --checkpoint=1 --checkpoint-action='sleep=1' \
-   --checkpoint-action='echo' -c -f archive.tar \
+   tar --blocking-factor=1 --checkpoint=1 --checkpoint-action='echo' \
+   --checkpoint-action='sleep=1' -c -f archive.tar \
--listed-incremental db -v dir dir/sub >/dev/null
 ],
 [2],
 [ignore],
 [tar: dir: Directory is new
 tar: dir/sub: Directory is new
+tar: dir/sub: file changed as we read it
 tar: dir/sub: Cannot open: No such file or directory
 tar: Exiting with failure status due to previous errors
 ],[],[],[gnu,posix])
-- 
2.14.3

Re: [Bug-tar] tar-1.30 released [stable]

[Pavel Raiskup]

On Wednesday, December 20, 2017 6:43:46 PM CET Joerg Schilling wrote:
> Pavel Raiskup <prais...@redhat.com> wrote:
> 
> > On Wednesday, December 20, 2017 1:41:38 PM CET Joerg Schilling wrote:
> > > There still is a --acl option but no support for ACLs.
> >
> > There's no support for NTFS/NFSv4 ACLs in GNU tar, to un-confuse the
> > statement.
> 
> But the withdrawn POSIX.4 ACL interface proposal does not seem to be 
> supported 
> either.
> 
> I am on a system that sports both - depending on the underlying filesystem.

Has ./configure detected the ACL support?

...
checking for library containing acl_get_file... -lacl
checking for library containing acl_get_fd... none required
checking for library containing acl_set_file... none required
checking for library containing acl_set_fd... none required
checking for library containing acl_to_text... none required
checking for library containing acl_from_text... none required
checking for library containing acl_delete_def_file... none required
checking for library containing acl_free... none required
...

Pavel

> BTW: With regard to the NTFS/NFSv4 ACLs, I recently enhanced star to use a 
> more 
> compact ACL format in this case.
> 
> Here is what the old definition was: 
> 
> 30 atime=1383676106.725425278
> 30 ctime=1383676333.651257344
> 30 mtime=1383676106.725425278
> 324 
> SCHILY.acl.ace=group:daemon:rwx---:---:allow:12,user:root:rwx---:---:allow:0,owner@:--x---:---:deny,owner@:rw-p---A-W-Co-:---:allow,group@:-wxp--:---:deny,group@:r-:---:allow,everyone@:-wxp---A-W-Co-:---:deny,everyone@:r-a-R-c--s:---:allow
> 23 SCHILY.dev=47775747
> 17 SCHILY.ino=11
> 18 SCHILY.nlink=1
> 27 SCHILY.filetype=regular
> 
> and here is the new compact format:
> 
> 30 atime=1383676106.725425278
> 30 ctime=1383676333.651257344
> 30 mtime=1383676106.725425278
> 186 
> SCHILY.acl.ace=group:daemon:rwx::allow:12,user:root:rwx::allow:0,owner@:x::deny,owner@:rwpAWCo::allow,group@:wxp::deny,group@:r::allow,everyone@:wxpAWCo::deny,everyone@:raRcs::allow
> 23 SCHILY.dev=47775747
> 17 SCHILY.ino=11
> 18 SCHILY.nlink=1
> 27 SCHILY.filetype=regular
> 
> that usually fits in a single 512 byte block.
> 
> Jörg
> 
> 

Re: [Bug-tar] tar-1.30 released [stable]

[Pavel Raiskup]

On Wednesday, December 20, 2017 1:41:38 PM CET Joerg Schilling wrote:
> There still is a --acl option but no support for ACLs.

There's no support for NTFS/NFSv4 ACLs in GNU tar, to un-confuse the
statement.

> Is this intented?

s/intended/not yet implemented/

Pavel

Re: [Bug-tar] infinite loop in sparse_dump_region()

[Pavel Raiskup]

Hi, the proposed patch [1] can still be applied, and the reproducer below is
still valid.  Please let me know I could fix something in the patch.

[1] https://www.mail-archive.com/bug-tar@gnu.org/msg04443.html

Pavel

On Sunday, March 16, 2014 10:55:38 PM CET Pavel Raiskup wrote:
> Hello François,
> 
> On Thursday, March 13, 2014 14:43:55 François Ouellet wrote:
> > When dumping a file which is being actively updated by an application
> > (in our case it was an outlook pst file on our samba server), safe_read()
> > can sometimes return 0.
> > 
> > When it happens, sparse_dump_region() goes into an infinite loop.
> > 
> > I don't know what the proper fix would be.  I just fixed it on our server
> > with this:
> 
> thanks for clear report!  You can try the attached patch if you wanted (but it
> has the same effects as yours one, just the message is little bit different).
> Reproducer:
> 
>   $ truncate -s 10M file
>   $ tar cSf archive file
>   $ truncate -s 5M file
>   $ tar df archive
> 
> Could we please apply at least something like the patch attached?
> Looking at the code, I don't like duplications.  I was thinking
> about safe_read/blocking_read wrapper with sth. like 'bool may_eof'
> argument (and others) to make it possible to deal with errors at one
> place (stopped once I realized that it will be somehow bigger code
> change).  Would you be interested in such patch?
> 
> Pavel

Re: [Bug-tar] [GNU tar 1.30] testsuite: 92 failed

[Pavel Raiskup]

On Monday, December 18, 2017 4:34:03 PM CET Sergey Poznyakoff wrote:
> David McInnis  ha escrit:
> 
> > I run Arch Linux , please let me know if you need more information.
> 
> Thanks, the information you supplied is quite enough. The following
> patch to the testsuite should fix it:

Partly this helps, please apply :-) thanks.  But there are other issues,
in Fedora build [1] in in new test-cases [2] (randomly happening on different
architectures [3):

117: directory removed before readingFAILED (dirrem01.at:37)
118: explicitly named directory removed before reading FAILED 
(dirrem02.at:34)
...
--- -   2017-12-18 12:13:22.658094565 +
+++ /builddir/build/BUILD/tar-1.30/tests/testsuite.dir/at-groups/117/stderr 
2017-12-18 12:13:22.646808763 +
@@ -1,4 +1,5 @@
 tar: dir: Directory is new
 tar: dir/sub: Directory is new
+tar: dir/sub: file changed as we read it
 tar: dir/sub: File removed before we read it

[1] https://koji.fedoraproject.org/koji/taskinfo?taskID=23777198
[2] https://kojipkgs.fedoraproject.org//work/tasks/7210/23777210/build.log
[3] https://koji.fedoraproject.org/koji/taskinfo?taskID=2378

Pavel

> diff --git a/tests/difflink.at b/tests/difflink.at
> index eadfb08..4e01176 100644
> --- a/tests/difflink.at
> +++ b/tests/difflink.at
> @@ -21,7 +21,7 @@ mkdir a
>  genfile -f a/x
>  ln -s x a/y
>  ln a/y a/z
> -tar cf a.tar a
> +tar cf a.tar a/x a/y a/z
>  rm a/z
>  ln -s x a/z
>  tar df a.tar
> 
> 
> Regards,
> Sergey
> 
> 

Re: [Bug-tar] Software Proposal

[Pavel Raiskup]

Hi Charlie,

On Monday, September 25, 2017 10:25:20 PM CEST Charlie Sale wrote:
> Does this mean that an API should not be developed for GNU tar, but
> instead for GNU paxutils?

I don't think that the library location matters too much.  I just think
that the library shouldn't be just "separate product", but also the
low-level cornerstone for /bin/tar.  And paxutils git more or less served
this purpose.

> If this is the case, should I be talking to paxutils instead?

That's the same upstream (Sergey and Paul here on the bug-tar list).

> Also, if I did go ahead and develop an API for GNU tar only, would I be
> able to just modify the existing modules, or do I have to reinvent the
> wheel and more or less rewrite the entire project? I would really just
> like to be able to add some data structures and remove global variables.

Dunno, that's implementation detail the patch author should look at;  as
the patch author is going to defend the patch.

Pavel

Re: [Bug-tar] Software Proposal

[Pavel Raiskup]

Hi Charlie,

On Monday, September 25, 2017 5:09:12 PM CEST Charlie Sale wrote:
> As for building the library, I would compile the tar source against a possible
> paxutils library and gnulib (if it has a shared library). I would also have to
> install some of the tar headers.
> 
> Please let me know what you think.

I don't claim it could not work as you draw it.  But the GNU tar binary IMO
should *use* the library which would be provided by GNU tar/GNU paxutils.

IOW, please build the binary against the new library (whereever it will live)
and not the other way around (library against tar's sources).

Pavel

Re: [Bug-tar] Software Proposal

[Pavel Raiskup]

On Saturday, September 23, 2017 4:14:54 PM CEST Charlie Sale wrote:
> My though was to develop a C API that would be part of the GNU tar package.

I believe something like this was planned to happen in paxutils [1], from
the README file of that project:

  | GNU paxutils aims to provide:
  |
  |   1. tar implementation, replacing current GNU tar,
  |   2. cpio implementation, replacing current GNU cpio,
  |   3. pax implementation, conforming to POSIX standard.
  |
  | All three implementations will be built around a common (presumably
  | shared) library.

So, I would suggest you to start there, and Sergey probably has some more
info.  Shared library would be invaluable, same as '/bin/pax'
implementation in GNU (yeah, cpio/tar are LEGACY utilities when you have a
look at POSIX specs..).

The problem is that most of the work is nowadays done in GNU tar only,
without paying too much attention to merge the stuff back to paxutils
project;  so I guess a lot of manpower is needed to get this work done.

> It would handle tasks that would be completed via the command line
> option.  My thinking is that it would convenient to have a built-in API
> instead of having to find outside dependencies.

This sounds like you plan to implement work-around to the fact that paxlib
has no shared lib;  so I would much rather see this implemented properly.
API built on top of execve('/bin/tar' ..) would be brittle, with a bit
unexpected output..

Pavel

Re: [Bug-tar] GCC 7 warnings

[Pavel Raiskup]

On Friday, September 8, 2017 12:06:22 PM CEST Sergey Poznyakoff wrote:
> Paul Eggert <egg...@cs.ucla.edu> ha escrit:
> 
> > I suggest ignoring the warning globally by pragma (i.e., in
> > configure.ac) since it's more trouble than it's worth for tar.
> 
> Agreed.

Please have a look at the attached patch then.

Pavel
>From 144ce3bc94bc38e9170bce7eb9cc870ed0638ac6 Mon Sep 17 00:00:00 2001
From: Pavel Raiskup <prais...@redhat.com>
Date: Fri, 8 Sep 2017 08:26:01 +0200
Subject: [PATCH] Fix/silence GCC 7 warnings

Per https://www.mail-archive.com/bug-tar@gnu.org/msg05347.html

* configure.ac: Lower GCC's -Wimplicit-fallthrough from 5 to 1 and
avoid using -Wformat-overflow.
* lib/wordsplit.c (wordsplit_perror): Add missing break.
* src/sparse.c (sparse_scan_file): Explicitly mention fallback.
* src/tar.c (parse_opt): Use abort to assure compiler that
argp_error doesn't return in this case.
* gnulib: Move to the latest version.
---
 configure.ac| 2 ++
 gnulib  | 2 +-
 lib/wordsplit.c | 1 +
 src/sparse.c| 1 +
 src/tar.c   | 1 +
 5 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index e89ed1d..be3809d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -151,6 +151,7 @@ if test "$gl_gcc_warnings" = yes; then
   nw="$nw -Wstrict-overflow"	# It's OK to optimize strictly.
   nw="$nw -Wsuggest-attribute=pure" # Too many warnings for now.
   nw="$nw -Wstack-protector"
+  nw="$nw -Wformat-overflow=2"
 
   gl_MANYWARN_ALL_GCC([ws])
   gl_MANYWARN_COMPLEMENT([ws], [$ws], [$nw])
@@ -164,6 +165,7 @@ if test "$gl_gcc_warnings" = yes; then
   
   gl_WARN_ADD([-fdiagnostics-show-option])
   gl_WARN_ADD([-funit-at-a-time])
+  gl_WARN_ADD([-Wimplicit-fallthrough=1]) # Be more tolerant to fall-through comments
   
 
   AC_SUBST([WARN_CFLAGS])
diff --git a/gnulib b/gnulib
index e210a3c..3ba4dba 16
--- a/gnulib
+++ b/gnulib
@@ -1 +1 @@
-Subproject commit e210a3cbaec0ee82a67ff8fc427e21bdd64dba1b
+Subproject commit 3ba4dbaefe671991083ff46a2714ff256adf75a1
diff --git a/lib/wordsplit.c b/lib/wordsplit.c
index 07d0f8a..f2ecada 100644
--- a/lib/wordsplit.c
+++ b/lib/wordsplit.c
@@ -1584,6 +1584,7 @@ wordsplit_perror (struct wordsplit *wsp)
 
 case WRDSE_NOSUPP:
   wsp->ws_error (_("command substitution is not yet supported"));
+  break;
 
 case WRDSE_USAGE:
   wsp->ws_error (_("invalid wordsplit usage"));
diff --git a/src/sparse.c b/src/sparse.c
index b3a3fd3..5bd2b01 100644
--- a/src/sparse.c
+++ b/src/sparse.c
@@ -361,6 +361,7 @@ sparse_scan_file (struct tar_sparse_file *file)
   /* fall back to "raw" for this and all other files */
   hole_detection = HOLE_DETECTION_RAW;
 #endif
+  /* fallthrough */
 case HOLE_DETECTION_RAW:
   if (sparse_scan_file_raw (file))
 	return true;
diff --git a/src/tar.c b/src/tar.c
index 07a6995..4006060 100644
--- a/src/tar.c
+++ b/src/tar.c
@@ -2068,6 +2068,7 @@ parse_opt (int key, char *arg, struct argp_state *state)
 		  _("Options '-[0-7][lmh]' not supported by *this* tar"));
 
 #endif /* not DEVICE_PREFIX */
+  abort ();
 
 case ARGP_KEY_ERROR:
   if (args->loc->source == OPTS_FILE)
-- 
2.13.5

[Bug-tar] GCC 7 warnings

[Pavel Raiskup]

Hello maintainers,

building from git with GCC 7 is a bit more complited again, as gcc 7 is more
-Werror sensitive.  I've attached a patch that looks like worth having
pushed to me.

There's one remaining issue:

list.c: In function ‘tartime’:
list.c:1053:33: error: ‘%02d’ directive writing between 2 and 11 bytes 
into a region of size between 4 and 29 [-Werror=format-overflow=]
sprintf (buffer, "%04ld-%02d-%02d %02d:%02d:%02d",
 ^~~~
list.c:1053:4: note: ‘sprintf’ output between 20 and 81 bytes into a 
destination of size 37
sprintf (buffer, "%04ld-%02d-%02d %02d:%02d:%02d",
^~
  tm->tm_year + 1900L, tm->tm_mon + 1, tm->tm_mday,
  ~
  tm->tm_hour, tm->tm_min, tm->tm_sec);
  
list.c:1059:31: error: ‘%02d’ directive writing between 2 and 11 bytes 
into a region of size between 4 and 29 [-Werror=format-overflow=]
  sprintf (buffer, "%04ld-%02d-%02d %02d:%02d",
   ^~~~
list.c:1059:2: note: ‘sprintf’ output between 17 and 69 bytes into a 
destination of size 37
  sprintf (buffer, "%04ld-%02d-%02d %02d:%02d",
  ^
tm->tm_year + 1900L, tm->tm_mon + 1, tm->tm_mday,
~
tm->tm_hour, tm->tm_min);


This is IMO false positive, as long as we can expect that if
"utc_option ? gmtime () : localtime ();" succeeds then it returns
valid 'struct tm'.

The warning can be silenced by e.g. min(0, max(, tm->tm_year)) hacks or by
ignoring the warning globally/locally by pragma.  Please silence this warning,
or suggest your preferred fix and I'll have a look at it.

Plus, there's gnulib issue [1].

[1] https://www.mail-archive.com/bug-gnulib@gnu.org/msg34265.html

Pavel
>From b0cda9f0926baf723858809dfbf7899fdd54b111 Mon Sep 17 00:00:00 2001
From: Pavel Raiskup <prais...@redhat.com>
Date: Fri, 8 Sep 2017 08:26:01 +0200
Subject: [PATCH] Fix some GCC warnings

* configure.ac: Use -Wimplicit-fallthrough=1.
* lib/wordsplit.c (wordsplit_perror): Fix missing break.
* src/sparse.c (sparse_scan_file): Explicitly mention fallback.
---
 configure.ac| 1 +
 lib/wordsplit.c | 1 +
 src/sparse.c| 1 +
 4 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index e89ed1d..fac44fa 100644
--- a/configure.ac
+++ b/configure.ac
@@ -164,6 +164,7 @@ if test "$gl_gcc_warnings" = yes; then
   
   gl_WARN_ADD([-fdiagnostics-show-option])
   gl_WARN_ADD([-funit-at-a-time])
+  gl_WARN_ADD([-Wimplicit-fallthrough=1]) # Be more tolerant to fall-through comments
   
 
   AC_SUBST([WARN_CFLAGS])
diff --git a/lib/wordsplit.c b/lib/wordsplit.c
index 07d0f8a..f2ecada 100644
--- a/lib/wordsplit.c
+++ b/lib/wordsplit.c
@@ -1584,6 +1584,7 @@ wordsplit_perror (struct wordsplit *wsp)
 
 case WRDSE_NOSUPP:
   wsp->ws_error (_("command substitution is not yet supported"));
+  break;
 
 case WRDSE_USAGE:
   wsp->ws_error (_("invalid wordsplit usage"));
diff --git a/src/sparse.c b/src/sparse.c
index b3a3fd3..5bd2b01 100644
--- a/src/sparse.c
+++ b/src/sparse.c
@@ -361,6 +361,7 @@ sparse_scan_file (struct tar_sparse_file *file)
   /* fall back to "raw" for this and all other files */
   hole_detection = HOLE_DETECTION_RAW;
 #endif
+  /* fallthrough */
 case HOLE_DETECTION_RAW:
   if (sparse_scan_file_raw (file))
 	return true;
-- 
2.13.5

Re: [Bug-tar] [PATCH] tests: more deterministic xattr07

[Pavel Raiskup]

Ping, I re-evaluated this issue again during work on back-patching.

Pavel

On Monday, November 28, 2016 9:01:03 AM CEST Pavel Raiskup wrote:
> * tests/xattr07.at: Define order of files within tested archive.
> ---
>  tests/xattr07.at | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/xattr07.at b/tests/xattr07.at
> index a834981..47d54b8 100644
> --- a/tests/xattr07.at
> +++ b/tests/xattr07.at
> @@ -36,7 +36,7 @@ setfattr -n user.test -v OurDirValue dir
>  setfattr -n user.test -v OurFileValue dir/file
>  setfattr -n user.test -v OurFileValue dir/file2
>  
> -tar --xattrs -cf archive.tar dir
> +tar --xattrs --no-recursion -cf archive.tar dir dir/file dir/file2
>  
>  setfattr -n user.test -v OurDirValue2 dir
>  setfattr -n user.test -v OurFileValue2 dir/file

Re: [Bug-tar] linux-backup-include-empty-swapfile (embedded)

[Pavel Raiskup]

On Tuesday, June 6, 2017 2:32:49 PM CEST Michael R. Lawrence CISSP wrote:
> hmm  may have to give that a go, as raspberry-pi3 isnt all that beefy ,
> 
>  /var/cache/swap/swap1 --exclude ... , and touch /tmp/fake-swap1 ,
> --transform to /var/cache/swap/swap1

Reading this ^^, and ..

> On Tue, Jun 6, 2017 at 5:07 AM, Pavel Raiskup <prais...@redhat.com> wrote:
> > That's definitely not an elegant way.  Also: I would be careful to use
> > --transform with --exclude, to my understanding --exclude should be applied
> > against _transformed_ name (but it doesn't happen ATM).

.. reading again my previous crabbed response, here is the fixed version:

Please don't rely on --transform and --exclude working together.  Currently
the --exclude excludes files based on names *before* transformation, but it
seems to be a bit unexpected behavior (IMO a bug).

The point is that probably nobody so far payed attention to "how --exclude &&
--transoform should work together", so it is undocumented (== undefined).

Pavel

Re: [Bug-tar] linux-backup-include-empty-swapfile (embedded)

[Pavel Raiskup]

On Saturday, June 3, 2017 5:40:31 AM CEST Michael R. Lawrence CISSP wrote:
> GNU tar can't be told to --empty-file* /var/cache/swap/swap1*

Ah, RFE for --empty-file.  That sounds like valid request to me.

> ADDING THE LIVE RUINING FILE WILL CRASH TAR/SYSTEM

Hm, tar shouldn't crash if some file is changed in parallel (that would be clear
bug and reproducer would be needed).  Documentation claims that you should avoid
such race conditions if you want to have _valid_ backup though.

> I'm just not sure how to get tar to add the 1 file as empty... or append
> archive as 1 empty file @ path...
> 
> *How to append the empty file in my backup script would be a a plus* , more
> elegant...

Have a look at '--transform' option:

$ tar --transform='s|/tmp/empty-file|/swp/file|' \
  /tmp/empty-file \
...

That's definitely not an elegant way.  Also: I would be careful to use
--transform with --exclude, to my understanding --exclude should be applied
against _transformed_ name (but it doesn't happen ATM).

Pavel

Re: [Bug-tar] [PATCH] Deterministic archive type detection

[Pavel Raiskup]

Hello, gently ping.  I rephrased the commit message (patch attached).

Pavel

On Thursday, March 30, 2017 2:05:07 PM CEST Pavel Raiskup wrote:
> Heuristic for guessing the archive/compression format works with
> archive magic and tar_checksum() which depends on the first block
> (BLOCKSIZE) of archive data in pre-fetched buffer.
> 
> Very small files though (compressed size is smaller than
> BLOCKSIZE) don't initialize a whole data block in buffer, so the
> heuristic calculated with uninitialized data previously.  In rare
> situations compressed archives were marked as non-compressed,
> which in turn caused extraction failure.
> 
> * src/buffer.c (check_compressed_archive): Don't assume archives
> smaller than BLOCKSIZE could be non-compressed, as tar header has
> always has at least one block.
> ---
>  src/buffer.c | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/src/buffer.c b/src/buffer.c
> index fb34834..f064d2a 100644
> --- a/src/buffer.c
> +++ b/src/buffer.c
> @@ -391,10 +391,12 @@ check_compressed_archive (bool *pshort)
>/* Restore global values */
>read_full_records = sfr;
>  
> -  if ((strcmp (record_start->header.magic, TMAGIC) == 0 ||
> -   strcmp (record_start->buffer + offsetof (struct posix_header, magic),
> -OLDGNU_MAGIC) == 0) &&
> -  tar_checksum (record_start, true) == HEADER_SUCCESS)
> +  if (record_start != record_end /* no files smaller than BLOCKSIZE */
> +  && (strcmp (record_start->header.magic, TMAGIC) == 0
> +  || strcmp (record_start->buffer + offsetof (struct posix_header,
> +  magic),
> + OLDGNU_MAGIC) == 0)
> +  && tar_checksum (record_start, true) == HEADER_SUCCESS)
>  /* Probably a valid header */
>  return ct_tar;
>  
> 

>From 3fe273c4ee7b0c29be6123cc5f46c02b3c6dd04a Mon Sep 17 00:00:00 2001
From: Pavel Raiskup <prais...@redhat.com>
Date: Thu, 30 Mar 2017 13:30:15 +0200
Subject: [PATCH] Fix non-deterministic archive type detection

Due to analysis of partly uninitialized read-ahead buffer
(short_read call), we sometimes mistakenly classified very small
compressed archives as non-compressed; which in turn caused
extraction failure.

* src/buffer.c (check_compressed_archive): Don't assume that
archives smaller than BLOCKSIZE could be non-compressed, as tar
header always has at least one block.
---
 src/buffer.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/buffer.c b/src/buffer.c
index fb34834..f064d2a 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -391,10 +391,12 @@ check_compressed_archive (bool *pshort)
   /* Restore global values */
   read_full_records = sfr;
 
-  if ((strcmp (record_start->header.magic, TMAGIC) == 0 ||
-   strcmp (record_start->buffer + offsetof (struct posix_header, magic),
-	   OLDGNU_MAGIC) == 0) &&
-  tar_checksum (record_start, true) == HEADER_SUCCESS)
+  if (record_start != record_end /* no files smaller than BLOCKSIZE */
+  && (strcmp (record_start->header.magic, TMAGIC) == 0
+  || strcmp (record_start->buffer + offsetof (struct posix_header,
+  magic),
+ OLDGNU_MAGIC) == 0)
+  && tar_checksum (record_start, true) == HEADER_SUCCESS)
 /* Probably a valid header */
 return ct_tar;
 
-- 
2.9.3

Re: [Bug-tar] tar (GNU tar) 1.29 bug in --no-recursion

[Pavel Raiskup]

Hi, thanks for the report.

On Friday, March 31, 2017 1:41:20 PM CEST N wrote:
> I'v recently updated my tar to 1.29.
> I use flexbackup and got a regression.
> The cause was ignoring --no-recursion parameter.
> So, I would like to report a bug.
> 
> I found, that tar behavior depends on parameter position.
> [...]
> It seems that bug is reproduced only if --no-recursion is applied after
> filenames.
> 
> Could you please confirm this bug?

That's expected behavior, the --(no-)recursion is possition sensitive option,
https://www.gnu.org/software/tar/manual/html_node/Position_002dSensitive-Options.html

Pavel

[Bug-tar] [PATCH] Deterministic archive type detection

[Pavel Raiskup]

Heuristic for guessing the archive/compression format works with
archive magic and tar_checksum() which depends on the first block
(BLOCKSIZE) of archive data in pre-fetched buffer.

Very small files though (compressed size is smaller than
BLOCKSIZE) don't initialize a whole data block in buffer, so the
heuristic calculated with uninitialized data previously.  In rare
situations compressed archives were marked as non-compressed,
which in turn caused extraction failure.

* src/buffer.c (check_compressed_archive): Don't assume archives
smaller than BLOCKSIZE could be non-compressed, as tar header has
always has at least one block.
---
 src/buffer.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/buffer.c b/src/buffer.c
index fb34834..f064d2a 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -391,10 +391,12 @@ check_compressed_archive (bool *pshort)
   /* Restore global values */
   read_full_records = sfr;
 
-  if ((strcmp (record_start->header.magic, TMAGIC) == 0 ||
-   strcmp (record_start->buffer + offsetof (struct posix_header, magic),
-  OLDGNU_MAGIC) == 0) &&
-  tar_checksum (record_start, true) == HEADER_SUCCESS)
+  if (record_start != record_end /* no files smaller than BLOCKSIZE */
+  && (strcmp (record_start->header.magic, TMAGIC) == 0
+  || strcmp (record_start->buffer + offsetof (struct posix_header,
+  magic),
+ OLDGNU_MAGIC) == 0)
+  && tar_checksum (record_start, true) == HEADER_SUCCESS)
 /* Probably a valid header */
 return ct_tar;
 
-- 
2.9.3

[Bug-tar] [PATCH] Test and document --keep-directory-symlink

[Pavel Raiskup]

* doc/tar.1: Document the option.
* tests/extrac20.at: New testcase.
* tests/Makefile.am: Mention extrac20.
* tests/testsuite.at: Likewise.
---
 doc/tar.1  |   3 ++
 tests/Makefile.am  |   1 +
 tests/extrac20.at  | 151 +
 tests/testsuite.at |   1 +
 4 files changed, 156 insertions(+)
 create mode 100644 tests/extrac20.at

diff --git a/doc/tar.1 b/doc/tar.1
index d66d3e6..f5c1fca 100644
--- a/doc/tar.1
+++ b/doc/tar.1
@@ -333,6 +333,9 @@ Don't replace existing files when extracting.
 \fB\-\-keep\-newer\-files\fR
 Don't replace existing files that are newer than their archive copies.
 .TP
+\fB\-\-keep\-directory\-symlink\fR
+Don't replace existing symlinks to directories when extracting.
+.TP
 \fB\-\-no\-overwrite\-dir\fR
 Preserve metadata of existing directories.
 .TP
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 502a37c..92516fb 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -107,6 +107,7 @@ TESTSUITE_AT = \
  extrac17.at\
  extrac18.at\
  extrac19.at\
+ extrac20.at\
  filerem01.at\
  filerem02.at\
  gzip.at\
diff --git a/tests/extrac20.at b/tests/extrac20.at
new file mode 100644
index 000..dd9b00d
--- /dev/null
+++ b/tests/extrac20.at
@@ -0,0 +1,151 @@
+# Process this file with autom4te to create testsuite. -*- Autotest -*-
+#
+# Test suite for GNU tar.
+# Copyright 2017 Free Software Foundation, Inc.
+
+# This file is part of GNU tar.
+
+# GNU tar is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+
+# GNU tar is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+
+AT_SETUP([keep-directory-symlink])
+AT_KEYWORDS([extrac20 extract old-files keep-old-files])
+
+AT_TAR_CHECK([
+AT_SORT_PREREQ
+
+for i in a b c
+do
+  dir=in$i
+  mkdir -p $dir/root/dir $dir/root/dirsymlink
+  touch $dir/root/dirsymlink/file$i
+  test $i != a && touch $dir/root/dirsymlink/file.conflict
+  tar cf archive$i.tar -C $dir root
+done
+
+prep()
+{
+  echo "== $1 =="
+  echo "== $1 ==" >&2
+  backup_dir=$1
+  dir=out
+  mkdir -p $dir/root/dir
+  ln -s dir $dir/root/dirsymlink
+  test $round = normal && cd $dir >/dev/null
+}
+
+clean()
+{
+  test $round = normal && cd .. >/dev/null
+  find $dir | sort
+  mv $dir $backup_dir
+}
+
+# Expand to '-f ../$1' or '-f $1 -C $dir' depending on $round variable
+file_spec()
+{
+  if test $round = normal
+  then
+echo "-f ../$1"
+  else
+echo "-f $1 -C $dir"
+  fi
+}
+
+for round in normal dir
+do
+  # Check that 'dirsymlink' replaces 'dir'
+  prep without_option_$round
+  tar -x `file_spec archivea.tar` || exit 1
+  tar -x `file_spec archiveb.tar` || exit 1
+  clean
+
+  # Keep directory symlink must keep root/dirsymlink
+  prep with_option_$round
+  tar -x --keep-directory-symlink `file_spec archivea.tar` || exit 1
+  tar -x --keep-directory-symlink `file_spec archiveb.tar` || exit 1
+  clean
+
+  prep collision_$round
+  tar -x --keep-directory-symlink `file_spec archivea.tar` --keep-old-files || 
exit 1
+  tar -x --keep-directory-symlink `file_spec archiveb.tar` --keep-old-files || 
exit 1
+  tar -x --keep-directory-symlink `file_spec archivec.tar` --keep-old-files && 
exit 1
+  clean
+done
+],
+[0],
+[== without_option_normal ==
+out
+out/root
+out/root/dir
+out/root/dirsymlink
+out/root/dirsymlink/file.conflict
+out/root/dirsymlink/filea
+out/root/dirsymlink/fileb
+== with_option_normal ==
+out
+out/root
+out/root/dir
+out/root/dir/file.conflict
+out/root/dir/filea
+out/root/dir/fileb
+out/root/dirsymlink
+== collision_normal ==
+out
+out/root
+out/root/dir
+out/root/dir/file.conflict
+out/root/dir/filea
+out/root/dir/fileb
+out/root/dir/filec
+out/root/dirsymlink
+== without_option_dir ==
+out
+out/root
+out/root/dir
+out/root/dirsymlink
+out/root/dirsymlink/file.conflict
+out/root/dirsymlink/filea
+out/root/dirsymlink/fileb
+== with_option_dir ==
+out
+out/root
+out/root/dir
+out/root/dir/file.conflict
+out/root/dir/filea
+out/root/dir/fileb
+out/root/dirsymlink
+== collision_dir ==
+out
+out/root
+out/root/dir
+out/root/dir/file.conflict
+out/root/dir/filea
+out/root/dir/fileb
+out/root/dir/filec
+out/root/dirsymlink
+],
+[== without_option_normal ==
+== with_option_normal ==
+== collision_normal ==
+tar: root/dirsymlink/file.conflict: Cannot open: File exists
+tar: Exiting with failure status due to previous errors
+== without_option_dir ==
+== with_option_dir ==
+== collision_dir ==
+tar: root/dirsymlink/file.conflict: Cannot open: File exists
+tar: Exiting with failure status due to previous 

Re: [Bug-tar] Unable to build tar with PGI compilers

[Pavel Raiskup]

[+cc gnulib, the selinux-at code comes from there]

Thanks for the report!

On Tuesday, February 14, 2017 10:16:02 AM CET Lorinczy Zsigmond wrote:
> It might be selinux (even if you didn't specify the platform you use).
> You could try to disable it with configure option '--without-selinux'

Yes, for some reason ./configure thinks that tar should be built with
selinux support (so SELinux is working on your box?) and the build fails later.
The --without-selinux really could work-around this issue.

Copying the original report:
> I'm trying to build GNU Tar 1.29 with the PGI 16.10 compilers, but it crashes
> during `make`:

I'm not familiar with this compiler, thus the errors below are not
readable to me (maybe undefined symbol because of broken system header?).
Perhaps others might decode better ..  but I think that the best guess
would be to provide a patch which fixes gnulib's SELinux module for your
system/compiler.

Pavel

> PGC-S-0040-Illegal use of symbol, security_context_t (getfilecon.c: 36)
> PGC-W-0156-Type not specified, 'int' assumed (getfilecon.c: 36)
> PGC-S-0040-Illegal use of symbol, security_context_t (getfilecon.c: 37)
> PGC-W-0156-Type not specified, 'int' assumed (getfilecon.c: 37)
> PGC-S-0040-Illegal use of symbol, security_context_t (getfilecon.c: 38)
> PGC-W-0156-Type not specified, 'int' assumed (getfilecon.c: 38)
> PGC-S-0040-Illegal use of symbol, security_context_t (getfilecon.c: 49)
> PGC-W-0156-Type not specified, 'int' assumed (getfilecon.c: 49)
> PGC-W-0095-Type cast required for this conversion (getfilecon.c: 57)
> PGC-W-0155-Pointer value created from a nonlong integral type  (getfilecon.c: 
> 57)
> PGC-W-0093-Type cast required for this conversion of constant (getfilecon.c: 
> 60)
> PGC-S-0040-Illegal use of symbol, security_context_t (getfilecon.c: 69)
> PGC-W-0156-Type not specified, 'int' assumed (getfilecon.c: 69)
> PGC-S-0040-Illegal use of symbol, security_context_t (getfilecon.c: 76)
> PGC-W-0156-Type not specified, 'int' assumed (getfilecon.c: 76)
> PGC-S-0040-Illegal use of symbol, security_context_t (getfilecon.c: 83)
> PGC-W-0156-Type not specified, 'int' assumed (getfilecon.c: 83)
> PGC/x86-64 Linux 16.10-0: compilation completed with severe errors
> make[4]: *** [getfilecon.o] Error 2

Re: [Bug-tar] exclude-vcs-ignores bug

[Pavel Raiskup]

On Thursday, December 15, 2016 11:09:39 AM CET Pavel Raiskup wrote:
> Anyways, it looks like this deserves separate library
> doing the right thing WRT .gitignore [1].
> 
> [1] https://git-scm.com/docs/gitignore

Seems like there's 'git check-ignore PATTERN' which is not really library, but
maybe it's better then reimplement gitignore pattern matching.

As a workaround, one can generate the list of files to be archived and use that
list as tar's '-T' parameter.

Pavel

Re: [Bug-tar] exclude-vcs-ignores bug

[Pavel Raiskup]

On Wednesday, December 7, 2016 9:23:10 AM CET Serge Matveenko wrote:
> Hello everybody!
> 
> It looks like I've found bug in tar.
> 
> When used to create archive with option `--exclude-vcs-ignores` tar treats
> `.gitignore` patterns as shell patterns while they are not.

That's right [1], it is a bit more complicated, though the shell patterns are
closer to that syntax than regexps.

> Consider the following example.
> 
> $ tar c --exclude-vcs-ignores -f ../tartest.tar .
> $ tar tf ../tartest.tar
> ./
> $ ls -1lA
> total 8
> 1
> .2
> .git
> .gitignore
> $ cat .gitignore
> .*
> 
> Expected result is for `1` to be found in the archive.
> 
> In this case git ignores `.2` and `.gitignore` files while tar ignores
> everything.
> `.*` means "ignore everything starting with .". This is not a regex.
> 
> It is expected for `--exclude-vcs-ignores` to mimic VCS' ignore logic.

This is not that tar threats '.*' as regex, but I think tar tries to match
'.*' pattern with './1' string for example, and it succeeds (so the file
is excluded).

The pattern '.*' should be probably transformed into './.*' in tar,
somewhere around:

  -> src/exclist.c:
  106 >---  if (add_exclude_fp (vcsfile->addfn, ex, fp,
  107 >--->--->---  EXCLUDE_WILDCARDS|EXCLUDE_ANCHORED, '\n',
  108 >--->--->---  vcsfile->data))
  109 >---{

But add_exclude_fp seems to be gnulib call, and 'fp' is FILE* pointer, so
I'm not sure how easily prepend the leading part of the pattern (without diving
too deep) ... maybe tar should avoid using add_exclude_fp() call ... and add the
exclude pattern manually.  Anyways, it looks like this deserves separate library
doing the right thing WRT .gitignore [1].

[1] https://git-scm.com/docs/gitignore

Pavel

[Bug-tar] [PATCH] tests: more deterministic xattr07

[Pavel Raiskup]

* tests/xattr07.at: Define order of files within tested archive.
---
 tests/xattr07.at | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/xattr07.at b/tests/xattr07.at
index a834981..47d54b8 100644
--- a/tests/xattr07.at
+++ b/tests/xattr07.at
@@ -36,7 +36,7 @@ setfattr -n user.test -v OurDirValue dir
 setfattr -n user.test -v OurFileValue dir/file
 setfattr -n user.test -v OurFileValue dir/file2
 
-tar --xattrs -cf archive.tar dir
+tar --xattrs --no-recursion -cf archive.tar dir dir/file dir/file2
 
 setfattr -n user.test -v OurDirValue2 dir
 setfattr -n user.test -v OurFileValue2 dir/file
-- 
2.9.3

Re: [Bug-tar] [PATCH] Bugfix - fix xattr exclude/include for archive create

[Pavel Raiskup]

On Friday, July 29, 2016 9:23:58 AM CEST Pavel Raiskup wrote:
> On Thursday, June 2, 2016 8:04:52 AM CEST Pavel Raiskup wrote:
> > Fixed the testsuite and re-attached.
> 
> Ping?

Ping again for:
http://www.mail-archive.com/bug-tar@gnu.org/msg05069.html

Pavel

Re: [Bug-tar] Multiple path headers mixing sparse and --posix

[Pavel Raiskup]

On Thursday, June 30, 2016 4:23:15 PM CEST Pavel Raiskup wrote:
> Re-attached with testcase.

Ping for http://www.mail-archive.com/bug-tar@gnu.org/msg05079.html

Pavel

Re: [Bug-tar] Tar hangs (blocks?) on existing file when extracting with `--skip-old-files` and `--xattrs`.

[Pavel Raiskup]

Hi Dawid, thanks for reporting this!

On Thursday, October 6, 2016 12:47:26 PM CEST Dawid wrote:
> I've tried to use `tar + ssh + tar` pipeline to copy files between two
> hosts. The only twist over many examples you can find with 2 minutes of
> online searching is that I need to copy `xattrs` and not overwrite the
> existing files.
>
> To my surprise it worked the first time, but on subsequent attempts it
> just hangs. After some investigation, it turned out that if one uses
> `--xattrs --skip-old-files` together, and `tar x ...` encouter existing
> file, it just hangs.

That's bug.  The set_xattr() function ignores RECOVER_SKIP.

> More investigation (when looking for workaround) revealed that
> `--keep-old-files` does not seem to preserve `xattrs` of the destination. The
> original destination file will be kept, but the xattrs will be overwritten
> anyway.

This is related bug too, can you please try the attached patch?

Pavel
>From f201b360e67f53fb0a0e367dc69e7e59f3a38d3f Mon Sep 17 00:00:00 2001
From: Pavel Raiskup <prais...@redhat.com>
Date: Fri, 7 Oct 2016 11:45:17 +0200
Subject: [PATCH] xattrs: don't set xattrs when --skip-old-files is used

* src/extract.c (set_xattr): Properly handle maybe_recoverable()
output.  Throw warnings to not complicate caller.
(extract_file): Don't handle set_xattr's error.
* tests/xattr06.at: New testcase.
* tests/Makefile.am: Mention new testcase.
* tests/testsuite.at: Likewise.
* THANKS: Dawid.
---
 THANKS |  1 +
 src/extract.c  | 41 +++---
 tests/Makefile.am  |  1 +
 tests/testsuite.at |  1 +
 tests/xattr06.at   | 73 ++
 5 files changed, 102 insertions(+), 15 deletions(-)
 create mode 100644 tests/xattr06.at

diff --git a/THANKS b/THANKS
index f1def93..5e8e8c9 100644
--- a/THANKS
+++ b/THANKS
@@ -138,6 +138,7 @@ David Nugent		dav...@blaze.net.au
 David Shaw		david.s...@alcatel.com.au
 David Steiner		dstei...@ispa.uni-osnabrueck.de
 David Taylor		tay...@think.com
+Dawid			d...@dpc.pw
 Dean Gaudet		dgau...@watdragon.uwaterloo.ca
 Demizu Noritoshi	nor...@is.aist-nara.ac.jp
 Denis Excoffier denis.excoff...@free.fr
diff --git a/src/extract.c b/src/extract.c
index f982433..67885d7 100644
--- a/src/extract.c
+++ b/src/extract.c
@@ -795,13 +795,13 @@ maybe_recoverable (char *file_name, bool regular, bool *interdir_made)
in advance dramatically improves the following  performance of reading and
writing a file).  If not restoring permissions, invert the INVERT_PERMISSIONS
bits from the file's current permissions.  TYPEFLAG specifies the type of the
-   file.  FILE_CREATED indicates set_xattr has created the file */
+   file.  Returns non-zero when error occurs (while un-available xattrs is not
+   an error, rather no-op).  Non-zero FILE_CREATED indicates set_xattr has
+   created the file. */
 static int
 set_xattr (char const *file_name, struct tar_stat_info const *st,
mode_t invert_permissions, char typeflag, int *file_created)
 {
-  int status = 0;
-
 #ifdef HAVE_XATTRS
   bool interdir_made = false;
 
@@ -809,17 +809,32 @@ set_xattr (char const *file_name, struct tar_stat_info const *st,
 {
   mode_t mode = current_stat_info.stat.st_mode & MODE_RWX & ~ current_umask;
 
-  do
-status = mknodat (chdir_fd, file_name, mode ^ invert_permissions, 0);
-  while (status && maybe_recoverable ((char *)file_name, false,
-  _made));
+  for (;;)
+{
+  if (!mknodat (chdir_fd, file_name, mode ^ invert_permissions, 0))
+{
+  /* Successfully created file */
+  xattrs_xattrs_set (st, file_name, typeflag, 0);
+  *file_created = 1;
+  return 0;
+}
 
-  xattrs_xattrs_set (st, file_name, typeflag, 0);
-  *file_created = 1;
+  switch (maybe_recoverable ((char *)file_name, false, _made))
+{
+  case RECOVER_OK:
+continue;
+  case RECOVER_NO:
+skip_member ();
+open_error (file_name);
+return 1;
+  case RECOVER_SKIP:
+return 0;
+}
+}
 }
 #endif
 
-  return(status);
+  return 0;
 }
 
 /* Fix the statuses of all directories whose statuses need fixing, and
@@ -1136,11 +1151,7 @@ extract_file (char *file_name, int typeflag)
   int file_created = 0;
   if (set_xattr (file_name, _stat_info, invert_permissions,
  typeflag, _created))
-{
-  skip_member ();
-  open_error (file_name);
-  return 1;
-}
+return 1;
 
   while ((fd = open_output_file (file_name, typeflag, mode,
  file_created, _mode,
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 5376180..06f2325 100644
--- a/tests/Make

Re: [Bug-tar] Circular symlinks lead to segfault

[Pavel Raiskup]

On Wednesday, February 10, 2016 10:53:51 AM CEST Pavel Raiskup wrote:
> I rebased the patches (mostly because of copyright year bump) and
> attaching again.

Rebased, some test-case caused patches did not apply cleanly.

I observe two test-case failures not related to this proposal, but rather
btrfs issues [1,2].  On ext4 the testsuite again passed for me.

[1] https://lists.gnu.org/archive/html/bug-tar/2016-07/msg0.html
[2] https://mail-archive.com/linux-btrfs@vger.kernel.org/msg55383.html

Pavel
>From cd3cd86e683a6050a97bf24a312cb372107bef45 Mon Sep 17 00:00:00 2001
From: Pavel Raiskup <prais...@redhat.com>
Date: Fri, 11 Jul 2014 07:36:58 +0200
Subject: [PATCH 1/3] tar: avoid recursion in directory traversal

This commit avoids unnecessary recursion from dump_file() call.
Directory "stack" is created on heap space instead to avoid
segfaults in case of rarely deep directory structures.  Note that
the recursion still exists in case of incremental archives.

Relevant tread:
http://www.mail-archive.com/bug-tar@gnu.org/msg04542.html

* src/create.c (dump_dir0): Rename to dump_dir as the wrapper
itself caused that additional calls to free() had to exist.
(dump_dir): Return bool.  Use the tour_t context and call
tour_plan_dir/tour_plan_file to emulate recursion from now.
(open_failure_recover): Rather dup() (sensitively) the directory
descriptor for fdopendir() purposes which allows us to save a lot
of memory in case of deep dir structures.
(create_archive): Use dump_file_incr for incremental dumps and
dump_member for usual dumps instead of dump_file in general.
(dump_file0): Reuse tour_t context.  Let the tar_stat_close on
"tour" mechanism.
(dump_file): Rename to dump_member.
(dump_member): The member name parameter is enough from now.  Use
the tour_t & tour_* handlers.
* update.c (update_archive): Call dump_member from now.
* src/tour.c: New file.
* src/common.h: Export needed symbols from xlist.c.
* gnulib.modules: Use array-list and xlist.
* src/Makefile.am: Compile also tour.c file.
* NEWS: Document.
---
 NEWS|   4 +
 gnulib.modules  |   2 +
 src/Makefile.am |   1 +
 src/common.h|  31 +++-
 src/create.c| 148 +--
 src/tour.c  | 234 
 src/unlink.c|   4 +-
 src/update.c|   2 +-
 8 files changed, 344 insertions(+), 82 deletions(-)
 create mode 100644 src/tour.c

diff --git a/NEWS b/NEWS
index caa77bc..26c8d7c 100644
--- a/NEWS
+++ b/NEWS
@@ -168,6 +168,10 @@ speed up archivation.
 
 * Tar refuses to read input from and write output to a tty device.
 
+* Tar avoids calling recursive rutines for directory traversal (for usual
+  scenarios) to save stack-space and thus avoid segfaults for rarely deep
+  archived directories.
+
 * Manpages
 
 This release includes official tar(1) and rmt(8) manpages.
diff --git a/gnulib.modules b/gnulib.modules
index 0e1de2f..d2639c1 100644
--- a/gnulib.modules
+++ b/gnulib.modules
@@ -23,6 +23,7 @@ areadlinkat-with-size
 argmatch
 argp
 argp-version-etc
+array-list
 backupfile
 closeout
 configmake
@@ -100,5 +101,6 @@ version-etc-fsf
 xalloc
 xalloc-die
 xgetcwd
+xlist
 xstrtoumax
 xvasprintf
diff --git a/src/Makefile.am b/src/Makefile.am
index 08fc24c..fa9f766 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -40,6 +40,7 @@ tar_SOURCES = \
  suffix.c\
  system.c\
  tar.c\
+ tour.c\
  transform.c\
  unlink.c\
  update.c\
diff --git a/src/common.h b/src/common.h
index 50c34cc..7f4103a 100644
--- a/src/common.h
+++ b/src/common.h
@@ -499,7 +499,8 @@ char *get_directory_entries (struct tar_stat_info *st);
 
 void create_archive (void);
 void pad_archive (off_t size_left);
-void dump_file (struct tar_stat_info *parent, char const *name,
+void dump_member (char const *member);
+void dump_file_incr (struct tar_stat_info *parent, char const *name,
 		char const *fullname);
 union block *start_header (struct tar_stat_info *st);
 void finish_header (struct tar_stat_info *st, union block *header,
@@ -970,5 +971,33 @@ int owner_map_translate (uid_t uid, uid_t *new_uid, char const **new_name);
 void group_map_read (char const *file);
 int group_map_translate (gid_t gid, gid_t *new_gid, char const **new_name);
 
+/* Module tour.c */
+
+/* hide TOUR definition in tour.c */
+typedef struct tour *tour_t;
+
+/* One tour_node_t represents single directory level in FS hierarchy,
+   i.e. list of files.  At one moment we keep in memory only one path
+   representation. */
+typedef struct
+{
+  struct tar_stat_info *parent; /* used to fill child's stat info */
+  char *items;  /* output from readdir */
+
+  /* per-directory volatile data to avoid reallocation for each file in
+   * directory */
+  const char   *item; /* processed filename (ptr into ITEMS) */
+  struct tar_stat_info  st;   /* stat of processed file */
+  char *namebuf;  /* buffer for constructing full 

Re: [Bug-tar] gnu tar incremental backup saves unchanged directories

[Pavel Raiskup]

On Tuesday, September 27, 2016 7:25:45 PM CEST Sergey Poznyakoff wrote:
> I'm ashamed.  It's my fault it wasn't incorporated then.

Please don't, I meant that I faced some real issues with the patch while
working on something else, probably that is just not complete fix.
Something says me that it is related to [1], but I don't see an obvious
issue now :/

[1] https://www.mail-archive.com/bug-tar@gnu.org/msg04542.html

Pavel

Re: [Bug-tar] gnu tar incremental backup saves unchanged directories

[Pavel Raiskup]

On Monday, September 26, 2016 2:11:45 PM CEST Sergey Poznyakoff wrote:
> Ivan Kalvachev  ha escrit:
> > http://lists.gnu.org/archive/html/bug-tar/2016-08/msg3.html ), but
> > so far I haven't gotten any reply.
> 
> I've pushed the attached patch.  Thanks for the report.

FYI, I've proposed the same patch several years ago, and we have it
applied in Feodra and RHEL for some time:
https://lists.gnu.org/archive/html/bug-tar/2012-02/msg7.html

After several pings :) I stopped re-proposing the patch, because I started
to have a feeling this is not 100% proper fix, but I don't really remember
why :(.

Pavel

Re: [Bug-tar] Bug with TAR_OPTIONS and exclude-from in tar 1.29

[Pavel Raiskup]

On Tuesday, September 20, 2016 5:07:14 PM CEST Jan Larres wrote:
> Is there any possibility that someone could have a look at this? Having a set
> of "standard" excludes with this option is quite useful to always exclude
> custom build files, swap files, backup files etc that aren't covered by the
> existing options. Thanks!

That's because 'name_list' elements point to temporary memory allocated by
wordsplit code while parsing TAR_OPTIONS.  That memory is however explicitly
removed right after the TAR_OPTIONS is parsed.  So the naive patch would be:

option-parser: fix use-after-free error

diff --git a/src/tar.c b/src/tar.c
index ba24c43..ddc5d33 100644
--- a/src/tar.c
+++ b/src/tar.c
@@ -2202,8 +2202,6 @@ parse_default_options (void)
   ws.ws_wordv[0] = (char*) program_name;
   more_options (ws.ws_offs + ws.ws_wordc, ws.ws_wordv, );
 }
-
-  wordsplit_free ();
 }

This is OK, because the function is called just once (we don't have to care
about memory leaks too much.  However, the wordsplit code is used multiple times
while parsing arguments, so I'm not sure .. it might ask for some systematic
long-term fix?

Pavel

Re: [Bug-tar] stat() on btrfs reports the st_blocks with delay (data loss in archivers)

[Pavel Raiskup]

On Monday, July 4, 2016 1:35:25 PM CEST Andreas Dilger wrote:
> I think in addition to fixing btrfs (because it needs to work with existing
> tar/rsync/etc. tools) it makes sense to *also* fix the heuristics of tar to
> handle this situation more robustly.

What I was rather thinking about is to remove the [2] heuristic.  As there is
now SEEK_HOLE implemented, the need for that check "completely sparse files"
might be considered less useful.  With [1], I'm not sure -- is it that bad to
face some false positive there? (it is documented that tar shouldn't be run
concurrently with other processes writing to archived files .., and waiting for
flush here is probably a very similar race condition).

> One option is if st_blocks == 0 then tar should also check if st_mtime is
> less than 60s in the past, and if yes then it should call fsync() on the
> file to flush any unwritten data to disk , or assume the file is not sparse
> and read the whole file, so that it doesn't incorrectly assume that the file
> is sparse and skip archiving the file data.

The reported fact 'st_blocks != 0' doesn't mean that the fsync() call is not
needed, so I'm not 100% we should special-case the 'st_blocks == 0' files.

--

As this effectively breaks tar's testsuite on btrfs, could we also explicitly
sync in 'genfile'?

Pavel

> 
> Cheers, Andreas
> 
> > [1] 
> > http://git.savannah.gnu.org/cgit/paxutils.git/tree/lib/system.h?id=ec72abd9dd63bbff4534ec77e97b1a6cadfc3cf8#n392
> > [2] 
> > http://git.savannah.gnu.org/cgit/tar.git/tree/src/sparse.c?id=ac065c57fdc1788a2769fb119ed0c8146e1b9dd6#n273

[Bug-tar] stat() on btrfs reports the st_blocks with delay (data loss in archivers)

[Pavel Raiskup]

There are optimizations in archivers (tar, rsync, ...) that rely on up2date
st_blocks info.  For example, in GNU tar there is optimization check [1]
whether the 'st_size' reports more data than the 'st_blocks' can hold --> then
tar considers that file is sparse (and does additional steps).

It looks like btrfs doesn't show correct value in 'st_blocks' until the data
are synced.  ATM, there happens that:

a) some "tool" creates sparse file
b) that tool does not sync explicitly and exits ..
c) tar is called immediately after that to archive the sparse file
d) tar considers [2] the file is completely sparse (because st_blocks is
   zero) and archives no data.  Here comes data loss.

Because we fixed 'btrfs' to report non-zero 'st_blocks' when the file data is
small and is in-lined (no real data blocks) -- I consider this is too bug in
btrfs worth fixing.

[1] 
http://git.savannah.gnu.org/cgit/paxutils.git/tree/lib/system.h?id=ec72abd9dd63bbff4534ec77e97b1a6cadfc3cf8#n392
[2] 
http://git.savannah.gnu.org/cgit/tar.git/tree/src/sparse.c?id=ac065c57fdc1788a2769fb119ed0c8146e1b9dd6#n273

Tested on kernel:
kernel-4.5.7-300.fc24.x86_64

Originally reported here, reproducer available there:
https://bugzilla.redhat.com/show_bug.cgi?id=1352061

Pavel

Re: [Bug-tar] Multiple path headers mixing sparse and xattrs

[Pavel Raiskup]

Re-attached with testcase.

Pavel
From 4f1de1f88a1c5e0c7482be02f40c5472368da792 Mon Sep 17 00:00:00 2001
From: Pavel Raiskup <prais...@redhat.com>
Date: Thu, 30 Jun 2016 16:17:29 +0200
Subject: [PATCH] sparse: fix pax extraction for unicode filenames

Make sure that 'GNU.sparse.name' header has higher priority than
(for sparse-purposes artificially modified) 'path' pax header.

Historically, the 'GNU.sparse.name' header comes before 'path';
this caused that modified 'path' header won and that is not what
we want in sparse "capable" tar implementation.

* src/tar.h (tar_stat_info): New argument sparse_name_done.
* src/xheader.c (raw_path_decoder): Move here the unconditional
code from path_decoder.
(path_decoder): Apply raw_path_decoder only if sparse_path_decoder
was not yet called.
(sparse_path_decoder): New wrapper around raw_path_decoder.
* tests/sparse07.at: New testcase.
* tests/testsuite.at: Mention new testcase.
* tests/Makefile.am: Likewise.
---
 src/tar.h  |  4 
 src/xheader.c  | 28 +++-
 tests/Makefile.am  |  1 +
 tests/sparse07.at  | 35 +++
 tests/testsuite.at |  1 +
 5 files changed, 64 insertions(+), 5 deletions(-)
 create mode 100644 tests/sparse07.at

diff --git a/src/tar.h b/src/tar.h
index 07b5bc1..f3e2c43 100644
--- a/src/tar.h
+++ b/src/tar.h
@@ -331,6 +331,10 @@ struct tar_stat_info
   int   real_size_set;  /* True when GNU.sparse.realsize is set in
 			   archived file */
 
+  bool  sparse_name_done;   /* Set to true if 'GNU.sparse.name' header was
+   processed pax header parsing.  Following 'path'
+   header (lower priority) will be ignored. */
+
   size_t xattr_map_size;   /* Size of the xattr map */
   struct xattr_array *xattr_map;
 
diff --git a/src/xheader.c b/src/xheader.c
index 8dda580..335ddaf 100644
--- a/src/xheader.c
+++ b/src/xheader.c
@@ -1291,16 +1291,34 @@ path_coder (struct tar_stat_info const *st, char const *keyword,
 }
 
 static void
-path_decoder (struct tar_stat_info *st,
-	  char const *keyword __attribute__((unused)),
-	  char const *arg,
-	  size_t size __attribute__((unused)))
+raw_path_decoder (struct tar_stat_info *st, char const *arg)
 {
   decode_string (>orig_file_name, arg);
   decode_string (>file_name, arg);
   st->had_trailing_slash = strip_trailing_slashes (st->file_name);
 }
 
+
+static void
+path_decoder (struct tar_stat_info *st,
+	  char const *keyword __attribute__((unused)),
+	  char const *arg,
+	  size_t size __attribute__((unused)))
+{
+  if (! st->sparse_name_done)
+raw_path_decoder (st, arg);
+}
+
+static void
+sparse_path_decoder (struct tar_stat_info *st,
+ char const *keyword __attribute__((unused)),
+ char const *arg,
+ size_t size __attribute__((unused)))
+{
+  st->sparse_name_done = true;
+  raw_path_decoder (st, arg);
+}
+
 static void
 size_coder (struct tar_stat_info const *st, char const *keyword,
 	struct xheader *xhdr, void const *data __attribute__ ((unused)))
@@ -1730,7 +1748,7 @@ struct xhdr_tab const xhdr_tab[] = {
   { "uname",uname_coder,uname_decoder,0, false },
 
   /* Sparse file handling */
-  { "GNU.sparse.name",   path_coder, path_decoder,
+  { "GNU.sparse.name",   path_coder, sparse_path_decoder,
 XHDR_PROTECTED, false },
   { "GNU.sparse.major",  sparse_major_coder, sparse_major_decoder,
 XHDR_PROTECTED, false },
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 0ea6d17..5f68583 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -214,6 +214,7 @@ TESTSUITE_AT = \
  sparse04.at\
  sparse05.at\
  sparse06.at\
+ sparse07.at\
  sparsemv.at\
  sparsemvp.at\
  spmvp00.at\
diff --git a/tests/sparse07.at b/tests/sparse07.at
new file mode 100644
index 000..8191c00
--- /dev/null
+++ b/tests/sparse07.at
@@ -0,0 +1,35 @@
+# Process this file with autom4te to create testsuite. -*- Autotest -*-
+
+# Test suite for GNU tar.
+# Copyright 2016 Free Software Foundation, Inc.
+
+# This file is part of GNU tar.
+
+# GNU tar is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+
+# GNU tar is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+AT_SETUP([sparse files with unicode names])
+AT_KEYWORDS([sparse sparse07 unicode])
+
+AT_TAR_CHECK([
+genfile --sparse --file žluť --block-siz

Re: [Bug-tar] Multiple path headers mixing sparse and xattrs

[Pavel Raiskup]

On Thursday, June 23, 2016 3:35:51 PM CEST Dominique Martinet wrote:
> Does anyone have an opinion on this ?

It is a bug in sparse and --posix (--xatts implies --posix).  Have a look at the
attached patch which should resolve this.

Pavel
>From d2d97a795ab1b5b04f19e9f60d39301e01f7218d Mon Sep 17 00:00:00 2001
From: Pavel Raiskup <prais...@redhat.com>
Date: Thu, 30 Jun 2016 15:47:41 +0200
Subject: [PATCH] sparse: fix pax extraction for unicode filenames

Make sure that 'GNU.sparse.name' header has higher priority than
(for sparse-purposes artificially modified) 'path' pax header.

Historically, the 'GNU.sparse.name' header comes before 'path';
this caused that modified 'path' header won and that is not what
we want in sparse "capable" tar implementation.

* src/tar.h (tar_stat_info): New argument sparse_name_done.
* src/xheader.c (raw_path_decoder): Move here the unconditional
code from path_decoder.
(path_decoder): Apply raw_path_decoder only if sparse_path_decoder
was not yet called.
(sparse_path_decoder): New wrapper around raw_path_decoder.
---
 src/tar.h |  4 
 src/xheader.c | 28 +++-
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/src/tar.h b/src/tar.h
index 07b5bc1..f3e2c43 100644
--- a/src/tar.h
+++ b/src/tar.h
@@ -331,6 +331,10 @@ struct tar_stat_info
   int   real_size_set;  /* True when GNU.sparse.realsize is set in
 			   archived file */
 
+  bool  sparse_name_done;   /* Set to true if 'GNU.sparse.name' header was
+   processed pax header parsing.  Following 'path'
+   header (lower priority) will be ignored. */
+
   size_t xattr_map_size;   /* Size of the xattr map */
   struct xattr_array *xattr_map;
 
diff --git a/src/xheader.c b/src/xheader.c
index 8dda580..335ddaf 100644
--- a/src/xheader.c
+++ b/src/xheader.c
@@ -1291,16 +1291,34 @@ path_coder (struct tar_stat_info const *st, char const *keyword,
 }
 
 static void
-path_decoder (struct tar_stat_info *st,
-	  char const *keyword __attribute__((unused)),
-	  char const *arg,
-	  size_t size __attribute__((unused)))
+raw_path_decoder (struct tar_stat_info *st, char const *arg)
 {
   decode_string (>orig_file_name, arg);
   decode_string (>file_name, arg);
   st->had_trailing_slash = strip_trailing_slashes (st->file_name);
 }
 
+
+static void
+path_decoder (struct tar_stat_info *st,
+	  char const *keyword __attribute__((unused)),
+	  char const *arg,
+	  size_t size __attribute__((unused)))
+{
+  if (! st->sparse_name_done)
+raw_path_decoder (st, arg);
+}
+
+static void
+sparse_path_decoder (struct tar_stat_info *st,
+ char const *keyword __attribute__((unused)),
+ char const *arg,
+ size_t size __attribute__((unused)))
+{
+  st->sparse_name_done = true;
+  raw_path_decoder (st, arg);
+}
+
 static void
 size_coder (struct tar_stat_info const *st, char const *keyword,
 	struct xheader *xhdr, void const *data __attribute__ ((unused)))
@@ -1730,7 +1748,7 @@ struct xhdr_tab const xhdr_tab[] = {
   { "uname",uname_coder,uname_decoder,0, false },
 
   /* Sparse file handling */
-  { "GNU.sparse.name",   path_coder, path_decoder,
+  { "GNU.sparse.name",   path_coder, sparse_path_decoder,
 XHDR_PROTECTED, false },
   { "GNU.sparse.major",  sparse_major_coder, sparse_major_decoder,
 XHDR_PROTECTED, false },
-- 
2.7.4

Re: [Bug-tar] tar-1.26-29.el7.x86_64 on CentOS

[Pavel Raiskup]

Hi Tero,

On Monday, June 27, 2016 9:27:43 AM CEST Tero Kirjavainen wrote:
> By default, extract destroys symbolic links on CentOS 7
> Older CentOS 6 and tar-1.23-13.el6.x86_64 this didn't happen.
> I don't know if this is CentOS specific or generic Tar behaviour.

I encourage you to contact CentOS bugzilla with CentOS related questions.
Upstream is usually not aware of distribution specific stuff..

But yes, older tar (even upstream) followed directory symlinks instead of
removing them.  This has been changed to remove dir symlinks (as any other
non-directory files) later and even after that fix the option
--keep-directory-option has been added to be compatible with the old behavior.

Pavel

Re: [Bug-tar] [GNU tar 1.29] testsuite: 70 76 90 failed

[Pavel Raiskup]

Hi Giulio,

On Saturday, June 11, 2016 11:32:12 PM CEST Giulio Agostini wrote:
> I tried to make the latest tar on my shared system (hosted on Hostmonster,
> CentOS 6.7), but some tests (make check) failed, see attached.
> The reason I was trying to upgrade was because I had problems installing
> Lilypond, which makes use of tar, and my old version (1.23) had problems
> with extracting symlinks.

the 1.29 testsuite fails exactly because your (file)system has troubles with
symlinks (upgrade to 1.29 will most probably won't help).

I can't tell why this does not work for you:

mkdir dst1
ln -s target1 dst1/file1
echo target1 >dst1/target1

Pavel

Re: [Bug-tar] [PATCH] Bugfix - fix xattr exclude/include for archive create

[Pavel Raiskup]

+[0],
+[])
+
+AT_CLEANUP
-- 
2.5.5

>From 07e1634b93f8e41c6ab80f53067cea87a25353e6 Mon Sep 17 00:00:00 2001
From: Pavel Raiskup <prais...@redhat.com>
Date: Thu, 2 Jun 2016 08:00:10 +0200
Subject: [PATCH 2/2] * tests/xattr06.at: Test include/exclude during
 archive/exctract.

---
 src/xattrs.c |  2 +-
 tests/xattr06.at | 19 ---
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/src/xattrs.c b/src/xattrs.c
index 655dd43..d9f1f8b 100644
--- a/src/xattrs.c
+++ b/src/xattrs.c
@@ -495,7 +495,7 @@ xattrs_xattrs_get (int parentfd, char const *file_name,
 
   if (aret != -1)
 {
-  if (!xattrs_masked_out(attr, true))
+  if (!xattrs_masked_out (attr, true))
 xheader_xattr_add (st, attr, val, aret);
 }
   else if (errno != ENOATTR)
diff --git a/tests/xattr06.at b/tests/xattr06.at
index 42204af..691f7e7 100644
--- a/tests/xattr06.at
+++ b/tests/xattr06.at
@@ -34,16 +34,21 @@ mkdir dir
 mkdir output
 genfile --file dir/file
 
-setfattr -n user.excludedxattr -v value dir/file
+for attr in excluded incla inclb inclc incl_excluded
+do
+  setfattr -n user.${attr} -v value dir/file || AT_SKIP_TEST
+done
 
-# exclude the attribute we just set
-tar --xattrs --xattrs-exclude=user.excludedxattr -cf archive.tar -C dir .
+tar --xattrs-include=user.incl'*' --xattrs-exclude=user.incl_excluded -cf archive.tar -C dir .
+tar -xf archive.tar --xattrs-include=user.incl[[ab]] --xattrs-exclude=user.inclb -C output
 
-tar --xattrs -xf archive.tar -C output
-getfattr -d output/file | grep -v excludedxattr > without
-getfattr -d output/file > with
+getfattr -d output/file | grep -v \
+-e excluded \
+-e inclb \
+-e inclc > filtered
+getfattr -d output/file > full
 # if they differ then the attribute is still present
-diff without with
+diff filtered full
 ],
 [0],
 [])
-- 
2.5.5

Re: [Bug-tar] numeric.at vs sudo

[Pavel Raiskup]

On Tuesday 17 of May 2016 11:29:59 Jack Howarth wrote:
>While testing the new tar 1.29 release on x86_64-apple-darwin15
> under the fink packaging system, I noticed a glitch with the
> numeric.at test case. The fink packaging system runs under sudo and
> that test fails in that case as...

Thanks for the report, Jack.  Does the patch from 
http://www.mail-archive.com/bug-tar@gnu.org/msg05067.html help?

Pavel

Re: [Bug-tar] tar 1.29: Test 30 numeric.at broken with BSD filesystem semantics

[Pavel Raiskup]

On Wednesday 01 of June 2016 23:05:52 Christian Weisgerber wrote:
> In GNU tar 1.29, this new regression test
>
>   30: numeric.at:18  --numeric-owner basic tests
>
> is itself broken on filesystems that have BSD group inheritance,
> i.e., where a newly created file gets the group of the directory
> rather than that of the user.
>
> Obviously, this affects (at least) all *BSD operating systems.

Then we should be able to switch the group ownership back to $GID, does
the attached patch help?

Pavel
>From a2a41ba4a3710a815f18a2b87d280066e7bd51d2 Mon Sep 17 00:00:00 2001
From: Pavel Raiskup <prais...@redhat.com>
Date: Thu, 2 Jun 2016 07:28:01 +0200
Subject: [PATCH] tests: fix numeric.at for BSD

While creating file, BSD kernels inherit the group ownership from
parent directory.
http://lists.gnu.org/archive/html/bug-tar/2016-06/msg0.html

* tests/numeric.at: Attempt to 'chown' the newly created directory
to proper group (at least on affected machines that command is
expected to succeed).
---
 THANKS   | 1 +
 tests/numeric.at | 9 ++---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/THANKS b/THANKS
index f1def93..8d00396 100644
--- a/THANKS
+++ b/THANKS
@@ -94,6 +94,7 @@ Christian Laubscher	christian.laubsc...@tiscalinet.ch
 Christian T. Dum	c...@mpe-garching.mpg.de
 Christian von Roques	roq...@pond.sub.org
 Christian Wetzel	wet...@phoenix-pacs.de
+Christian Weisgerber	na...@mips.inka.de
 Christoph Litauer	lita...@mailhost.uni-koblenz.de
 Christophe Colle	co...@krtkg1.rug.ac.be
 Christophe Kalt		christophe.k...@kbcfp.com
diff --git a/tests/numeric.at b/tests/numeric.at
index 2fcd7ea..7a97248 100644
--- a/tests/numeric.at
+++ b/tests/numeric.at
@@ -27,14 +27,17 @@ tar $1 -vvf a dir |dnl
 ])
 
 AT_TAR_CHECK([
-mkdir dir
-genfile --file dir/file
-
 MYUID=$(id -u) || AT_SKIP_TEST
 MYGID=$(id -g) || AT_SKIP_TEST
 MYUSR=$(id -un) || AT_SKIP_TEST
 MYGRP=$(id -gn) || AT_SKIP_TEST
 
+mkdir dir
+# Ensure correct group id on BSDs.
+chown :$MYGID dir >/dev/null 2>/dev/null
+genfile --file dir/file
+
+
 TESTOP([--create])
 TESTOP([--list])
 TESTOP([--diff])
-- 
2.5.5

Re: [Bug-tar] [PATCH 0/5] Several changes in multi-volume archive handling

[Pavel Raiskup]

On Sunday 06 of December 2015 22:28:54 Sergey Poznyakoff wrote:
> Hi Pavel,
> 
> 1 - 4 applied (with slight modifications - 0a93c16c & 239441b5).
> 
> As to 5, I have serious doubts.  Of course hinting about the ? key
> is a nice idea.  However, the "Prepare volume" prompt is in its
> present form for long enough time, so that some other software
> might rely on its wording.

Definitely the prompt is not ideal now :/.  If we want to be
super-cautious, we could have additional option or environment variable to
enforce the old prompt format?

Do we know which software parses tar's prompt?

Pavel

Re: [Bug-tar] [PATCH] Bugfix - fix xattr exclude/include for archive create

[Pavel Raiskup]

Thanks, I would just update the testcase to be more general, but otherwise
it looks OK to me.  Sergey, can you have a look please?

On Monday 30 of May 2016 17:11:35 Ian McLeod wrote:
> * src/xattrs.c (xattrs_xattrs_get): apply exclude/include mask when
>   fetching extended attributes
> * tests/Makefile.am: Add new test case
> * tests/testsuite.at: ditto
> 
> This makes archive create behavior consistent with the documentation.
> Without this change xattr include/exclude options are accepted when
> creating an archive but are silently ignored.

This is nit, but we usually put description above the particular changelog
entries.

> ---
>  src/xattrs.c   | 15 ++-
>  tests/Makefile.am  |  1 +
>  tests/testsuite.at |  1 +
>  tests/xattr06.at   | 51 +++
>  4 files changed, 63 insertions(+), 5 deletions(-)
>  create mode 100644 tests/xattr06.at
> 
> diff --git a/src/xattrs.c b/src/xattrs.c
> index 8e56168..655dd43 100644
> --- a/src/xattrs.c
> +++ b/src/xattrs.c
> @@ -434,8 +434,12 @@ xattrs_clear_setup (void)
>clear_mask_map (_setup.excl);
>  }
>  
> -/* get all xattrs from file given by FILE_NAME or FD (when non-zero).  This
> -   includes all the user.*, security.*, system.*, etc. available domains */
> +static bool xattrs_masked_out (const char *kw, bool archiving);
> +
> +/* get xattrs from file given by FILE_NAME or FD (when non-zero)
> +   xattrs are checked against the user supplied include/exclude mask
> +   if no mask is given this includes all the user.*, security.*, system.*,
> +   etc. available domains */
>  void
>  xattrs_xattrs_get (int parentfd, char const *file_name,
> struct tar_stat_info *st, int fd)
> @@ -480,8 +484,6 @@ xattrs_xattrs_get (int parentfd, char const *file_name,
>size_t len = strlen (attr);
>ssize_t aret = 0;
>  
> -  /* Archive all xattrs during creation, decide at extraction 
> time
> -   * which ones are of interest/use for the target filesystem. */
>while (((fd == 0)
>? ((aret = lgetxattrat (parentfd, file_name, attr,
>val, asz)) == -1)
> @@ -492,7 +494,10 @@ xattrs_xattrs_get (int parentfd, char const *file_name,
>  }
>  
>if (aret != -1)
> -xheader_xattr_add (st, attr, val, aret);
> +{
> +  if (!xattrs_masked_out(attr, true))

s/xattrs_masked_out(/xattrs_masked_out (/

> +xheader_xattr_add (st, attr, val, aret);
> +}
>else if (errno != ENOATTR)
>  call_arg_warn ((fd == 0) ? "lgetxattrat"
> : "fgetxattr", file_name);
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 0ea6d17..5c13c8f 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -244,6 +244,7 @@ TESTSUITE_AT = \
>   xattr03.at\
>   xattr04.at\
>   xattr05.at\
> + xattr06.at\
>   acls01.at\
>   acls02.at\
>   acls03.at\
> diff --git a/tests/testsuite.at b/tests/testsuite.at
> index 11c39c9..126725c 100644
> --- a/tests/testsuite.at
> +++ b/tests/testsuite.at
> @@ -443,6 +443,7 @@ m4_include([xattr02.at])
>  m4_include([xattr03.at])
>  m4_include([xattr04.at])
>  m4_include([xattr05.at])
> +m4_include([xattr06.at])
>  
>  m4_include([acls01.at])
>  m4_include([acls02.at])
> diff --git a/tests/xattr06.at b/tests/xattr06.at
> new file mode 100644
> index 000..42204af
> --- /dev/null
> +++ b/tests/xattr06.at
> @@ -0,0 +1,51 @@
> +# Process this file with autom4te to create testsuite. -*- Autotest -*-
> +#
> +# Test suite for GNU tar.
> +# Copyright 2012-2014, 2016 Free Software Foundation, Inc.
> +
> +# This file is part of GNU tar.
> +
> +# GNU tar is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +
> +# GNU tar is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see .
> +#
> +# Test description:  Test for exclude of xattr during archive creation
> +#
> +# Relevant mailing list thread:
> +#
> +# http://lists.gnu.org/archive/html/bug-tar/2016-05/msg00031.html
+
> +AT_SETUP([xattrs: exclude xattrs on create ])
> +AT_KEYWORDS([xattrs xattr06])
> +
> +AT_TAR_CHECK([
> +AT_XATTRS_PREREQ
> +
> +mkdir dir
> +mkdir output
> +genfile --file dir/file
> +
> +setfattr -n user.excludedxattr -v value dir/file
> +
> +# exclude the attribute we just set
> +tar --xattrs 

Re: [Bug-tar] [PATCH] Bugfix - fix xattr exclude/include for archive create

[Pavel Raiskup]

Hi Ian, thanks for the update!

On Monday 30 of May 2016 17:23:33 Ian McLeod wrote:
> On 05/30/2016 02:10 AM, Pavel Raiskup wrote:
> > Could you please post smaller patch just having the important part:
> >
> > +  if (!xattrs_masked_out(attr, true /* archiving */))
> > +xheader_xattr_add (st, attr, val, aret);
> >
> > (+ one prototype above).  And could you please add one new testcase for
> > --xattrs-{exclude,include}?
>
> Done.
>
> Note that the added test only exercises exclude.  As best I can tell,
> --xattrs-include can never actually be meaningful when doing a create.
> Any --xattrs-include implies --xattrs which, for create, starts with a
> default of saving all xattrs.

If there is occurrence of --xattrs-include, tar does not store all
xattrs but only those which are specified:

  620 static bool
  621 xattrs_kw_included (const char *kw, bool archiving)
  622 {
  623   if (xattrs_setup.incl.size)
  624 return xattrs_matches_mask (kw, _setup.incl);
  ^^
  625   else if (archiving)
  626 return true;
  ^^ default case for 'tar -c --xattrs'
  627   else
  628 return strncmp (kw, USER_DOT_PFX, sizeof (USER_DOT_PFX) - 1) == 0;
  629 }

So the --xattrs-include is to some extent useful (and testable), too.

Pavel

Re: [Bug-tar] [PATCH] Bugfix - fix xattr exclude/include for archive create

[Pavel Raiskup]

Hi Ian,

On Sunday 29 of May 2016 16:20:00 Ian McLeod wrote:
> * src/xattrs.c (xattrs_xattrs_get): apply exclude/include mask when
>   fetching extended attributes
> 
> This makes archive create behavior consistent with the documentation.
> Without this change xattr include/exclude options are accepted when
> creating an archive but are silently ignored.

Good catch, exclude/include is ignored during archive creation.

Could you please post smaller patch just having the important part:

+  if (!xattrs_masked_out(attr, true /* archiving */))
+xheader_xattr_add (st, attr, val, aret);

(+ one prototype above).  And could you please add one new testcase for
--xattrs-{exclude,include}?

Thanks!
Pavel


> ---
>  src/xattrs.c | 103 
> ++-
>  1 file changed, 53 insertions(+), 50 deletions(-)
> 
> diff --git a/src/xattrs.c b/src/xattrs.c
> index 8e56168..a272af7 100644
> --- a/src/xattrs.c
> +++ b/src/xattrs.c
> @@ -434,8 +434,55 @@ xattrs_clear_setup (void)
>clear_mask_map (_setup.excl);
>  }
>  
> -/* get all xattrs from file given by FILE_NAME or FD (when non-zero).  This
> -   includes all the user.*, security.*, system.*, etc. available domains */
> +static bool
> +xattrs_matches_mask (const char *kw, struct xattrs_mask_map *mm)
> +{
> +  int i;
> +
> +  if (!mm->size)
> +return false;
> +
> +  for (i = 0; i < mm->used; i++)
> +if (fnmatch (mm->masks[i], kw, 0) == 0)
> +  return true;
> +
> +  return false;
> +}
> +
> +#define USER_DOT_PFX "user."
> +
> +static bool
> +xattrs_kw_included (const char *kw, bool archiving)
> +{
> +  if (xattrs_setup.incl.size)
> +return xattrs_matches_mask (kw, _setup.incl);
> +  else if (archiving)
> +return true;
> +  else
> +return strncmp (kw, USER_DOT_PFX, sizeof (USER_DOT_PFX) - 1) == 0;
> +}
> +
> +static bool
> +xattrs_kw_excluded (const char *kw, bool archiving)
> +{
> +  return xattrs_setup.excl.size ?
> +xattrs_matches_mask (kw, _setup.excl) : false;
> +}
> +
> +/* Check whether the xattr with keyword KW should be discarded from list of
> +   attributes that are going to be archived/excluded (set ARCHIVING=true for
> +   archiving, false for excluding) */
> +static bool
> +xattrs_masked_out (const char *kw, bool archiving)
> +{
> +  return xattrs_kw_included (kw, archiving) ?
> +xattrs_kw_excluded (kw, archiving) : true;
> +}
> +
> +/* get xattrs from file given by FILE_NAME or FD (when non-zero)
> +   xattrs are checked against the user supplied include/exclude mask
> +   if no mask is given this includes all the user.*, security.*, system.*,
> +   etc. available domains */
>  void
>  xattrs_xattrs_get (int parentfd, char const *file_name,
> struct tar_stat_info *st, int fd)
> @@ -480,8 +527,6 @@ xattrs_xattrs_get (int parentfd, char const *file_name,
>size_t len = strlen (attr);
>ssize_t aret = 0;
>  
> -  /* Archive all xattrs during creation, decide at extraction 
> time
> -   * which ones are of interest/use for the target filesystem. */
>while (((fd == 0)
>? ((aret = lgetxattrat (parentfd, file_name, attr,
>val, asz)) == -1)
> @@ -492,7 +537,10 @@ xattrs_xattrs_get (int parentfd, char const *file_name,
>  }
>  
>if (aret != -1)
> -xheader_xattr_add (st, attr, val, aret);
> +{
> +  if (!xattrs_masked_out(attr, true))
> +xheader_xattr_add (st, attr, val, aret);
> +}
>else if (errno != ENOATTR)
>  call_arg_warn ((fd == 0) ? "lgetxattrat"
> : "fgetxattr", file_name);
> @@ -595,51 +643,6 @@ xattrs_selinux_set (struct tar_stat_info const *st,
>  }
>  }
>  
> -static bool
> -xattrs_matches_mask (const char *kw, struct xattrs_mask_map *mm)
> -{
> -  int i;
> -
> -  if (!mm->size)
> -return false;
> -
> -  for (i = 0; i < mm->used; i++)
> -if (fnmatch (mm->masks[i], kw, 0) == 0)
> -  return true;
> -
> -  return false;
> -}
> -
> -#define USER_DOT_PFX "user."
> -
> -static bool
> -xattrs_kw_included (const char *kw, bool archiving)
> -{
> -  if (xattrs_setup.incl.size)
> -return xattrs_matches_mask (kw, _setup.incl);
> -  else if (archiving)
> -return true;
> -  else
> -return strncmp (kw, USER_DOT_PFX, sizeof (USER_DOT_PFX) - 1) == 0;
> -}
> -
> -static bool
> -xattrs_kw_excluded (const char *kw, bool archiving)
> -{
> -  return xattrs_setup.excl.size ?
> -xattrs_matches_mask (kw, _setup.excl) : false;
> -}
> -
> -/* Check whether the xattr with keyword KW should be discarded from list of
> -   attributes that are going to be archived/excluded (set ARCHIVING=true for
> -   archiving, false for excluding) */
> -static bool
> -xattrs_masked_out (const char *kw, bool archiving)

Re: [Bug-tar] On upcoming release

[Pavel Raiskup]

On Thursday 17 of March 2016 10:45:28 Sergey Poznyakoff wrote:
> I plan to release 1.29 within 7-10 days.  I'm aware of several still
> pending issues, but these don't seem to be important enough to justify
> further delays.  Please, correct me if I'm wrong.  Paul, Pavel - your
> opinions?

There is one infinite loop issue which could potentially deserve a minute:
http://lists.gnu.org/archive/html/bug-tar/2014-03/msg00052.html

Also the slightly adjusted (attached) patch for xattrs configure-detection
might help at least to OS X (no need to pass --without-xattrs?):
http://lists.gnu.org/archive/html/bug-tar/2014-08/msg3.html

Other patches (I touched) can probably wait for 1.30, I'll check.

The v1.29 would bring the SEEK_HOLE SEEK_DATA implementation, so sparse
file archiving might be good candidate for pre-release testing.

Thanks,
Pavel
>From c58fa39d546dc7284f03398e2beb8d0fce614ef0 Mon Sep 17 00:00:00 2001
From: Pavel Raiskup <address@hidden>
Date: Mon, 4 Aug 2014 13:19:49 +0200
Subject: [PATCH] xattrs: fix bug in configure

Define HAVE_XATTRS only when all needed xattr-related functions
are available (either in libc or libattr).

Reported independently by Denis Excoffier and Dominyk Tille.

* acinclude.m4 (TAR_HEADERS_ATTR_XATTR_H): Check for each xattr
function separately.  Don't AC_CHECK_LIB (LIBS is filled by
AC_SEARCH_LIBS when necessary).
* src/Makefile.am: The LDADD -lattr was redundant.
---
 acinclude.m4| 42 ++
 src/Makefile.am |  4 
 2 files changed, 14 insertions(+), 32 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 812340b..ca90285 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -40,37 +40,23 @@ AC_DEFUN([TAR_HEADERS_ATTR_XATTR_H],
   # First check for 
   AC_CHECK_HEADERS([sys/xattr.h])
   AM_CONDITIONAL([TAR_COND_XATTR_H],[test "$ac_cv_header_sys_xattr_h" = yes])
-  AM_CONDITIONAL([TAR_LIB_ATTR],[false])
-  if test "$ac_cv_header_sys_xattr_h" = yes; then
-AC_CHECK_FUNCS(getxattr  fgetxattr  lgetxattr \
-   setxattr  fsetxattr  lsetxattr \
-   listxattr flistxattr llistxattr,
-# only when functions are present
-AC_DEFINE([HAVE_SYS_XATTR_H], [1],
-[define to 1 if we have  header])
-if test "$with_xattrs" != no; then
-  AC_DEFINE([HAVE_XATTRS],,[Define when we have working linux xattrs.])
-fi
-)
-  fi
-
-  # If  is not found, then check for 
   if test "$ac_cv_header_sys_xattr_h" != yes; then
 AC_CHECK_HEADERS([attr/xattr.h])
 AM_CONDITIONAL([TAR_COND_XATTR_H],[test "$ac_cv_header_attr_xattr_h" = yes])
-AC_CHECK_LIB([attr],[fgetxattr])
-AM_CONDITIONAL([TAR_LIB_ATTR],[test "$ac_cv_lib_attr_fgetxattr" = yes])
-if test "$ac_cv_header_attr_xattr_h" = yes; then
-  AC_CHECK_FUNCS(getxattr  fgetxattr  lgetxattr \
- setxattr  fsetxattr  lsetxattr \
- listxattr flistxattr llistxattr,
-  # only when functions are present
-  AC_DEFINE([HAVE_ATTR_XATTR_H], [1],
-  [define to 1 if we have  header])
-  if test "$with_xattrs" != no; then
-AC_DEFINE([HAVE_XATTRS],,[Define when we have working linux xattrs.])
-  fi
-  )
+  fi
+
+  if test "$with_xattrs" != no; then
+for i in getxattr  fgetxattr  lgetxattr \
+ setxattr  fsetxattr  lsetxattr \
+ listxattr flistxattr llistxattr
+do
+  AC_SEARCH_LIBS($i, attr)
+  eval found=\$ac_cv_search_$i
+  test "$found" = "no" && break
+done
+
+if test "$found" != no; then
+  AC_DEFINE([HAVE_XATTRS],,[Define when we have working linux xattrs.])
 fi
   fi
 ])
diff --git a/src/Makefile.am b/src/Makefile.am
index dc05d5c..08fc24c 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -53,7 +53,3 @@ AM_CFLAGS = $(WARN_CFLAGS) $(WERROR_CFLAGS)
 LDADD = ../lib/libtar.a ../gnu/libgnu.a $(LIBINTL) $(LIBICONV)
 
 tar_LDADD = $(LIBS) $(LDADD) $(LIB_CLOCK_GETTIME) $(LIB_EACCESS) $(LIB_SELINUX)
-
-if TAR_LIB_ATTR
-tar_LDADD += -lattr
-endif
-- 
2.5.5

Re: [Bug-tar] Circular symlinks lead to segfault

[Pavel Raiskup]

On Sunday 15 of November 2015 17:13:43 Sergey Poznyakoff wrote:
> Pavel Raiskup <prais...@redhat.com> ha escrit:
> 
> > issues like that (bug in library call) should be fixed within
> > gnulib;
> 
> In general - yes, I agree.  But in this particular case it was exactly
> because it fixes remfiles08b,remfiles09b that I pushed it to the repo.

I rebased the patches (mostly because of copyright year bump) and
attaching again.

Pavel
>From 6d3f62a3935810bb36dd4a1d7fc887e84d5b4d71 Mon Sep 17 00:00:00 2001
From: Pavel Raiskup <prais...@redhat.com>
Date: Fri, 11 Jul 2014 07:36:58 +0200
Subject: [PATCH 1/3] tar: avoid recursion in directory traversal

This commit avoids unnecessary recursion from dump_file() call.
Directory "stack" is created on heap space instead to avoid
segfaults in case of rarely deep directory structures.  Note that
the recursion still exists in case of incremental archives.

Relevant tread:
http://www.mail-archive.com/bug-tar@gnu.org/msg04542.html

* src/create.c (dump_dir0): Rename to dump_dir as the wrapper
itself caused that additional calls to free() had to exist.
(dump_dir): Return bool.  Use the tour_t context and call
tour_plan_dir/tour_plan_file to emulate recursion from now.
(open_failure_recover): Rather dup() (sensitively) the directory
descriptor for fdopendir() purposes which allows us to save a lot
of memory in case of deep dir structures.
(create_archive): Use dump_file_incr for incremental dumps and
dump_member for usual dumps instead of dump_file in general.
(dump_file0): Reuse tour_t context.  Let the tar_stat_close on
"tour" mechanism.
(dump_file): Rename to dump_member.
(dump_member): The member name parameter is enough from now.  Use
the tour_t & tour_* handlers.
* update.c (update_archive): Call dump_member from now.
* src/tour.c: New file.
* src/common.h: Export needed symbols from xlist.c.
* gnulib.modules: Use array-list and xlist.
* src/Makefile.am: Compile also tour.c file.
* NEWS: Document.
---
 NEWS|   4 +
 gnulib.modules  |   2 +
 src/Makefile.am |   1 +
 src/common.h|  31 +++-
 src/create.c| 148 +--
 src/tour.c  | 234 
 src/unlink.c|   4 +-
 src/update.c|   2 +-
 8 files changed, 344 insertions(+), 82 deletions(-)
 create mode 100644 src/tour.c

diff --git a/NEWS b/NEWS
index 52e3f57..4941fe5 100644
--- a/NEWS
+++ b/NEWS
@@ -130,6 +130,10 @@ speed up archivation.
 
 * Tar refuses to read input from and write output to a tty device.
 
+* Tar avoids calling recursive rutines for directory traversal (for usual
+  scenarios) to save stack-space and thus avoid segfaults for rarely deep
+  archived directories.
+
 * Manpages
 
 This release includes official tar(1) and rmt(8) manpages.
diff --git a/gnulib.modules b/gnulib.modules
index fe5ab73..f2ce146 100644
--- a/gnulib.modules
+++ b/gnulib.modules
@@ -23,6 +23,7 @@ areadlinkat-with-size
 argmatch
 argp
 argp-version-etc
+array-list
 backupfile
 closeout
 configmake
@@ -102,5 +103,6 @@ version-etc-fsf
 xalloc
 xalloc-die
 xgetcwd
+xlist
 xstrtoumax
 xvasprintf
diff --git a/src/Makefile.am b/src/Makefile.am
index dc05d5c..2ef5637 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -40,6 +40,7 @@ tar_SOURCES = \
  suffix.c\
  system.c\
  tar.c\
+ tour.c\
  transform.c\
  unlink.c\
  update.c\
diff --git a/src/common.h b/src/common.h
index 09e42e1..4537e5a 100644
--- a/src/common.h
+++ b/src/common.h
@@ -492,7 +492,8 @@ char *get_directory_entries (struct tar_stat_info *st);
 
 void create_archive (void);
 void pad_archive (off_t size_left);
-void dump_file (struct tar_stat_info *parent, char const *name,
+void dump_member (char const *member);
+void dump_file_incr (struct tar_stat_info *parent, char const *name,
 		char const *fullname);
 union block *start_header (struct tar_stat_info *st);
 void finish_header (struct tar_stat_info *st, union block *header,
@@ -963,5 +964,33 @@ int owner_map_translate (uid_t uid, uid_t *new_uid, char const **new_name);
 void group_map_read (char const *file);
 int group_map_translate (gid_t gid, gid_t *new_gid, char const **new_name);
 
+/* Module tour.c */
+
+/* hide TOUR definition in tour.c */
+typedef struct tour *tour_t;
+
+/* One tour_node_t represents single directory level in FS hierarchy,
+   i.e. list of files.  At one moment we keep in memory only one path
+   representation. */
+typedef struct
+{
+  struct tar_stat_info *parent; /* used to fill child's stat info */
+  char *items;  /* output from readdir */
+
+  /* per-directory volatile data to avoid reallocation for each file in
+   * directory */
+  const char   *item; /* processed filename (ptr into ITEMS) */
+  struct tar_stat_info  st;   /* stat of processed file */
+  char *namebuf;  /* buffer for constructing full name */
+  size_tbuflen;   

Re: [Bug-tar] Apply ignore-failed-read to "file changed as we read it" case

[Pavel Raiskup]

Hi Stefanos,

On Tuesday 05 of January 2016 15:54:57 Stefanos Harhalakis wrote:
> Can you consider the attached patch?
> 
> This will apply the --ignore-failed-read flag to the case where a file
> was changed as we read it, thus preventing tar from exiting with a
> failure.

this does not look like the right approach.  The --ignore-failed-read is
related to different problems, from man page:

   --ignore-failed-read
  Do not exit with nonzero on unreadable files.

.. and the file is not unreadable.

Pavel

Re: [Bug-tar] Apply ignore-failed-read to "file changed as we read it" case

[Pavel Raiskup]

On Wednesday 06 of January 2016 01:02:24 Stefanos Harhalakis wrote:
> Aren't these two cases pretty similar?

Those are similar, yes.  In both cases the input file is _not unreadable_
so tar should behave similarly with regards to --ignore-failed-read.

Anyways, it would be fine to have rather a better option for it, something
similar to "--warning", but the effect would be
dont-switch-to-nonzero-status.

Other than that;  If the file have shrunk/grew, it _is_ serious issue from
the consistency POV!  And returning non-zero status is IMO really worth.
I can imagine that in really *rare* situation you can expect that the
archived file may be restored a safe way.  And to me, --ignore-failed-read
ignores way too much (orthogonal) situations.

Pavel

Re: [Bug-tar] tar-1.28 make check failure

[Pavel Raiskup]

Hi Ketil,

On Saturday 26 of December 2015 11:47:19 Ketil Wright wrote:
> I am going to (perhaps naively) assume, that as my entire partition is
> only 8GB, I should expect this failure, and proceed with LFS.

that particular test should not have big storage footprint, based on the
testsuite.log:

  | @@ -0,0 +1,2 @@
  | +tar: sparsefile: Cannot open: Value too large for defined data type
  | +tar: Exiting with failure status due to previous errors
  | stdout:

please have a look at:
https://lists.gnu.org/archive/html/bug-tar/2015-12/msg1.html

Pavel

[Bug-tar] [PATCH] sparse: fix use of indeterminate value

[Pavel Raiskup]

Detected by cppcheck 1.70 (Fedora 23) and gcc 4.1.2 (RHEL5), newer
gcc does not complain.

* src/xheader.c (sparse_map_decoder): Move 'e' up from loop-block.
---
 src/xheader.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/xheader.c b/src/xheader.c
index a5452a1..8e62383 100644
--- a/src/xheader.c
+++ b/src/xheader.c
@@ -1456,13 +1456,13 @@ sparse_map_decoder (struct tar_stat_info *st,
size_t size __attribute__((unused)))
 {
   int offset = 1;
+  struct sp_array e;
 
   st->sparse_map_avail = 0;
   while (1)
 {
   intmax_t u;
   char *delim;
-  struct sp_array e;
 
   if (!ISDIGIT (*arg))
{
-- 
2.5.0

[Bug-tar] [PATCH] exclude: avoid one cppcheck puzzle warning

[Pavel Raiskup]

Per report from Kamil Dudka.

* src/exclist.c (hg_initfn): Make the initialization of data
pointer here rather than expecting caller will do it.
(vcs_ignore_file): Fix the initfn prototype.
(info_attach_exclist): Let the initfn do the initialization.
---
 src/exclist.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/exclist.c b/src/exclist.c
index c24a00c..c788a7d 100644
--- a/src/exclist.c
+++ b/src/exclist.c
@@ -30,7 +30,7 @@ struct vcs_ignore_file
   char const *filename;
   int flags;
   add_fn addfn;
-  void *(*initfn) (void *);
+  void (*initfn) (void *);
   void *data;
 };
 
@@ -101,7 +101,7 @@ info_attach_exclist (struct tar_stat_info *dir)
  vcsfile = get_vcs_ignore_file (file->name);
 
  if (vcsfile->initfn)
-   vcsfile->data = vcsfile->initfn (vcsfile->data);
+   vcsfile->initfn (>data);
 
  if (add_exclude_fp (vcsfile->addfn, ex, fp,
  EXCLUDE_WILDCARDS|EXCLUDE_ANCHORED, '\n',
@@ -247,17 +247,17 @@ bzr_addfn (struct exclude *ex, char const *pattern, int 
options, void *data)
   add_exclude (ex, pattern, options);
 }
 
-static void *
+static void
 hg_initfn (void *data)
 {
-  int *hgopt;
   static int hg_options;
 
-  if (!data)
-hgopt = _options;
+  /* make data pointer point to static hg_options */
+  int **hgopt = data;
+  if (!*hgopt)
+*hgopt = _options;
 
-  *hgopt = EXCLUDE_REGEX;
-  return hgopt;
+  hg_options = EXCLUDE_REGEX;
 }
 
 static void
-- 
2.5.0

Re: [Bug-tar] [GNU tar 1.28] testsuite: 128 failed

[Pavel Raiskup]

On Wednesday 02 of December 2015 08:15:41 Pavel Raiskup wrote:
> On Wednesday 02 of December 2015 01:04:37 Budi Setiawan wrote:
> > plz check
> > need answer for my lfs project thx
> 
> Please don't attach uncompressed testsuite.log [1].
> 
> | uname -m = i686
> | uname -r = 3.19.0-33-generic
> | uname -s = Linux
> | uname -v = #38~14.04.1-Ubuntu SMP Fri Nov 6 18:17:49 UTC 2015
> |
> |
> | 128. sparse03.at:21: testing storing sparse files > 8G ...
> | ./sparse03.at:29:
> | mkdir posix
> | (cd posix
> | TEST_TAR_FORMAT=posix
> | export TEST_TAR_FORMAT
> | TAR_OPTIONS="-H posix"
> | export TAR_OPTIONS
> | rm -rf *
> |
> | genfile --length 1000 --file begin
> | genfile --length 1000 --file end
> | genfile --sparse --file sparsefile --block-size 512 8G A || exit 77
> | tar -c -f archive --sparse begin sparsefile end || exit 1
> | echo separator
> |
> | tar tfv archive
> | echo separator
> | mkdir directory
> | tar Cxf directory archive
> | genfile --stat=name,size sparsefile
> | cmp sparsefile directory/sparsefile
> | )
> | --- /dev/null   2015-12-01 14:34:19.258291399 +
> | +++ /mnt/lfs/sources/tar-1.28/tests/testsuite.dir/at-groups/128/stderr  
> 2015-12-01 17:51:17.521183405 +
> | @@ -0,0 +1,2 @@
> | +tar: sparsefile: Cannot open: Value too large for defined data type
> | +tar: Exiting with failure status due to previous errors
> 
> Looks like tar is not able to open 'sparsefile' (big sparse file).
> Could you check why '#define _FILE_OFFSET_BITS 64' is not used in your
> config.h (or why, for some reason, _FILE_OFFSET_BITS=64 is not used) [2]?
> Based on the testsuite.log, it looks _FILE_OFFSET_BITS=64 is defined.
> 
> [1] https://lists.gnu.org/archive/html/bug-tar/2014-01/msg00021.html
> [2] 
> http://www.gnu.org/software/coreutils/faq/coreutils-faq.html#Value-too-large-for-defined-data-type

JFYI: I observed this behavior recently on Fedora 23 i386:
https://bugzilla.redhat.com/show_bug.cgi?id=1288662

# rpm -q glibc
glibc-2.22-5.fc23.i686

Pavel

[Bug-tar] [PATCH] genfile: remove unused variable

[Pavel Raiskup]

* tests/genfile.c (generate_sparse_file): Remove unused 'i'.
---
 tests/genfile.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/genfile.c b/tests/genfile.c
index d41336b..4699d21 100644
--- a/tests/genfile.c
+++ b/tests/genfile.c
@@ -557,7 +557,6 @@ make_fragment (int fd, char *offstr, char *mapstr)
 static void
 generate_sparse_file (int argc, char **argv)
 {
-  int i;
   int fd;
   int flags = O_CREAT | O_RDWR | O_BINARY;
 
-- 
2.5.0

Re: [Bug-tar] [GNU tar 1.28] testsuite: 128 failed

[Pavel Raiskup]

On Wednesday 02 of December 2015 01:04:37 Budi Setiawan wrote:
> plz check
> need answer for my lfs project thx

Please don't attach uncompressed testsuite.log [1].

| uname -m = i686
| uname -r = 3.19.0-33-generic
| uname -s = Linux
| uname -v = #38~14.04.1-Ubuntu SMP Fri Nov 6 18:17:49 UTC 2015
|
|
| 128. sparse03.at:21: testing storing sparse files > 8G ...
| ./sparse03.at:29:
| mkdir posix
| (cd posix
| TEST_TAR_FORMAT=posix
| export TEST_TAR_FORMAT
| TAR_OPTIONS="-H posix"
| export TAR_OPTIONS
| rm -rf *
|
| genfile --length 1000 --file begin
| genfile --length 1000 --file end
| genfile --sparse --file sparsefile --block-size 512 8G A || exit 77
| tar -c -f archive --sparse begin sparsefile end || exit 1
| echo separator
|
| tar tfv archive
| echo separator
| mkdir directory
| tar Cxf directory archive
| genfile --stat=name,size sparsefile
| cmp sparsefile directory/sparsefile
| )
| --- /dev/null 2015-12-01 14:34:19.258291399 +
| +++ /mnt/lfs/sources/tar-1.28/tests/testsuite.dir/at-groups/128/stderr
2015-12-01 17:51:17.521183405 +
| @@ -0,0 +1,2 @@
| +tar: sparsefile: Cannot open: Value too large for defined data type
| +tar: Exiting with failure status due to previous errors

Looks like tar is not able to open 'sparsefile' (big sparse file).
Could you check why '#define _FILE_OFFSET_BITS 64' is not used in your
config.h (or why, for some reason, _FILE_OFFSET_BITS=64 is not used) [2]?
Based on the testsuite.log, it looks _FILE_OFFSET_BITS=64 is defined.

[1] https://lists.gnu.org/archive/html/bug-tar/2014-01/msg00021.html
[2] 
http://www.gnu.org/software/coreutils/faq/coreutils-faq.html#Value-too-large-for-defined-data-type

Pavel