patchgit-submodule add: Record branch name in .gitmodules

From: "W. Trevor King" <wking@tremily.us>

This removes a configuration step if you're trying to setup Ævar's

  $ git submodule foreach 'git checkout $(git config --file $toplevel/.gitmodules submodule.$name.branch) && git pull'

workflow from

  commit f030c96d8643fa0a1a9b2bd9c2f36a77721fb61f
  Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
  Date:   Fri May 21 16:10:10 2010 +0000

    git-submodule foreach: Add $toplevel variable

If you're not using that workflow, I see no harm in recording the branch used to determine the original submodule commit.

Signed-off-by: W. Trevor King <wking@tremily.us>
---
 Documentation/git-submodule.txt | 3 ++-
 git-submodule.sh                | 4 ++++
 t/t7400-submodule-basic.sh      | 1 +
 3 files changed, 7 insertions(+), 1 deletion(-)

On 2012-10-22 09:34, W. Trevor King wrote:

IMHO, the problem is that this works only for a very specific workflow. Normal git usage can very easily break this feature.

For instance, the module may later be updated to a commit in branch B instead of branch A. Unless you remember to also update .gitmodule, you have then inconsistent information.

A similar issue arises if branch A is deleted from the module later (for instance because it has been merged in the master branch and is not useful anymore). Then .gitmodule points to a non-existant branch. Same thing if a branch is renamed.

Last issue, the branch that exists in your local repository may not exist in someone else's repository, either because the branch is purely local, or because it has a different name on the remote repo.

I think a better place to store that kind of information is using git-notes. That way, if the branch is renamed or deleted, you can easily update the old notes to use the new name instead.

	Nahor

On Tue, Oct 23, 2012 at 12:16:22PM -0700, Nahor wrote:

But you're explicitly *using* the configured setting in

  git config --file $toplevel/.gitmodules submodule.$name.branch

That should be a reminder that the configuration is important, and you'll remember to change it. Plus, the text from git-pull should clearly display the branch you are pulling, which gives you a second change to notice if something is going wrong.

All of these are possible, and all would reqire manual intervention to pick out a new branch to follow. Plus, this patch is not even about the auto-pull application, it's about storing the initially cloned branch name. What you do with that name afterwards is up to you ;).

Interesting.  What would you attach the note too?

Trevor

On 2012-10-23 12:44, W. Trevor King wrote:

 From my experience with my colleagues at work, that's not going to 
happen. People are very good at forgetting to do something ;)

That's assuming that the user knows the branch that should be pulled and that he's paying attention to the output and not just quick-scanning for errors/warnings. Again, from my experience, that's not going to be the case.

Plus, there isn't always a human being behind a git-pull, e.g. build bots.

To the commits in the super-repo where a module branch is selected, i.e.:
- where a module was added
- where the module changed branch
- where a super-branch was merged and there was a conflict on the 
module's branch name

Am 23.10.2012 21:16, schrieb Nahor:

Except recording the branch name might raise expectations about what git will do with it. And as far as this patch goes, git won't do anything with it (yet).

Those are valid points. The next "git submodule update" will leave the submodule with a detached HEAD, making the branch configuration rather meaningless (except for your "git submodule foreach ..." use case of course).

You'll always face this kind of problems with commits too when using submodules, so I don't see that as a problem here.

I can't comment on that, as I have never been using notes so far.

But I'd rather see a patch series properly implementing the always-tip mode Ævar mentions in f030c96d86 (and which is requested by some users), especially the interesting parts: What should git record as commit in that case and how are "git status" and "git diff" going to handle submodules which shall follow a specific branch. I assume "git submodule update" is the right point in time to fetch that branch again and check out a newer branch tip if necessary, but should that commit be added to the superproject for that submodule automagically or not? This patch falls short of this, as it does the easy part but not the interesting ones ;-)

On Tue, Oct 23, 2012 at 10:36:44PM +0200, Jens Lehmann wrote:

As Phil pointed out, doing anything with this variable is ambiguous:

On Mon, Oct 22, 2012 at 06:03:53PM -0400, Phil Hord wrote:

On Tue, Oct 23, 2012 at 10:36:44PM +0200, Jens Lehmann wrote:

