* git-submodule.sh respects submodule.$name.update in .git/config but not .gitmodules @ 2013-12-06 15:48 Charlie Dyson 2013-12-09 22:35 ` Heiko Voigt 0 siblings, 1 reply; 10+ messages in thread From: Charlie Dyson @ 2013-12-06 15:48 UTC (permalink / raw) To: git 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. 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. Perhaps the right thing to do is read from .git/config and fall back to .gitmodules using get_submodule_config(). Cheers, Charlie ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: git-submodule.sh respects submodule.$name.update in .git/config but not .gitmodules 2013-12-06 15:48 git-submodule.sh respects submodule.$name.update in .git/config but not .gitmodules Charlie Dyson @ 2013-12-09 22:35 ` Heiko Voigt 2013-12-09 23:40 ` Junio C Hamano 0 siblings, 1 reply; 10+ messages in thread From: Heiko Voigt @ 2013-12-09 22:35 UTC (permalink / raw) To: Charlie Dyson; +Cc: git, Jens Lehmann On Fri, Dec 06, 2013 at 03:48:46PM +0000, Charlie Dyson wrote: > 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: git-submodule.sh respects submodule.$name.update in .git/config but not .gitmodules 2013-12-09 22:35 ` Heiko Voigt @ 2013-12-09 23:40 ` Junio C Hamano 2013-12-11 22:26 ` Jens Lehmann 0 siblings, 1 reply; 10+ messages in thread From: Junio C Hamano @ 2013-12-09 23:40 UTC (permalink / raw) To: Heiko Voigt; +Cc: Charlie Dyson, git, Jens Lehmann Heiko Voigt <hvoigt@hvoigt.net> writes: > 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. I think you need to be a bit careful here. I do not think everything should blindly default to .gitmodules (otherwise there are obvious security implications as well as usability ones). > 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. And 'update', as it allows any custom way to run it, is unlikely to be allowed to be used blindly from .gitmodules, no? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: git-submodule.sh respects submodule.$name.update in .git/config but not .gitmodules 2013-12-09 23:40 ` Junio C Hamano @ 2013-12-11 22:26 ` Jens Lehmann 2013-12-11 22:44 ` W. Trevor King 0 siblings, 1 reply; 10+ messages in thread From: Jens Lehmann @ 2013-12-11 22:26 UTC (permalink / raw) To: Junio C Hamano, Heiko Voigt; +Cc: Charlie Dyson, git Am 10.12.2013 00:40, schrieb Junio C Hamano: > Heiko Voigt <hvoigt@hvoigt.net> writes: > >> 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. > > I think you need to be a bit careful here. I do not think > everything should blindly default to .gitmodules (otherwise there > are obvious security implications as well as usability ones). I believe everything except the URL should default to .gitmodules, for the same reasons we already do that for fetch and ignore [1]. But it should not do so blindly and take precautions that this only happens for safe values. The only current exception is the update setting, but I'd like to change that in two steps: 1) Teach git-submodule.sh to fall back to the update setting from .gitmodules if none is found in .git/config (more details below) 2) Wait some time and remove the copying on init later (to make life easier for people who are working on the same checkout with different versions of git). >> 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. > > And 'update', as it allows any custom way to run it, is unlikely to > be allowed to be used blindly from .gitmodules, no? Definitely not. And while thinking about it it might make sense to pass a list of allowed values for all configurations to the get_submodule_config() function, making sure that fallbacks are only used when they are safe. [1] http://thread.gmane.org/gmane.comp.version-control.git/161193/focus=161357 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Re: git-submodule.sh respects submodule.$name.update in .git/config but not .gitmodules 2013-12-11 22:26 ` Jens Lehmann @ 2013-12-11 22:44 ` W. Trevor King 2013-12-12 1:16 ` Junio C Hamano 0 siblings, 1 reply; 10+ messages in thread From: W. Trevor King @ 2013-12-11 22:44 UTC (permalink / raw) To: Jens Lehmann; +Cc: Junio C Hamano, Heiko Voigt, Charlie Dyson, git [-- Attachment #1: Type: text/plain, Size: 1741 bytes --] On Wed, Dec 11, 2013 at 11:26:17PM +0100, Jens Lehmann wrote: > Am 10.12.2013 00:40, schrieb Junio C Hamano: > > Heiko Voigt <hvoigt@hvoigt.net> writes: > >> 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. > > > > I think you need to be a bit careful here. I do not think > > everything should blindly default to .gitmodules (otherwise there > > are obvious security implications as well as usability ones). > > I believe everything except the URL should default to .gitmodules, > for the same reasons we already do that for fetch and ignore [1]. > But it should not do so blindly and take precautions that this only > happens for safe values. I think the "safety of .git/config vs. convenience of .gitmodules" balance is going to break down differently for different folks. That's why I proposed get_submodule_config() [1] and submodule.<name>.active as an activation marker [2,3]. Then users can move as much or as little of the submodule config from .gitmodules into .git/config to strike the balance they feel is appropriate. For safety, maybe the default `init` should copy *everything* into .git/config, after which users can remove stuff they'd like to delegate to .gitmodules. Cheers, Trevor [1]: http://article.gmane.org/gmane.comp.version-control.git/210930 [2]: http://article.gmane.org/gmane.comp.version-control.git/211014 [3]: http://article.gmane.org/gmane.comp.version-control.git/211042 -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: git-submodule.sh respects submodule.$name.update in .git/config but not .gitmodules 2013-12-11 22:44 ` W. Trevor King @ 2013-12-12 1:16 ` Junio C Hamano 2013-12-12 18:57 ` Jens Lehmann 0 siblings, 1 reply; 10+ messages in thread From: Junio C Hamano @ 2013-12-12 1:16 UTC (permalink / raw) To: W. Trevor King; +Cc: Jens Lehmann, Heiko Voigt, Charlie Dyson, git "W. Trevor King" <wking@tremily.us> writes: > For > safety, maybe the default `init` should copy *everything* into > .git/config, after which users can remove stuff they'd like to > delegate to .gitmodules. Copying everything into config is "be unsafe and inconvenient by default for everybody", isn't it? Folks who want safety are forced to inspect the resulting entries in their config file (which is more inconvenent if you compare with the design where nothing is copied and nothing dynamically defaults to what then-current .gitmodules happens to contain). Folks who trust those who update .gitmodules for them are forced to update their config every time upstream decides to use different settings in .gitmodules, because they have stale values in their config that mask what are in .gitmodules. I think the solution we want is to copy only minimum to the config (and that "minimum" may turn out to be "nothing"), and to default keys that are only absolutely safe to .gitmodules file. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: git-submodule.sh respects submodule.$name.update in .git/config but not .gitmodules 2013-12-12 1:16 ` Junio C Hamano @ 2013-12-12 18:57 ` Jens Lehmann 2013-12-12 19:14 ` W. Trevor King ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Jens Lehmann @ 2013-12-12 18:57 UTC (permalink / raw) To: Junio C Hamano, W. Trevor King; +Cc: Heiko Voigt, Charlie Dyson, git Am 12.12.2013 02:16, schrieb Junio C Hamano: > "W. Trevor King" <wking@tremily.us> writes: > >> For >> safety, maybe the default `init` should copy *everything* into >> .git/config, after which users can remove stuff they'd like to >> delegate to .gitmodules. > > Copying everything into config is "be unsafe and inconvenient by > default for everybody", isn't it? Folks who want safety are forced > to inspect the resulting entries in their config file (which is more > inconvenent if you compare with the design where nothing is copied > and nothing dynamically defaults to what then-current .gitmodules > happens to contain). Folks who trust those who update .gitmodules > for them are forced to update their config every time upstream > decides to use different settings in .gitmodules, because they have > stale values in their config that mask what are in .gitmodules. > > I think the solution we want is to copy only minimum to the config > (and that "minimum" may turn out to be "nothing"), and to default > keys that are only absolutely safe to .gitmodules file. I agree and will prepare a patch for that. What about teaching "git submodule sync" the "--url", "--update", "--fetch", "--ignore", "--branch" and "--all" options to allow the user to copy the current settings he wants from .gitmodules to .git/config (but only safe values of course)? Trevor, would you be ok to issue another command to copy everything to .git/config? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: git-submodule.sh respects submodule.$name.update in .git/config but not .gitmodules 2013-12-12 18:57 ` Jens Lehmann @ 2013-12-12 19:14 ` W. Trevor King 2013-12-12 19:23 ` Jonathan Nieder 2013-12-12 21:17 ` Junio C Hamano 2 siblings, 0 replies; 10+ messages in thread From: W. Trevor King @ 2013-12-12 19:14 UTC (permalink / raw) To: Jens Lehmann; +Cc: Junio C Hamano, Heiko Voigt, Charlie Dyson, git [-- Attachment #1: Type: text/plain, Size: 1121 bytes --] On Thu, Dec 12, 2013 at 07:57:51PM +0100, Jens Lehmann wrote: > Am 12.12.2013 02:16, schrieb Junio C Hamano: > > I think the solution we want is to copy only minimum to the config > > (and that "minimum" may turn out to be "nothing"), and to default > > keys that are only absolutely safe to .gitmodules file. > > I agree and will prepare a patch for that. > > What about teaching "git submodule sync" the "--url", "--update", > "--fetch", "--ignore", "--branch" and "--all" options to allow the > user to copy the current settings he wants from .gitmodules to > .git/config (but only safe values of course)? Trevor, would you be > ok to issue another command to copy everything to .git/config? Sounds good to me. The more stuff I can leave in .gitmodules, the happier I'll be ;). My second step will be removing “unsafe” values from .git/config where possible, and I'll trust myself to check .gitmodules after pulls. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: git-submodule.sh respects submodule.$name.update in .git/config but not .gitmodules 2013-12-12 18:57 ` Jens Lehmann 2013-12-12 19:14 ` W. Trevor King @ 2013-12-12 19:23 ` Jonathan Nieder 2013-12-12 21:17 ` Junio C Hamano 2 siblings, 0 replies; 10+ messages in thread From: Jonathan Nieder @ 2013-12-12 19:23 UTC (permalink / raw) To: Jens Lehmann Cc: Junio C Hamano, W. Trevor King, Heiko Voigt, Charlie Dyson, git Jens Lehmann wrote: > Am 12.12.2013 02:16, schrieb Junio C Hamano: >> I think the solution we want is to copy only minimum to the config >> (and that "minimum" may turn out to be "nothing"), and to default >> keys that are only absolutely safe to .gitmodules file. > > I agree and will prepare a patch for that. > > What about teaching "git submodule sync" the "--url", "--update", > "--fetch", "--ignore", "--branch" and "--all" options to allow the > user to copy the current settings he wants from .gitmodules to > .git/config (but only safe values of course)? Why would I want to copy settings to .git/config when they are safe enough to already be used directly from .gitmodules and the value from .gitmodules is the one chosen to make sense when working with the current revision? URLs are kind of special because generally the newest value is the most meaningful one, even when looking back over old history. Thanks, Jonathan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: git-submodule.sh respects submodule.$name.update in .git/config but not .gitmodules 2013-12-12 18:57 ` Jens Lehmann 2013-12-12 19:14 ` W. Trevor King 2013-12-12 19:23 ` Jonathan Nieder @ 2013-12-12 21:17 ` Junio C Hamano 2 siblings, 0 replies; 10+ messages in thread From: Junio C Hamano @ 2013-12-12 21:17 UTC (permalink / raw) To: Jens Lehmann; +Cc: W. Trevor King, Heiko Voigt, Charlie Dyson, git Jens Lehmann <Jens.Lehmann@web.de> writes: > Am 12.12.2013 02:16, schrieb Junio C Hamano: >> "W. Trevor King" <wking@tremily.us> writes: >> >>> For >>> safety, maybe the default `init` should copy *everything* into >>> .git/config, after which users can remove stuff they'd like to >>> delegate to .gitmodules. >> >> Copying everything into config is "be unsafe and inconvenient by >> default for everybody", isn't it? Folks who want safety are forced >> to inspect the resulting entries in their config file (which is more >> inconvenent if you compare with the design where nothing is copied >> and nothing dynamically defaults to what then-current .gitmodules >> happens to contain). Folks who trust those who update .gitmodules >> for them are forced to update their config every time upstream >> decides to use different settings in .gitmodules, because they have >> stale values in their config that mask what are in .gitmodules. >> >> I think the solution we want is to copy only minimum to the config >> (and that "minimum" may turn out to be "nothing"), and to default >> keys that are only absolutely safe to .gitmodules file. > > I agree and will prepare a patch for that. > > What about teaching "git submodule sync" the "--url", "--update", > "--fetch", "--ignore", "--branch" and "--all" options to allow the > user to copy the current settings he wants from .gitmodules to > .git/config (but only safe values of course)? An option per variable, which forms an unbounded set over time? From the syntax point of view, "--copy-config=url,update,..." probably is a better option, but I think that misses the point. Copying will freeze the choice in stone. Also, as long as the copying is deliberately done with such an option, copying potentially "unsafe" ones is perfectly fine. Reading and using what are not copied from the .gitmodules file _is_ a lot more severe security risk, so your "only safe ones, of course" should apply more heavily on that side. In principle, by default, we should use *nothing* from .gitmodules, and make exceptions on case by case basis, allowing only the safe ones. What is missing is a support for those like W. Trevor who trust what are in .gitmodules, and want to use values from there for ones we do not add to that default list of exceptions. They are not helped by such an option to say "copy these keys from .gitmodules to my config". They do not want to freeze values to what was in there at one point. They want to just follow along, whatever values happen to be set in the .gitmodules file of the day. So I _think_ a better approach would be to let users say something like: [submodule "frotz"] useInTreeSetting = update ignore in their .git/config file in the repository of the top-level project, to tell Git: When 'submodule.frotz.update' or 'submodule.frotz.ignore' is needed, please read from the .gitmodules file to grab the value for that setting. I trust the project as a whole to set a suitable value for me. and copy almost nothing to .git/config file upon 'init' time. If we were to go this route, I would envision that this new variable would be a list of keys to additionally allow defaulting to the values found in .gitmodules; if we hardcode 'branch', for example, as one of the keys that we fallback to .gitmodules, and if the user does *not* want to follow along to the project's recommendation, the user can just set "submodule.frotz.branch = <desired value>" in the .git/config file, and there is no need for the useIntreeSetting variable to support "Git by default may allow this variable 'branch' to be read from .gitmodules but I do not like that". ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-12-12 21:17 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-12-06 15:48 git-submodule.sh respects submodule.$name.update in .git/config but not .gitmodules Charlie Dyson 2013-12-09 22:35 ` Heiko Voigt 2013-12-09 23:40 ` Junio C Hamano 2013-12-11 22:26 ` Jens Lehmann 2013-12-11 22:44 ` W. Trevor King 2013-12-12 1:16 ` Junio C Hamano 2013-12-12 18:57 ` Jens Lehmann 2013-12-12 19:14 ` W. Trevor King 2013-12-12 19:23 ` Jonathan Nieder 2013-12-12 21:17 ` Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).