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'. This patch attemps to add all the missing error checks to git-filter-branch, and removes an existing $ret variable that did nothing. I've tested this patch using t/t7003-filter-branch.sh, and it passes all the existing tests. This patch also causes 'git filter-branch' to fail if the --commit-filter argument returns an error. A test case for this behavior is included. 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 Feedback on this patch was provided by Johannes Sixt, Johannes Schindelin and Junio C Hamano. Thomas Rast helped with pipeline error handling. 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(-) v4: Call test_must_fail from inside test_expect_success v3: Replaced 'exit 1' with 'exit' to use exit status of last command Use test_must_fail for unit test expecting failure 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 diff --git a/git-filter-branch.sh b/git-filter-branch.sh index 86eef56..27b57b8 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 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 +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 + git update-index --add --replace --remove --stdin \ + < "$tempdir"/tree-state || exit 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 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 fi -exit $ret +exit 0 diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh index cb04743..56b5ecc 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' ' + test_must_fail 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