I agree that I'm not working on always-tip. I'm just making that easier. For people that aren't interested in always-tip submodules (e.g. Gerrit folks), this patch is still useful. It would certainly be possible to build an always-tip implementation on top of submodule.$name.branch (as Ævar's one-liner does), but that would be another patch series.

Personally, I think truly updates should be made explicitly, with a hand written commit message about why the updates are occuring. I also think that setting up and running auto-updates should be easy one-liners, not long, complicated ones ;).

On 2012-10-23 13:36, Jens Lehmann wrote:

 >> exist in someone else's repository, either because the branch is
 >> purely local, or because it has a different name on the remote repo.

Commits can't change or disappear during "normal" git operation (i.e. without using "git push -f" or "git branch -D"). A commit also has the same id in all the clones repository so there is no issue of a different name between the local and the remote repositories.

Am 24.10.2012 00:02, schrieb Nahor:

But if you forget to push a submodule commit it won't exist in someone else's repository and so he won't be able to update the submodule after checking out a commit in the superproject that records that unpushed submodule commit.

On Mon, Oct 22, 2012 at 06:03:53PM -0400, Phil Hord wrote:

For those to lazy to hunt down a reference, Gerrit uses submodule.$name.branch to

  “indicate the branch of a submodule project that when updated will
  trigger automatic update of its registered gitlink.” [1]

They also have some extra sauce:

  “The branch value could be '.' if the submodule project branch has
  the same name as the destination branch of the commit having
  gitlinks/.gitmodules file.
  …
  Any git submodules which are added and not have the branch field
  available in the .gitmodules file will not be subscribed by Gerrit
  to automatically update the superproject.”

Ugh, I don't want to retype the branch name whenever I do this.

<brainstorming>

How about -r/--record, with the recorded name being optional?

  --record-branch[=<recorded_name>]

This would satisfy Gerrit users that wanted to use '.', but also satisfy me with:

  git submodule add -rb=master foo bar

However, there is a change that people would see that, and then use

  git submodule add -r -b=master foo bar

which would checkout the HEAD from foo and store `-b=master` in submodule.$name.branch.

A more verbose, but less dangerous, option would be a boolean -r/--record that enables the recording of whatever was passed to -b/--branch. This looses the flexibility of running something like

  git submodule add --branch=master --record-branch=. foo bar

but the Gerrit folks have gotten along OK without any branch recording so far ;). They can keep setting '.' the same way they always have.

</brainstorming>

On a tangentially related note, it would be nice to set environment variables for each of the settings in submodule.$name during a foreach call. Then you could use

  git submodule foreach 'git checkout $branch && git pull'

Perhaps you'd want to blacklist/whitelist or downcase settings names to avoid things like

  [submodule "foo"]
        PATH = /my/rootkit/

but the update line is much cleaner.

Cheers, Trevor

[1]: https://gerrit.googlesource.com/gerrit/+/master/Documentation/user-submodules.txt

On Mon, Oct 22, 2012 at 6:55 PM, W. Trevor King <wking@tremily.us> wrote:

This is ugly as can be, but it works in my meagre testing. It defines "submodule_path=/my/rootkit/" for the above bit of .gitmodules. That is, it adds definitions for 'submodule_<var-name>' for each <var-name> in .gitmodules in submodule.$name.<var-name>, but <var-name> is lowercased and defanged (everything that's not already [a-z0-9] is replaced with underscore). For example,

  git submodule foreach 'echo $submodule_url'

--- >8 ---

On Wed, Oct 24, 2012 at 02:12:18PM -0400, Phil Hord wrote:

Is there a reason why you use printf instead of echo?

Also, this sort of name cleaning should probably live in a new function:

  clean_environment_variable()

or some such. Is there a git-utility-functions.sh library hiding somewhere in the source? ;)

On Wed, Oct 24, 2012 at 09:12:27PM -0400, W. Trevor King wrote:

Sadly, echo is not entirely portable for arbitrary content. We have run into problems with backslashes being interpreted differently (bash requires "echo -e" to interpret them, dash will always do it, and POSIX leaves it vague).

git-sh-setup?

-Peff