threads / discuss / 56802

git aliases and GIT_PREFIX

Subject: git aliases and GIT_PREFIX

## tl;dr

5 messages between Oct 28, 2021 and Nov 4, 2021.

replies: 4people: 3as markdown or json

Federico Kircheis· Oct 28, 2021, 19:03 UTC · lore
Hello to everyone,
today I reported what I believed to be a bug on
  https://github.com/git-for-windows/git/issues/3496
and learned about GIT_DIR when working with aliases and git worktree.

It's annoying that GIT_DIR it is defined only if (as far as I've understood) working from a worktrees or submodule, as it does not seem to be related to those type of repositories.

This is also irritating because apparently working aliases breaks when being executed from those repositories.

I believe it would be better if GIT_DIR it's either always set or never (could someone enlighten me why the variable is needed in first place?).

If it is consistently set, it would make it much easier to notice that an alias is actually broken.

Best
Federico
PS: I'm not subscribed to the mailing list (yet), so please keep me in CC.
Ævar Arnfjörð Bjarmason· Oct 29, 2021, 10:59 UTC · re: Federico Kircheis · lore

Re: git aliases and GIT_PREFIX

On Thu, Oct 28 2021, Federico Kircheis wrote:
Show 21 quoted lines
> Hello to everyone,
>
> today I reported what I believed to be a bug on
>
>  https://github.com/git-for-windows/git/issues/3496
>
> and learned about GIT_DIR when working with aliases and git worktree.
>
>
>
> It's annoying that GIT_DIR it is defined only if (as far as I've
> understood) working from a worktrees or submodule, as it does not seem 
> to be related to those type of repositories.
>
> This is also irritating because apparently working aliases breaks when
> being executed from those repositories.
>
>
> I believe it would be better if GIT_DIR it's either always set or
> never (could someone enlighten me why the variable is needed in first
> place?).

I don't know the full story, but a good place to start is to apply this patch:

diff --git a/cache.h b/cache.h
index eba12487b99..84d4c8da167 100644
--- a/cache.h
+++ b/cache.h
@@ -486,7 +486,7 @@ static inline enum object_type object_type(unsigned int mode)
 }
 
 /* Double-check local_repo_env below if you add to this list. */
-#define GIT_DIR_ENVIRONMENT "GIT_DIR"
+#define GIT_DIR_ENVIRONMENT "POISON_GIT_DIR"
 #define GIT_COMMON_DIR_ENVIRONMENT "GIT_COMMON_DIR"
 #define GIT_NAMESPACE_ENVIRONMENT "GIT_NAMESPACE"
 #define GIT_WORK_TREE_ENVIRONMENT "GIT_WORK_TREE"

If you then run the full test suite that comes with git you should get a
pretty good picture of why/how not having GIT_DIR breaks.

Surely some of those reasons are fixable, e.g. are we invoking our own
"git" with "GIT_DIR=<path> git [...]", then we can just use "git
--git-dir=<path>", some of the others might be tricky.

When I did that the failed tests were the following:

Test Summary Report
-------------------
t9401-git-cvsserver-crlf.sh                      (Wstat: 256 Tests: 18 Failed: 16)
  Failed tests:  2-8, 10-18
  Non-zero exit status: 1
t9400-git-cvsserver-server.sh                    (Wstat: 256 Tests: 45 Failed: 23)
  Failed tests:  2, 20, 22-25, 27-41, 43, 45
  Non-zero exit status: 1
t9200-git-cvsexportcommit.sh                     (Wstat: 256 Tests: 15 Failed: 13)
  Failed tests:  1-2, 4-8, 10-15
  Non-zero exit status: 1
t9402-git-cvsserver-refs.sh                      (Wstat: 256 Tests: 37 Failed: 25)
  Failed tests:  7-12, 14-24, 27-29, 31-32, 34, 36-37
  Non-zero exit status: 1
t5516-fetch-push.sh                              (Wstat: 256 Tests: 103 Failed: 3)
  Failed tests:  101-103
  Non-zero exit status: 1
t5500-fetch-pack.sh                              (Wstat: 256 Tests: 373 Failed: 6)
  Failed tests:  7, 38-42
  Non-zero exit status: 1
