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

2013-12-12 Thread Junio C Hamano
Jens Lehmann  writes:

> Am 12.12.2013 02:16, schrieb Junio C Hamano:
>> "W. Trevor King"  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 = " 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".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-12-12 Thread Jonathan Nieder
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-12-12 Thread W. Trevor King
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


signature.asc
Description: OpenPGP digital signature


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

2013-12-12 Thread Jens Lehmann
Am 12.12.2013 02:16, schrieb Junio C Hamano:
> "W. Trevor King"  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?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-12-11 Thread Junio C Hamano
"W. Trevor King"  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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-12-11 Thread W. Trevor King
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  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..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


signature.asc
Description: OpenPGP digital signature


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

2013-12-11 Thread Jens Lehmann
Am 10.12.2013 00:40, schrieb Junio C Hamano:
> Heiko Voigt  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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-12-09 Thread Junio C Hamano
Heiko Voigt  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?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-12-09 Thread Heiko Voigt
On Fri, Dec 06, 2013 at 03:48:46PM +, 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-12-06 Thread Charlie Dyson
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html