[PATCH] git-clean: handle errors if removing files fails
From: Miklos Vajna <hidden>
Date: 2016-06-15 22:44:15
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-06-15 · [PATCH] git-clean: handle errors if removing files fails · Miklos Vajna <hidden>
git-clean simply ignored errors if removing a file or directory failed. This patch makes it raise a warning and the exit code also greater than zero if there are remaining files. Signed-off-by: Miklos Vajna <redacted> --- On Wed, Feb 20, 2008 at 04:12:16PM -0800, Junio C Hamano [off-list ref] wrote:
That's quite different style from the other commit log messages in the project, isn't it?
right, i rewrote it.
While I agree reporting an error is definitely an improvement, I do not think dying in the middle is the right thing to do. Shouldn't it note the error, remove other cruft, and then finally signal the error by exiting non-zero?
something like this? i also noticed that we need a chmod 755 after the last test, otherwise removing the trash directory will not be possible. i haven't hit this bug by running the test only once. builtin-clean.c | 22 ++++++++++++++-------- t/t7300-clean.sh | 10 ++++++++++ 2 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/builtin-clean.c b/builtin-clean.c
index eb853a3..5ed57d7 100644
--- a/builtin-clean.c
+++ b/builtin-clean.c@@ -29,7 +29,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix) { int i; int show_only = 0, remove_directories = 0, quiet = 0, ignored = 0; - int ignored_only = 0, baselen = 0, config_set = 0; + int ignored_only = 0, baselen = 0, config_set = 0, errors = 0; struct strbuf directory; struct dir_struct dir; const char *path, *base;
@@ -137,12 +137,15 @@ int cmd_clean(int argc, const char **argv, const char *prefix) if (show_only && (remove_directories || matches)) { printf("Would remove %s\n", directory.buf + prefix_offset); - } else if (quiet && (remove_directories || matches)) { - remove_dir_recursively(&directory, 0); } else if (remove_directories || matches) { - printf("Removing %s\n", - directory.buf + prefix_offset); - remove_dir_recursively(&directory, 0); + if (!quiet) + printf("Removing %s\n", + directory.buf + prefix_offset); + if (remove_dir_recursively(&directory, 0) != 0) { + warning("failed to remove '%s'", + directory.buf + prefix_offset); + errors++; + } } else if (show_only) { printf("Would not remove %s\n", directory.buf + prefix_offset);
@@ -162,11 +165,14 @@ int cmd_clean(int argc, const char **argv, const char *prefix) printf("Removing %s\n", ent->name + prefix_offset); } - unlink(ent->name); + if (unlink(ent->name) != 0) { + warning("failed to remove '%s'", ent->name); + errors++; + } } } free(seen); strbuf_release(&directory); - return 0; + return errors; }
diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index dfd1188..3840364 100755
--- a/t/t7300-clean.sh
+++ b/t/t7300-clean.sh@@ -316,4 +316,14 @@ test_expect_success 'core.excludesfile' ' ' +test_expect_success 'removal failure' ' + + mkdir foo && + touch foo/bar && + chmod 0 foo && + ! git clean -f -d + +' +chmod 755 foo + test_done
--
1.5.4.1