t7003-filter-branch.sh                           (Wstat: 256 Tests: 48 Failed: 10)
  Failed tests:  6-7, 9-14, 16, 19
  Non-zero exit status: 1
t7406-submodule-update.sh                        (Wstat: 256 Tests: 57 Failed: 1)
  Failed test:  13
  Non-zero exit status: 1
t9902-completion.sh                              (Wstat: 256 Tests: 212 Failed: 2)
  Failed tests:  11-12
  Non-zero exit status: 1
t5601-clone.sh                                   (Wstat: 256 Tests: 107 Failed: 3)
  Failed tests:  12-13, 35
  Non-zero exit status: 1
t1510-repo-setup.sh                              (Wstat: 256 Tests: 109 Failed: 26)
  Failed tests:  3-4, 6, 8-9, 14-17, 21, 23-24, 29-30, 55
                57, 59-60, 67, 69-70, 73-74, 78, 80-81
  Non-zero exit status: 1
t5310-pack-bitmaps.sh                            (Wstat: 256 Tests: 73 Failed: 1)
  Failed test:  48
  Non-zero exit status: 1
t5401-update-hooks.sh                            (Wstat: 256 Tests: 13 Failed: 9)
  Failed tests:  3-10, 12
  Non-zero exit status: 1
t5531-deep-submodule-push.sh                     (Wstat: 256 Tests: 27 Failed: 22)
  Failed tests:  2-3, 5-15, 18-20, 22-27
  Non-zero exit status: 1
t2400-worktree-add.sh                            (Wstat: 256 Tests: 71 Failed: 2)
  Failed tests:  61-62
  Non-zero exit status: 1
t3430-rebase-merges.sh                           (Wstat: 256 Tests: 25 Failed: 1)
  Failed test:  11
  Non-zero exit status: 1
t5801-remote-helpers.sh                          (Wstat: 256 Tests: 31 Failed: 28)
  Failed tests:  2, 4-17, 19-31
  Non-zero exit status: 1
t7401-submodule-summary.sh                       (Wstat: 256 Tests: 23 Failed: 1)
  Failed test:  14
  Non-zero exit status: 1
t0001-init.sh                                    (Wstat: 256 Tests: 61 Failed: 3)
  Failed tests:  10, 13, 36
  Non-zero exit status: 1
t1500-rev-parse.sh                               (Wstat: 256 Tests: 75 Failed: 7)
  Failed tests:  35-36, 45, 49-51, 59
  Non-zero exit status: 1
t1501-work-tree.sh                               (Wstat: 256 Tests: 39 Failed: 19)
  Failed tests:  4-7, 9-11, 13-15, 17-18, 24-26, 31, 33-34
                39
  Non-zero exit status: 1
t5509-fetch-push-namespaces.sh                   (Wstat: 256 Tests: 14 Failed: 1)
  Failed test:  14
  Non-zero exit status: 1
t4201-shortlog.sh                                (Wstat: 256 Tests: 25 Failed: 1)
  Failed test:  9
  Non-zero exit status: 1
t5519-push-alternates.sh                         (Wstat: 256 Tests: 8 Failed: 8)
  Failed tests:  1-8
  Non-zero exit status: 1
t6060-merge-index.sh                             (Wstat: 256 Tests: 7 Failed: 2)
  Failed tests:  6-7
  Non-zero exit status: 1
t7409-submodule-detached-work-tree.sh            (Wstat: 256 Tests: 2 Failed: 2)
  Failed tests:  1-2
  Non-zero exit status: 1
t5403-post-checkout-hook.sh                      (Wstat: 256 Tests: 8 Failed: 1)
  Failed test:  8
  Non-zero exit status: 1
t2201-add-update-typechange.sh                   (Wstat: 256 Tests: 6 Failed: 2)
  Failed tests:  5-6
  Non-zero exit status: 1
t5402-post-merge-hook.sh                         (Wstat: 256 Tests: 6 Failed: 4)
  Failed tests:  3-6
  Non-zero exit status: 1
t1515-rev-parse-outside-repo.sh                  (Wstat: 256 Tests: 4 Failed: 1)
  Failed test:  3
  Non-zero exit status: 1
