{"thread":{"id":"56802","subject":"git aliases and GIT_PREFIX","startedAt":"2021-10-28T19:07:32Z","lastAt":"2021-11-04T00:02:32Z","messageCount":5,"participants":["Federico Kircheis","Ævar Arnfjörð Bjarmason","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"439923","messageId":"2e328484-d0e3-8801-61da-07544cc93eef@gmail.com","threadId":"56802","inReplyTo":null,"subject":"git aliases and GIT_PREFIX","fromName":"Federico Kircheis","fromEmail":"federico.kircheis@gmail.com","sentAt":"2021-10-28T19:03:08Z","receivedAt":"2021-10-28T19:07:32Z","isPatch":false,"sender":{"key":"federico.kircheis@gmail.com","avatar":null},"body":"\nHello to everyone,\n\ntoday I reported what I believed to be a bug on\n\n  https://github.com/git-for-windows/git/issues/3496\n\nand learned about GIT_DIR when working with aliases and git worktree.\n\n\n\nIt's annoying that GIT_DIR it is defined only if (as far as I've \nunderstood) working from a worktrees or submodule, as it does not seem \nto be related to those type of repositories.\n\nThis is also irritating because apparently working aliases breaks when \nbeing executed from those repositories.\n\n\nI believe it would be better if GIT_DIR it's either always set or never \n(could someone enlighten me why the variable is needed in first place?).\n\nIf it is consistently set, it would make it much easier to notice that \nan alias is actually broken.\n\n\n\nBest\n\nFederico\n\n\nPS: I'm not subscribed to the mailing list (yet), so please keep me in CC.\n"},{"id":"439993","messageId":"211029.867ddww0vb.gmgdl@evledraar.gmail.com","threadId":"56802","inReplyTo":"2e328484-d0e3-8801-61da-07544cc93eef@gmail.com","subject":"Re: git aliases and GIT_PREFIX","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-29T10:59:20Z","receivedAt":"2021-10-29T11:01:50Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Oct 28 2021, Federico Kircheis wrote:\n\n> Hello to everyone,\n>\n> today I reported what I believed to be a bug on\n>\n>  https://github.com/git-for-windows/git/issues/3496\n>\n> and learned about GIT_DIR when working with aliases and git worktree.\n>\n>\n>\n> It's annoying that GIT_DIR it is defined only if (as far as I've\n> understood) working from a worktrees or submodule, as it does not seem \n> to be related to those type of repositories.\n>\n> This is also irritating because apparently working aliases breaks when\n> being executed from those repositories.\n>\n>\n> I believe it would be better if GIT_DIR it's either always set or\n> never (could someone enlighten me why the variable is needed in first\n> place?).\n\nI don't know the full story, but a good place to start is to apply this\npatch:\n\ndiff --git a/cache.h b/cache.h\nindex eba12487b99..84d4c8da167 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -486,7 +486,7 @@ static inline enum object_type object_type(unsigned int mode)\n }\n \n /* Double-check local_repo_env below if you add to this list. */\n-#define GIT_DIR_ENVIRONMENT \"GIT_DIR\"\n+#define GIT_DIR_ENVIRONMENT \"POISON_GIT_DIR\"\n #define GIT_COMMON_DIR_ENVIRONMENT \"GIT_COMMON_DIR\"\n #define GIT_NAMESPACE_ENVIRONMENT \"GIT_NAMESPACE\"\n #define GIT_WORK_TREE_ENVIRONMENT \"GIT_WORK_TREE\"\n\nIf you then run the full test suite that comes with git you should get a\npretty good picture of why/how not having GIT_DIR breaks.\n\nSurely some of those reasons are fixable, e.g. are we invoking our own\n\"git\" with \"GIT_DIR=<path> git [...]\", then we can just use \"git\n--git-dir=<path>\", some of the others might be tricky.\n\nWhen I did that the failed tests were the following:\n\nTest Summary Report\n-------------------\nt9401-git-cvsserver-crlf.sh                      (Wstat: 256 Tests: 18 Failed: 16)\n  Failed tests:  2-8, 10-18\n  Non-zero exit status: 1\nt9400-git-cvsserver-server.sh                    (Wstat: 256 Tests: 45 Failed: 23)\n  Failed tests:  2, 20, 22-25, 27-41, 43, 45\n  Non-zero exit status: 1\nt9200-git-cvsexportcommit.sh                     (Wstat: 256 Tests: 15 Failed: 13)\n  Failed tests:  1-2, 4-8, 10-15\n  Non-zero exit status: 1\nt9402-git-cvsserver-refs.sh                      (Wstat: 256 Tests: 37 Failed: 25)\n  Failed tests:  7-12, 14-24, 27-29, 31-32, 34, 36-37\n  Non-zero exit status: 1\nt5516-fetch-push.sh                              (Wstat: 256 Tests: 103 Failed: 3)\n  Failed tests:  101-103\n  Non-zero exit status: 1\nt5500-fetch-pack.sh                              (Wstat: 256 Tests: 373 Failed: 6)\n  Failed tests:  7, 38-42\n  Non-zero exit status: 1\nt7003-filter-branch.sh                           (Wstat: 256 Tests: 48 Failed: 10)\n  Failed tests:  6-7, 9-14, 16, 19\n  Non-zero exit status: 1\nt7406-submodule-update.sh                        (Wstat: 256 Tests: 57 Failed: 1)\n  Failed test:  13\n  Non-zero exit status: 1\nt9902-completion.sh                              (Wstat: 256 Tests: 212 Failed: 2)\n  Failed tests:  11-12\n  Non-zero exit status: 1\nt5601-clone.sh                                   (Wstat: 256 Tests: 107 Failed: 3)\n  Failed tests:  12-13, 35\n  Non-zero exit status: 1\nt1510-repo-setup.sh                              (Wstat: 256 Tests: 109 Failed: 26)\n  Failed tests:  3-4, 6, 8-9, 14-17, 21, 23-24, 29-30, 55\n                57, 59-60, 67, 69-70, 73-74, 78, 80-81\n  Non-zero exit status: 1\nt5310-pack-bitmaps.sh                            (Wstat: 256 Tests: 73 Failed: 1)\n  Failed test:  48\n  Non-zero exit status: 1\nt5401-update-hooks.sh                            (Wstat: 256 Tests: 13 Failed: 9)\n  Failed tests:  3-10, 12\n  Non-zero exit status: 1\nt5531-deep-submodule-push.sh                     (Wstat: 256 Tests: 27 Failed: 22)\n  Failed tests:  2-3, 5-15, 18-20, 22-27\n  Non-zero exit status: 1\nt2400-worktree-add.sh                            (Wstat: 256 Tests: 71 Failed: 2)\n  Failed tests:  61-62\n  Non-zero exit status: 1\nt3430-rebase-merges.sh                           (Wstat: 256 Tests: 25 Failed: 1)\n  Failed test:  11\n  Non-zero exit status: 1\nt5801-remote-helpers.sh                          (Wstat: 256 Tests: 31 Failed: 28)\n  Failed tests:  2, 4-17, 19-31\n  Non-zero exit status: 1\nt7401-submodule-summary.sh                       (Wstat: 256 Tests: 23 Failed: 1)\n  Failed test:  14\n  Non-zero exit status: 1\nt0001-init.sh                                    (Wstat: 256 Tests: 61 Failed: 3)\n  Failed tests:  10, 13, 36\n  Non-zero exit status: 1\nt1500-rev-parse.sh                               (Wstat: 256 Tests: 75 Failed: 7)\n  Failed tests:  35-36, 45, 49-51, 59\n  Non-zero exit status: 1\nt1501-work-tree.sh                               (Wstat: 256 Tests: 39 Failed: 19)\n  Failed tests:  4-7, 9-11, 13-15, 17-18, 24-26, 31, 33-34\n                39\n  Non-zero exit status: 1\nt5509-fetch-push-namespaces.sh                   (Wstat: 256 Tests: 14 Failed: 1)\n  Failed test:  14\n  Non-zero exit status: 1\nt4201-shortlog.sh                                (Wstat: 256 Tests: 25 Failed: 1)\n  Failed test:  9\n  Non-zero exit status: 1\nt5519-push-alternates.sh                         (Wstat: 256 Tests: 8 Failed: 8)\n  Failed tests:  1-8\n  Non-zero exit status: 1\nt6060-merge-index.sh                             (Wstat: 256 Tests: 7 Failed: 2)\n  Failed tests:  6-7\n  Non-zero exit status: 1\nt7409-submodule-detached-work-tree.sh            (Wstat: 256 Tests: 2 Failed: 2)\n  Failed tests:  1-2\n  Non-zero exit status: 1\nt5403-post-checkout-hook.sh                      (Wstat: 256 Tests: 8 Failed: 1)\n  Failed test:  8\n  Non-zero exit status: 1\nt2201-add-update-typechange.sh                   (Wstat: 256 Tests: 6 Failed: 2)\n  Failed tests:  5-6\n  Non-zero exit status: 1\nt5402-post-merge-hook.sh                         (Wstat: 256 Tests: 6 Failed: 4)\n  Failed tests:  3-6\n  Non-zero exit status: 1\nt1515-rev-parse-outside-repo.sh                  (Wstat: 256 Tests: 4 Failed: 1)\n  Failed test:  3\n  Non-zero exit status: 1\nFiles=940, Tests=24858, 144 wallclock secs ( 6.34 usr  1.64 sys + 654.49 cusr 307.66 csys = 970.13 CPU)\nResult: FAIL\nmake: *** [Makefile:53: prove] Error 1\n"},{"id":"440313","messageId":"nycvar.QRO.7.76.6.2111021520370.56@tvgsbejvaqbjf.bet","threadId":"56802","inReplyTo":"2e328484-d0e3-8801-61da-07544cc93eef@gmail.com","subject":"Re: git aliases and GIT_PREFIX","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-11-02T14:26:05Z","receivedAt":"2021-11-02T14:26:10Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Federico,\n\nOn Thu, 28 Oct 2021, Federico Kircheis wrote:\n\n> today I reported what I believed to be a bug on\n>\n>  https://github.com/git-for-windows/git/issues/3496\n>\n> and learned about GIT_DIR when working with aliases and git worktree.\n>\n> It's annoying that GIT_DIR it is defined only if (as far as I've understood)\n> working from a worktrees or submodule, as it does not seem to be related to\n> those type of repositories.\n\nTo clarify: `GIT_DIR` is set when executing an alias in a worktree other\nthan the primary one (and probably also in submodules), but not when\nexecuting in a primary worktree.\n\n> This is also irritating because apparently working aliases breaks when being\n> executed from those repositories.\n\nTo clarify: an alias that wants to switch to a different repository and\nexecute Git commands there works well in a primary worktree. But when you\nswitch to a different repository while executing an alias from a secondary\nworktree, it will fail because of `GIT_DIR` having been set.\n\n> I believe it would be better if GIT_DIR it's either always set or never\n> (could someone enlighten me why the variable is needed in first place?).\n\nThe fact that `GIT_DIR` is not set when calling an alias in a primary\nworktree suggests that the behavior in secondary worktrees is not by\ndesign. We should therefore be able to stop setting it there.\n\nThe question is: what code is responsible for setting it only in some\ncircumstances but not others?\n\nFederico, do you have any experience in debugging C code? If so, it would\nbe good if you could take a crack at investigating this.\n\nCiao,\nJohannes\n"},{"id":"440344","messageId":"20211102172644.cnsro5p3m6o6b6po@gmail.com","threadId":"56802","inReplyTo":"nycvar.QRO.7.76.6.2111021520370.56@tvgsbejvaqbjf.bet","subject":"Re: git aliases and GIT_PREFIX","fromName":"Federico Kircheis","fromEmail":"federico.kircheis@gmail.com","sentAt":"2021-11-02T17:26:44Z","receivedAt":"2021-11-02T17:31:12Z","isPatch":false,"sender":{"key":"federico.kircheis@gmail.com","avatar":null},"body":"\nSorry for the late reply.\n\nOn Tue, Nov 02, 2021 at 03:26:05PM +0100, Johannes Schindelin wrote:\n>Hi Federico,\n>\n>On Thu, 28 Oct 2021, Federico Kircheis wrote:\n>\n>> today I reported what I believed to be a bug on\n>>\n>>  https://github.com/git-for-windows/git/issues/3496\n>>\n>> and learned about GIT_DIR when working with aliases and git worktree.\n>>\n>> It's annoying that GIT_DIR it is defined only if (as far as I've understood)\n>> working from a worktrees or submodule, as it does not seem to be related to\n>> those type of repositories.\n>\n>To clarify: `GIT_DIR` is set when executing an alias in a worktree other\n>than the primary one (and probably also in submodules), but not when\n>executing in a primary worktree.\n>\n>> This is also irritating because apparently working aliases breaks when being\n>> executed from those repositories.\n>\n>To clarify: an alias that wants to switch to a different repository and\n>execute Git commands there works well in a primary worktree. But when you\n>switch to a different repository while executing an alias from a secondary\n>worktree, it will fail because of `GIT_DIR` having been set.\n>\n>> I believe it would be better if GIT_DIR it's either always set or never\n>> (could someone enlighten me why the variable is needed in first place?).\n\nYes, sorry if I was not clear, your clarification are what I meant to \nsay.\n\n>The fact that `GIT_DIR` is not set when calling an alias in a primary\n>worktree suggests that the behavior in secondary worktrees is not by\n>design. We should therefore be able to stop setting it there.\n>\n>The question is: what code is responsible for setting it only in some\n>circumstances but not others?\n>\n>Federico, do you have any experience in debugging C code? If so, it would\n>be good if you could take a crack at investigating this.\n>\n>Ciao,\n>Johannes\n\nYes, I have some experience, but never looked at the git codebase.\n\nOn GitHub (https://github.com/git-for-windows/git/issues/3496) there is \nalready an hint where those variables are set:\n\n   https://github.com/git/git/blob/v2.33.1/git.c#L354\n\nOr do you mean if I could investigate the test cases Ævar Arnfjörð \nBjarmason mentioned?\n\n"},{"id":"440412","messageId":"nycvar.QRO.7.76.6.2111040047540.56@tvgsbejvaqbjf.bet","threadId":"56802","inReplyTo":"20211102172644.cnsro5p3m6o6b6po@gmail.com","subject":"Re: git aliases and GIT_PREFIX","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-11-04T00:02:26Z","receivedAt":"2021-11-04T00:02:32Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Federico,\n\nOn Tue, 2 Nov 2021, Federico Kircheis wrote:\n\n> On Tue, Nov 02, 2021 at 03:26:05PM +0100, Johannes Schindelin wrote:\n> >\n> >On Thu, 28 Oct 2021, Federico Kircheis wrote:\n> >\n> > > today I reported what I believed to be a bug on\n> > >\n> > >  https://github.com/git-for-windows/git/issues/3496\n> > >\n> > > and learned about GIT_DIR when working with aliases and git worktree.\n> > >\n> > > It's annoying that GIT_DIR it is defined only if (as far as I've\n> > > understood)\n> > > working from a worktrees or submodule, as it does not seem to be related\n> > > to\n> > > those type of repositories.\n> >\n> >To clarify: `GIT_DIR` is set when executing an alias in a worktree other\n> >than the primary one (and probably also in submodules), but not when\n> >executing in a primary worktree.\n> >\n> > > This is also irritating because apparently working aliases breaks when\n> > > being\n> > > executed from those repositories.\n> >\n> >To clarify: an alias that wants to switch to a different repository and\n> >execute Git commands there works well in a primary worktree. But when you\n> >switch to a different repository while executing an alias from a secondary\n> >worktree, it will fail because of `GIT_DIR` having been set.\n> >\n> > > I believe it would be better if GIT_DIR it's either always set or never\n> > > (could someone enlighten me why the variable is needed in first place?).\n>\n> Yes, sorry if I was not clear, your clarification are what I meant to say.\n>\n> >The fact that `GIT_DIR` is not set when calling an alias in a primary\n> >worktree suggests that the behavior in secondary worktrees is not by\n> >design. We should therefore be able to stop setting it there.\n> >\n> >The question is: what code is responsible for setting it only in some\n> >circumstances but not others?\n> >\n> >Federico, do you have any experience in debugging C code? If so, it would\n> >be good if you could take a crack at investigating this.\n> >\n> >Ciao,\n> >Johannes\n>\n> Yes, I have some experience, but never looked at the git codebase.\n>\n> On GitHub (https://github.com/git-for-windows/git/issues/3496) there is\n> already an hint where those variables are set:\n>\n>   https://github.com/git/git/blob/v2.33.1/git.c#L354\n\nWell, that's a call to `setup_git_directory_gently()`, and obviously we\nalready know a couple of scenarios where the `GIT_DIR` environment\nvariable is _not_ set in this function.\n\nUnfortunately, this function is a bit long and calls other functions, too:\nhttps://github.com/git/git/blob/v2.33.1/setup.c#L1206-L1339\n\nI actually believe that the `GIT_DIR` is set (_when_ it is set) in\n`set_git_dir_1()`:\nhttps://github.com/git/git/blob/v2.33.1/environment.c#L332. This function\nis called, unconditionally, in `set_git_dir()`, so my best bet is that the\n`set_git_dir()` function calls in `setup_git_directory_gently()` are the\nreason why `GIT_DIR` is set sometimes, but not always.\n\nAnd it is further my suspicion that calls to `set_git_dir()` are used to\npotentially turn a relative path into an absolute one, by passing a\nnon-zero value for `make_realpath`. And this might be the reason for the\ncalls in the first place, not the `GIT_DIR` variable.\n\nSo the first step would probably be to write a test case (see the scripts\nin t/) that creates a secondary worktree, then calls an alias that\nverifies that `GIT_DIR` is not set when run in that worktree. This test\ncase would be marked with `test_expect_failure`.\n\nThen, you need to debug that test case (e.g. by using the `debug` function\nto run a given Git command through `gdb` and then setting a breakpoint on\n`setenv()`). Once you have the call path of the offending `setenv()`, you\nmight find an elegant way to avoid setting the environment variable.\n\nOn the other hand, you could also simply unset `GIT_DIR` and friends\nexplicitly, by adding something like this after\nhttps://github.com/git/git/blob/v2.33.1/git.c#L364:\n\n\tstrvec_push(&child.env, \"GIT_DIR\");\n\nBut then, you might actually break things. The `GIT_PREFIX` variable is\nrequired to let aliases know in which subdirectory (if any) they were\nstarted. For example, if you run this command in Git's `Documentation/`\ndirectory, it will actually print the path of the top-level directory:\n\n\tgit -c alias.pwd='!pwd' pwd\n\nThis is long-established behavior, and the only way to go back to the\ndirectory from where the alias was started is to call `cd \"$GIT_PREFIX\"`.\nThat's behavior, therefore, that you cannot change.\n\nAnd if `GIT_PREFIX` needs to be set, maybe there are scenarios where\n`GIT_DIR` actually _does_ need to be set?\n\nHopefully these explanations make some sense to you.\n\nCiao,\nJohannes\n"}]}