threads / patch / 17821

patchfilter-branch -d: Export GIT_DIR earlier

Subject: [PATCH] filter-branch -d: Export GIT_DIR earlier

## tl;dr

5 messages between Feb 16, 2009 and Feb 17, 2009. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Lars Noschinski· Feb 16, 2009, 13:09 UTC · lore

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 error message comes from git for-each-ref in line 224. GIT_DIR is set correctly by git-sh-setup (to the foo.git repository), but not exported (yet). ---

This fix is not yet ready for commit, as it lacks a test case.

Writing a test case for this bug, I wonder about the preferred way to use a directory outside any git repository in a test: Using some directory below git/t/ will not work, as (in most cases) git is a git repository.

Using the system's temp directory via mktemp() or so directory would work most of the time - but not always. Any ideas?

  - Lars.

  git-filter-branch.sh |   12 ++++++------
  1 files changed, 6 insertions(+), 6 deletions(-)

Show changes to git-filter-branch.sh +6 −6

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 27b57b8..9a09ba1 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -220,6 +220,12 @@ die ""
  # Remove tempdir on exit
  trap 'cd ../..; rm -rf "$tempdir"' 0
  
+ORIG_GIT_DIR="$GIT_DIR"
+ORIG_GIT_WORK_TREE="$GIT_WORK_TREE"
+ORIG_GIT_INDEX_FILE="$GIT_INDEX_FILE"
+GIT_WORK_TREE=.
+export GIT_DIR GIT_WORK_TREE
+
  # Make sure refs/original is empty
  git for-each-ref > "$tempdir"/backup-refs || exit
  while read sha1 type name
@@ -234,12 +240,6 @@ do
  	esac
  done < "$tempdir"/backup-refs
  
-ORIG_GIT_DIR="$GIT_DIR"
-ORIG_GIT_WORK_TREE="$GIT_WORK_TREE"
-ORIG_GIT_INDEX_FILE="$GIT_INDEX_FILE"
-GIT_WORK_TREE=.
-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 "$@" > "$tempdir"/raw-heads || exit

-- 
1.6.2.rc0.90.g0753.dirty

Johannes Schindelin· Feb 16, 2009, 13:42 UTC · re: Lars Noschinski · lore

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

Hi,

On Mon, 16 Feb 2009, Lars Noschinski wrote:

Show 18 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 error message comes from git for-each-ref in line 224. GIT_DIR is
> set correctly by git-sh-setup (to the foo.git repository), but not
> exported (yet).

Thanks.

Show 7 quoted lines

> ---
> 
> This fix is not yet ready for commit, as it lacks a test case.
> 
> Writing a test case for this bug, I wonder about the preferred way to use a
> directory outside any git repository in a test: Using some directory below
> git/t/ will not work, as (in most cases) git is a git repository.

How about using a filter-branch call with a filter that echoes GIT_DIR into a file, then fails, and then checking the exact contents of GIT_DIR?

Ciao, Dscho

Lars Noschinski· Feb 16, 2009, 14:51 UTC · re: Johannes Schindelin · lore

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

Hello,

* Johannes Schindelin <Johannes.Schindelin@gmx.de> [09-02-16 15:42]:

Show 8 quoted lines

>> This fix is not yet ready for commit, as it lacks a test case.
>> 
>> Writing a test case for this bug, I wonder about the preferred way to use a
>> directory outside any git repository in a test: Using some directory below
>> git/t/ will not work, as (in most cases) git is a git repository.
>
>How about using a filter-branch call with a filter that echoes GIT_DIR 
>into a file, then fails, and then checking the exact contents of GIT_DIR?

This would not catch this bug: for-each-ref is the only git command called after changing to the temporary directory and before exporting GIT_DIR.

It would help only if GIT_DIR would never be exported.

 - Lars.

Johannes Schindelin· Feb 16, 2009, 15:39 UTC · re: Lars Noschinski · lore

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

Hi,

On Mon, 16 Feb 2009, Lars Noschinski wrote:

Show 14 quoted lines

> * Johannes Schindelin <Johannes.Schindelin@gmx.de> [09-02-16 15:42]:
> > > This fix is not yet ready for commit, as it lacks a test case.
> > > 
> > > Writing a test case for this bug, I wonder about the preferred way to use
> > > a
> > > directory outside any git repository in a test: Using some directory below
> > > git/t/ will not work, as (in most cases) git is a git repository.
> >
> >How about using a filter-branch call with a filter that echoes GIT_DIR into a
> >file, then fails, and then checking the exact contents of GIT_DIR?
> 
> This would not catch this bug: for-each-ref is the only git command
> called after changing to the temporary directory and before exporting
> GIT_DIR.

Right.

Next try: is it not true that we can check that "$tempdir/backup-refs" is correct, i.e. identical to what 'for-each-ref' outputs for the desired directory?

So we could make a subdirectory, set up a dummy branch with one commit, call filter-branch with a tmpdir that is in _another_ subdirectory, with a filter that just returns false, so that the tmpdir is not deleted, and verify the contents of $tmpdir/backup-refs, no?

Ciao, Dscho

Lars Noschinski· Feb 17, 2009, 08:17 UTC · re: Johannes Schindelin · lore

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

* Johannes Schindelin <Johannes.Schindelin@gmx.de> [09-02-16 18:52]:

Show 11 quoted lines

>Hi,
>
>On Mon, 16 Feb 2009, Lars Noschinski wrote:
>
>> * Johannes Schindelin <Johannes.Schindelin@gmx.de> [09-02-16 15:42]:
>> > > This fix is not yet ready for commit, as it lacks a test case.
>> > > 
>> > > Writing a test case for this bug, I wonder about the preferred way to use
>> > > a
>> > > directory outside any git repository in a test: Using some directory below
>> > > git/t/ will not work, as (in most cases) git is a git repository.

[...]

>Next try: is it not true that we can check that "$tempdir/backup-refs" 
>is correct, i.e. identical to what 'for-each-ref' outputs for the desired 
>directory?

True.

>So we could make a subdirectory, set up a dummy branch with one commit, 
>call filter-branch with a tmpdir that is in _another_ subdirectory, with a 
>filter that just returns false, so that the tmpdir is not deleted, and 
>verify the contents of $tmpdir/backup-refs, no?

AFAICS $tmpdir is always deleted, filter-branch sets a trap to ensure this. But using a filter to copy backup-refs elsewhere works. Patch follows.

  - Lars.