Files=940, Tests=24858, 144 wallclock secs ( 6.34 usr  1.64 sys + 654.49 cusr 307.66 csys = 970.13 CPU)
Result: FAIL
make: *** [Makefile:53: prove] Error 1
Johannes Schindelin· Nov 2, 2021, 14:26 UTC · re: Federico Kircheis · lore

Re: git aliases and GIT_PREFIX

Hi Federico,
On Thu, 28 Oct 2021, Federico Kircheis wrote:
Show 9 quoted lines
> today I reported what I believed to be a bug on
>
>  https://github.com/git-for-windows/git/issues/3496
>
> and learned about GIT_DIR when working with aliases and git worktree.
>
> It's annoying that GIT_DIR it is defined only if (as far as I've understood)
> working from a worktrees or submodule, as it does not seem to be related to
> those type of repositories.

To clarify: `GIT_DIR` is set when executing an alias in a worktree other than the primary one (and probably also in submodules), but not when executing in a primary worktree.

> This is also irritating because apparently working aliases breaks when being
> executed from those repositories.

To clarify: an alias that wants to switch to a different repository and execute Git commands there works well in a primary worktree. But when you switch to a different repository while executing an alias from a secondary worktree, it will fail because of `GIT_DIR` having been set.

> I believe it would be better if GIT_DIR it's either always set or never
> (could someone enlighten me why the variable is needed in first place?).

The fact that `GIT_DIR` is not set when calling an alias in a primary worktree suggests that the behavior in secondary worktrees is not by design. We should therefore be able to stop setting it there.

The question is: what code is responsible for setting it only in some circumstances but not others?

Federico, do you have any experience in debugging C code? If so, it would be good if you could take a crack at investigating this.

Ciao, Johannes

Federico Kircheis· Nov 2, 2021, 17:26 UTC · re: Johannes Schindelin · lore

Re: git aliases and GIT_PREFIX

Sorry for the late reply.
On Tue, Nov 02, 2021 at 03:26:05PM +0100, Johannes Schindelin wrote:
Show 28 quoted lines
>Hi Federico,
>
>On Thu, 28 Oct 2021, Federico Kircheis wrote:
>
>> today I reported what I believed to be a bug on
>>
>>  https://github.com/git-for-windows/git/issues/3496
>>
>> and learned about GIT_DIR when working with aliases and git worktree.
>>
>> It's annoying that GIT_DIR it is defined only if (as far as I've understood)
>> working from a worktrees or submodule, as it does not seem to be related to
>> those type of repositories.
>
>To clarify: `GIT_DIR` is set when executing an alias in a worktree other
>than the primary one (and probably also in submodules), but not when
>executing in a primary worktree.
>
>> This is also irritating because apparently working aliases breaks when being
>> executed from those repositories.
>
>To clarify: an alias that wants to switch to a different repository and
>execute Git commands there works well in a primary worktree. But when you
>switch to a different repository while executing an alias from a secondary
>worktree, it will fail because of `GIT_DIR` having been set.
>
>> I believe it would be better if GIT_DIR it's either always set or never
>> (could someone enlighten me why the variable is needed in first place?).

Yes, sorry if I was not clear, your clarification are what I meant to say.

Show 12 quoted lines
>The fact that `GIT_DIR` is not set when calling an alias in a primary
>worktree suggests that the behavior in secondary worktrees is not by
>design. We should therefore be able to stop setting it there.
>
>The question is: what code is responsible for setting it only in some
>circumstances but not others?
>
>Federico, do you have any experience in debugging C code? If so, it would
>be good if you could take a crack at investigating this.
>
>Ciao,
>Johannes
Yes, I have some experience, but never looked at the git codebase.

