All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jens Lehmann <Jens.Lehmann@web.de>
Cc: "W. Trevor King" <wking@tremily.us>,
	Heiko Voigt <hvoigt@hvoigt.net>,
	Charlie Dyson <charlie@charliedyson.net>,
	git@vger.kernel.org
Subject: Re: git-submodule.sh respects submodule.$name.update in .git/config but not .gitmodules
Date: Thu, 12 Dec 2013 13:17:48 -0800	[thread overview]
Message-ID: <xmqqppp1sr9f.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <52AA072F.2000105@web.de> (Jens Lehmann's message of "Thu, 12 Dec 2013 19:57:51 +0100")

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".

      parent reply	other threads:[~2013-12-12 21:17 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqppp1sr9f.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Jens.Lehmann@web.de \
    --cc=charlie@charliedyson.net \
    --cc=git@vger.kernel.org \
    --cc=hvoigt@hvoigt.net \
    --cc=wking@tremily.us \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.