Hi, I'd like consult some defects in tar --update feature. There are
several problems I have found in --update mode until now:
* cooperation with -C option. I have mentioned it some time before:
http://lists.gnu.org/archive/html/bug-tar/2012-02/msg00007.html
(my *previous* mail client deformed the second patch MIME, it is also
plain text patch so don't worry to open the proposed fix)
* problem with --wildcards option #1. Mentioned here:
https://bugzilla.redhat.com/show_bug.cgi?id=848230
and yet before here:
http://forums.fedoraforum.org/showthread.php?p=1597330#post1597330
Proposed test/fix is in attachment. I'm not sure that it is proper
solution so I'm writing here. The problem is that regular file name
stored in archive is interpreted later on like fnmatch pattern -- and
if the stored file name contains special characters like '*', '[' and
others, it causes problems. The proposed fix just disables fnmatch()
for added file. Second possibility would be to escape all special
characters in filenames but it requires some hacks later on.
* Problem with default behavior of --update when --wildcards is turned
'on' for some files. See the following:
1) prepare
$ mkdir zzz
$ touch zzz/aaa
$ mkdir zz
$ touch zz/oh
$ tar -cvf test.tar zzz
zzz/
zzz/aaa
2a)
$ touch zzz/aaa
$ tar -uvf test.tar --wildcards 'zz*'
zzz/aaa
** BUT **
2b)
$ tar -uvf test.tar --wildcards 'zz'
zz/
zz/oh
-> note that tar is adding directory 'zz' even if it does not exist
in archive. If it is correct way than the 'zz' directory should be
added into archive even if the 'zz*' wildcard is passed.
(sorry --> commands may contain typos -> I'm C&P them)
* another problem is maybe here:
| @@ -146,15 +146,16 @@ update_archive (void)
| && (name = name_scan (current_stat_info.file_name)) != NULL)
| {
| struct stat s;
| + const char *target_name = current_stat_info.file_name;
| _
| chdir_do (name->change_dir);
| - if (deref_stat (current_stat_info.file_name, &s) == 0)
| + if (deref_stat (target_name, &s) == 0)
| {
| if (S_ISDIR (s.st_mode))
| {
| char *p, *dirp;
| DIR *stream = NULL;
| - int fd = openat (chdir_fd, name->name,
| + int fd = openat (chdir_fd, target_name,
| open_read_flags | O_DIRECTORY);
| if (fd < 0)
| open_error (name->name);
| @@ -163,7 +164,7 @@ update_archive (void)
| savedir_error (name->name);
| else
| {
| - namebuf_t nbuf = namebuf_create (name->name);
| + namebuf_t nbuf = namebuf_create (target_name);
|
| for (p = dirp; *p; p += strlen (p) + 1)
| /* we are adding new file name based on actual
... the deref_stat () may be done on different string than following
openat() call.
* my last problem is when:
$ mkdir zzz
$ touch zzz/aaa
$ touch zzzfile
$ tar -cvf test.tar zzz zzzfile
zzz/
zzz/aaa
zzzfile
$ touch zzzfile
$ tar -uvf test.tar --wildcards zzz
$ # prints nothing ^^^ ! but the 'zzzfile' was touched.
The problem here is the 'rmname()' call even for wildcard patterns.
It would like to ask you whether it would be possible to add another list
with "whitelisted" files that are going to be updated aside the
'namelist'. The 'namelist' is noticeably unusable for --update purposes.
Are there any other possibilities?
Would it be possible to add some notes about mentioned circumstances into
info documentation?
Thanks for your help,
Pavel
>From 3bfe5f52f469ab8ea1ae5fb6a2974955d33c8705 Mon Sep 17 00:00:00 2001
From: Pavel Raiskup <prais...@redhat.com>
Date: Wed, 15 Aug 2012 16:55:27 +0200
Subject: [PATCH 1/3] Test for bad cooperation of --wildcard and --update
options.
---
tests/Makefile.am | 1 +
tests/testsuite.at | 1 +
tests/update03.at | 43 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 45 insertions(+)
create mode 100644 tests/update03.at
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 4601e0e..b20b86e 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -160,6 +160,7 @@ TESTSUITE_AT = \
update.at\
update01.at\
update02.at\
+ update03.at\
volsize.at\
volume.at\
verbose.at\
diff --git a/tests/testsuite.at b/tests/testsuite.at
index 97f5e41..c7585e9 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -245,6 +245,7 @@ m4_include([spmvp10.at])
m4_include([update.at])
m4_include([update01.at])
m4_include([update02.at])
+m4_include([update03.at])
m4_include([verify.at])
diff --git a/tests/update03.at b/tests/update03.at
new file mode 100644
index 0000000..511dabb
--- /dev/null
+++ b/tests/update03.at
@@ -0,0 +1,43 @@
+# Process this file with autom4te to create testsuite. -*- Autotest -*-
+
+# Test suite for GNU tar.
+# Copyright (C) 2012 Free Software Foundation, Inc.
+
+# This program 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, or (at your option)
+# any later version.
+
+# This program 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, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301, USA.
+
+# Description: Test for bad wildcard-matching while updating an archive.
+
+AT_SETUP([update non-changed files with --wildcards])
+AT_KEYWORDS([update update03])
+
+AT_TAR_CHECK([
+AT_SORT_PREREQ
+mkdir zzz
+touch zzz/foo@<:@bar@:>@
+
+tar -cvf archive.tar --wildcards zzz
+echo "separator"
+
+# this should print nothing
+tar -uvf archive.tar --wildcards zzz
+],
+[0],
+[zzz/
+zzz/foo@<:@bar@:>@
+separator]
+)
+
+AT_CLEANUP
--
1.7.11.2
>From e222c514347afbc1230015df59a1fe1c521645a4 Mon Sep 17 00:00:00 2001
From: Pavel Raiskup <prais...@redhat.com>
Date: Wed, 15 Aug 2012 16:56:03 +0200
Subject: [PATCH 2/3] Fix for bad cooperation of --wildcard and --update.
---
src/common.h | 3 ++-
src/names.c | 15 ++++++++-------
src/tar.c | 2 +-
src/update.c | 5 ++++-
4 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/src/common.h b/src/common.h
index c51ab11..7b2e785 100644
--- a/src/common.h
+++ b/src/common.h
@@ -668,7 +668,8 @@ void name_term (void);
const char *name_next (int change_dirs);
void name_gather (void);
struct name *addname (char const *string, int change_dir,
- bool cmdline, struct name *parent);
+ bool cmdline, struct name *parent, int
+ mask_matching_flags);
void remname (struct name *name);
bool name_match (const char *name);
void names_notfound (void);
diff --git a/src/names.c b/src/names.c
index eee6bde..bdb2de5 100644
--- a/src/names.c
+++ b/src/names.c
@@ -414,7 +414,7 @@ name_gather (void)
namelist = nametail = buffer;
}
else if (change_dir)
- addname (0, change_dir, false, NULL);
+ addname (0, change_dir, false, NULL, 0);
}
else
{
@@ -428,11 +428,11 @@ name_gather (void)
change_dir = chdir_arg (xstrdup (ep->v.name));
if (ep)
- addname (ep->v.name, change_dir, true, NULL);
+ addname (ep->v.name, change_dir, true, NULL, 0);
else
{
if (change_dir != change_dir0)
- addname (NULL, change_dir, false, NULL);
+ addname (NULL, change_dir, false, NULL, 0);
break;
}
}
@@ -441,14 +441,15 @@ name_gather (void)
/* Add a name to the namelist. */
struct name *
-addname (char const *string, int change_dir, bool cmdline, struct name *parent)
+addname (char const *string, int change_dir, bool cmdline, struct name *parent,
+ int mask_matching_flags)
{
struct name *name = make_name (string);
name->prev = nametail;
name->next = NULL;
name->found_count = 0;
- name->matching_flags = matching_flags;
+ name->matching_flags = matching_flags & ~mask_matching_flags;
name->change_dir = change_dir;
name->directory = NULL;
name->parent = parent;
@@ -829,7 +830,7 @@ add_hierarchy_to_namelist (struct tar_stat_info *st, struct name *name)
namebuf = xrealloc (namebuf, allocated_length + 1);
}
strcpy (namebuf + name_length, string + 1);
- np = addname (namebuf, change_dir, false, name);
+ np = addname (namebuf, change_dir, false, name, 0);
if (!child_head)
child_head = np;
else
@@ -934,7 +935,7 @@ collect_and_sort_names (void)
name_gather ();
if (!namelist)
- addname (".", 0, false, NULL);
+ addname (".", 0, false, NULL, 0);
if (listed_incremental_option)
{
diff --git a/src/tar.c b/src/tar.c
index 8c4f656..ddcd700 100644
--- a/src/tar.c
+++ b/src/tar.c
@@ -1553,7 +1553,7 @@ parse_opt (int key, char *arg, struct argp_state *state)
case 'K':
starting_file_option = true;
- addname (arg, 0, true, NULL);
+ addname (arg, 0, true, NULL, 0);
break;
case ONE_FILE_SYSTEM_OPTION:
diff --git a/src/update.c b/src/update.c
index 17f9e05..89d2a38 100644
--- a/src/update.c
+++ b/src/update.c
@@ -158,8 +158,11 @@ update_archive (void)
namebuf_t nbuf = namebuf_create (name->name);
for (p = dirp; *p; p += strlen (p) + 1)
+ /* we are adding new file name based on actual
+ directory contents - we don't want to use
+ wildcards later on. */
addname (namebuf_name (nbuf, p),
- 0, false, NULL);
+ 0, false, NULL, EXCLUDE_WILDCARDS);
namebuf_free (nbuf);
free (dirp);
--
1.7.11.2