On GitHub (https://github.com/git-for-windows/git/issues/3496) there is already an hint where those variables are set:

   https://github.com/git/git/blob/v2.33.1/git.c#L354

Or do you mean if I could investigate the test cases Ævar Arnfjörð Bjarmason mentioned?

Johannes Schindelin· Nov 4, 2021, 00:02 UTC · re: Federico Kircheis · lore

Re: git aliases and GIT_PREFIX

Hi Federico,
On Tue, 2 Nov 2021, Federico Kircheis wrote:
Show 53 quoted lines
> On Tue, Nov 02, 2021 at 03:26:05PM +0100, Johannes Schindelin wrote:
> >
> >On Thu, 28 Oct 2021, Federico Kircheis wrote:
> >
> > > today I reported what I believed to be a bug on
> > >
> > >  https://github.com/git-for-windows/git/issues/3496
> > >
> > > and learned about GIT_DIR when working with aliases and git worktree.
> > >
> > > It's annoying that GIT_DIR it is defined only if (as far as I've
> > > understood)
> > > working from a worktrees or submodule, as it does not seem to be related
> > > to
> > > those type of repositories.
> >
> >To clarify: `GIT_DIR` is set when executing an alias in a worktree other
> >than the primary one (and probably also in submodules), but not when
> >executing in a primary worktree.
> >
> > > This is also irritating because apparently working aliases breaks when
> > > being
> > > executed from those repositories.
> >
> >To clarify: an alias that wants to switch to a different repository and
> >execute Git commands there works well in a primary worktree. But when you
> >switch to a different repository while executing an alias from a secondary
> >worktree, it will fail because of `GIT_DIR` having been set.
> >
> > > I believe it would be better if GIT_DIR it's either always set or never
> > > (could someone enlighten me why the variable is needed in first place?).
>
> Yes, sorry if I was not clear, your clarification are what I meant to say.
>
> >The fact that `GIT_DIR` is not set when calling an alias in a primary
> >worktree suggests that the behavior in secondary worktrees is not by
> >design. We should therefore be able to stop setting it there.
> >
> >The question is: what code is responsible for setting it only in some
> >circumstances but not others?
> >
> >Federico, do you have any experience in debugging C code? If so, it would
> >be good if you could take a crack at investigating this.
> >
> >Ciao,
> >Johannes
>
> Yes, I have some experience, but never looked at the git codebase.
>
> On GitHub (https://github.com/git-for-windows/git/issues/3496) there is
> already an hint where those variables are set:
>
>   https://github.com/git/git/blob/v2.33.1/git.c#L354

Well, that's a call to `setup_git_directory_gently()`, and obviously we already know a couple of scenarios where the `GIT_DIR` environment variable is _not_ set in this function.

Unfortunately, this function is a bit long and calls other functions, too: https://github.com/git/git/blob/v2.33.1/setup.c#L1206-L1339

I actually believe that the `GIT_DIR` is set (_when_ it is set) in `set_git_dir_1()`: https://github.com/git/git/blob/v2.33.1/environment.c#L332. This function is called, unconditionally, in `set_git_dir()`, so my best bet is that the `set_git_dir()` function calls in `setup_git_directory_gently()` are the reason why `GIT_DIR` is set sometimes, but not always.

And it is further my suspicion that calls to `set_git_dir()` are used to potentially turn a relative path into an absolute one, by passing a non-zero value for `make_realpath`. And this might be the reason for the calls in the first place, not the `GIT_DIR` variable.

So the first step would probably be to write a test case (see the scripts in t/) that creates a secondary worktree, then calls an alias that verifies that `GIT_DIR` is not set when run in that worktree. This test case would be marked with `test_expect_failure`.

Then, you need to debug that test case (e.g. by using the `debug` function to run a given Git command through `gdb` and then setting a breakpoint on `setenv()`). Once you have the call path of the offending `setenv()`, you might find an elegant way to avoid setting the environment variable.

On the other hand, you could also simply unset `GIT_DIR` and friends explicitly, by adding something like this after https://github.com/git/git/blob/v2.33.1/git.c#L364:

	strvec_push(&child.env, "GIT_DIR");

But then, you might actually break things. The `GIT_PREFIX` variable is required to let aliases know in which subdirectory (if any) they were started. For example, if you run this command in Git's `Documentation/` directory, it will actually print the path of the top-level directory:

	git -c alias.pwd='!pwd' pwd

This is long-established behavior, and the only way to go back to the directory from where the alias was started is to call `cd "$GIT_PREFIX"`. That's behavior, therefore, that you cannot change.

And if `GIT_PREFIX` needs to be set, maybe there are scenarios where `GIT_DIR` actually _does_ need to be set?

Hopefully these explanations make some sense to you.

Ciao, Johannes

← back to recent threads