Re: [PATCH v2] filter-branch -d: Export GIT_DIR earlier

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Feb 17, 2009, 12:44 UTC
Message-ID: <alpine.DEB.1.00.0902171327270.6185@intel-tinevez-2-302>
In-Reply-To: <20090217083146.GA7338@lars.home.noschinski.de>

Hi,

On Tue, 17 Feb 2009, Lars Noschinski wrote:

Show 14 quoted lines

> The improved error handling catches a bug in filter-branch when using
> -d pointing to a path outside any git repository:
> 
> $ mkdir foo
> $ cd foo
> $ git init
> $ touch bar
> $ git add bar
> $ git commit -m bar
> $ cd ..
> $ git clone --bare foo
> $ cd foo.git
> $ git filter-branch -d /tmp/filter master
> fatal: Not a git repository (or any of the parent directories): .git

This could be written as

	$ cd .git
	$ git filter-branch -d /tmp/bla master

Right?

>  git-filter-branch.sh     |   12 ++++++------
>  t/t7003-filter-branch.sh |    9 +++++++++
>  2 files changed, 15 insertions(+), 6 deletions(-)

Funny, git am -3 reports:

	Did you hand edit your patch?
	It does not apply to blobs recorded in its index.
	Cannot fall back to three-way merge.

After realizing that the common lines were prefixed with a double space, and applying my l33t patch m0nkey ski77z, I could verify that it works as expected (in addition to looking at the patch and deeming it correct).

Show 11 quoted lines

> diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
> index 56b5ecc..446700b 100755
> --- a/t/t7003-filter-branch.sh
> +++ b/t/t7003-filter-branch.sh
> @@ -48,6 +48,15 @@ test_expect_success 'result is really identical' '
>  	test $H = $(git rev-parse HEAD)
>  '
>  
> +TRASHDIR=$(pwd)
> +test_expect_success 'correct GIT_DIR while using -d' '
> +        mkdir drepo && cd drepo && git init && make_commit drepo &&

I usually prefer those to be on one line, each.

> +        git filter-branch -d "$TRASHDIR/dfoo" \
> +            --index-filter "cp \"$TRASHDIR\"/dfoo/backup-refs \"$TRASHDIR\""
> &&
> +        cd .. &&

We try to avoid cd'ing back, by using constructs like this:

	(cd drepo &&
	 ...
	) &&

After those two (maybe three) changes and your SOB: ACK.

BTW the reason I wanted to test this thing is that I suspected that you meant test_commit instead of make_commit. But then, I realized that there exists a make_commit in t7003... which shares the shortcoming of our previous implementation of test_commit in that it adds ambiguities on case-insensitive filesystems.

So I _had_ to look who introduced make_commit:

	$ git blame -L '/make_commit/,/}/' t/t7003*

Making a fool out of yourself -- priceless.

Ciao, Dscho