Re: [PATCH v6 1/4] submodule: add get_submodule_config helper funtion

On Mon, Dec 03, 2012 at 11:30:12AM -0800, Junio C Hamano wrote:

quoted

+get_submodule_config()
+{

style (see CodingGuidelines):

	get_submodule_config ()	{

Will fix.  I was generally just copying the surrounding code.

quoted

+	name="$1"
+	option="$2"
+	default="$3"
+	value=$(git config submodule."$name"."$option")

This will get unwieldy quickly once we have submodule.$name.$var
that takes a boolean option, as there are different ways to spell
boolean and "git config --bool" is the way to ask for canonicalized
"true" or "false".

If we never query any boolean via this helper function, it is
obviously not an issue, though.

We do in my submodule.<name>.active branch, and I adjusted the
function in

  submodule: add submodule.<name>.active [1]

to add additional options passed through to `git config`.  You do have
to pick a default to use the extra options though.  If that becomes a
problem, I'd suggest extending git config itself to add a file above
or below the usual series of files.  Then get_submodule_config could
be

  git config --bottom-file .gitmodules submodule."$name"."$option"

or something, without needing a separate shell function.

On Mon, Dec 03, 2012 at 12:29:05PM -0800, Junio C Hamano wrote:

"W. Trevor King" [off-list ref] writes:

quoted

As an example to make this clearer:

  $ cat .gitmodules
  [submodule "sub1"]
    path = sub1
    url = git://example.com/sub1.git
    remote = remote1
    branch = branch1
    update-source = submodule-upstream
    update = rebase
  [submodule "sub2"]
  ...

Maybe it is just me but that "remote = remote1" sticks out like a
sore thumb.

If you are showing the .gitmodules file to be shared as hints to
project participants, why does it even need to have both URL and
remote1?

The remote name will probably only ever get configured locally in
.git/config.  I put it in (as a separate patch) mostly because Phil
suggested something like it:

On Thu, Nov 29, 2012 at 10:27:19PM -0500, W. Trevor King wrote:

On Thu, Nov 29, 2012 at 08:11:20PM -0500, Phil Hord wrote:

quoted

I've always felt that the "origin" defaults are broken and are simply
being ignored because most users do not trip over them.  But ISTR that
submodule commands use the remote indicated by the superproject's
current remote-tracking configuration, with a fallback to 'origin' if
there is none.  Sort of a "best effort" algorithm, I think.  Am I
remembering that wrong?

The current code uses a bare "git-fetch".  I'm not sure what that
defaults to if you're on a detached head.  If it bothers you, I'm fine
adding the submodule.<name>.remote option in v6.

and I hadn't heard any comments against it.  I'm not really attached
to that patch though, so feel free to leave it out (unless Phil chimes
in with stronger motivation?).

On Mon, Dec 03, 2012 at 12:29:05PM -0800, Junio C Hamano wrote:

But I do not see any reason to require or even suggest any local
nickname that is to be used to call the remote.  It really is a
local matter.  Why should .gitmodules have "remote = ..." line?

The idea for configuring it at all probably goes something like “I
don't like where upstream (origin) is taking this submodule.  I want
to follow *my* upstream, but I've called it something besides origin.
Look, a submodule.<name>.remote option!  Now I don't have to rename
my-remote→origin→original-remote.”  I don't think this will come up
all that often.

On the other hand, if you meant the above as an excerpt from
$GIT_DIR/config, it also does not make sense.  At that point, the
participant own the file and updating url to point at whatever
different repository without changing the remote name is sufficient.

Unless they still want to keep an the origin remote to track the
original submodule upstream.  Maybe they'll want to switch back to
following that remote later.  As I hinted at above, if they have
remotes `alice`, `bob`, etc., it's easier to flip between them by
configuring submodule.<name>.remote

  $ git config submodule.submod.remote alice

than it is to reconfigure the submodule's origin:

  $ cd submod
  $ git remote rename origin charlie
  $ git remote rename alice origin

It looks way over-engineered for unclear/dubious benefit.

I'm not going to push for submodule.<name>.remote.  Drop at will.

Cheers,
Trevor

[1]: https://github.com/wking/git/commit/fbe2d8419902700a6b0b40defaa5801811b887f7#L0R288

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