'git submodules update' ignores credential.helper config of the parent repository

'git submodules update' ignores credential.helper config of the parent repository

[Dmitry Neverov]

I'm checking out a repository in a non-interactive environment and
would like to disable interactive credential helpers. According to [1]
it can be done by specifying an empty helper in a local config:

  [credential]
    helper =

But the submodule update command ignores the helper specified in the
config of the parent repository. To reproduce it, fetch a repository
with submodules requiring authentication and run:

  git submodule init;
  git submodule sync;
  git submodule update;

the 'git submodule update' runs a default credential helper. The only
way to disable it is specify helper in command-line:

  git -c credential.helper= submodule update

Is it by design?

[1] http://marc.info/?l=git&m=147136396024768&w=2

Re: 'git submodules update' ignores credential.helper config of the parent repository

[Stefan Beller]

On Mon, Feb 27, 2017 at 5:33 AM, Dmitry Neverov
<dmitry.neverov@gmail.com> wrote:>
>   git -c credential.helper= submodule update
>
> Is it by design?

A similar question came up w.r.t. submodule configuration
recently. It is about url.<URLISH>.insteadOf[1] that is set
in the super project and is expected to work in the submodules.
More reading on some background there, as it is the very same
problem: Which configuration should propagate to the submodules,
how do we tell the users and can the user influence if some
articular settings are propagated?

For both these settings (url...insteadOf and the credentialHelper)
one might think that they absolutely should be propagated
to the submodules, but that may not be true; e.g. a submodule
might be hosted at a different hosting provider, needing a different
credentials setup. (The submodule might be an open source library
that you use, which may even require no credentials at all)

So I think we have to come up with a generic solution to respect
certain settings of the superproject instead of e.g. hard coding
credential.helper to be looked up in the superproject.

So the current proposal (in that mentioned thread) is
to borrow the idea from worktrees that have a similar problem:
split up the config into multiple files and each file applies to
a different worktree or in our case we would have
(A) a config file that applies to the superproject;
(B) a config file that applies to both superproject
     and all submodules
(C) and each submodule has its own config file as well.

---
For worktrees these multiple config files sounded like
the obvious solution, but I wonder if there was also
some bike shedding about other solutions?

I could imagine that we would want to have attributes
for specific configuration, e.g.:

--8<--
[core]
    repositoryformatversion = 0
    filemode = true
    bare = false
    logallrefupdates = true
