git-filter-branch exits early

Johannes Schindelin· Jul 10, 2007, 21:57 UTC · re: Alex Riesen · lore

Re: git-filter-branch exits early

Hi,

On Tue, 10 Jul 2007, Alex Riesen wrote:

> I have a Debian system where git-filter-branch exits immediately after
> "unset CDPATH" in git-sh-setup (the command exits with 1, as CDPATH is
> not defined). The system still has bash-2.05a.
> 
> git-filter-branch has "set -e", which is why the script finishes
> prematurely. If this is not really needed, maybe it can be removed?
> 
> I'll see if the system can be upgraded, but I suspect someone can get
> a similar problem.

Using rewrite-commits would have a couple of advantages:

- the name is much better,

- since we have reflogs enabled by default now, there is really no good 
  reason why you should have to copy the rewritten branches back to their 
  original name (we would need a way to redirect that, though, for 
  example for subdirectory filters),

- it is faster,

- since it is a C program, it should be more stable, eventually, than a 
  shell script, where you have to work around limitations all the time,

- with the trick I described in the mail to skimo, you can have 
  convenience functions (think "map") in the commit filter, too,

- and it would be less work for me.

	--index-filter "git read-tree $COMMIT_SHA1:sub/directory/"

The second half of it is to add "-- sub/directory/" to the command line.

Alex Riesen· Jul 11, 2007, 06:02 UTC · re: Johannes Schindelin · lore

Re: git-filter-branch exits early

On 7/10/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

> On Tue, 10 Jul 2007, Alex Riesen wrote:
>
> > I have a Debian system where git-filter-branch exits immediately after
> > "unset CDPATH" in git-sh-setup (the command exits with 1, as CDPATH is
> > not defined). The system still has bash-2.05a.
> >
> > git-filter-branch has "set -e", which is why the script finishes
> > prematurely. If this is not really needed, maybe it can be removed?
> >
> > I'll see if the system can be upgraded, but I suspect someone can get
> > a similar problem.
>
> I do not really understand why "unset CDPATH" should trigger an error.  I
> guess that this is one of the nice braindamages in dash, right?

in bash. Bash-2.05.a

> Anyway, "set -e" was one thing I wanted to fix. ...

BTW, what can break if I just go on and remove it?

> - since it is a C program, it should be more stable, eventually, than a
>   shell script, where you have to work around limitations all the time,

right :)

Re: git-filter-branch exits early

On Tue, Jul 10, 2007 at 10:57:44PM +0100, Johannes Schindelin wrote:

> Anyway, "set -e" was one thing I wanted to fix.  But I'm not sure I want 
> to work on filter-branch now, what with skimo (possibly? hopefully?) 
> working on my wishlist for rewrite-commits.

I'm working on it, but only evenings (and weekends).

skimo

Johannes Schindelin· Jul 18, 2007, 13:17 UTC · re: Alex Riesen · lore

[PATCH] filter-branch: get rid of "set -e"

So get rid of "set -e".

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Tue, 10 Jul 2007, Alex Riesen wrote:

	> I have a Debian system where git-filter-branch exits immediately 
	> after "unset CDPATH" in git-sh-setup (the command exits with 1, 
	> as CDPATH is not defined). The system still has bash-2.05a.
	> 
	> git-filter-branch has "set -e", which is why the script finishes
	> prematurely. If this is not really needed, maybe it can be 
	> removed?

	I hope I got all of the interesting places equipped by the 
	appropriate conditional die() calls.

 git-filter-branch.sh |   31 +++++++++++++++++--------------
 1 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 5401970..0d000ed 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -8,8 +8,6 @@
 # a new branch. You can specify a number of filters to modify the commits,
 # files and trees.
 
-set -e
-
 USAGE="git-filter-branch [-d TEMPDIR] [FILTERS] DESTBRANCH [REV-RANGE]"
 . git-sh-setup
 
@@ -141,9 +139,10 @@ git show-ref "refs/heads/$dstbranch" 2> /dev/null &&
 	die "branch $dstbranch already exists"
 
 test ! -e "$tempdir" || die "$tempdir already exists, please remove it"
