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