Re: [RFC] How to pass Git config command line instructions to Submodule commands?
- From
Jeff King <peff@peff.net>
- Date
- Apr 28, 2016, 12:05 UTC
- Message-ID
- <20160428120504.GA22399@sigill.intra.peff.net>
- In-Reply-To
- <20160428112511.GA11522@sigill.intra.peff.net>
On Thu, Apr 28, 2016 at 07:25:11AM -0400, Jeff King wrote:
Show 26 quoted lines
> > diff --git a/git-submodule.sh b/git-submodule.sh
> > index 2a84d7e..b02f5b9 100755
> > --- a/git-submodule.sh
> > +++ b/git-submodule.sh
> > @@ -199,7 +199,7 @@ sanitize_submodule_env()
> > {
> > sanitized_config=$(git submodule--helper sanitize-config)
> > clear_local_git_env
> > - GIT_CONFIG_PARAMETERS=$sanitized_config
> > + export GIT_CONFIG_PARAMETERS=$sanitized_config
> > }
>
> If you already have $GIT_CONFIG_PARAMETERS exported when we enter the
> function, then we should not need to re-export it when changing the
> value in the final line (the export bit is retained by the shell). But
> if you don't have it set already, then $sanitized_config must by
> definition be empty.
>
> So it should do the right thing without the export.
>
> At the same time, clear_local_git_env() will call "unset" on
> GIT_CONFIG_PARAMETERS. Which would clear the export bit, meaning the
> final line doesn't ever have any impact on sub-programs, and the whole
> thing is totally broken. But then, why does the test in t5550 pass?
>
> Confused...Ah. t5550 passes because it does not exercise this code path at all. We try a recursive clone, which calls "git submodule update --init", which does not seem to clear the config at all. So it works even without 14111fc49.
I tried to improve the test by adding git-fetch (note that I also fixed a bug where we use $HTTP_URL instead of $HTTPD_URL, and added some whitespace to make the result more readable):
diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh index 69ef388..6ec3ba3 100755 --- a/t/t5550-http-fetch-dumb.sh +++ b/t/t5550-http-fetch-dumb.sh @@ -103,12 +103,23 @@ test_expect_success 'cmdline credential config passes into submodules' ' git submodule add "$HTTPD_URL/auth/dumb/repo.git" sub && git commit -m "add submodule" ) && + set_askpass wrong pass@host && test_must_fail git clone --recursive super super-clone && rm -rf super-clone && + set_askpass wrong pass@host && - git -c "credential.$HTTP_URL.username=user@host" \ + git -c "credential.$HTTPD_URL.username=user@host" \ clone --recursive super super-clone && + expect_askpass pass user@host && + + set_askpass wrong pass@host && + test_must_fail git -C super-clone fetch --recurse-submodules && + + set_askpass wrong pass@host && + git -C super-clone \ + -c "credential.$HTTPD_URL.username=user@host" \ + fetch --recurse-submodules && expect_askpass pass user@host ' but that doesn't pass, even with the export fix! That's because fetch doesn't go through git-submodule at all; it calls "git fetch" itself, and uses local_repo_env, which clears the config. It needs to learn to use the same mechanism that sanitize_submodule_env() does. So AFAICT 14111fc49 is totally broken. It doesn't actually work for git-submodule (because of the missing export), nor for git-fetch (because that skips the shell script), and the one case we are testing already worked without it (but probably _should_ be sanitizing the config, so is buggy, too). -Peff