-mkdir -p "$tempdir/t"
-cd "$tempdir/t"
-workdir="$(pwd)"
+mkdir -p "$tempdir/t" &&
+cd "$tempdir/t" &&
+workdir="$(pwd)" ||
+die ""
 
 case "$GIT_DIR" in
 /*)
@@ -155,12 +154,12 @@ esac
 export GIT_DIR GIT_WORK_TREE=.
 
 export GIT_INDEX_FILE="$(pwd)/../index"
-git read-tree # seed the index file
+git read-tree || die "Could not seed the index"
 
 ret=0
 
-
-mkdir ../map # map old->new commit ids for rewriting parents
+# map old->new commit ids for rewriting parents
+mkdir ../map || die "Could not create map/ directory"
 
 case "$filter_subdir" in
 "")
@@ -170,7 +169,7 @@ case "$filter_subdir" in
 *)
 	git rev-list --reverse --topo-order --default HEAD \
 		--parents --full-history "$@" -- "$filter_subdir"
-esac > ../revs
+esac > ../revs || die "Could not get the commits"
 commits=$(wc -l <../revs | tr -d " ")
 
 test $commits -eq 0 && die "Found nothing to rewrite"
@@ -186,10 +185,11 @@ while read commit parents; do
 		;;
 	*)
 		git read-tree -i -m $commit:"$filter_subdir"
-	esac
+	esac || die "Could not initialize the index"
 
 	export GIT_COMMIT=$commit
-	git cat-file commit "$commit" >../commit
+	git cat-file commit "$commit" >../commit ||
+		die "Cannot read commit $commit"
 
 	eval "$(set_ident AUTHOR <../commit)" ||
 		die "setting author failed for commit $commit"
@@ -199,7 +199,8 @@ while read commit parents; do
 		die "env filter failed: $filter_env"
 
 	if [ "$filter_tree" ]; then
-		git checkout-index -f -u -a
+		git checkout-index -f -u -a ||
+			die "Could not checkout the index"
 		# files that $commit removed are now still in the working tree;
 		# remove them, else they would be added again
 		git ls-files -z --others | xargs -0 rm -f
@@ -240,7 +241,8 @@ case "$target_head" in
 	echo Nothing rewritten
 	;;
 *)
-	git update-ref refs/heads/"$dstbranch" $target_head
+	git update-ref refs/heads/"$dstbranch" $target_head ||
+		die "Could not update $dstbranch with $target_head"
 	if [ $(wc -l <../map/$src_head) -gt 1 ]; then
 		echo "WARNING: Your commit filter caused the head commit to expand to several rewritten commits. Only the first such commit was recorded as the current $dstbranch head but you will need to resolve the situation now (probably by manually merging the other commits). These are all the commits:" >&2
 		sed 's/^/	/' ../map/$src_head >&2
@@ -277,7 +279,8 @@ if [ "$filter_tag_name" ]; then
 			warn "unreferencing tag object $sha1t"
 		fi
 
-		git update-ref "refs/tags/$new_ref" "$new_sha1"
+		git update-ref "refs/tags/$new_ref" "$new_sha1" ||
+			die "Could not write tag $new_ref"
 	done
 fi
 

-- 
1.5.3.rc1.16.g9d6f-dirty

David Kastrup· Jul 18, 2007, 13:27 UTC · re: Johannes Schindelin · lore

Re: [PATCH] filter-branch: get rid of "set -e"

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> It was reported by Alex Riesen that "set -e" can break something as 
> trivial as "unset CDPATH" in bash.

> So get rid of "set -e".

I'd rather write

[ "X$CDPATH" = "X" ] || unset CDPATH

-- 
David Kastrup

Johannes Schindelin· Jul 18, 2007, 14:05 UTC · re: David Kastrup · lore

Re: [PATCH] filter-branch: get rid of "set -e"

Hi,

[please Cc me]

On Wed, 18 Jul 2007, David Kastrup wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > It was reported by Alex Riesen that "set -e" can break something as 
> > trivial as "unset CDPATH" in bash.
> 
> Not in bash.  Just tried it.  But there are Bourne shells that do get
> annoyed.

> > So get rid of "set -e".
> 
> I'd rather write
> 
> [ "X$CDPATH" = "X" ] || unset CDPATH

We would write that as "test", not "[".

But it is much cleaner to get rid of "set -e" anyway.