Re: [PATCH/RFC] Pinning of submodules

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Sep 6, 2015, 23:43 UTC
Message-ID: <CAPig+cRwAjTF6_rT8+nhbsZTYcZqneWSZ_LTCo9z2m5+SEGwWg@mail.gmail.com>
In-Reply-To: <55ECB956.1080909@gmail.com>

On Sun, Sep 6, 2015 at 6:08 PM, Anders Ro <anders.ronnbrant@gmail.com> wrote:

Show 21 quoted lines

> On 04/09/15 07:02, Eric Sunshine wrote:
>> On Wed, Sep 2, 2015 at 7:34 PM, Anders Ro <anders.ronnbrant@gmail.com> wrote:
>>> git-submodule.sh: pin submodule when branch name is '@'
>>>
>>> Setting branch name to '@' for a submodule will disable 'git submodule
>>> update --remote' calls for that specific submodule. I.e. instead of
>>> follow the unspecified default choice of master, nothing is being
>>> updated. This is useful when multiple submodules exist but not all
>>> should follow the remote branch head.
>>
>> With the disclaimer that I'm not a submodule user (to which the
>> answer might be obvious): What benefit is there in using a magic
>> value like this ("@") over, say, an explicit configuration setting?
>
> From what I have understood (not a submodule expert yet) the '@' is an
> invalid branch name and should therefore not collide with any current
> branches. My idea was to disable the '--remote' option when the user
> have explicitly set an invalid branch name to not modify any current
> behaviour. Though having an explicit option is of course more
> clarifying. The current behaviour though is that empty branch name means
> "follow master" which is somewhat unintuitive.

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.

Show 10 quoted lines

>>> +test_expect_success 'set branch name to "@" for submodule pinned' '
>>> +(cd super && git config --replace-all submodule.pinned.branch "@") &&
>>> +test "$(cd super && git config --get submodule.pinned.branch)" = "@"
>>
>> What is the last line testing? It appears to be testing the behavior
>> of git-config, which is outside the scope of this test script.
>>
>> Once combined, you can use test_config rather than git-config, since
>> test_config will ensure that the config setting is undone when the
>> test exits.

In light of this recent thread[1] which shows that test_when_finished() and, consequently, test_config() are non-functional in subshells, I have to retract the advice to use test_config() in this situation. Instead, at this time, it probably would be best to continue using "git-config" as you do here...

Show 5 quoted lines

>>> +test_expect_success 'remove branch name "@" for submodule pinned (unpin)' '
>>> +(cd super && git config --unset-all submodule.pinned.branch) &&
>>
>> If you take the above advice and use test_config in the previous
>> test, then this won't be needed.

and "git-config --unset-all" here.

Later, once [1] has landed in "master" (or perhaps "next"), this can be revisited and updated to use "test_config -C"[2].

[1]: http://thread.gmane.org/gmane.comp.version-control.git/277323/focus=277370 [2]: http://thread.gmane.org/gmane.comp.version-control.git/277323/focus=277372