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. In two places, I've had to break apart pipelines in order to check the error code for the first stage of the pipeline, as discussed here: http://kerneltrap.org/mailarchive/git/2009/1/28/4835614 Thank you to charon on #git for pointing me in the right direction. This patch causes 'git filter-branch' to fail if the --commit-filter argument returns an error. A test case for this behavior is included. Feedback on the original version of this patch was provided by Johannes Sixt and Johannes Schindelin. v2: Remove useless $ret variable Correctly check the first command in a pipeline, not the second Replace verbose 'die' messages with 'exit 1' in most cases Signed-off-by: Eric Kidd <git@xxxxxxxxxxxxxxx> --- git-filter-branch.sh | 26 ++++++++++++++------------ t/t7003-filter-branch.sh | 4 ++++ 2 files changed, 18 insertions(+), 12 deletions(-) Thank you for the feedback! I've tried to incorporate everybody's suggestions. Please let me know if some of those 'exit 1' statements should be changed back to 'die'. -Eric diff --git a/git-filter-branch.sh b/git-filter-branch.sh index 86eef56..fff07c8 100755 --- a/git-filter-branch.sh +++ b/git-filter-branch.sh @@ -221,7 +221,7 @@ die "" trap 'cd ../..; rm -rf "$tempdir"' 0 # Make sure refs/original is empty -git for-each-ref > "$tempdir"/backup-refs +git for-each-ref > "$tempdir"/backup-refs || exit 1 while read sha1 type name do case "$force,$name" in @@ -241,8 +241,9 @@ GIT_WORK_TREE=. 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 +git rev-parse --no-flags --revs-only --symbolic-full-name \ + --default HEAD "$@" > "$tempdir"/raw-heads || exit 1 +sed -e '/^^/d' "$tempdir"/raw-heads >"$tempdir"/heads test -s "$tempdir"/heads || die "Which ref do you want to rewrite?" @@ -251,8 +252,6 @@ GIT_INDEX_FILE="$(pwd)/../index" export GIT_INDEX_FILE git read-tree || die "Could not seed the index" -ret=0 - # map old->new commit ids for rewriting parents mkdir ../map || die "Could not create map/ directory" @@ -315,10 +314,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 + ) > "$tempdir"/tree-state || exit 1 + git update-index --add --replace --remove --stdin \ + < "$tempdir"/tree-state || exit 1 fi eval "$filter_index" < /dev/null || @@ -339,7 +339,8 @@ while read commit parents; do eval "$filter_msg" > ../message || die "msg filter failed: $filter_msg" @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 # In case of a subdirectory filter, it is possible that a specified head @@ -407,7 +408,8 @@ do die "Could not rewrite $ref" ;; esac - git update-ref -m "filter-branch: backup" "$orig_namespace$ref" $sha1 + git update-ref -m "filter-branch: backup" "$orig_namespace$ref" $sha1 || + exit 1 done < "$tempdir"/heads # TODO: This should possibly go, with the semantics that all positive given @@ -483,7 +485,7 @@ test -z "$ORIG_GIT_INDEX_FILE" || { } if [ "$(is_bare_repository)" = false ]; then - git read-tree -u -m HEAD + git read-tree -u -m HEAD || exit 1 fi -exit $ret +exit 0 diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh index cb04743..39affd9 100755 --- a/t/t7003-filter-branch.sh +++ b/t/t7003-filter-branch.sh @@ -48,6 +48,10 @@ test_expect_success 'result is really identical' ' test $H = $(git rev-parse HEAD) ' +test_expect_success 'Fail if commit filter fails' ' + ! git filter-branch -f --commit-filter "exit 1" HEAD +' + test_expect_success 'rewrite, renaming a specific file' ' git filter-branch -f --tree-filter "mv d doh || :" HEAD ' -- 1.6.0.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
