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
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
--
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
