|
View:
New views
6 Messages
—
Rating Filter:
Alert me
|
|
|
--remove-files deletes files even if archive couldn't be created, when used with compressionusing tar (GNU tar) 1.20, 1.22, or recent git checkout, on ubuntu 9.04/x86_64:
$ ../src/tar --version tar (GNU tar) 1.22.90 Copyright (C) 2009 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>. This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Written by John Gilmore and Jay Fenlason. $ mkdir a $ mkdir b $ touch b/c $ ../src/tar cfv a --remove-files b ../src/tar: a: Cannot open: Is a directory ../src/tar: Error is not recoverable: exiting now $ ls b c $ ../src/tar cfvz a --remove-files b ../src/tar: a: Cannot open: Is a directory ../src/tar: Error is not recoverable: exiting now b/ b/c ../src/tar: Child returned status 2 ../src/tar: Exiting with failure status due to previous errors $ ls b ls: cannot access b: No such file or directory $ ls a/ -- 3b |
|
|
Re: --remove-files deletes files even if archive couldn't be created, when used with compressionBart Botta <00003b@...> ha escrit:
> $ ../src/tar cfv a --remove-files b > ../src/tar: a: Cannot open: Is a directory Argument to the -f option cannot be a directory. See http://www.gnu.org/software/tar/manual/html_node/Creating-the-archive.html for the correct ways to create an archive. Regards, Sergey |
|
|
Re: --remove-files deletes files even if archive couldn't be created, when used with compressionOn Sun, Oct 4, 2009 at 4:16 AM, Sergey Poznyakoff <gray@...> wrote:
> Bart Botta <00003b@...> ha escrit: > >> $ ../src/tar cfv a --remove-files b >> ../src/tar: a: Cannot open: Is a directory > > Argument to the -f option cannot be a directory. See > Right, I wasn't expecting it to actually create an archive, that was just a convenient way to make it fail. If you prefer, here is another version, which fails due to permissions: $ mkdir a $ chmod -xw a $ mkdir b $ touch b/c $ tar cfv a/d.tar --remove-files b tar: a/d.tar: Cannot open: Permission denied tar: Error is not recoverable: exiting now $ ls b c $ tar cfvz a/d.tar --remove-files b tar: a/d.tar: Cannot open: Permission denied tar: Error is not recoverable: exiting now b/ b/c tar: Child returned status 2 tar: Error exit delayed from previous errors $ ls b ls: cannot access b: No such file or directory $ ls a/ the point is that tar should actually 'exit now' when it says 'exiting now' instead of proceeding to delete the files without putting them in a usable archive. -- 3b |
|
|
Re: --remove-files deletes files even if archive couldn't be created, when used with compressionSergey Poznyakoff wrote:
> Bart Botta <00003b@...> ha escrit: > >> $ ../src/tar cfv a --remove-files b >> ../src/tar: a: Cannot open: Is a directory > > Argument to the -f option cannot be a directory. See... ---- That doesn't address the problem. I feel that tar should be more robust in handling 'source' files that are going into an archive, and should NOT delete the source if can't open (and move them) into the target archive. Similarly, I wouldn't expect 'mv' to delete a file from a source dir unless it could successfully move it to a target directory. No? |
|
|
Re: --remove-files deletes files even if archive couldn't be created, when used with compressionHi Bart,
> If you prefer, here is another version, which fails due to permissions: Ah, indeed. I've misinterpreted your report. Thanks for clarifying! > the point is that tar should actually 'exit now' when it says 'exiting > now' instead of proceeding to delete the files without putting them in > a usable archive. Yes, of course it should! I'll fix it. Regards, Sergey |
|
|
Re: --remove-files deletes files even if archive couldn't be created, when used with compressionHi Bart,
> the point is that tar should actually 'exit now' when it says 'exiting > now' instead of proceeding to delete the files without putting them in > a usable archive. It is fixed in the repository. The patch is attached. Regards, Sergey From 4dfcd6c054a5e9e1a371c822a3be90564dd9b690 Mon Sep 17 00:00:00 2001 From: Sergey Poznyakoff <gray@...> Date: Wed, 7 Oct 2009 16:42:06 +0300 Subject: [PATCH] Fix bugs in handling the --remove-files option. Make sure the files are deleted only if they were succesfully stored to the archive. * src/exit.c: New file. * src/unlink.c: New file. * src/Makefile.am (tar_SOURCES): Add exit.c and unlink.c. * src/common.h: Include progname.h (program_name): Remove global. (records_written): New extern. (queue_deferred_unlink, finish_deferred_unlinks): New prototypes. (fatal_exit_hook): New extern. * src/create.c (create_archive): Call finish_deferred_unlinks. (dump_hard_link, dump_file0): Don't actually unlink the file, queue it to deferred_unlinks instead. * src/delete.c (records_written): Remove extern: declared in common.h. * src/extract.c (extract_archive): Set fatal_exit_hook. (fatal_exit, xalloc_die): Move to exit.c * src/system.c (sys_wait_for_child): Exit immediately if the child dies or exits with a non-zero status. (sys_child_open_for_compress) (sys_child_open_for_uncompress): Use set_program_name, instead of setting program_name directly. * src/tar.c (main): Use set_program_name, instead of setting program_name directly. * tests/Makefile.am (TESTSUITE_AT): Add remfiles01.at and remfiles02.at. * tests/testsuite.at: Likewise. * tests/gzip.at: Reflect the above changes. --- src/Makefile.am | 2 + src/common.h | 13 +++- src/create.c | 30 ++-------- src/delete.c | 1 - src/exit.c | 37 ++++++++++++ src/extract.c | 17 +----- src/system.c | 14 ++-- src/tar.c | 2 +- src/unlink.c | 158 +++++++++++++++++++++++++++++++++++++++++++++++++++ tests/Makefile.am | 2 + tests/gzip.at | 7 +- tests/remfiles01.at | 57 ++++++++++++++++++ tests/remfiles02.at | 58 +++++++++++++++++++ tests/testsuite.at | 3 + 14 files changed, 347 insertions(+), 54 deletions(-) create mode 100644 src/exit.c create mode 100644 src/unlink.c create mode 100644 tests/remfiles01.at create mode 100644 tests/remfiles02.at diff --git a/src/Makefile.am b/src/Makefile.am index 9f268c3..fcbac33 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -27,6 +27,7 @@ tar_SOURCES = \ compare.c\ create.c\ delete.c\ + exit.c\ extract.c\ xheader.c\ incremen.c\ @@ -38,6 +39,7 @@ tar_SOURCES = \ system.c\ tar.c\ transform.c\ + unlink.c\ update.c\ utf8.c\ warning.c diff --git a/src/common.h b/src/common.h index 1a6ca8b..c2a92d2 100644 --- a/src/common.h +++ b/src/common.h @@ -60,6 +60,7 @@ #define obstack_chunk_alloc xmalloc #define obstack_chunk_free free #include <obstack.h> +#include <progname.h> #include <paxlib.h> @@ -70,9 +71,6 @@ /* Information gleaned from the command line. */ -/* Name of this program. */ -GLOBAL const char *program_name; - /* Main command option. */ enum subcommand @@ -400,6 +398,7 @@ extern char *volume_label; extern char *continued_file_name; extern uintmax_t continued_file_size; extern uintmax_t continued_file_offset; +extern off_t records_written; size_t available_space_after (union block *pointer); off_t current_block_ordinal (void); @@ -827,3 +826,11 @@ extern int warning_option; } \ while (0) +/* Module unlink.c */ + +void queue_deferred_unlink (const char *name, bool is_dir); +void finish_deferred_unlinks (void); + +/* Module exit.c */ +extern void (*fatal_exit_hook) (void); + diff --git a/src/create.c b/src/create.c index 79c80ce..d4b9ae7 100644 --- a/src/create.c +++ b/src/create.c @@ -1333,7 +1333,7 @@ create_archive (void) write_eot (); close_archive (); - + finish_deferred_unlinks (); if (listed_incremental_option) write_directory_file (); } @@ -1413,8 +1413,8 @@ dump_hard_link (struct tar_stat_info *st) blk->header.typeflag = LNKTYPE; finish_header (st, blk, block_ordinal); - if (remove_files_option && unlink (st->orig_file_name) != 0) - unlink_error (st->orig_file_name); + if (remove_files_option) + queue_deferred_unlink (st->orig_file_name, false); return true; } @@ -1680,18 +1680,7 @@ dump_file0 (struct tar_stat_info *st, const char *p, } if (ok && remove_files_option) - { - if (is_dir) - { - if (rmdir (p) != 0 && errno != ENOTEMPTY) - rmdir_error (p); - } - else - { - if (unlink (p) != 0) - unlink_error (p); - } - } + queue_deferred_unlink (p, is_dir); return; } @@ -1727,10 +1716,8 @@ dump_file0 (struct tar_stat_info *st, const char *p, /* nothing more to do to it */ if (remove_files_option) - { - if (unlink (p) == -1) - unlink_error (p); - } + queue_deferred_unlink (p, false); + file_count_links (st); return; } @@ -1782,10 +1769,7 @@ dump_file0 (struct tar_stat_info *st, const char *p, finish_header (st, header, block_ordinal); if (remove_files_option) - { - if (unlink (p) == -1) - unlink_error (p); - } + queue_deferred_unlink (p, false); } void diff --git a/src/delete.c b/src/delete.c index d59a857..a67993c 100644 --- a/src/delete.c +++ b/src/delete.c @@ -35,7 +35,6 @@ extern union block *current_block; extern union block *recent_long_name; extern union block *recent_long_link; extern off_t records_read; -extern off_t records_written; /* The number of records skipped at the start of the archive, when passing over members that are not deleted. */ diff --git a/src/exit.c b/src/exit.c new file mode 100644 index 0000000..ad4d27c --- /dev/null +++ b/src/exit.c @@ -0,0 +1,37 @@ +/* This file is part of GNU tar. + Copyright (C) 2009 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. */ + +#include <system.h> +#include "common.h" + +void (*fatal_exit_hook) (void); + +void +fatal_exit (void) +{ + if (fatal_exit_hook) + fatal_exit_hook (); + error (TAREXIT_FAILURE, 0, _("Error is not recoverable: exiting now")); + abort (); +} + +void +xalloc_die (void) +{ + error (0, 0, "%s", _("memory exhausted")); + fatal_exit (); +} diff --git a/src/extract.c b/src/extract.c index 3c92e53..5f12cf9 100644 --- a/src/extract.c +++ b/src/extract.c @@ -1242,6 +1242,8 @@ extract_archive (void) char typeflag; tar_extractor_t fun; + fatal_exit_hook = extract_finish; + /* Try to disable the ability to unlink a directory. */ priv_set_remove_linkdir (); @@ -1406,18 +1408,3 @@ rename_directory (char *src, char *dst) } return true; } - -void -fatal_exit (void) -{ - extract_finish (); - error (TAREXIT_FAILURE, 0, _("Error is not recoverable: exiting now")); - abort (); -} - -void -xalloc_die (void) -{ - error (0, 0, "%s", _("memory exhausted")); - fatal_exit (); -} diff --git a/src/system.c b/src/system.c index 7df8122..cf39dbd 100644 --- a/src/system.c +++ b/src/system.c @@ -174,11 +174,11 @@ sys_wait_for_child (pid_t child_pid, bool eof) { int sig = WTERMSIG (wait_status); if (!(!eof && sig == SIGPIPE)) - ERROR ((0, 0, _("Child died with signal %d"), sig)); + FATAL_ERROR ((0, 0, _("Child died with signal %d"), sig)); } else if (WEXITSTATUS (wait_status) != 0) - ERROR ((0, 0, _("Child returned status %d"), - WEXITSTATUS (wait_status))); + FATAL_ERROR ((0, 0, _("Child returned status %d"), + WEXITSTATUS (wait_status))); } } @@ -330,7 +330,7 @@ sys_child_open_for_compress (void) /* The new born child tar is here! */ - program_name = _("tar (child)"); + set_program_name (_("tar (child)")); xdup2 (parent_pipe[PREAD], STDIN_FILENO); xclose (parent_pipe[PWRITE]); @@ -374,7 +374,7 @@ sys_child_open_for_compress (void) { /* The newborn grandchild tar is here! Launch the compressor. */ - program_name = _("tar (grandchild)"); + set_program_name (_("tar (grandchild)")); xdup2 (child_pipe[PWRITE], STDOUT_FILENO); xclose (child_pipe[PREAD]); @@ -473,7 +473,7 @@ sys_child_open_for_uncompress (void) /* The newborn child tar is here! */ - program_name = _("tar (child)"); + set_program_name (_("tar (child)")); xdup2 (parent_pipe[PWRITE], STDOUT_FILENO); xclose (parent_pipe[PREAD]); @@ -508,7 +508,7 @@ sys_child_open_for_uncompress (void) { /* The newborn grandchild tar is here! Launch the uncompressor. */ - program_name = _("tar (grandchild)"); + set_program_name (_("tar (grandchild)")); xdup2 (child_pipe[PREAD], STDIN_FILENO); xclose (child_pipe[PWRITE]); diff --git a/src/tar.c b/src/tar.c index e3300fb..0c59352 100644 --- a/src/tar.c +++ b/src/tar.c @@ -2452,7 +2452,7 @@ int main (int argc, char **argv) { set_start_time (); - program_name = argv[0]; + set_program_name (argv[0]); setlocale (LC_ALL, ""); bindtextdomain (PACKAGE, LOCALEDIR); diff --git a/src/unlink.c b/src/unlink.c new file mode 100644 index 0000000..2af6f99 --- /dev/null +++ b/src/unlink.c @@ -0,0 +1,158 @@ +/* This file is part of GNU tar. + Copyright (C) 2009 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. */ + +#include <system.h> +#include "common.h" +#include <quotearg.h> + +struct deferred_unlink + { + struct deferred_unlink *next; /* Next unlink in the queue */ + char *file_name; /* Absolute name of the file to unlink */ + bool is_dir; /* True if file_name is a directory */ + off_t records_written; /* Number of records written when this + entry got added to the queue */ + }; + +/* The unlink queue */ +static struct deferred_unlink *dunlink_head, *dunlink_tail; + +/* Number of entries in the queue */ +static size_t dunlink_count; + +/* List of entries available for allocation */ +static struct deferred_unlink *dunlink_avail; + +/* Delay (number of records written) between adding entry to the + list and its actual removal. */ +size_t deferred_unlink_delay = 0; + +static struct deferred_unlink * +dunlink_alloc () +{ + struct deferred_unlink *p; + if (dunlink_avail) + { + p = dunlink_avail; + dunlink_avail = p->next; + p->next = NULL; + } + else + p = xmalloc (sizeof (*p)); + return p; +} + +static void +dunlink_reclaim (struct deferred_unlink *p) +{ + free (p->file_name); + p->next = dunlink_avail; + dunlink_avail = p; +} + +static void +flush_deferred_unlinks (bool force) +{ + struct deferred_unlink *p, *prev = NULL; + + for (p = dunlink_head; p; ) + { + struct deferred_unlink *next = p->next; + if (force + || records_written > p->records_written + deferred_unlink_delay) + { + if (p->is_dir) + { + if (rmdir (p->file_name) != 0) + { + switch (errno) + { + case ENOENT: + /* nothing to worry about */ + break; + case ENOTEMPTY: + if (!force) + { + /* Keep the record in list, in the hope we'll + be able to remove it later */ + prev = p; + p = next; + continue; + } + /* fall through */ + default: + rmdir_error (p->file_name); + } + } + } + else + { + if (unlink (p->file_name) != 0 && errno != ENOENT) + unlink_error (p->file_name); + } + dunlink_reclaim (p); + dunlink_count--; + p = next; + if (prev) + prev->next = p; + else + dunlink_head = p; + } + else + { + prev = p; + p = next; + } + } + if (!dunlink_head) + dunlink_tail = NULL; +} + +void +finish_deferred_unlinks () +{ + flush_deferred_unlinks (true); + while (dunlink_avail) + { + struct deferred_unlink *next = dunlink_avail->next; + free (dunlink_avail); + dunlink_avail = next; + } +} + +void +queue_deferred_unlink (const char *name, bool is_dir) +{ + struct deferred_unlink *p; + + if (dunlink_head + && records_written > dunlink_head->records_written + deferred_unlink_delay) + flush_deferred_unlinks (false); + + p = dunlink_alloc (); + p->next = NULL; + p->file_name = normalize_filename (name); + p->is_dir = is_dir; + p->records_written = records_written; + + if (dunlink_tail) + dunlink_tail->next = p; + else + dunlink_head = p; + dunlink_tail = p; + dunlink_count++; +} diff --git a/tests/Makefile.am b/tests/Makefile.am index 51ad526..787b9d0 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -113,6 +113,8 @@ TESTSUITE_AT = \ rename03.at\ rename04.at\ rename05.at\ + remfiles01.at\ + remfiles02.at\ same-order01.at\ same-order02.at\ shortfile.at\ diff --git a/tests/gzip.at b/tests/gzip.at index eb43030..38d3538 100644 --- a/tests/gzip.at +++ b/tests/gzip.at @@ -1,7 +1,7 @@ # Process this file with autom4te to create testsuite. -*- Autotest -*- # Test suite for GNU tar. -# Copyright (C) 2004, 2007 Free Software Foundation, Inc. +# Copyright (C) 2004, 2007, 2009 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 @@ -28,14 +28,13 @@ unset TAR_OPTIONS AT_CHECK([ AT_GZIP_PREREQ tar xfvz /dev/null -test $? = 2 || exit 1 ], -[0], +[2], [], [ gzip: stdin: unexpected end of file tar: Child returned status 1 -tar: Exiting with failure status due to previous errors +tar: Error is not recoverable: exiting now ], [],[]) diff --git a/tests/remfiles01.at b/tests/remfiles01.at new file mode 100644 index 0000000..8a668a7 --- /dev/null +++ b/tests/remfiles01.at @@ -0,0 +1,57 @@ +# Process this file with autom4te to create testsuite. -*- Autotest -*- + +# Test suite for GNU tar. +# Copyright (C) 2009 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: When called with --create --remove-files and a compression +# options tar (v. <= 1.22.90) would remove files even if it had failed +# to store them in the archive. +# +# References: <77cb99c00910020940k6ce15da4wb564d2418ec52cfb@...> +# http://lists.gnu.org/archive/html/bug-tar/2009-10/msg00005.html + +AT_SETUP([remove-files with compression]) +AT_KEYWORDS([create remove-files remfiles01 gzip]) + +unset TAR_OPTIONS +AT_CHECK([ +AT_GZIP_PREREQ +AT_SORT_PREREQ + +mkdir dir +cd dir +genfile --file a --length 0 +chmod 0 a +genfile --file b +mkdir c + +tar -c -f a -z --remove-files b c + +find . | sort +], +[0], +[. +./a +./b +./c +], +[tar (child): a: Cannot open: Permission denied +tar (child): Error is not recoverable: exiting now +]) + +AT_CLEANUP diff --git a/tests/remfiles02.at b/tests/remfiles02.at new file mode 100644 index 0000000..d137433 --- /dev/null +++ b/tests/remfiles02.at @@ -0,0 +1,58 @@ +# Process this file with autom4te to create testsuite. -*- Autotest -*- + +# Test suite for GNU tar. +# Copyright (C) 2009 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: When called with --create --remove-files and a compression +# options tar (v. <= 1.22.90) would remove files even if it had failed +# to store them in the archive. +# +# References: <77cb99c00910020940k6ce15da4wb564d2418ec52cfb@...> +# http://lists.gnu.org/archive/html/bug-tar/2009-10/msg00005.html + +AT_SETUP([remove-files with compression: grand-child]) +AT_KEYWORDS([create remove-files remfiles02 gzip]) + +unset TAR_OPTIONS +AT_CHECK([ +AT_GZIP_PREREQ +AT_SORT_PREREQ + +mkdir dir +cd dir +mkdir a +genfile --file b +mkdir c + +tar -c -f a -z --remove-files b c + +find . | sort +], +[0], +[. +./a +./b +./c +], +[tar (child): a: Cannot open: Is a directory +tar (child): Error is not recoverable: exiting now +tar: Child returned status 2 +tar: Error is not recoverable: exiting now +]) + +AT_CLEANUP diff --git a/tests/testsuite.at b/tests/testsuite.at index 325f9d0..17bec7e 100644 --- a/tests/testsuite.at +++ b/tests/testsuite.at @@ -218,6 +218,9 @@ m4_include([shortupd.at]) m4_include([truncate.at]) m4_include([grow.at]) +m4_include([remfiles01.at]) +m4_include([remfiles02.at]) + m4_include([star/gtarfail.at]) m4_include([star/gtarfail2.at]) -- 1.6.4.2 |
| Free embeddable forum powered by Nabble | Forum Help |