Re: [PATCH] git-filter-branch: Add more error-handling
From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:46:09
Hi, On Wed, 11 Feb 2009, Eric Kidd wrote:
In commit 9273b56278e64dd47b1a96a705ddf46aeaf6afe3, I fixed an error that had slipped by the test suites because of a missing check on 'git read-tree -u -m HEAD'. I mentioned to Johannes Schindelin that there were several bugs of this type in git-filter-branch, and he suggested that I send a patch. I've tested this patch using t/t7003-filter-branch.sh, and it passes all the existing tests. But it's entirely possible that this patch contains errors, and I would love input from people who have more experience with sh and who know more about git-filter-branch. In particular, the following hunk may change the public UI to git-filter-branch, although I'm not sure whether the change is for better or for worse. As I understand it, this hunk would allow $filter_commit to abort the rewriting process by returning a non-0 exit status: @SHELL_PATH@ -c "$filter_commit" "git commit-tree" \ - $(git write-tree) $parentstr < ../message > ../map/$commit + $(git write-tree) $parentstr < ../message > ../map/$commit || + die "could not write rewritten commit" done <../revs I'd be happy to add a test case for what happens when $filter_commit returns a non-0 exit status. Is the old behavior preferable? ---
Thanks. Although it lacks a Sign-off, and part of the commit message is actually a personal comment that belongs after the three dashes.
quoted hunk ↗ jump to hunk
diff --git a/git-filter-branch.sh b/git-filter-branch.sh index 86eef56..9d50978 100755 --- a/git-filter-branch.sh +++ b/git-filter-branch.sh@@ -242,7 +242,7 @@ export GIT_DIR GIT_WORK_TREE # The refs should be updated if their heads were rewritten git rev-parse --no-flags --revs-only --symbolic-full-name --default HEAD "$@" | -sed -e '/^^/d' >"$tempdir"/heads +sed -e '/^^/d' >"$tempdir"/heads || die "Can't make list of original refs"
This will catch errors in the sed invocation, but not in rev-parse.
quoted hunk ↗ jump to hunk
@@ -315,10 +315,11 @@ while read commit parents; do die "tree filter failed: $filter_tree" ( - git diff-index -r --name-only $commit + git diff-index -r --name-only $commit && git ls-files --others ) | - git update-index --add --replace --remove --stdin + git update-index --add --replace --remove --stdin || + die "unable to update index with results of tree filter"
This will catch errors in the update-index call, but neither in diff-index nor ls-files. Otherwise, the patch looks good to me. Ciao, Dscho