Re: git-submodule.sh respects submodule.$name.update in .git/config but not .gitmodules

From: Heiko Voigt <hvoigt@hvoigt.net>
Date: Dec 9, 2013, 22:35 UTC
Message-ID: <20131209223506.GF9606@sandbox-ub>
In-Reply-To: <CABYr9QtSeX=Euf73MZPq6suo+GpVA=f+tH73Ct0tP-3LYogh9w@mail.gmail.com>

On Fri, Dec 06, 2013 at 03:48:46PM +0000, Charlie Dyson wrote:

Show 17 quoted lines

> gitmodules(5) states that submodule.$name.update should be defined in
> .gitmodules. However in cmd_update() in git-submodule.sh, git config
> is used with "-f .gitmodules". Consequently this flag is only
> respected in .git/config
> 
> Tested against: 1.8.2.1 [sorry! I've checked the relevant bit of
> source and it's the same]
> 
> Steps to reproduce:
> $ git init
> $ git submodule add -b master someproject
> $ git config -f .gitmodules --add submodule.someproject.update merge
> $ # Go to someproject and commit something
> $ git submodule update --remote
> 
> The latter does not perform a merge, and behaviour is visibly
> different to adding --merge.

This is because of histerical reasons. When submodules were first implemented the notion was that .gitmodules should only be used as a starting point to initialise the configuration in .git/config when init was called.

This notion has changed in a way that only the url (by that the name) should be copied on init. The default for everything else should come from .gitmodules or gits own default.

The update configuration option was implemented before we realized that. So currently it is still copied when submodule init is called. The main reason is that we have not found the time to change that.

> I would submit a patch but I'm not completely sure what the behaviour
> would be - simply adding "-f .gitmodules" would hurt users that have
> adopted the practice of specifying their update preference in
> .git/config.

Well .gitmodules should only be the fallback if there is nothing in .git/config.

> Perhaps the right thing to do is read from .git/config and fall back
> to .gitmodules using get_submodule_config().

Yes IMO that is the right thing to do. If you implement it that way (and remove the automatic copying on init) you should not break peoples expectations. So if you want to go ahead please send a patch.

Cheers Heiko