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: Eric Sunshine <sunshine@sunshineco.com>,
	Anders Ro <anders.ronnbrant@gmail.com>,
	Git List <git@vger.kernel.org>, Heiko Voigt <hvoigt@hvoigt.net>,
	Johan Herland <johan@herland.net>,
	Mark Levedahl <mlevedahl@gmail.com>,
	"W. Trevor King" <wking@tremily.us>
Subject: Re: [PATCH/RFC] Pinning of submodules
Date: Tue, 08 Sep 2015 09:55:49 -0700	[thread overview]
Message-ID: <xmqq37yoam0a.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <55EDEFDC.3040605@web.de> (Jens Lehmann's message of "Mon, 7 Sep 2015 22:13:16 +0200")

Jens Lehmann <Jens.Lehmann@web.de> writes:

> Am 07.09.2015 um 01:43 schrieb Eric Sunshine:
>
>> My concern in asking was that some future person might come up with
>> another scenario which also wants to use a "magic value" and would
>> have to invent / arrive at another "illegal" representation. Hence, an
>> explicit setting might be more appropriate. However, as stated, I
>> don't even use submodules, so I may be far off the mark. I've cc'd a
>> few of the submodule maintainers with the hope that they will chime
>> in.
>
> Added Trevor to the CC, who is the original author of --remote (see
> 06b1abb5b).
>
> While I believe that adding such functionality makes perfect sense,
> I do not find it terribly obvious that setting the branch to '@' will
> make --remote skip this submodule. I wouldn't care so much if we'd
> only use this value internally, but this is user visible (and has to
> be set by the user if she wants to skip a submodule in --remote).
>
> Setting the branch to an empty value feels a bit more natural, but
> I'm not so sure our config handling supports that well (we seem to
> assume in quite some places that empty equals unset). So I tend to
> prefer a new option for that.

As I stare at both the code change and log message of 06b1abb5
(submodule update: add --remote for submodule's upstream changes,
2012-12-19), I cannot shake this feeling that the change to default
submodule.$name.branch to 'master' is doubly misdesigned.

The stated goal of users of --remote is to declare "This submodule
does not care what the top-level integrator happened to have seen at
the tip when the integration of the history of submodule.  It always
follows the tip of a specific branch at the upstream."

If we were to use any default, the only justification would be "the
users of --remote would want this mode of update to happen for all
submodules, and having to specify which specific branch to be
followed is too cumbersome".  But if that is the case, defaulting to
'master' does not make any sense---if it defaulted to 'HEAD' for
each submodule, it may have made some sense, as that is the usual
convention to denote which branch is the default branch in a
repository.

Also Anders' proposal refutes that "when --remote is used, all
submodules should behave that way" assumption.

I wonder if it is a sensible way forward to introduce a new
configuration variable 'submodule.remoteFallBackBranch' so that the
users can customize this line (near l.800 in git-submodule.sh)

     branch=$(get_submodule_config "$name" branch master)

The possible values for that new configuration value would be:

   - an empty string: disable "update --remote" for any submodule
     '$name' for which submodule.$name.branch is not set.

   - 'master': behave the same way as the current code; we can make
     this the default, when submodule.remoteFallBackBranch is unset,
     to ease transition.

   - any user-chosen branch name. On notable example may be 'HEAD',
     which 06b1abb5 (submodule update: add --remote for submodule's
     upstream changes, 2012-12-19) should have chosen as the
     default.

But I am not the target audience of "update --remote", so let's hear
what the real-world use cases are.

      parent reply	other threads:[~2015-09-08 16:55 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <55E78522.1030107@gmail.com>
2015-09-02 23:34 ` [PATCH/RFC] Pinning of submodules Anders Ro
2015-09-04  5:02   ` Eric Sunshine
2015-09-06 22:08     ` Anders Ro
2015-09-06 23:43       ` Eric Sunshine
2015-09-07 20:13         ` Jens Lehmann
2015-09-07 23:27           ` Anders Ro
2015-09-08 16:55           ` 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=xmqq37yoam0a.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Jens.Lehmann@web.de \
    --cc=anders.ronnbrant@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=hvoigt@hvoigt.net \
    --cc=johan@herland.net \
    --cc=mlevedahl@gmail.com \
    --cc=sunshine@sunshineco.com \
    --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.