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? --- git-filter-branch.sh | 17 ++++++++++------- 1 files changed, 10 insertions(+), 7 deletions(-)
I'm trying to do the constructive thing, and send patches instead of bug reports. :-) -Eric
Show changes to git-filter-branch.sh +10 −7
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 @@ -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 || die "Can't back up refs" while read sha1 type name do case "$force,$name" in @@ -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" test -s "$tempdir"/heads || die "Which ref do you want to rewrite?" @@ -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" fi eval "$filter_index" < /dev/null || @@ -339,7 +340,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 +409,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 || + die "Could not back up branch ref" done < "$tempdir"/heads # TODO: This should possibly go, with the semantics that all positive given @@ -483,7 +486,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 || die "Unable to checkout rewritten tree" fi exit $ret
-- 1.6.0.4
