{"thread":{"id":"56135","subject":"Bug: All git operations fail when .git contains a non-existent gitdir","startedAt":"2021-07-21T09:28:31Z","lastAt":"2021-08-31T14:16:16Z","messageCount":17,"participants":["Tom Cook","brian m. carlson","Ævar Arnfjörð Bjarmason","Junio C Hamano","Andrei Rybak","Atharva Raykar","David Aguilar"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"430745","messageId":"CAFSh4UzCWMCpOGZUYnrxwK79F6jN3irdNs=J7O6RMCeJbBxFBw@mail.gmail.com","threadId":"56135","inReplyTo":null,"subject":"Bug: All git operations fail when .git contains a non-existent gitdir","fromName":"Tom Cook","fromEmail":"tom.k.cook@gmail.com","sentAt":"2021-07-21T09:17:36Z","receivedAt":"2021-07-21T09:28:31Z","isPatch":false,"sender":{"key":"tom.k.cook@gmail.com","avatar":null},"body":"What did you do before the bug happened? (Steps to reproduce your issue)\n\nAdd a git submodule to a git repository.\nOverlay-mount that submodule to another place in the filesystem.\nAttempt any git operation in the overlay-mounted path.\n\nWhat did you expect to happen? (Expected behaviour)\n\ngit operations that don't depend on a repository should succeed.\n\nWhat happened instead? (Actual behaviour)\n\nAll git operations fail.  Even `git bugreport` fails.  More importantly,\n`git ls-remote` on an unrelated repository fails.\n\n```\n$ git bugreport\nfatal: not a git repository:\n/home/tkcook/git/Veea/demo.git/VHP-3375-veeadb-on-vhe09/build/iesv10/programs/veeadb/../../../../../../.git/worktree>\n```\n\nWhat's different between what you expected and what actually happened?\n\nCommands that should succeed actually fail with the above message.\n\nAnything else you want to add:\n\nPlease review the rest of the bug report below.\nYou can delete any lines you don't wish to share.\n\n\n[System Info]\ngit version:\ngit version 2.30.2\ncpu: x86_64\nno commit associated with this build\nsizeof-long: 8\nsizeof-size_t: 8\nshell-path: /bin/sh\nuname: Linux 5.13.0 #1 SMP Mon Jun 28 11:38:29 UTC 2021 x86_64\ncompiler info: gnuc: 10.2\nlibc info: glibc: 2.33\n$SHELL (typically, interactive shell): /bin/bash\n\n\n[Enabled Hooks]\nnot run from a git repository - no hooks to show\n"},{"id":"430820","messageId":"YPimnEtNgKD32r2o@camp.crustytoothpaste.net","threadId":"56135","inReplyTo":"CAFSh4UzCWMCpOGZUYnrxwK79F6jN3irdNs=J7O6RMCeJbBxFBw@mail.gmail.com","subject":"Re: Bug: All git operations fail when .git contains a non-existent gitdir","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-07-21T22:58:36Z","receivedAt":"2021-07-21T22:58:47Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-07-21 at 09:17:36, Tom Cook wrote:\n> What did you do before the bug happened? (Steps to reproduce your issue)\n> \n> Add a git submodule to a git repository.\n> Overlay-mount that submodule to another place in the filesystem.\n> Attempt any git operation in the overlay-mounted path.\n\nI'm not sure about what you mean by an overlay-mount operation.  Can you\nprovide some specific commands that we can run at a shell that reproduce\nthe issue?\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"430928","messageId":"CAFSh4Uyr5v9Ao-j0j7yO_HkUZSovBmSg7ADia7XCNZfsspFUYg@mail.gmail.com","threadId":"56135","inReplyTo":"YPimnEtNgKD32r2o@camp.crustytoothpaste.net","subject":"Re: Bug: All git operations fail when .git contains a non-existent gitdir","fromName":"Tom Cook","fromEmail":"tom.k.cook@gmail.com","sentAt":"2021-07-22T13:13:46Z","receivedAt":"2021-07-22T13:14:01Z","isPatch":false,"sender":{"key":"tom.k.cook@gmail.com","avatar":null},"body":"The easiest way to reproduce it is this:\n\n$ mkdir test\n$ cd test\n$ echo \"gitdir: /foo/bar\" > .git\n$ git ls-remote https://github.com/torvalds/linux\n\nWe happen to use overlay mounts in our build system in a way that maps\na git submodule from one place to another so that its \"gitdir\" is\ninvalid and then attempt a `git ls-remote` from that location which\nunexpectedly fails.  But the above reproduces the problem well enough.\n\nRegards,\nTom\n\nOn Wed, Jul 21, 2021 at 11:59 PM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n>\n> On 2021-07-21 at 09:17:36, Tom Cook wrote:\n> > What did you do before the bug happened? (Steps to reproduce your issue)\n> >\n> > Add a git submodule to a git repository.\n> > Overlay-mount that submodule to another place in the filesystem.\n> > Attempt any git operation in the overlay-mounted path.\n>\n> I'm not sure about what you mean by an overlay-mount operation.  Can you\n> provide some specific commands that we can run at a shell that reproduce\n> the issue?\n> --\n> brian m. carlson (he/him or they/them)\n> Toronto, Ontario, CA\n"},{"id":"430934","messageId":"patch-1.1-fc26c46d39-20210722T140648Z-avarab@gmail.com","threadId":"56135","inReplyTo":"CAFSh4Uyr5v9Ao-j0j7yO_HkUZSovBmSg7ADia7XCNZfsspFUYg@mail.gmail.com","subject":"[PATCH] setup: only die on invalid .git under RUN_SETUP","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-07-22T14:07:39Z","receivedAt":"2021-07-22T14:07:46Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change RUN_SETUP_GENTLY to stop dying if e.g. the .git is \"not a\nrepo\". This means that we now recover in cases like:\n\n    $ echo \"gitdir: /foo/bar\" > .git\n    $ git ls-remote https://github.com/torvalds/linux\n    [... ls-remote output ...]\n\nBut not (as intended):\n\n    $ git rev-parse HEAD\n    fatal: not a git repository: /foo/bar\n\nThe relevant setup_git_directory_gently_1() invocation was added in\n01017dce546 (setup_git_directory_gently_1(): avoid die()ing,\n2017-03-13), but I could reproduce this as far back as Git v1.8.0. I\ndon't know if this ever worked, but it should.\n\nLet's also use the compiler to check enum arms for us, instead of\nhaving a \"default\" fall-though case, this changes code added in\nce9b8aab5d9 (setup_git_directory_1(): avoid changing global state,\n2017-03-13).\n\nReported-by: Tom Cook <tom.k.cook@gmail.com>\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n setup.c            | 27 ++++++++++++++++++++++-----\n t/t0002-gitfile.sh |  8 ++++++--\n 2 files changed, 28 insertions(+), 7 deletions(-)\n\ndiff --git a/setup.c b/setup.c\nindex eb9367ca5c..6ff145d58b 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -1033,7 +1033,8 @@ enum discovery_result {\n \t/* these are errors */\n \tGIT_DIR_HIT_CEILING = -1,\n \tGIT_DIR_HIT_MOUNT_POINT = -2,\n-\tGIT_DIR_INVALID_GITFILE = -3\n+\tGIT_DIR_INVALID_GITFILE = -3,\n+\tGIT_DIR_GITFILE_NOT_A_REPO = -4,\n };\n \n /*\n@@ -1118,8 +1119,11 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,\n \t\t\t\t/* NEEDSWORK: fail if .git is not file nor dir */\n \t\t\t\tif (is_git_directory(dir->buf))\n \t\t\t\t\tgitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;\n-\t\t\t} else if (error_code != READ_GITFILE_ERR_STAT_FAILED)\n+\t\t\t} else if (error_code == READ_GITFILE_ERR_NOT_A_REPO) {\n+\t\t\t\treturn GIT_DIR_GITFILE_NOT_A_REPO;\n+\t\t\t} else if (error_code != READ_GITFILE_ERR_STAT_FAILED) {\n \t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n+\t\t\t}\n \t\t}\n \t\tstrbuf_setlen(dir, offset);\n \t\tif (gitdirenv) {\n@@ -1209,6 +1213,8 @@ const char *setup_git_directory_gently(int *nongit_ok)\n \tstruct strbuf dir = STRBUF_INIT, gitdir = STRBUF_INIT;\n \tconst char *prefix = NULL;\n \tstruct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;\n+\tint die_on_error = !nongit_ok;\n+\tenum discovery_result discovery;\n \n \t/*\n \t * We may have read an incomplete configuration before\n@@ -1231,7 +1237,9 @@ const char *setup_git_directory_gently(int *nongit_ok)\n \t\tdie_errno(_(\"Unable to read current working directory\"));\n \tstrbuf_addbuf(&dir, &cwd);\n \n-\tswitch (setup_git_directory_gently_1(&dir, &gitdir, 1)) {\n+\tdiscovery = setup_git_directory_gently_1(&dir, &gitdir, die_on_error);\n+\n+\tswitch (discovery) {\n \tcase GIT_DIR_EXPLICIT:\n \t\tprefix = setup_explicit_git_dir(gitdir.buf, &cwd, &repo_fmt, nongit_ok);\n \t\tbreak;\n@@ -1259,6 +1267,16 @@ const char *setup_git_directory_gently(int *nongit_ok)\n \t\t\t    dir.buf);\n \t\t*nongit_ok = 1;\n \t\tbreak;\n+\tcase GIT_DIR_GITFILE_NOT_A_REPO:\n+\t\tif (!nongit_ok)\n+\t\t\tdie(_(\"not a git repository: %s\"), dir.buf);\n+\t\t*nongit_ok = 1;\n+\t\tbreak;\n+\tcase GIT_DIR_INVALID_GITFILE:\n+\t\tif (!nongit_ok)\n+\t\t\tdie(_(\"invalid .git file: %s\"), dir.buf);\n+\t\t*nongit_ok = 1;\n+\t\tbreak;\n \tcase GIT_DIR_NONE:\n \t\t/*\n \t\t * As a safeguard against setup_git_directory_gently_1 returning\n@@ -1266,8 +1284,7 @@ const char *setup_git_directory_gently(int *nongit_ok)\n \t\t * set startup_info->have_repository to 1 when we did nothing to\n \t\t * find a repository.\n \t\t */\n-\tdefault:\n-\t\tBUG(\"unhandled setup_git_directory_1() result\");\n+\t\tBUG(\"setup_git_directory_1() should not return GIT_DIR_NONE\");\n \t}\n \n \t/*\ndiff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh\nindex 8440e6add1..176dc8c9dc 100755\n--- a/t/t0002-gitfile.sh\n+++ b/t/t0002-gitfile.sh\n@@ -21,13 +21,17 @@ test_expect_success 'initial setup' '\n test_expect_success 'bad setup: invalid .git file format' '\n \techo \"gitdir $REAL\" >.git &&\n \ttest_must_fail git rev-parse 2>.err &&\n-\ttest_i18ngrep \"invalid gitfile format\" .err\n+\ttest_i18ngrep \"invalid gitfile format\" .err &&\n+\n+\tgit ls-remote \"file://$REAL\"\n '\n \n test_expect_success 'bad setup: invalid .git file path' '\n \techo \"gitdir: $REAL.not\" >.git &&\n \ttest_must_fail git rev-parse 2>.err &&\n-\ttest_i18ngrep \"not a git repository\" .err\n+\ttest_i18ngrep \"not a git repository\" .err &&\n+\n+\tgit ls-remote \"file://$REAL\"\n '\n \n test_expect_success 'final setup + check rev-parse --git-dir' '\n-- \n2.32.0.954.g5a3a1483ade\n\n"},{"id":"430946","messageId":"xmqq5yx29nj3.fsf@gitster.g","threadId":"56135","inReplyTo":"patch-1.1-fc26c46d39-20210722T140648Z-avarab@gmail.com","subject":"Re: [PATCH] setup: only die on invalid .git under RUN_SETUP","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-22T19:06:08Z","receivedAt":"2021-07-22T19:06:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n\n> Change RUN_SETUP_GENTLY to stop dying if e.g. the .git is \"not a\n> repo\". This means that we now recover in cases like:\n>\n>     $ echo \"gitdir: /foo/bar\" > .git\n>     $ git ls-remote https://github.com/torvalds/linux\n>     [... ls-remote output ...]\n>\n> But not (as intended):\n>\n>     $ git rev-parse HEAD\n>     fatal: not a git repository: /foo/bar\n\nI am of two minds.  ls-remote is benign in that it behaves more or\nless identically when given certain types of args, and the above may\nbe a strict improvement (but it does fail if you did not use URL but\nuse a remote nickname you thought you configured in the repository\nin such a situation).  There however are a few niche commands that\nwork inside and outside a repository and they work differently.  For\nexample, if you do\n\n  $ git diff file1 file2\n\nin such a corrupt repository, I'd prefer to see the command _fail_\nto nudge the user to look into the situation, instead of taking the\noutput (which degenerates to \"git diff --no-index file1 file2\"\noutside a repository) blindly as a patch that shows the changes\nrelative to the index for these two paths.\n\n"},{"id":"430947","messageId":"8fbf200c-2d88-dce2-84c3-ead330e975e8@gmail.com","threadId":"56135","inReplyTo":"patch-1.1-fc26c46d39-20210722T140648Z-avarab@gmail.com","subject":"Re: [PATCH] setup: only die on invalid .git under RUN_SETUP","fromName":"Andrei Rybak","fromEmail":"rybak.a.v@gmail.com","sentAt":"2021-07-22T20:50:52Z","receivedAt":"2021-07-22T20:50:59Z","isPatch":true,"sender":{"key":"rybak.a.v@gmail.com","avatar":"https://avatars.githubusercontent.com/u/624072?v=4"},"body":"On 22/07/2021 16:07, Ævar Arnfjörð Bjarmason wrote:\n> Change RUN_SETUP_GENTLY to stop dying if e.g. the .git is \"not a\n> repo\". This means that we now recover in cases like:\n> \n>      $ echo \"gitdir: /foo/bar\" > .git\n>      $ git ls-remote https://github.com/torvalds/linux\n>      [... ls-remote output ...]\n> \n> But not (as intended):\n> \n>      $ git rev-parse HEAD\n>      fatal: not a git repository: /foo/bar\n> \n> The relevant setup_git_directory_gently_1() invocation was added in\n> 01017dce546 (setup_git_directory_gently_1(): avoid die()ing,\n> 2017-03-13), but I could reproduce this as far back as Git v1.8.0. I\n> don't know if this ever worked, but it should.\n> \n> Let's also use the compiler to check enum arms for us, instead of\n> having a \"default\" fall-though case, this changes code added in\n> ce9b8aab5d9 (setup_git_directory_1(): avoid changing global state,\n> 2017-03-13).\n> \n> Reported-by: Tom Cook <tom.k.cook@gmail.com>\n> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> ---\n>   setup.c            | 27 ++++++++++++++++++++++-----\n>   t/t0002-gitfile.sh |  8 ++++++--\n>   2 files changed, 28 insertions(+), 7 deletions(-)\n> \n> diff --git a/setup.c b/setup.c\n> index eb9367ca5c..6ff145d58b 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -1033,7 +1033,8 @@ enum discovery_result {\n>   \t/* these are errors */\n>   \tGIT_DIR_HIT_CEILING = -1,\n>   \tGIT_DIR_HIT_MOUNT_POINT = -2,\n> -\tGIT_DIR_INVALID_GITFILE = -3\n> +\tGIT_DIR_INVALID_GITFILE = -3,\n> +\tGIT_DIR_GITFILE_NOT_A_REPO = -4,\n>   };\n>   \n>   /*\n> @@ -1118,8 +1119,11 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,\n>   \t\t\t\t/* NEEDSWORK: fail if .git is not file nor dir */\n>   \t\t\t\tif (is_git_directory(dir->buf))\n>   \t\t\t\t\tgitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;\n> -\t\t\t} else if (error_code != READ_GITFILE_ERR_STAT_FAILED)\n> +\t\t\t} else if (error_code == READ_GITFILE_ERR_NOT_A_REPO) {\n> +\t\t\t\treturn GIT_DIR_GITFILE_NOT_A_REPO;\n> +\t\t\t} else if (error_code != READ_GITFILE_ERR_STAT_FAILED) {\n>   \t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n> +\t\t\t}\n>   \t\t}\n>   \t\tstrbuf_setlen(dir, offset);\n>   \t\tif (gitdirenv) {\n> @@ -1209,6 +1213,8 @@ const char *setup_git_directory_gently(int *nongit_ok)\n>   \tstruct strbuf dir = STRBUF_INIT, gitdir = STRBUF_INIT;\n>   \tconst char *prefix = NULL;\n>   \tstruct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;\n> +\tint die_on_error = !nongit_ok;\n> +\tenum discovery_result discovery;\n>   \n>   \t/*\n>   \t * We may have read an incomplete configuration before\n> @@ -1231,7 +1237,9 @@ const char *setup_git_directory_gently(int *nongit_ok)\n>   \t\tdie_errno(_(\"Unable to read current working directory\"));\n>   \tstrbuf_addbuf(&dir, &cwd);\n>   \n> -\tswitch (setup_git_directory_gently_1(&dir, &gitdir, 1)) {\n> +\tdiscovery = setup_git_directory_gently_1(&dir, &gitdir, die_on_error);\n> +\n> +\tswitch (discovery) {\n>   \tcase GIT_DIR_EXPLICIT:\n>   \t\tprefix = setup_explicit_git_dir(gitdir.buf, &cwd, &repo_fmt, nongit_ok);\n>   \t\tbreak;\n> @@ -1259,6 +1267,16 @@ const char *setup_git_directory_gently(int *nongit_ok)\n>   \t\t\t    dir.buf);\n>   \t\t*nongit_ok = 1;\n>   \t\tbreak;\n> +\tcase GIT_DIR_GITFILE_NOT_A_REPO:\n> +\t\tif (!nongit_ok)\n> +\t\t\tdie(_(\"not a git repository: %s\"), dir.buf);\n> +\t\t*nongit_ok = 1;\n> +\t\tbreak;\n> +\tcase GIT_DIR_INVALID_GITFILE:\n> +\t\tif (!nongit_ok)\n\nVariable die_on_error could be used in two `if`s above.\n\n> +\t\t\tdie(_(\"invalid .git file: %s\"), dir.buf);\n> +\t\t*nongit_ok = 1;\n> +\t\tbreak;\n>   \tcase GIT_DIR_NONE:\n>   \t\t/*\n>   \t\t * As a safeguard against setup_git_directory_gently_1 returning\n> @@ -1266,8 +1284,7 @@ const char *setup_git_directory_gently(int *nongit_ok)\n>   \t\t * set startup_info->have_repository to 1 when we did nothing to\n>   \t\t * find a repository.\n>   \t\t */\n> -\tdefault:\n> -\t\tBUG(\"unhandled setup_git_directory_1() result\");\n> +\t\tBUG(\"setup_git_directory_1() should not return GIT_DIR_NONE\");\n>   \t}\n>   \n>   \t/*\n> diff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh\n> index 8440e6add1..176dc8c9dc 100755\n> --- a/t/t0002-gitfile.sh\n> +++ b/t/t0002-gitfile.sh\n> @@ -21,13 +21,17 @@ test_expect_success 'initial setup' '\n>   test_expect_success 'bad setup: invalid .git file format' '\n>   \techo \"gitdir $REAL\" >.git &&\n>   \ttest_must_fail git rev-parse 2>.err &&\n> -\ttest_i18ngrep \"invalid gitfile format\" .err\n> +\ttest_i18ngrep \"invalid gitfile format\" .err &&\n> +\n> +\tgit ls-remote \"file://$REAL\"\n>   '\n>   \n>   test_expect_success 'bad setup: invalid .git file path' '\n>   \techo \"gitdir: $REAL.not\" >.git &&\n>   \ttest_must_fail git rev-parse 2>.err &&\n> -\ttest_i18ngrep \"not a git repository\" .err\n> +\ttest_i18ngrep \"not a git repository\" .err &&\n> +\n> +\tgit ls-remote \"file://$REAL\"\n>   '\n>   \n>   test_expect_success 'final setup + check rev-parse --git-dir' '\n> \n\n"},{"id":"430951","messageId":"87mtqet5aw.fsf@evledraar.gmail.com","threadId":"56135","inReplyTo":"xmqq5yx29nj3.fsf@gitster.g","subject":"Re: [PATCH] setup: only die on invalid .git under RUN_SETUP","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-07-22T21:08:23Z","receivedAt":"2021-07-22T21:19:25Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Jul 22 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n>\n>> Change RUN_SETUP_GENTLY to stop dying if e.g. the .git is \"not a\n>> repo\". This means that we now recover in cases like:\n>>\n>>     $ echo \"gitdir: /foo/bar\" > .git\n>>     $ git ls-remote https://github.com/torvalds/linux\n>>     [... ls-remote output ...]\n>>\n>> But not (as intended):\n>>\n>>     $ git rev-parse HEAD\n>>     fatal: not a git repository: /foo/bar\n>\n> I am of two minds.  ls-remote is benign in that it behaves more or\n> less identically when given certain types of args, and the above may\n> be a strict improvement (but it does fail if you did not use URL but\n> use a remote nickname you thought you configured in the repository\n> in such a situation).  There however are a few niche commands that\n> work inside and outside a repository and they work differently.  For\n> example, if you do\n>\n>   $ git diff file1 file2\n>\n> in such a corrupt repository, I'd prefer to see the command _fail_\n> to nudge the user to look into the situation, instead of taking the\n> output (which degenerates to \"git diff --no-index file1 file2\"\n> outside a repository) blindly as a patch that shows the changes\n> relative to the index for these two paths.\n\nYes it comes down to what we think RUN_SETUP_GENTLY and\nsetup_git_directory_gently() should be doing.\n\nI.e. is &nongit_ok supposed to be a binary \"repo you can use\"/\"[...]\ncan't use\", or a tri-state of that plus \"this looks like it's supposed\nto be a repo, but it's broken, so let's die\".\n\nAnyway, if you're not happy with this pretty much as-is consider it\ndropped from my side, because I think the next step would be to do some\nmore work on e.g. split up RUN_SETUP_GENTLY into a mode that makes sense\nfor \"diff\", and another for \"bugreport\" and \"ls-remote\". I figured this\nwas an easy bugfix, but if not perhaps Tom Cooks wants to pick this\nup...\n\nI guess another easy alternative would be to issue a warning() in this\ncase, which is a relatively light refactoring of passing an error\nmessage up from the relevant function(s) instead of having it die()\ndirectly.\n"},{"id":"430980","messageId":"xmqqfsw5rdsq.fsf@gitster.g","threadId":"56135","inReplyTo":"87mtqet5aw.fsf@evledraar.gmail.com","subject":"Re: [PATCH] setup: only die on invalid .git under RUN_SETUP","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-23T01:59:01Z","receivedAt":"2021-07-23T01:59:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> I guess another easy alternative would be to issue a warning() in this\n> case, which is a relatively light refactoring of passing an error\n> message up from the relevant function(s) instead of having it die()\n> directly.\n\nI think the end users deserve a warning even when they are running\nls-remote or bugreport in such a corrupt repository-lookalike.  It\nmight be sufficient to make \"git diff [--no-index]\" safe enough.\n\nThanks.\n\n"},{"id":"430992","messageId":"3D8703D8-54E6-4CF0-9E9F-CCAFFAA8914C@gmail.com","threadId":"56135","inReplyTo":"CAFSh4Uyr5v9Ao-j0j7yO_HkUZSovBmSg7ADia7XCNZfsspFUYg@mail.gmail.com","subject":"Re: Bug: All git operations fail when .git contains a non-existent gitdir","fromName":"Atharva Raykar","fromEmail":"raykar.ath@gmail.com","sentAt":"2021-07-23T08:23:36Z","receivedAt":"2021-07-23T08:23:42Z","isPatch":false,"sender":{"key":"raykar.ath@gmail.com","avatar":"https://avatars.githubusercontent.com/u/24277692?v=4"},"body":"On 22-Jul-2021, at 18:43, Tom Cook <tom.k.cook@gmail.com> wrote:\n> On Wed, Jul 21, 2021 at 11:59 PM brian m. carlson\n> <sandals@crustytoothpaste.net> wrote:\n>> \n>> On 2021-07-21 at 09:17:36, Tom Cook wrote:\n>>> What did you do before the bug happened? (Steps to reproduce your issue)\n>>> \n>>> Add a git submodule to a git repository.\n>>> Overlay-mount that submodule to another place in the filesystem.\n>>> Attempt any git operation in the overlay-mounted path.\n>> \n>> I'm not sure about what you mean by an overlay-mount operation.  Can you\n>> provide some specific commands that we can run at a shell that reproduce\n>> the issue?\n>> --\n>> brian m. carlson (he/him or they/them)\n>> Toronto, Ontario, CA\n> \n> The easiest way to reproduce it is this:\n> \n> $ mkdir test\n> $ cd test\n> $ echo \"gitdir: /foo/bar\" > .git\n> $ git ls-remote https://github.com/torvalds/linux\n> \n> We happen to use overlay mounts in our build system in a way that maps\n> a git submodule from one place to another so that its \"gitdir\" is\n> invalid and then attempt a `git ls-remote` from that location which\n> unexpectedly fails.  But the above reproduces the problem well enough.\n\n'ls-remote' needs a valid git directory for the case where the URL is not\nexplicitly supplied (to read the git config and learn the default remote).\n\nMaking a special case for when an explicit URL is not given is not as\nstraightforward as it seems, because by the time 'ls-remote' even knows about\nits arguments, it already takes a worktree prefix and sets up the environment,\nfor which a valid Git repository path is required.\n\nI am not too familiar with this area, and I don't know how feasible it is to\ndelay setting up the environment until after looking at the 'ls-remote'\narguments. At a cursory glance, it looks difficult to do without large\nstructural changes to the code.\n\nThis might have been less of a problem with old-form submodules, where '.git'\nwas an actual directory, rather than a text file pointer [1], but newer\nversions of Git discourage their usage.\n\n[1] https://git-scm.com/docs/gitsubmodules#_forms\n\nPS: we prefer bottom posting or inline replies :)\n\n---\nPointers for others who might be interested in looking into this:\n\nThe immediate cause of this seems to be 'setup.c:setup_gitdir_gently()' [2]\nwhich calls 'setup_gitdir_gently_1()' with the 'die_on_error' argument set\nto true. This function then calls 'read_gitfile_gently()' with the same flag,\nwhich errors out when it runs 'is_git_directory()' [3], because the path in\nthe gitfile is not a valid repository.\n\n[2] https://github.com/git/git/blob/eb27b338a3e71c7c4079fbac8aeae3f8fbb5c687/setup.c#L1234\n[3] https://github.com/git/git/blob/eb27b338a3e71c7c4079fbac8aeae3f8fbb5c687/setup.c#L784-L799\n\n---\nAtharva Raykar\nಅಥರ್ವ ರಾಯ್ಕರ್\nअथर्व रायकर\n\n"},{"id":"430994","messageId":"CAFSh4UySxQLS10M4XJHyNQ313wuLu1rg2Jnahf-Dk2uxJkw17w@mail.gmail.com","threadId":"56135","inReplyTo":"3D8703D8-54E6-4CF0-9E9F-CCAFFAA8914C@gmail.com","subject":"Re: Bug: All git operations fail when .git contains a non-existent gitdir","fromName":"Tom Cook","fromEmail":"tom.k.cook@gmail.com","sentAt":"2021-07-23T08:39:58Z","receivedAt":"2021-07-23T08:40:12Z","isPatch":false,"sender":{"key":"tom.k.cook@gmail.com","avatar":null},"body":"On Fri, Jul 23, 2021 at 9:23 AM Atharva Raykar <raykar.ath@gmail.com> wrote:\n> I am not too familiar with this area, and I don't know how feasible it is to\n> delay setting up the environment until after looking at the 'ls-remote'\n> arguments. At a cursory glance, it looks difficult to do without large\n> structural changes to the code.\n\nUnderstood.  This happened to bite us particularly hard, because the\ncontext where we found it is during a `go mod download` and `go`\nswallows the error, attempts something else and then reports that the\nsomething else failed with no indication what the underlying error is.\nIt took us a while to track down what was going on (see\nhttps://github.com/golang/go/issues/47311).\n"},{"id":"430995","messageId":"CAFSh4Uw0H-HHfEYZbqkzXvFu29oisRL71x7qp+=vu3mtoVMnkw@mail.gmail.com","threadId":"56135","inReplyTo":"87mtqet5aw.fsf@evledraar.gmail.com","subject":"Re: [PATCH] setup: only die on invalid .git under RUN_SETUP","fromName":"Tom Cook","fromEmail":"tom.k.cook@gmail.com","sentAt":"2021-07-23T08:42:28Z","receivedAt":"2021-07-23T08:42:41Z","isPatch":true,"sender":{"key":"tom.k.cook@gmail.com","avatar":null},"body":"On Thu, Jul 22, 2021 at 10:19 PM Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n\n> Anyway, if you're not happy with this pretty much as-is consider it\n> dropped from my side, because I think the next step would be to do some\n> more work on e.g. split up RUN_SETUP_GENTLY into a mode that makes sense\n> for \"diff\", and another for \"bugreport\" and \"ls-remote\". I figured this\n> was an easy bugfix, but if not perhaps Tom Cooks wants to pick this\n> up...\n\nJust to note that I am following this conversation.  I don't have any\ntime available to look into this immediately but probably will in a\ncouple of weeks.  If it's not resolved by then, then I can pick it up.\nBut please continue the conversation; I've no familiarity at all with\ngit internals so the more detail that comes out here, the better.\n\nRegards,\nTom\n"},{"id":"431011","messageId":"87tukls7ax.fsf@evledraar.gmail.com","threadId":"56135","inReplyTo":"8fbf200c-2d88-dce2-84c3-ead330e975e8@gmail.com","subject":"Re: [PATCH] setup: only die on invalid .git under RUN_SETUP","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-07-23T09:33:30Z","receivedAt":"2021-07-23T09:33:40Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Jul 22 2021, Andrei Rybak wrote:\n\n> On 22/07/2021 16:07, Ævar Arnfjörð Bjarmason wrote:\n>> Change RUN_SETUP_GENTLY to stop dying if e.g. the .git is \"not a\n>> repo\". This means that we now recover in cases like:\n>>      $ echo \"gitdir: /foo/bar\" > .git\n>>      $ git ls-remote https://github.com/torvalds/linux\n>>      [... ls-remote output ...]\n>> But not (as intended):\n>>      $ git rev-parse HEAD\n>>      fatal: not a git repository: /foo/bar\n>> The relevant setup_git_directory_gently_1() invocation was added in\n>> 01017dce546 (setup_git_directory_gently_1(): avoid die()ing,\n>> 2017-03-13), but I could reproduce this as far back as Git v1.8.0. I\n>> don't know if this ever worked, but it should.\n>> Let's also use the compiler to check enum arms for us, instead of\n>> having a \"default\" fall-though case, this changes code added in\n>> ce9b8aab5d9 (setup_git_directory_1(): avoid changing global state,\n>> 2017-03-13).\n>> Reported-by: Tom Cook <tom.k.cook@gmail.com>\n>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n>> ---\n>>   setup.c            | 27 ++++++++++++++++++++++-----\n>>   t/t0002-gitfile.sh |  8 ++++++--\n>>   2 files changed, 28 insertions(+), 7 deletions(-)\n>> diff --git a/setup.c b/setup.c\n>> index eb9367ca5c..6ff145d58b 100644\n>> --- a/setup.c\n>> +++ b/setup.c\n>> @@ -1033,7 +1033,8 @@ enum discovery_result {\n>>   \t/* these are errors */\n>>   \tGIT_DIR_HIT_CEILING = -1,\n>>   \tGIT_DIR_HIT_MOUNT_POINT = -2,\n>> -\tGIT_DIR_INVALID_GITFILE = -3\n>> +\tGIT_DIR_INVALID_GITFILE = -3,\n>> +\tGIT_DIR_GITFILE_NOT_A_REPO = -4,\n>>   };\n>>     /*\n>> @@ -1118,8 +1119,11 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,\n>>   \t\t\t\t/* NEEDSWORK: fail if .git is not file nor dir */\n>>   \t\t\t\tif (is_git_directory(dir->buf))\n>>   \t\t\t\t\tgitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;\n>> -\t\t\t} else if (error_code != READ_GITFILE_ERR_STAT_FAILED)\n>> +\t\t\t} else if (error_code == READ_GITFILE_ERR_NOT_A_REPO) {\n>> +\t\t\t\treturn GIT_DIR_GITFILE_NOT_A_REPO;\n>> +\t\t\t} else if (error_code != READ_GITFILE_ERR_STAT_FAILED) {\n>>   \t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n>> +\t\t\t}\n>>   \t\t}\n>>   \t\tstrbuf_setlen(dir, offset);\n>>   \t\tif (gitdirenv) {\n>> @@ -1209,6 +1213,8 @@ const char *setup_git_directory_gently(int *nongit_ok)\n>>   \tstruct strbuf dir = STRBUF_INIT, gitdir = STRBUF_INIT;\n>>   \tconst char *prefix = NULL;\n>>   \tstruct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;\n>> +\tint die_on_error = !nongit_ok;\n>> +\tenum discovery_result discovery;\n>>     \t/*\n>>   \t * We may have read an incomplete configuration before\n>> @@ -1231,7 +1237,9 @@ const char *setup_git_directory_gently(int *nongit_ok)\n>>   \t\tdie_errno(_(\"Unable to read current working directory\"));\n>>   \tstrbuf_addbuf(&dir, &cwd);\n>>   -\tswitch (setup_git_directory_gently_1(&dir, &gitdir, 1)) {\n>> +\tdiscovery = setup_git_directory_gently_1(&dir, &gitdir, die_on_error);\n>> +\n>> +\tswitch (discovery) {\n>>   \tcase GIT_DIR_EXPLICIT:\n>>   \t\tprefix = setup_explicit_git_dir(gitdir.buf, &cwd, &repo_fmt, nongit_ok);\n>>   \t\tbreak;\n>> @@ -1259,6 +1267,16 @@ const char *setup_git_directory_gently(int *nongit_ok)\n>>   \t\t\t    dir.buf);\n>>   \t\t*nongit_ok = 1;\n>>   \t\tbreak;\n>> +\tcase GIT_DIR_GITFILE_NOT_A_REPO:\n>> +\t\tif (!nongit_ok)\n>> +\t\t\tdie(_(\"not a git repository: %s\"), dir.buf);\n>> +\t\t*nongit_ok = 1;\n>> +\t\tbreak;\n>> +\tcase GIT_DIR_INVALID_GITFILE:\n>> +\t\tif (!nongit_ok)\n>\n> Variable die_on_error could be used in two `if`s above.\n\nRe-reading my own code I think it's better just to drop die_on_error\nentirely and use !nongit_ok consistently, as the rest of the function\ndoes. What do yo think?\n"},{"id":"431039","messageId":"xmqq8s1xqcnq.fsf@gitster.g","threadId":"56135","inReplyTo":"87tukls7ax.fsf@evledraar.gmail.com","subject":"Re: [PATCH] setup: only die on invalid .git under RUN_SETUP","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-23T15:21:13Z","receivedAt":"2021-07-23T15:21:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n>>> +\tint die_on_error = !nongit_ok;\n>>> +\tenum discovery_result discovery;\n>>> ...    \t/*\n>>> +\tcase GIT_DIR_GITFILE_NOT_A_REPO:\n>>> +\t\tif (!nongit_ok)\n>>> +\t\t\tdie(_(\"not a git repository: %s\"), dir.buf);\n>>> +\t\t*nongit_ok = 1;\n>>> +\t\tbreak;\n>>> +\tcase GIT_DIR_INVALID_GITFILE:\n>>> +\t\tif (!nongit_ok)\n>>\n>> Variable die_on_error could be used in two `if`s above.\n>\n> Re-reading my own code I think it's better just to drop die_on_error\n> entirely and use !nongit_ok consistently, as the rest of the function\n> does. What do yo think?\n\nI think \"not X_ok\" means we do not consider X is OK, and agree with\nyou that the code is clearer without an extra indirection (I do not\nknow if you meant to address me, though).\n\n\n"},{"id":"431041","messageId":"87o8atrpz7.fsf@evledraar.gmail.com","threadId":"56135","inReplyTo":"3D8703D8-54E6-4CF0-9E9F-CCAFFAA8914C@gmail.com","subject":"Re: Bug: All git operations fail when .git contains a non-existent gitdir","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-07-23T15:47:18Z","receivedAt":"2021-07-23T15:47:56Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Jul 23 2021, Atharva Raykar wrote:\n\n> On 22-Jul-2021, at 18:43, Tom Cook <tom.k.cook@gmail.com> wrote:\n>> On Wed, Jul 21, 2021 at 11:59 PM brian m. carlson\n>> <sandals@crustytoothpaste.net> wrote:\n>>> \n>>> On 2021-07-21 at 09:17:36, Tom Cook wrote:\n>>>> What did you do before the bug happened? (Steps to reproduce your issue)\n>>>> \n>>>> Add a git submodule to a git repository.\n>>>> Overlay-mount that submodule to another place in the filesystem.\n>>>> Attempt any git operation in the overlay-mounted path.\n>>> \n>>> I'm not sure about what you mean by an overlay-mount operation.  Can you\n>>> provide some specific commands that we can run at a shell that reproduce\n>>> the issue?\n>>> --\n>>> brian m. carlson (he/him or they/them)\n>>> Toronto, Ontario, CA\n>> \n>> The easiest way to reproduce it is this:\n>> \n>> $ mkdir test\n>> $ cd test\n>> $ echo \"gitdir: /foo/bar\" > .git\n>> $ git ls-remote https://github.com/torvalds/linux\n>> \n>> We happen to use overlay mounts in our build system in a way that maps\n>> a git submodule from one place to another so that its \"gitdir\" is\n>> invalid and then attempt a `git ls-remote` from that location which\n>> unexpectedly fails.  But the above reproduces the problem well enough.\n>\n> 'ls-remote' needs a valid git directory for the case where the URL is not\n> explicitly supplied (to read the git config and learn the default remote).\n>\n> Making a special case for when an explicit URL is not given is not as\n> straightforward as it seems, because by the time 'ls-remote' even knows about\n> its arguments, it already takes a worktree prefix and sets up the environment,\n> for which a valid Git repository path is required.\n>\n> I am not too familiar with this area, and I don't know how feasible it is to\n> delay setting up the environment until after looking at the 'ls-remote'\n> arguments. At a cursory glance, it looks difficult to do without large\n> structural changes to the code.\n>\n> This might have been less of a problem with old-form submodules, where '.git'\n> was an actual directory, rather than a text file pointer [1], but newer\n> versions of Git discourage their usage.\n>\n> [1] https://git-scm.com/docs/gitsubmodules#_forms\n>\n> PS: we prefer bottom posting or inline replies :)\n>\n> ---\n> Pointers for others who might be interested in looking into this:\n>\n> The immediate cause of this seems to be 'setup.c:setup_gitdir_gently()' [2]\n> which calls 'setup_gitdir_gently_1()' with the 'die_on_error' argument set\n> to true. This function then calls 'read_gitfile_gently()' with the same flag,\n> which errors out when it runs 'is_git_directory()' [3], because the path in\n> the gitfile is not a valid repository.\n>\n> [2] https://github.com/git/git/blob/eb27b338a3e71c7c4079fbac8aeae3f8fbb5c687/setup.c#L1234\n> [3] https://github.com/git/git/blob/eb27b338a3e71c7c4079fbac8aeae3f8fbb5c687/setup.c#L784-L799\n\nThe timing of this reply after [1] suggests that you may not have seen\nthat patch that fixes this issue (sans Junio's outstanding comments on\nit).\n\nPerhaps your E-Mail client is forcing threading by subject only, not\nIn-Reply-To chains?\n\n1. https://lore.kernel.org/git/patch-1.1-fc26c46d39-20210722T140648Z-avarab@gmail.com/\n"},{"id":"431058","messageId":"11ECE7FC-F85E-4731-9EE8-4DAAC7A6A08A@gmail.com","threadId":"56135","inReplyTo":"87o8atrpz7.fsf@evledraar.gmail.com","subject":"Re: Bug: All git operations fail when .git contains a non-existent gitdir","fromName":"Atharva Raykar","fromEmail":"raykar.ath@gmail.com","sentAt":"2021-07-23T17:02:27Z","receivedAt":"2021-07-23T17:02:36Z","isPatch":false,"sender":{"key":"raykar.ath@gmail.com","avatar":"https://avatars.githubusercontent.com/u/24277692?v=4"},"body":"On 23-Jul-2021, at 21:17, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n> \n> [...]\n> The timing of this reply after [1] suggests that you may not have seen\n> that patch that fixes this issue (sans Junio's outstanding comments on\n> it).\n> \n> Perhaps your E-Mail client is forcing threading by subject only, not\n> In-Reply-To chains?\n> \n> 1. https://lore.kernel.org/git/patch-1.1-fc26c46d39-20210722T140648Z-avarab@gmail.com/\n\nYes, that is exactly what happened, and I noticed your patch a few minutes \nafter I sent that message."},{"id":"434043","messageId":"CAJDDKr7vXRxhwuTN=xFxqQkt_zt_Hpi3bYfc7rotcUAFjau9Sg@mail.gmail.com","threadId":"56135","inReplyTo":"CAFSh4Uyr5v9Ao-j0j7yO_HkUZSovBmSg7ADia7XCNZfsspFUYg@mail.gmail.com","subject":"Re: Bug: All git operations fail when .git contains a non-existent gitdir","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2021-08-30T00:38:21Z","receivedAt":"2021-08-30T00:39:02Z","isPatch":false,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Thu, Jul 22, 2021 at 6:16 AM Tom Cook <tom.k.cook@gmail.com> wrote:\n>\n> The easiest way to reproduce it is this:\n>\n> $ mkdir test\n> $ cd test\n> $ echo \"gitdir: /foo/bar\" > .git\n> $ git ls-remote https://github.com/torvalds/linux\n>\n> We happen to use overlay mounts in our build system in a way that maps\n> a git submodule from one place to another so that its \"gitdir\" is\n> invalid and then attempt a `git ls-remote` from that location which\n> unexpectedly fails.  But the above reproduces the problem well enough.\n\nI'm making a guess about your setup. To me it sounds like tweaking the\noverlayfs or bind mounts to also map the .git directory of the parent\nrepository (at just the right relative location inside the build\nenvironment) is the right approach towards making the .git-file links\nhappy.\n\nSubmodules point to the parent repository's .git/modules/ directory\nthese days so poking those paths through into the mapped file system\nis the solution that's going to lead to the least amount of\nhard-to-diagnose issues in the future.\n\nThe .git-file uses relative paths so it is possible to remap and\nrelocate them as you're doing, but it's going to require another mount\nin the right relative location to do it.\n\nIt seems doable, even for most edge cases. For example if you end up\nmapping some/deeply/nested/submod into /shallow inside your\ncontainer(?) and the .git-file contains\n\"../../../../.git/modules/submod\", that might seem impossible to\nresolve from a \"/shallow\" directory, but actually it's fine.\n\nThe solution in that situation is to map the parent repo's .git/ dir\nto the \"/.git\" filesystem root inside the build environment. Unix\ntreats \"/../../../../../../\" as equivalent to \"/\" so if the .git-file\nhas a bunch of \"../../../\" parent-dir traversing going on then it'll\nstill work from a one-level deep \"/shallow\" directory. The parent git\nrepo directory would be expected to be found at \"/.git\" (with a\n\"modules\" directory inside it) because the parent directory of \"/\" is\nalso \"/\".\n\nThis should mesh right in with the setup you described because you're\nalready using overlay mounts to synthesize a filesystem.\n\nThere are edge cases where this approach breaks down, though. One of\nthese cases is submodules inside submodules. Luckily it doesn't sound\nlike you have that problem.\n\nThere are patches now that special-case being able to run ls-remote\n<url> in a broken setup, but the real fix is to extend the build\nenvironment's filesystem view so that the submodules have access to\nthe underlying .git/modules/ storage.\n\nHopefully it's just a --mount or --volume docker(?) command-line flag\nthat has to be extended to poke the parent .git/ repository contents\ninto the environment. That's the avenue I would explore if I were\nalready down a path of using mount tricks.\n\nI hope that helps and maybe sparks an idea that you can run with. I\ncertainly don't mean it to be a dismissive, \"go change your setup\"\nkind of advice, but it would be interesting to know more details since\nit seems like achieving correctness is going to require doing that\nbased on the limited information in this thread about the specifics of\nthe setup.\n\nAlso, I wouldn't be surprised if, even after the \"git ls-remote\"\nrobustness fixes are applied, you'll just stumble onto the next git\ncommand that can only be resolved by mapping in the parent repo's .git\ndirectory.\n\n--\nDavid\n"},{"id":"434282","messageId":"CAFSh4UynPZ9YvS=_RZX2Grr38h=Ly2D830Q1XXJ0yqUj7z8GgA@mail.gmail.com","threadId":"56135","inReplyTo":"CAJDDKr7vXRxhwuTN=xFxqQkt_zt_Hpi3bYfc7rotcUAFjau9Sg@mail.gmail.com","subject":"Re: Bug: All git operations fail when .git contains a non-existent gitdir","fromName":"Tom Cook","fromEmail":"tom.k.cook@gmail.com","sentAt":"2021-08-31T14:16:02Z","receivedAt":"2021-08-31T14:16:16Z","isPatch":false,"sender":{"key":"tom.k.cook@gmail.com","avatar":null},"body":"On Mon, Aug 30, 2021 at 1:38 AM David Aguilar <davvid@gmail.com> wrote:\n\n> I hope that helps and maybe sparks an idea that you can run with. I\n> certainly don't mean it to be a dismissive, \"go change your setup\"\n> kind of advice, but it would be interesting to know more details since\n> it seems like achieving correctness is going to require doing that\n> based on the limited information in this thread about the specifics of\n> the setup.\n>\n> Also, I wouldn't be surprised if, even after the \"git ls-remote\"\n> robustness fixes are applied, you'll just stumble onto the next git\n> command that can only be resolved by mapping in the parent repo's .git\n> directory.\n\nWe map the source code into a build tree in order to build\nexecutables.  We don't expect to be using git on that build tree.  But\none of our executables is written in go and uses go modules.  In our\nsituation, `go mod download` was trying to use `git ls-remote` to\ninspect dependencies before downloading them with `git clone`,\nswallowing the git error and then dying horribly on something else\nthat appeared to be unrelated.  I raised a related bug report on `go`\n(I think I linked it elsewhere in this thread).  So for us, easily the\nworst thing about these two related bugs was just figuring out what\nwas wrong in the first place, and the go bug was definitely more\nsevere than the git bug.\n\nWe could modify our build system to make the git local repo valid but\nthere just doesn't seem any point - we never do anything to the local\nrepo in the build tree.  The next engineer who comes along and looks\nat our build system would inevitably ask why on earth we need to make\ngit work in the build tree and it would be a fair question.\n\nAs an end user, it still appears to make little sense for a git\noperation that will complete happily with no local git repo at all and\nwhich doesn't depend on the local repo in any way to nonetheless die\nbecause the local repo is not valid.  As a software engineer I can see\nwhy it's that way but as a user it's unhelpful.\n\nRegards,\nTom\n"}]}