Hello,
my Team and I found a behaviour of tar which I assume is a bug or a
possibility for an improvement. I tested with tar versions: 1.27
and 1.29.
Current Behaviour (1.27/1.29):
------------------------------
In case a directory in a deep directory is removed during
creation of an incremental tar file, tar exits with a
TAREXIT_FAILURE (==2).
This is the same issue as tested by test-case 'filerem01.at' of
the tar testsuite. However, the test-case tests the deletion of
a file and not of a directory.
Expected Behaviour:
-------------------
Tar should exit with TAREXIT_DIFFERS (==1).
Reason: I have use-cases where not only files, but also complete
folders vanish during creation of the archive.
To be able to distinguish these cases from other serious
issues like actual I/O errors I would like to have the
same behaviour as for vanishing files
(i.e., exit-code == 1).
Test-cases:
===========
During my experiments to reproduce the issues I experienced, I
created two test-cases dirrem01.at and dirrem02.at based on the
test-cases filerem01/02.at:
dirrem01.at: Exit with TAREXIT_DIFFERS for the scenario
described above
dirrem02.at: Still exit with TAREXIT_FAILURE if an explicitly
named directory vanishes
Attached file:
0002-Test-handling-of-directories-removed-during-incremen.patch
A Trivial Patch Idea:
=====================
Based on these two test-cases I created a patch modifying
create_archive() to produce the behaviour I expected:
Attached file:
0001-Fix-handling-of-directories-removed-during-increment.patch
(The patch is based on release_1_29 /
20b55f0679d314568ec21ae6db1ea635494e292b)
This was the first time I took a look into the gnu tar
source-code, therefore I try to provide a rather detailed
explanation on why I assume this patch could be a valid
modification.
If I missed anything I am more than happy to hear your comments.
Behaviour when create_archive(...) fails to open or stat a
directory:
- Remark: '->' depicts a function call
- Old Behaviour: open_diag()/stat_diag() is called.
-------------------------------------------------
open_diag()/stat_diag():
if(!ignore_failed_read_option)
-----------------------------
-> open_error()/stat_error() -> call_arg_error()
-> ERROR()<i> -> error()
=> <i>: exit_status = PAXEXIT_FAILURE (==2);
=> processing is continued
(<i>: call to func ptr error_hook / func
checkpoint_flush_actions not further discussed here)
else
----
-> open_warn()/stat_warn() -> call_arg_warn() -> WARN()
-> error()
=> no modification of exit_status
=> processing is continued
- New Behaviour: file_removed_diag() is called.
---------------------------------------------
file_removed_diag():
if(!top_level && errno == ENOENT))
----------------------------------
-> set_exit_status(TAREXIT_DIFFERS) <ii>
=> <ii>: exit_status = TAREXIT_DIFFERS (if not already >=
TAREXIT_DIFFERS)
=> processing is continued
else
----
-> open_diag()/stat_diag() [=> old behaviour]
=> Both, the patched and the non-patched code do not immediately
terminate the execution, the difference is the exit-code.
As far as I can see, this is the only difference (besides the
modified text-output).
Table I: Exit-codes of patch / non-patched code
|Case depicted | Case depicted in dirrem01.at + |
|in dirrem01.at | --ignore-failed-read |
-----------------------------------------------------------------
Non-Patched : | 2 | 0 |
-----------------------------------------------------------------
Patched : | 1 | 1 |
Open issues I am aware of:
--------------------------
- Wrong text output (File vs. Directory)
Output of dirrem01.at: "dir/sub: File removed before we read
it"
I assume a function dir_removed_diag should be added and
used, similar to file_removed_diag.
- Test-cases are only executed for gnu
Open questions from my side:
----------------------------
- Are the modified locations sufficient? (i.e., are there other
locations in the code which I may have missed; in particular code
which will be triggered on subsequent incremental tar runs)
- Assuming I did not miss anything and the only relevant
difference between the patched / non-patched code is the
modified exit-code:
The patch leads to a scenario (dirrem01.at) where tar was
returning TAREXIT_FAILURE and now returns TAREXIT_DIFFERS
instead.
The content of the resulting archive should be the same
(see assumption).
Is it save to assume the resulting archive is a valid archive
(and can be unpacked)?
An argument supporting the archive should be valid:
The non-patched tar with option '--ignore-failed-read' leads
to the same archive and exit-code 0.
Therefore reporting exit-code 1 for the same archive seems
not to be completely unreasonable.
- Should the modified behaviour only be enabled by a command
line option?
(Reason: Preserve old behaviour if this report is considered
to be an improvement and not to be a bug)
Regards and thanks for reading,
Jonas
---
Jonas Julino
1&1 Internet SE
Hauptsitz Montabaur, Amtsgericht Montabaur, HRB 24498
Vorstand: Christian Genis Bigatà Joseph, Robert Hoffmann, Hans-Henning
Kettler, Uwe Lamnek
Aufsichtsratsvorsitzender: Michael Scheeren
>From 864821bc4c0be0f9d5320bc00530d53aa2d1d151 Mon Sep 17 00:00:00 2001
From: Jonas Julino <[email protected]>
Date: Mon, 10 Apr 2017 16:01:53 +0200
Subject: [PATCH 2/2] Test handling of directories removed during incremental
tar run
---
tests/Makefile.am | 2 ++
tests/dirrem01.at | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
tests/dirrem02.at | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++
tests/testsuite.at | 4 +++
4 files changed, 156 insertions(+)
create mode 100644 tests/dirrem01.at
create mode 100644 tests/dirrem02.at
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 3e94276..1ae145c 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -108,6 +108,8 @@ TESTSUITE_AT = \
extrac19.at\
filerem01.at\
filerem02.at\
+ dirrem01.at\
+ dirrem02.at\
gzip.at\
grow.at\
incremental.at\
diff --git a/tests/dirrem01.at b/tests/dirrem01.at
new file mode 100644
index 0000000..d67d8e1
--- /dev/null
+++ b/tests/dirrem01.at
@@ -0,0 +1,76 @@
+# Process this file with autom4te to create testsuite. -*- Autotest -*-
+
+# Test suite for GNU tar.
+# Copyright 2009, 2013 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/>.
+
+# Description:
+#
+# When a directory in a deep directory disappeared during creation
+# of incremental dump, tar <= 1.29 exits with TAREXIT_FAILURE (2).
+#
+# In case of files tar does not return TAREXIT_FAILURE, but instead
+# it prints a warning and exits with TAREXIT_DIFFERS.
+#
+# This test checks whether this behaviour is mimicked for directories, too.
+
+# Remark: This file is based on 'filerem01.at', which is the test-case for
+# a vanishing file.
+
+AT_SETUP([dir removed as we read it (ca. 8 seconds)])
+AT_KEYWORDS([create incremental listed dirchange dirrem dirrem01])
+
+AT_TAR_CHECK([
+mkdir dir
+mkdir dir/sub
+genfile --file dir/file1
+genfile --file dir/sub/file2
+
+genfile --run --checkpoint=3 --exec='rmdir dir/sub' --checkpoint=3 --unlink dir/sub/file2 -- \
+ tar --blocking-factor=1 --checkpoint=1 --checkpoint-action='sleep=1' \
+ --checkpoint-action='echo' -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 removed before we read it
+],[],[],[gnu])
+
+# Timing information:
+#
+# For -Hgnu the above command line takes about 8 seconds to execute and
+# produces:
+#
+# tar: dir: Directory is new
+# tar: dir/sub: Directory is new
+# dir/
+# tar: Write checkpoint 1
+# tar: Write checkpoint 2
+# dir/sub/
+# tar: Write checkpoint 3
+# tar: Write checkpoint 4
+# dir/file1
+# tar: Write checkpoint 5
+# dir/sub/file2
+# tar: Write checkpoint 6
+# tar: Write checkpoint 7
+# tar: Write checkpoint 8
+
+AT_CLEANUP
+
diff --git a/tests/dirrem02.at b/tests/dirrem02.at
new file mode 100644
index 0000000..b011725
--- /dev/null
+++ b/tests/dirrem02.at
@@ -0,0 +1,74 @@
+# Process this file with autom4te to create testsuite. -*- Autotest -*-
+
+# Test suite for GNU tar.
+# Copyright 2009, 2013 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/>.
+
+# Description:
+#
+# When an explicitley named directory disappears during creation
+# of incremental dump, tar should still exit with TAREXIT_FAILURE (2).
+#
+# For further details see dirrem01.at
+
+# Remark: This file is based on 'filerem01/02.at', which are test-cases for
+# vanishing files.
+
+AT_SETUP([explicitley named dir removed as we read it (ca. 8 seconds)])
+AT_KEYWORDS([create incremental listed dirchange dirrem dirrem02])
+
+AT_TAR_CHECK([
+mkdir dir
+mkdir dir/sub
+genfile --file dir/file1
+genfile --file dir/sub/file2
+
+genfile --run --checkpoint=3 --exec='rmdir dir/sub' --checkpoint=3 --unlink dir/sub/file2 -- \
+ tar --blocking-factor=1 --checkpoint=1 --checkpoint-action='sleep=1' \
+ --checkpoint-action='echo' -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: Cannot open: No such file or directory
+tar: Exiting with failure status due to previous errors
+],[],[],[gnu])
+
+# Timing information:
+#
+# For -Hgnu the above command line takes about 8 seconds to execute and
+# produces:
+#
+# tar: dir: Directory is new
+# tar: dir/sub: Directory is new
+# dir/
+# tar: Write checkpoint 1
+# tar: Write checkpoint 2
+# dir/sub/
+# tar: Write checkpoint 3
+# tar: Write checkpoint 4
+# dir/file1
+# tar: Write checkpoint 5
+# dir/sub/file2
+# tar: Write checkpoint 6
+# tar: Write checkpoint 7
+# tar: Write checkpoint 8
+
+AT_CLEANUP
+
diff --git a/tests/testsuite.at b/tests/testsuite.at
index cf4b2fd..604b737 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -331,6 +331,10 @@ AT_BANNER([Files removed while archiving])
m4_include([filerem01.at])
m4_include([filerem02.at])
+AT_BANNER([Directories removed while archiving])
+m4_include([dirrem01.at])
+m4_include([dirrem02.at])
+
AT_BANNER([Renames])
m4_include([rename01.at])
m4_include([rename02.at])
--
2.7.4
>From 09eb4b3272e9e50fd3053aac9ec36016b1f39f90 Mon Sep 17 00:00:00 2001
From: Jonas Julino <[email protected]>
Date: Mon, 10 Apr 2017 16:37:39 +0200
Subject: [PATCH 1/2] Fix handling of directories removed during incremental
tar run
---
src/create.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/create.c b/src/create.c
index 3a0f2dc..66c939f 100644
--- a/src/create.c
+++ b/src/create.c
@@ -1391,13 +1391,13 @@ create_archive (void)
open_searchdir_flags);
if (fd < 0)
{
- open_diag (p->name);
+ file_removed_diag(p->name, !p->parent, open_diag);
break;
}
st.fd = fd;
if (fstat (fd, &st.stat) != 0)
{
- stat_diag (p->name);
+ file_removed_diag(p->name, !p->parent, stat_diag);
break;
}
st.orig_file_name = xstrdup (p->name);
--
2.7.4