[remote "origin"]
    url = git://github.com/gitster/git
    fetch = +refs/heads/*:refs/remotes/origin/*
[attribute "submodules"]
    read = true
# this will be read and respected by submodules as well:
[url."internal-git-miror"]
    insteadOf = github.com
[attribute "submodules"]
    read = false
# This (and the beginning of this file) will not be respected
# by submodules
[credential]
    helper =
-->8--

This would change the semantics of a config file as the attribute for
each setting depends on the location (was attribute.FOO.read =
{true, false} read before).

This would be read-compatible with older versions of Git, and it seems
as if it were write compatible as well. Just writing a new value with a specifc
attribute would be interesting to implement.

Thanks,
Stefan


[1] https://public-inbox.org/git/84fcb0bd-85dc-0142-dd58-47a04eaa7c2b@durchholz.org/

Re: 'git submodules update' ignores credential.helper config of the parent repository

[Jeff King]

On Mon, Feb 27, 2017 at 11:09:12AM -0800, Stefan Beller wrote:

> For worktrees these multiple config files sounded like
> the obvious solution, but I wonder if there was also
> some bike shedding about other solutions?
> 
> I could imagine that we would want to have attributes
> for specific configuration, e.g.:
> 
> --8<--
> [core]
>     repositoryformatversion = 0
>     filemode = true
>     bare = false
>     logallrefupdates = true
> [remote "origin"]
>     url = git://github.com/gitster/git
>     fetch = +refs/heads/*:refs/remotes/origin/*
> [attribute "submodules"]
>     read = true
> # this will be read and respected by submodules as well:
> [url."internal-git-miror"]
>     insteadOf = github.com
> [attribute "submodules"]
>     read = false
> # This (and the beginning of this file) will not be respected
> # by submodules
> [credential]
>     helper =
> -->8--
> 
> This would change the semantics of a config file as the attribute for
> each setting depends on the location (was attribute.FOO.read =
> {true, false} read before).

I'm not enthused by this, just because there is a hidden dependency
between attribute.* sections and other ones. They _look_ like regular
config keys, but they really aren't.

I have a feeling that something like this would create unwelcome corner
cases in the config-writer, which is otherwise does not have to care
about which existing section of a file it adds a key to.

-Peff

Re: 'git submodules update' ignores credential.helper config of the parent repository

[Stefan Beller]

On Tue, Feb 28, 2017 at 6:37 AM, Jeff King <peff@peff.net> wrote:
>>
>> This would change the semantics of a config file as the attribute for
>> each setting depends on the location (was attribute.FOO.read =
>> {true, false} read before).
>
> I'm not enthused by this, just because there is a hidden dependency
> between attribute.* sections and other ones. They _look_ like regular
> config keys, but they really aren't.

True.

> I have a feeling that something like this would create unwelcome corner
> cases in the config-writer, which is otherwise does not have to care
> about which existing section of a file it adds a key to.

Yeah the writer would become a lot more involved, if we're not going
the stupid way (add these sections for nearly all keys. that would be
a mess but easy to implement)

So I guess then we rather settle with multiple config files or a white/blacklist
of config options to propagate from the superproject to its submodules.

Re: 'git submodules update' ignores credential.helper config of the parent repository

[Jeff King]

On Tue, Feb 28, 2017 at 10:05:24AM -0800, Stefan Beller wrote:

> > I have a feeling that something like this would create unwelcome corner
> > cases in the config-writer, which is otherwise does not have to care
> > about which existing section of a file it adds a key to.
> 
> Yeah the writer would become a lot more involved, if we're not going
> the stupid way (add these sections for nearly all keys. that would be
> a mess but easy to implement)
> 
> So I guess then we rather settle with multiple config files or a white/blacklist
> of config options to propagate from the superproject to its submodules.

I'm still open to the idea that we simply improve the documentation to
make it clear that per-repo config really is per-repo, and is not shared
between super-projects and submodules. And then something like Duy's
proposed conditional config lets you set global config that flexibly
covers a set of repos.

-Peff

Re: 'git submodules update' ignores credential.helper config of the parent repository

[Stefan Beller]

On Tue, Feb 28, 2017 at 12:08 PM, Jeff King <peff@peff.net> wrote:
> On Tue, Feb 28, 2017 at 10:05:24AM -0800, Stefan Beller wrote:
>
>> > I have a feeling that something like this would create unwelcome corner
>> > cases in the config-writer, which is otherwise does not have to care
>> > about which existing section of a file it adds a key to.
>>
>> Yeah the writer would become a lot more involved, if we're not going
>> the stupid way (add these sections for nearly all keys. that would be
>> a mess but easy to implement)
>>
>> So I guess then we rather settle with multiple config files or a white/blacklist
>> of config options to propagate from the superproject to its submodules.
>
> I'm still open to the idea that we simply improve the documentation to
> make it clear that per-repo config really is per-repo, and is not shared
> between super-projects and submodules. And then something like Duy's
> proposed conditional config lets you set global config that flexibly
> covers a set of repos.

How would the workflow for that look like?
My naive thought on that is:

  (1)  $EDIT .git/config_to_be_included
  (2)  $ git config add-config-inclusion .git/config_to_be_included
  (3)  $ git submodule foreach git config add-inclusion-config
.git/config_to_be_included

which sounds a bit cumbersome to me.
So I guess we'd want some parts of that as part of another command, e.g.
(3) could be part of (2).

--
I am also open and willing to document this better; but were would
we want to put documentation? Obviously we would not want to put it
alongside each potentially useful config option to be inherited to
submodules. (that would imply repeating ourselves quite a lot in
the config man page).

I guess putting it into "man gitmodules" that I was writing tentatively
would make sense.
C.f
https://public-inbox.org/git/20161227234310.13264-4-sbeller@google.com/
(or search for "background story" in your emails)

Thanks,
Stefan

Re: 'git submodules update' ignores credential.helper config of the parent repository

[Jeff King]

On Tue, Feb 28, 2017 at 12:21:57PM -0800, Stefan Beller wrote:

> > I'm still open to the idea that we simply improve the documentation to
> > make it clear that per-repo config really is per-repo, and is not shared
> > between super-projects and submodules. And then something like Duy's
> > proposed conditional config lets you set global config that flexibly
> > covers a set of repos.
> 
> How would the workflow for that look like?
> My naive thought on that is:
> 
>   (1)  $EDIT .git/config_to_be_included
>   (2)  $ git config add-config-inclusion .git/config_to_be_included
>   (3)  $ git submodule foreach git config add-inclusion-config
> .git/config_to_be_included
> 
> which sounds a bit cumbersome to me.
> So I guess we'd want some parts of that as part of another command, e.g.
> (3) could be part of (2).

I think it would be more like:

  (1) $EDIT ~/.gitconfig-super
  (2) git config --global \
        includeIf.gitdir:/path/to/super.path .gitconfig-super

I know that is probably a bit more cumbersome to figure out than
treating the super/sub relationship in a special way. But I suspect for
a lot of cases that it actually ends up even better, because the
situation is more like:

  (1) $EDIT ~/.gitconfig-work
  (2) git config --global includeIf.gitdir:~/work.path .gitconfig-work

and then it covers all of your projects in ~/work, whether they are
super-projects, submodules, or regular repos.

> I am also open and willing to document this better; but were would
> we want to put documentation? Obviously we would not want to put it
> alongside each potentially useful config option to be inherited to
> submodules. (that would imply repeating ourselves quite a lot in
> the config man page).
> 
> I guess putting it into "man gitmodules" that I was writing tentatively
> would make sense.

Yeah, I think it is worth mentioning in "gitmodules", and probably in
git-config where we define per-repo config.

It may also be worth calling it out especially for url.insteadOf, just
because it is not clear there when the URL rewriting happens (it's not
insane to think that it happens in the super-project, that just doesn't
happen to be how it's implemented).

-Peff