git-submodule add: Add -r/--record option.

Quoted 4 lines · Nahor

On 2012-10-22 09:34, W. Trevor King wrote:
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.

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.

Quoted 3 lines · W. Trevor King

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

--record-branch[=<recorded_name>]

I like that just fine.

Quoted 5 lines

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

git submodule add -rb=master foo bar

Quoted 6 lines

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.

I don't think it would.

Though I see in rev-parse--parseopts that the use of optional-argument options "is discouraged".

From: "W. Trevor King" <[email protected]>

This option allows you to record a submodule.<name>.branch option in .gitmodules. Git does not currently use this configuration option for anything, but users have used it for several things, so it makes sense to add some syntactic sugar for initializing the value.

Current consumers:

Ævar uses this setting to designate the upstream branch for pulling submodule updates:

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

as he describes in

commit f030c96d8643fa0a1a9b2bd9c2f36a77721fb61f
Author: Ævar Arnfjörð Bjarmason <[email protected]>
Date: Fri May 21 16:10:10 2010 +0000

git-submodule foreach: Add $toplevel variable

Gerrit uses this setting to

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

I'm not clear on what that means, but they accept special values like '.', so their usage is not compatible with Ævar's proposal.

By remaining agnostic on the variable usage, this patch makes submodule setup more convenient for all parties.

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

Signed-off-by W. Trevor King <[email protected]>

---
 Documentation/git-submodule.txt | 11 ++++++++++-
 git-submodule.sh                | 19 ++++++++++++++++++-
 t/t7400-submodule-basic.sh      | 25 +++++++++++++++++++++++++
 3 files changed, 53 insertions(+), 2 deletions(-)

Documentation/git-submodule.txt +10 -1

index b4683bb..f9c74d6 100644
@@ -9,7 +9,7 @@ git-submodule - Initialize, update or inspect submodules
 SYNOPSIS
 --------
 [verse]
-'git submodule' [--quiet] add [-b branch] [-f|--force]
+'git submodule' [--quiet] add [-b branch] [--record[=<branch>]] [-f|--force]
 	      [--reference <repository>] [--] <repository> [<path>]
 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
 'git submodule' [--quiet] init [--] [<path>...]

@@ -209,6 +209,15 @@ OPTIONS
 --branch::
 	Branch of repository to add as submodule.
 
+-r::
+--record::
+  Record a branch name used as `submodule.<path>.branch` in
+  `.gitmodules` for future reference.  If you do not list an explicit
+  name here, the name given with `--branch` will be recorded.  If that
+  is not set either, `HEAD` will be recorded.  Because the branch name
+  is optional, you must use the equal-sign form (`-r=<branch>`), not
+  `-r <branch>`.
+
 -f::
 --force::
 	This option is only valid for add and update commands.

git-submodule.sh +18 -1

index ab6b110..bc33112 100755
@@ -5,7 +5,7 @@
 # Copyright (c) 2007 Lars Hjemli
 
 dashless=$(basename "$0" | sed -e 's/-/ /')
-USAGE="[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <repository> [<path>]
+USAGE="[--quiet] add [-b branch] [--record[=<branch>]] [-f|--force] [--reference <repository>] [--] <repository> [<path>]
    or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] init [--] [<path>...]
    or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]

@@ -20,6 +20,8 @@ require_work_tree
 
 command=
 branch=
+record_branch=
+record_branch_empty=
 force=
 reference=
 cached=

@@ -257,6 +259,12 @@ cmd_add()
 			branch=$2
 			shift
 			;;
+		-r | --record)
+			record_branch_empty=true
+			;;
+		-r=* | --record=*)
+			record_branch="${1#*=}"
+			;;
 		-f | --force)
 			force=$1
 			;;

@@ -328,6 +336,11 @@ cmd_add()
 	git ls-files --error-unmatch "$sm_path" > /dev/null 2>&1 &&
 	die "$(eval_gettext "'\$sm_path' already exists in the index")"
 
+	if test -z "$record_branch" && test "$record_branch_empty" = "true"
+	then
+		record_branch="${branch:=HEAD}"
+	fi
+
 	if test -z "$force" && ! git add --dry-run --ignore-missing "$sm_path" > /dev/null 2>&1
 	then
 		eval_gettextln "The following path is ignored by one of your .gitignore files:

@@ -366,6 +379,10 @@ Use -f if you really want to add it." >&2
 
 	git config -f .gitmodules submodule."$sm_path".path "$sm_path" &&
 	git config -f .gitmodules submodule."$sm_path".url "$repo" &&
+	if test -n "$branch"
+	then
+		git config -f .gitmodules submodule."$sm_path".branch "$record_branch"
+	fi &&
 	git add --force .gitmodules ||
 	die "$(eval_gettext "Failed to register submodule '\$sm_path'")"
 }

t/t7400-submodule-basic.sh +25 -0

index 5397037..88ae74c 100755
@@ -133,6 +133,7 @@ test_expect_success 'submodule add --branch' '
 	(
 		cd addtest &&
 		git submodule add -b initial "$submodurl" submod-branch &&
+		test -z "$(git config -f .gitmodules submodule.submod-branch.branch)" &&
 		git submodule init
 	) &&
 

@@ -211,6 +212,30 @@ test_expect_success 'submodule add with ./, /.. and // in path' '
 	test_cmp empty untracked
 '
 
+test_expect_success 'submodule add --record' '
+	(
+		cd addtest &&
+		git submodule add -r "$submodurl" submod-record-head &&
+		test "$(git config -f .gitmodules submodule.submod-record-head.branch)" = "HEAD"
+	)
+'
+
+test_expect_success 'submodule add --record --branch' '
+	(
+		cd addtest &&
+		git submodule add -r -b initial "$submodurl" submod-auto-record &&
+		test "$(git config -f .gitmodules submodule.submod-auto-record.branch)" = "initial"
+	)
+'
+
+test_expect_success 'submodule add --record=<name> --branch' '
+	(
+		cd addtest &&
+		git submodule add -r=final -b initial "$submodurl" submod-record &&
+		test "$(git config -f .gitmodules submodule.submod-record.branch)" = "final"
+	)
+'
+
 test_expect_success 'setup - add an example entry to .gitmodules' '
 	GIT_CONFIG=.gitmodules \
 	git config submodule.example.url git://example.com/init.git

I still fail to see what adding that functionality to the submodule command buys us (unless we also add code which really uses the branch setting). What's wrong with doing a simple:

git config -f .gitmodules submodule.<path>.branch <record_branch>

on the command line when you want to use the branch setting for your own purposes? You could easily wrap that into a helper script, no?

Quoted 6 lines

I still fail to see what adding that functionality to the submodule command buys us (unless we also add code which really uses the branch setting). What's wrong with doing a simple:

git config -f .gitmodules submodule.<path>.branch <record_branch>

Quoted 2 lines

on the command line when you want to use the branch setting for your own purposes? You could easily wrap that into a helper script, no?

Sure. But why maintain my own helper script if I can edit git-submodules.sh? It seems like a number of people are using this config option, and they generally store the same name in it that they use to create the submodule. This way I can save them time too.

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

Quoted 5 lines · Phil Hord

Some projects now use the 'branch' config value to record the tracking branch for the submodule. Some ascribe different meaning to the configuration if the value is given vs. undefined. For example, see the Gerrit submodule-subscription mechanism. This change will cause those workflows to behave differently than they do now.

Am 25.10.2012 02:53, schrieb W. Trevor King:

Quoted 10 lines · Jens Lehmann

Quoted 8 lines

I still fail to see what adding that functionality to the submodule command buys us (unless we also add code which really uses the branch setting). What's wrong with doing a simple:

git config -f .gitmodules submodule.<path>.branch <record_branch>

on the command line when you want to use the branch setting for your own purposes? You could easily wrap that into a helper script, no?

Quoted 4 lines

Sure. But why maintain my own helper script if I can edit git-submodules.sh? It seems like a number of people are using this config option, and they generally store the same name in it that they use to create the submodule. This way I can save them time too.

But people are already using the "branch" setting in *different* ways:

Am 23.10.2012 22:55, schrieb W. Trevor King:

Quoted 8 lines

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

Quoted 5 lines · Phil Hord

Some projects now use the 'branch' config value to record the tracking branch for the submodule. Some ascribe different meaning to the configuration if the value is given vs. undefined. For example, see the Gerrit submodule-subscription mechanism. This change will cause those workflows to behave differently than they do now.

I don't have a problem with the amount or complexity of the code being added, But by adding that option we may be giving the impression that it is officially sanctioned, or that it will be kept up to date by further submodule commands.

I added Shawn to the CC, maybe he can comment on how the "branch" setting is used in Gerrit and what he thinks about adding code to set that with "git submodule add -r <branch> ..." to core git.

Am 23.10.2012 22:55, schrieb W. Trevor King:

Quoted 8 lines

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

Quoted 5 lines · Phil Hord

Some projects now use the 'branch' config value to record the tracking branch for the submodule. Some ascribe different meaning to the configuration if the value is given vs. undefined. For example, see the Gerrit submodule-subscription mechanism. This change will cause those workflows to behave differently than they do now.

I don't have a problem with the amount or complexity of the code being added, But by adding that option we may be giving the impression that it is officially sanctioned, or that it will be kept up to date by further submodule commands. I added Shawn to the CC, maybe he can comment on how the "branch" setting is used in Gerrit and what he thinks about adding code to set that with "git submodule add -r <branch> ..." to core git.

I'm not clear on what that means, but they accept special values like '.', so their usage is not compatible with Ævar's proposal.

Looks like the Gerrit meaning is basically the same as Ævar's. Gerrit updates the parent project as if you had done:

  $ git submodule foreach 'git checkout $(git config --file
$toplevel/.gitmodules submodule.$name.branch) && git pull'
  $ git commit -a -m "Updated submodules"
  $ git push

Quoted 2 lines

I'm not clear on what that means, but they accept special values like '.', so their usage is not compatible with Ævar's proposal.

"." is a special value to mean use the parent project's branch name. So its more like this:

  $ git submodule foreach 'git checkout $(git --git-dir $toplevel/.git
read-ref HEAD | sed s,^refs/heads/,,) && git pull'
  $ git commit -a -m "Updated submodules"
  $ git push

We use "." in Gerrit to make branching an entire forest of projects easier. Setting up dev-fix-yy in the parent project will automatically track dev-fix-yy in each submodule.

Quoted 7 lines · Shawn Pearce

Looks like the Gerrit meaning is basically the same as Ævar's. Gerrit updates the parent project as if you had done:

  $ git submodule foreach 'git checkout $(git config --file
$toplevel/.gitmodules submodule.$name.branch) && git pull'
  $ git commit -a -m "Updated submodules"
  $ git push

Ah, good, then we *are* all using the option for the same thing.

Quoted 9 lines · W. Trevor King

Quoted 7 lines · Shawn Pearce

Looks like the Gerrit meaning is basically the same as Ævar's. Gerrit updates the parent project as if you had done:

  $ git submodule foreach 'git checkout $(git config --file
$toplevel/.gitmodules submodule.$name.branch) && git pull'
  $ git commit -a -m "Updated submodules"
  $ git push

Quoted 1 line

Ah, good, then we *are* all using the option for the same thing.

That makes me more comfortable. Your patch adds support for setting the variable initially. Does it need any special magic for maintenance, or is it something that would always be updated by hand?

Quoted 5 lines

Quoted 1 line

Ah, good, then we *are* all using the option for the same thing.

That makes me more comfortable. Your patch adds support for setting the variable initially. Does it need any special magic for maintenance, or is it something that would always be updated by hand?

Everyone we've heard from so far interprets the setting as “pull from $branch in the remote repository $url to update the submodule”. With Phil's export, that would become

  $ git submodule foreach 'git checkout "$submodule_branch" && git pull'
  $ git commit -a -m "Updated submodules"
  $ git push

As Nahor mentioned on the 23rd, there are a number of ways that the upstream branch could disappear, but Git has no way to know what the new branch setting should be. This means that even if we make “pull from $branch” interpretation official, we still couldn't do anything slick about updating it. So, yes, it will be updated by hand.

Can you send an updated version of the patch that summarizes the situation in the commit message?

Quoted 2 lines · Jeff King

Can you send an updated version of the patch that summarizes the situation in the commit message?

Sure. Should I include Phil's $submodule_<var-name> export, or would you rather have that be a separate series?

Quoted 6 lines · W. Trevor King

Quoted 2 lines · Jeff King

Can you send an updated version of the patch that summarizes the situation in the commit message?

Sure. Should I include Phil's $submodule_<var-name> export, or would you rather have that be a separate series?

I think it probably makes sense as a separate patch in the same series, since it is meant to support the same workflows.

I am not sure it is sufficient as-is, though. It does not seem to ever clear variables, only set them, which means that values could leak across iterations of the loop, [...] E.g., when
the first submodule has submodule.*.foo set but the second one does not, you will still end up with $submodule_foo set when you process the second one.

or down to recursive calls.

Quoted 6 lines

I am not sure it is sufficient as-is, though. It does not seem to ever clear variables, only set them, which means that values could leak across iterations of the loop, [...] E.g., when
the first submodule has submodule.*.foo set but the second one does not, you will still end up with $submodule_foo set when you process the second one.

Good point. That should not happen.

Quoted 1 line

or down to recursive calls.

Frankly, I consider that to be a feature. However, I can see how it would be considered inconsistent in many ways, so it's probably best to squash it. :-\

Quoted 7 lines · Phil Hord

Quoted 6 lines

I am not sure it is sufficient as-is, though. It does not seem to ever clear variables, only set them, which means that values could leak across iterations of the loop, [...] E.g., when
the first submodule has submodule.*.foo set but the second one does not, you will still end up with $submodule_foo set when you process the second one.

Quoted 7 lines

Good point. That should not happen.

Quoted 1 line

or down to recursive calls.

Frankly, I consider that to be a feature. However, I can see how it would be considered inconsistent in many ways, so it's probably best to squash it. :-\

I think it would depend on the semantics of the option. Some options would probably make sense to apply recursively, and some not.

Maybe instead of blindly converting config into the environment, it should forward or clear specific known-meaning config.

Quoted 2 lines

Maybe instead of blindly converting config into the environment, it should forward or clear specific known-meaning config.

Well, that's where we started. I was aiming for the more generic "never needs updating" direction.

git-submodule.sh Use -f if you really want to add it." >&2 · line 388

Quoted 11 lines · W. Trevor King

git-submodule.sh +4 -0

@@ -366,6 +379,10 @@ Use -f if you really want to add it." >&2
 
 	git config -f .gitmodules submodule."$sm_path".path "$sm_path" &&
 	git config -f .gitmodules submodule."$sm_path".url "$repo" &&
+	if test -n "$branch"
+	then
+		git config -f .gitmodules submodule."$sm_path".branch "$record_branch"
+	fi &&
 	git add --force .gitmodules ||
 	die "$(eval_gettext "Failed to register submodule '\$sm_path'")"
 }

Should the if condition test that $record_branch is not the empty string instead of testing that $branch is not the empty string? It seems like this will set submodule."$sm_path".branch to the empty string if -b is specified and no -r option is specified.

By remaining agnostic on the variable usage, this patch makes submodule setup more convenient for all parties.

Quoted 2 lines · W. Trevor King

By remaining agnostic on the variable usage, this patch makes submodule setup more convenient for all parties.

I personally do not think "remaining agnostic on the usage" is a good thing, at least for any option to commands at the higher level on the stack, such as "git submodule". I am afraid that giving an easier way to set up a variable with undefined semantics may make setup more confusing for all parties. One party gives one specific meaning to the field, while another party uses it for something slightly different.

I would not object to "git config submodule.$name.branch $value", on the other hand. "git config" can be used to set a piece of data that has specific meaning, but as a low-level tool, it is not _limited_ to variables that have defined meaning.

Quoted 2 lines · W. Trevor King

By remaining agnostic on the variable usage, this patch makes submodule setup more convenient for all parties.

I personally do not think "remaining agnostic on the usage" is a good thing, at least for any option to commands at the higher level on the stack, such as "git submodule". I am afraid that giving an easier way to set up a variable with undefined semantics may make setup more confusing for all parties. One party gives one specific meaning to the field, while another party uses it for something slightly different.

I would not object to "git config submodule.$name.branch $value", on the other hand. "git config" can be used to set a piece of data that has specific meaning, but as a low-level tool, it is not _limited_ to variables that have defined meaning.

I think we should agree on a behavior for this option and implement it the same time when add learns about it. When we were discussing floating submodules as an important option for the gerrit people I already started to implement a proof of concept. Please have a look here:

https://github.com/hvoigt/git/commits/hv/floating_submodules

  $ git submodule pull-branch

Quoted 5 lines · Heiko Voigt

I think we should agree on a behavior for this option and implement it the same time when add learns about it. When we were discussing floating submodules as an important option for the gerrit people I already started to implement a proof of concept. Please have a look here:

Quoted 1 line

https://github.com/hvoigt/git/commits/hv/floating_submodules

After skimming through this, something like

  $ git submodule update --pull

would probably be better than introducing a new command:

Quoted 1 line

  $ git submodule pull-branch

I think "floating submodules" is a misleading name for this feature though, since the checkout SHA is explicitly specified. We're just making it more convenient to explicitly update the SHA. How about "tracking submodules"?

Quoted 4 lines · Junio C Hamano

I would not object to "git config submodule.$name.branch $value", on the other hand. "git config" can be used to set a piece of data that has specific meaning, but as a low-level tool, it is not _limited_ to variables that have defined meaning.

This is what I'm doing now:

  $ git submodule add -b <branch> <repo> <path>
  $ git config --file .gitmodules submodule.<path>.branch <branch>
  $ git submodule foreach 'git checkout $(git config --file $toplevel/.gitmodules submodule.$name.branch) && git pull'

With my second patch (Phil's config export), that becomes

  $ git submodule add -b <branch> <repo> <path>
  $ git config --file .gitmodules submodule.<path>.branch <branch>
  $ git submodule foreach 'git checkout $submodule_branch && git pull'

With my first patch, that becomes

  $ git submodule add -rb <branch> <repo> <path>
  $ git submodule foreach 'git checkout $submodule_branch && git pull'

This seems pretty useful to me,...

The change seems to think "branch" is the _only_ thing the user might want to record per submodule upon "git submodule add".

On the other hand, if this were one small part of a series to define the "tip following mode" where (at least)

(1) "git submodule update [$path]" makes sure that the checkout of the submodule at $path matches the commit at the tip of the branch named by submodule.$name.branch in .gitmodules of the superproject, instead of the commit that is recorded in the index of the superproject; and

(2) "git diff [$path]" and friends in the superproject compares the HEAD of the checkout of the submodule at $path with the tip of the branch named by submodule.$name.branch in .gitmodules of the superproject, instead of the commit that is recorded in the index of the superproject.

and the option were called something like "--follow-branch=$branch", …

On Sun, Nov 11, 2012 at 02:33:45AM -0800, Junio C Hamano wrote:
In order to avoid losing (or creating) local-only submodule commits, I'll probably bail (with an error) on non-fast-forward pulls. Can anyone else think of other safety concerns?

This means that I'll probably drop Phil's $submodule_* export in v4, because the only explicit use we have for it is this branch tracking. I still think it is a useful idea, but it may not be useful enough to be worth the complexity.

Quoted 6 lines

(2) "git diff [$path]" and friends in the superproject compares the HEAD of the checkout of the submodule at $path with the tip of the branch named by submodule.$name.branch in .gitmodules of the superproject, instead of the commit that is recorded in the index of the superproject.

Hmm. ???git diff??? compares the working tree with the local HEAD (just a SHA for submodules), so I don't think it should care about the status of a remote branch. This sounds like you want something like:

  $ git submodule foreach 'git diff origin/$submodule_branch'

Perhaps this is enough motivation for keeping $submodule_* exports?

Quoted 2 lines

and the option were called something like "--follow-branch=$branch", ???

From what I have heard of projects using this: They usually still have

I'll replace -r/--record with --follow-branch in v4.

Quoted 7 lines · W. Trevor King

Quoted 1 line

  $ git submodule pull-branch

I think "floating submodules" is a misleading name for this feature though, since the checkout SHA is explicitly specified. We're just making it more convenient to explicitly update the SHA. How about "tracking submodules"?

Until now we have always called this workflow floating submodules. I imaging since the submodule floats to the newest revision (whatever the user chooses that to be) instead of staying at the recorded sha1.

"tracking submodules" sounds strange to me since the term tracked in git is mainly used in combination with exact recorded history (e.g. tracking branch). Since it is about *not* checking out the recorded sha1 but something that can change I think that could cause confusion.

I think floating is a more unambiguous term and already known on the list.

Quoted 15 lines

Quoted 6 lines

(2) "git diff [$path]" and friends in the superproject compares the HEAD of the checkout of the submodule at $path with the tip of the branch named by submodule.$name.branch in .gitmodules of the superproject, instead of the commit that is recorded in the index of the superproject.

Hmm. ???git diff??? compares the working tree with the local HEAD (just a SHA for submodules), so I don't think it should care about the status of a remote branch. This sounds like you want something like:

  $ git submodule foreach 'git diff origin/$submodule_branch'

Perhaps this is enough motivation for keeping $submodule_* exports?

Quoted 2 lines

Quoted 2 lines

and the option were called something like "--follow-branch=$branch", ???

I am not sure if hiding changes to the recorded SHA1 from the user is such a useful thing. In the first step I would like it if it was kept simple and only the submodule update machinery learned to follow a branch. If that results in local changes that should be shown. The user is still in charge of recording the updated SHA1 in his commit.

From what I have heard of projects using this: They usually still have something that records the SHA1s on a regular basis. Thinking further, why not record them in git? We could add an option to update which creates such a commit.

Quoted 18 lines · Heiko Voigt

Quoted 17 lines

Quoted 6 lines

(2) "git diff [$path]" and friends in the superproject compares the HEAD of thecheckout of the submodule at $path with the tip of the branch named by submodule.$name.branch in .gitmodules of the superproject, instead of the commit that is recorded in the index of the superproject.

Hmm. ???git diff??? compares the working tree with the local HEAD (just a SHA for submodules), so I don't think it should care about the status of a remote branch. This sounds like you want something like:

  $ git submodule foreach 'git diff origin/$submodule_branch'

Perhaps this is enough motivation for keeping $submodule_* exports?

Quoted 2 lines

and the option were called something like "--follow-branch=$branch", ???

Quoted 5 lines

I am not sure if hiding changes to the recorded SHA1 from the user is such a useful thing. In the first step I would like it if it was kept simple and only the submodule update machinery learned to follow a branch. If that results in local changes that should be shown. The user is still in charge of recording the updated SHA1 in his commit.

I understand what you're warning against here, or what it has to do with "git diff".

Quoted 4 lines

From what I have heard of projects using this: They usually still have something that records the SHA1s on a regular basis. Thinking further, why not record them in git? We could add an option to update which creates such a commit.

I think it's best to have users craft their own commit messages explaining why the branch was updated. That said, an auto-generated hint (a la "git merge") would probably be a useful extra feature.

Quoted 25 lines · W. Trevor King

Quoted 23 lines · Heiko Voigt

Quoted 7 lines

Quoted 6 lines

(2) "git diff [$path]" and friends in the superproject compares the HEAD of thecheckout of the submodule at $path with the tip of the branch named by submodule.$name.branch in .gitmodules of the superproject, instead of the commit that is recorded in the index of the superproject.

Quoted 10 lines

Hmm. ???git diff??? compares the working tree with the local HEAD (just a SHA for submodules), so I don't think it should care about the status of a remote branch. This sounds like you want something like:

  $ git submodule foreach 'git diff origin/$submodule_branch'

Perhaps this is enough motivation for keeping $submodule_* exports?

Quoted 2 lines

and the option were called something like "--follow-branch=$branch", ???

I am not sure if hiding changes to the recorded SHA1 from the user is such a useful thing. In the first step I would like it if it was kept simple and only the submodule update machinery learned to follow a branch. If that results in local changes that should be shown. The user is still in charge of recording the updated SHA1 in his commit.

Quoted 2 lines

I understand what you're warning against here, or what it has to do with "git diff".

Is there a not missing here?

What I am talking about is the suggestion of Junio. Instead of showing a diff if the SHA1 is different we show a diff if the checkout in the worktree is different from the tip of the configured branch. That would hide the fact that a submodule has changed during a submodule update operation.

Quoted 4 lines

From what I have heard of projects using this: They usually still have something that records the SHA1s on a regular basis. Thinking further, why not record them in git? We could add an option to update which creates such a commit.

I think it's best to have users craft their own commit messages explaining why the branch was updated. That said, an auto-generated hint (a la "git merge") would probably be a useful extra feature.

Quoted 8 lines · W. Trevor King

Quoted 4 lines

From what I have heard of projects using this: They usually still have something that records the SHA1s on a regular basis. Thinking further, why not record them in git? We could add an option to update which creates such a commit.

I think it's best to have users craft their own commit messages explaining why the branch was updated. That said, an auto-generated hint (a la "git merge") would probably be a useful extra feature.

I am not quite sure I agree. When the project says "Use the tip of 'bar' branch for the submodule 'foo'" at the top-level, does an individual user who is not working on the submodule 'foo' but merely is using it have any clue as to why the submodule's 'foo' branch 'foo' moved, or does he necessarily even care?

Now, since somebody created the top-level commit you have just pulled and checked out, other people may have worked on submodule 'foo' [*1*]. What should happen on "git submodule update foo"?

What should appear in "git diff"? The working tree taken as a whole is different from what the superproject's commit describes (which is the state the person who created the superproject wanted to record) even though this user does not have anything to do with the change at 'foo' from the recorded commit to the current tip of 'bar'. What would his description for the reason why the branch was updated?

Quoted 6 lines · Junio C Hamano

"W. Trevor King" <[email protected]> writes:
...

Quoted 3 lines

I think it's best to have users craft their own commit messages explaining why the branch was updated. That said, an auto-generated hint (a la "git merge") would probably be a useful extra feature.

Quoted 5 lines

I am not quite sure I agree. When the project says "Use the tip of 'bar' branch for the submodule 'foo'" at the top-level, does an individual user who is not working on the submodule 'foo' but merely is using it have any clue as to why the submodule's 'foo' branch 'foo' moved, or does he necessarily even care?

If he doesn't care, why is he updating the submodule gitlink?

Quoted 7 lines · W. Trevor King

Quoted 6 lines · Junio C Hamano

"W. Trevor King" <[email protected]> writes:
...

Quoted 3 lines

I think it's best to have users craft their own commit messages explaining why the branch was updated. That said, an auto-generated hint (a la "git merge") would probably be a useful extra feature.

Quoted 7 lines

Quoted 5 lines

I am not quite sure I agree. When the project says "Use the tip of 'bar' branch for the submodule 'foo'" at the top-level, does an individual user who is not working on the submodule 'foo' but merely is using it have any clue as to why the submodule's 'foo' branch 'foo' moved, or does he necessarily even care?

If he doesn't care, why is he updating the submodule gitlink?

He may not be updating the gitlink with "git add foo" at the top-level superproject level. He is just using that submodule as part of the larger whole as he is working on either the top-level or some other submodule. And checkout of 'foo' is necessary in the working tree for him to work in the larger context of the project, and 'foo' is set to float at the tip of its 'bar' branch. And that checkout results in a commit that is different from the commit the gitlink suggests, perhaps because somebody worked in 'foo' submodule and advanced the tip of branch 'bar'.

Or are you envisioning that such a check-out will and should show a local difference at the submodule 'foo' by leaving the index of the top-level superproject unchanged,

and the user should refrain from using "git commit -a" to avoid having to describe the changes made on the 'bar' branch in the meantime in his top-level commit?

That is certainly fine by me (I am no a heavy submodule user to begin with), but I am not sure if that is useful and helpful to the submodule users.

The superproject gitlink should only be updated after

  $ git submodule update --pull

A plain

  $ git submodule update

would still checkout the previously-recorded SHA, not the new upstream tip.

Quoted 6 lines · W. Trevor King

The superproject gitlink should only be updated after

  $ git submodule update --pull

A plain

Quoted 4 lines

  $ git submodule update

would still checkout the previously-recorded SHA, not the new upstream tip.

Hrm, doesn't it make the "float at the tip of a branch" mode useless, though?

I am still a little bit undecided about an automatically crafted commit.

At $dayjob we sometimes update submodules to their tip without any superproject changes just to make sure we use the newest version. Most of the time the commit messages are along the lines of "updated submodule x to master".

On one hand Junio is right that the person updating to the newest submodule stuff has no clue what to write in this message.

The benefit is that Ævar's

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

becomes

  $ git submodule update --pull

Quoted 13 lines · Junio C Hamano

Quoted 10 lines · W. Trevor King

The superproject gitlink should only be updated after

  $ git submodule update --pull

A plain

  $ git submodule update

would still checkout the previously-recorded SHA, not the new upstream tip.

Quoted 2 lines

Hrm, doesn't it make the "float at the tip of a branch" mode useless, though?

How about having a branch config option and reusing our submodule.$name.update option for specifying whether the user wants to always float to the tip of the branch?

1. If submodule.$name.update is pull it would checkout the specified tip.

2. If submodule.$name.update is checkout or none it would do the usual thing and you need to specify --pull to get the tip.

So currently I am more on the "have an automatically generated commit message" side. Its in a similar corner like merge commits, that are also generated, for me. We could have it as the default and a --no-commit option (like merge) for people that want to stage submodules individually.

Quoted 4 lines · W. Trevor King

The benefit is that Ævar's

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

Quoted 3 lines

becomes

  $ git submodule update --pull

There is an important question still unanswered here for me: How does the submodule get the configuration what the local branch tracks on the remote side?

Quoted 3 lines · Heiko Voigt

There is an important question still unanswered here for me: How does the submodule get the configuration what the local branch tracks on the remote side?

A good point ;). I'm actaully using submodule.<name>.branch to store the submodule's local branch name. The remote branch name for the pull is implicit, and defaults to something setup according to branch.autosetupmerge (I think). If you want to get more complicated than this, we'll probably have to add submodule.<name>.branch and submodule.<name>.remote sections to augment the submodule.<name>.branch setting. I'm not sure this is worth it.

Quoted 17 lines · Heiko Voigt

Quoted 15 lines · Junio C Hamano

Quoted 10 lines · W. Trevor King

The superproject gitlink should only be updated after

  $ git submodule update --pull

A plain

  $ git submodule update

would still checkout the previously-recorded SHA, not the new upstream tip.

Hrm, doesn't it make the "float at the tip of a branch" mode useless, though?

Quoted 3 lines

How about having a branch config option and reusing our submodule.$name.update option for specifying whether the user wants to always float to the tip of the branch?

I'm adding "update --pull" as one of the update options in v4, which I am writing up as we speak ;).

We could add

  $ git submodule update --branch

to checkout the gitlinked SHA1 as submodule.<name>.branch in each of the submodules, leaving the submodules on the .gitmodules-configured branch. Effectively (for each submodule):

  $ git branch -f $branch $sha1
  $ git checkout $branch

From: "W. Trevor King" <[email protected]>

This allows users to override the .gitmodules value with a per-repository value.

Signed-off-by W. Trevor King <[email protected]>

---
 Documentation/config.txt   |  9 +++++----
 git-submodule.sh           |  7 +++++++
 t/t7400-submodule-basic.sh | 18 ++++++++++++++++++
 3 files changed, 30 insertions(+), 4 deletions(-)

Documentation/config.txt +5 -4

index 11f320b..1304499 100644
@@ -1994,10 +1994,11 @@ status.submodulesummary::
 submodule.<name>.path::
 submodule.<name>.url::
 submodule.<name>.update::
-	The path within this project, URL, and the updating strategy
-	for a submodule.  These variables are initially populated
-	by 'git submodule init'; edit them to override the
-	URL and other values found in the `.gitmodules` file.  See
+submodule.<name>.branch::
+	The path within this project, URL, the updating strategy, and the
+	local branch name for a submodule.  These variables are initially
+	populated by 'git submodule init'; edit them to override the URL and
+	other values found in the `.gitmodules` file.  See
 	linkgit:git-submodule[1] and linkgit:gitmodules[5] for details.
 
 submodule.<name>.fetchRecurseSubmodules::

git-submodule.sh +7 -0

index 6eed008..c51b6ae 100755
@@ -505,6 +505,13 @@ cmd_init()
 		test -n "$(git config submodule."$name".update)" ||
 		git config submodule."$name".update "$upd" ||
 		die "$(eval_gettext "Failed to register update mode for submodule path '\$sm_path'")"
+
+		# Copy "branch" setting when it is not set yet
+		branch="$(git config -f .gitmodules submodule."$name".branch)"
+		test -z "$branch" ||
+		test -n "$(git config submodule."$name".branch)" ||
+		git config submodule."$name".branch "$branch" ||
+		die "$(eval_gettext "Failed to register branch for submodule path '\$sm_path'")"
 	done
 }
 

t/t7400-submodule-basic.sh +18 -0

index fc08647..3dc8237 100755
@@ -236,6 +236,24 @@ test_expect_success 'submodule add --local-branch=<name> --branch' '
 	)
 '
 
+test_expect_success 'init should register submodule branch in .git/config' '
+	(
+		cd addtest &&
+		git submodule init &&
+		test "$(git config submodule.submod-follow.branch)" = "final"
+	)
+'
+
+test_expect_success 'local config should override .gitmodules branch' '
+	(
+		cd addtest &&
+		rm -fr submod-follow &&
+		git config submodule.submod-follow.branch initial
+		git submodule init &&
+		test "$(git config submodule.submod-follow.branch)" = "initial"
+	)
+'
+
 test_expect_success 'setup - add an example entry to .gitmodules' '
 	GIT_CONFIG=.gitmodules \
 	git config submodule.example.url git://example.com/init.git

Am 26.11.2012 22:00, schrieb W. Trevor King:

Quoted 4 lines

From: "W. Trevor King" <[email protected]>

This allows users to override the .gitmodules value with a per-repository value.

[snip problems]. You should use the branch setting from .git/config if present and fall back to the branch setting from .gitmodules if not, which would enable the user to have her own setting if she doesn't like what upstream provides but would still enable others to follow different submodule branches in different superproject branches.

From: "W. Trevor King" <[email protected]>

This allows users to checkout the current superproject-recorded-submodule-sha as a branch, avoiding the detached head state that the standard submodule update creates. This may be useful for the existing --rebase/--merge workflows which already avoid detached heads.

It is also useful if you want easy tracking of upstream branches. The particular upstream branch to be tracked is configured locally with .git/modules/<name>/config. With the new option Ævar's suggested

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

reduces to a

  $ git submodule update --branch

after each supermodule .gitmodules edit, and a

  $ git submodule foreach 'git pull'

whenever you feel like updating the submodules. Your still on you're own to commit (or not) the updated submodule hashes in the superproject's .gitmodules.

Signed-off-by W. Trevor King <[email protected]>

---
 Documentation/git-submodule.txt | 20 +++++++++++------
 git-submodule.sh                | 48 +++++++++++++++++++++++++++++----------
 t/t7406-submodule-update.sh     | 50 ++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 98 insertions(+), 20 deletions(-)

Documentation/git-submodule.txt +6 -6

index d0b4436..34392a1 100644
@@ -13,7 +13,7 @@ SYNOPSIS
 	      [-f|--force] [--reference <repository>] [--] <repository> [<path>]
 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
 'git submodule' [--quiet] init [--] [<path>...]
-'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
+'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--branch] [--rebase]
 	      [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]
 	      [commit] [--] [<path>...]

@@ -136,11 +136,11 @@ init::
 
 update::
 	Update the registered submodules, i.e. clone missing submodules and
-	checkout the commit specified in the index of the containing repository.
-	This will make the submodules HEAD be detached unless `--rebase` or
-	`--merge` is specified or the key `submodule.$name.update` is set to
-	`rebase`, `merge` or `none`. `none` can be overridden by specifying
-	`--checkout`.
+	checkout the commit specified in the index of the containing
+	repository.  This will make the submodules HEAD be detached unless
+	`--branch`, `--rebase`, `--merge` is specified or the key
+	`submodule.$name.update` is set to `branch`, `rebase`, `merge` or
+	`none`. `none` can be overridden by specifying `--checkout`.
 +
 If the submodule is not yet initialized, and you just want to use the
 setting as stored in .gitmodules, you can automatically initialize the

@@ -207,7 +207,13 @@ OPTIONS
 

git-submodule.sh +6 -1

 		then
 			subforce=$force
 			# If we don't already have a -f flag and the submodule has never been checked out

@@ -650,16 +654,21 @@ Maybe you want to use 'update --init'?")"
 			case ";$cloned_modules;" in
 			*";$name;"*)
 				# then there is no local change to integrate
-				update_module= ;;
+				case "$update_module" in
+					rebase|merge)
+						update_module=
+						;;
+				esac
+				;;
 			esac

git-submodule.sh +0 -0

 				;;
 			merge)
 				command="git merge"

@@ -674,15 +683,30 @@ Maybe you want to use 'update --init'?")"
 				;;

Documentation/git-submodule.txt OPTIONS · line 216

Quoted 10 lines · W. Trevor King

-b::
--branch::

Documentation/git-submodule.txt +7 -1

-	Branch of repository to add as submodule.
+	When used with the add command, gives the branch of repository to
+	add as submodule.
++
+When used with the update command, checks out a branch named
+`submodule.<name>.branch` (as set by `--local-branch`) pointing at the
+current HEAD SHA-1.  This is useful for commands like `update
+--rebase` that do not work on detached heads.

Since you are reusing this option for update it further convinces me that reusing it for add makes sense and simplifies the logic for users.

I think an optional argument for --branch would be nice in the update case:

	$ git submodule update --branch=master

would then allow a user that has not configured anything (except the branch tracking info in the submodule of course) to pull all submodules master branches.

Quoted 8 lines

git-submodule.sh +0 -0

index c51b6ae..28eb4b1 100755
@@ -627,7 +631,7 @@ Maybe you want to use 'update --init'?")"
 			die "$(eval_gettext "Unable to find current revision in submodule path '\$sm_path'")"
 		fi
 

git-submodule.sh Maybe you want to use 'update --init'?")" · line 634

Quoted 2 lines

git-submodule.sh +1 -1

-		if test "$subsha1" != "$sha1" -o -n "$force"
+		if test "$subsha1" != "$sha1" -o -n "$force" -o "$update_module" = "branch"

As said before I think separating your code from the current update logic will simplify the handling below.

git-submodule.sh Maybe you want to use 'update --init'?")" · line 671

Quoted 9 lines

git-submodule.sh +0 -0

 			must_die_on_failure=
 			case "$update_module" in
 			rebase)

git-submodule.sh +2 -2

 				command="git rebase"
-				die_msg="$(eval_gettext "Unable to rebase '\$sha1' in submodule path '\$sm_path'")"
+				die_msg="$(eval_gettext "Unable to rebase '\$sha1' in submodule path '\$sm_path'")"	
 				say_msg="$(eval_gettext "Submodule path '\$sm_path': rebased into '\$sha1'")"
-				must_die_on_failure=yes
+			must_die_on_failure=yes

Please always cleanup whitespace changes.

git-submodule.sh Maybe you want to use 'update --init'?")" · line 704

Quoted 8 lines

git-submodule.sh +3 -4

 			then
-				die_with_status 2 "$die_msg"
-			else
-				err="${err};$die_msg"
-				continue
+				if (clear_local_git_env; cd "$sm_path" &&
+					git branch -f "$branch" "$sha1" &&
+					git checkout "$branch")

You wrote in earlier emails that you wanted to protect the user from non-fastforward changes. So I would expect a

	$ git pull --ff-only

BTW, I am more and more convinced that an automatically manufactured commit on update with --branch should be the default.

BTW, I am more and more convinced that an automatically manufactured commit on update with --branch should be the default. What do other think? Sascha raised a concern that he would not want this, but as far as I understood he let the CI-server do that so I see no downside to natively adding that to git. People who want to manually craft those commits can still amend the generated commit. Since this is all about helping people keeping their submodules updated why not go the full way?

+ test "a" = "b"

Quoted 15 lines · Heiko Voigt

Quoted 5 lines · W. Trevor King

Because you need to recurse through submodules for update --branch even if "$subsha1" == "$sha1", I had to amend the conditional controlling that block. This broke one of the existing tests, which I "fixed" in patch 4. I think a proper fix would involve rewriting

Quoted 8 lines

  (clear_local_git_env; cd "$sm_path" &&
   ( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) &&
    test -z "$rev") || git-fetch)) ||
  die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"

but I'm not familiar enough with rev-list to want to dig into that yet. If feedback for the earlier three patches is positive, I'll work up a clean fix and resubmit.

Quoted 6 lines

You probably need to separate your handling here. The comparison of the currently checked out sha1 and the recorded sha1 is an optimization which skips unnecessary fetching in case the submodules commits are already correct. This code snippet checks whether the to be checked out sha1 is already local and also skips the fetch if it is. We should not break that.

Agreed. However, determining if the target $sha1 is local should have nothing to do with the current checked out $subsha1.

From: "W. Trevor King" <[email protected]>

Quoted 8 lines · W. Trevor King

We could add

  $ git submodule update --branch

to checkout the gitlinked SHA1 as submodule.<name>.branch in each of the submodules, leaving the submodules on the .gitmodules-configured branch. Effectively (for each submodule):

Quoted 2 lines

  $ git branch -f $branch $sha1
  $ git checkout $branch

I haven't gotten any feedback on this as an idea, but perhaps someone will comment on it as a patch series ;).

Changes since v3:

* --record=??? is now --local-branch=???
* Dropped patches 2 ($submodule_ export) and 3 (motivating documentation)
* Added local git-config overrides of .gitmodules' submodule.<name>.branch
* Added submodule update --branch

Because you need to recurse through submodules for update --branch even if "$subsha1" == "$sha1", I had to amend the conditional controlling that block. This broke one of the existing tests, which I "fixed" in patch 4. I think a proper fix would involve rewriting

  (clear_local_git_env; cd "$sm_path" &&
   ( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) &&
    test -z "$rev") || git-fetch)) ||
  die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"

but I'm not familiar enough with rev-list to want to dig into that yet. If feedback for the earlier three patches is positive, I'll work up a clean fix and resubmit.

Quoted 14 lines · W. Trevor King

From: "W. Trevor King" <[email protected]>

Quoted 10 lines · W. Trevor King

We could add

  $ git submodule update --branch

to checkout the gitlinked SHA1 as submodule.<name>.branch in each of the submodules, leaving the submodules on the .gitmodules-configured branch. Effectively (for each submodule):

  $ git branch -f $branch $sha1
  $ git checkout $branch

Quoted 2 lines

I haven't gotten any feedback on this as an idea, but perhaps someone will comment on it as a patch series ;).

I am not sure I understand you correctly. You are suggesting that the branch option as an alias for the registered SHA1 in the superproject?

I though the goal of your series was that you want to track submodules branch which come from the remote side?

I would think more of some convention like:

	$ git checkout -t origin/$branch

when first initialising the submodule with e.g.

	$ git submodule update --init --branch

Then later calls of

	$ git submodule update --branch

would have a branch configured to pull from. I imagine that results in a similar behavior gerrit is doing on the server side?

Quoted 6 lines

Changes since v3:

* --record=??? is now --local-branch=???
* Dropped patches 2 ($submodule_ export) and 3 (motivating documentation)
* Added local git-config overrides of .gitmodules' submodule.<name>.branch
* Added submodule update --branch

I would prefer if we could squash all these commits together into one since it seems to me one logical step, using the new variable for update belongs together with its configuration on initialization.

How about reusing the -b|--branch option for add? Since we only change the behavior when submodule.$name.update is set to branch it seems reasonable to me. Opinions?

Quoted 5 lines

Because you need to recurse through submodules for update --branch even if "$subsha1" == "$sha1", I had to amend the conditional controlling that block. This broke one of the existing tests, which I "fixed" in patch 4. I think a proper fix would involve rewriting

Quoted 8 lines

  (clear_local_git_env; cd "$sm_path" &&
   ( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) &&
    test -z "$rev") || git-fetch)) ||
  die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"

but I'm not familiar enough with rev-list to want to dig into that yet. If feedback for the earlier three patches is positive, I'll work up a clean fix and resubmit.

You probably need to separate your handling here. The comparison of the currently checked out sha1 and the recorded sha1 is an optimization which skips unnecessary fetching in case the submodules commits are already correct. This code snippet checks whether the to be checked out sha1 is already local and also skips the fetch if it is. We should not break that.

On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote:
The v4 series leaves the remote branch amigious, but it helps you point the local branch at the right hash so that future calls to

  $ git submodule foreach 'git pull'

can use the branch's .git/modules/<name>/config settings.

Quoted 10 lines

I would think more of some convention like:

	$ git checkout -t origin/$branch

when first initialising the submodule with e.g.

	$ git submodule update --init --branch

Then later calls of

Quoted 4 lines

	$ git submodule update --branch

would have a branch configured to pull from. I imagine that results in a similar behavior gerrit is doing on the server side?

That sounds like it's doing pretty much the same thing. Can you think of a test that would distinguish it from my current v4 implementation?

Quoted 3 lines

How about reusing the -b|--branch option for add? Since we only change the behavior when submodule.$name.update is set to branch it seems reasonable to me. Opinions?

That was the approach I used in v1, but people were concerned that we would be stomping on previously unclaimed config space. Since noone has pointed out other uses besides Gerrit's very similar case, I'm not sure if that is still an issue.

Quoted 14 lines

Quoted 5 lines

Because you need to recurse through submodules for update --branch even if "$subsha1" == "$sha1", I had to amend the conditional controlling that block. This broke one of the existing tests, which I "fixed" in patch 4. I think a proper fix would involve rewriting

Quoted 8 lines

  (clear_local_git_env; cd "$sm_path" &&
   ( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) &&
    test -z "$rev") || git-fetch)) ||
  die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"

but I'm not familiar enough with rev-list to want to dig into that yet. If feedback for the earlier three patches is positive, I'll work up a clean fix and resubmit.

Quoted 6 lines

You probably need to separate your handling here. The comparison of the currently checked out sha1 and the recorded sha1 is an optimization which skips unnecessary fetching in case the submodules commits are already correct. This code snippet checks whether the to be checked out sha1 is already local and also skips the fetch if it is. We should not break that.

Agreed. However, determining if the target $sha1 is local should have nothing to do with the current checked out $subsha1.

Quoted 7 lines · W. Trevor King

On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote:
The v4 series leaves the remote branch amigious, but it helps you point the local branch at the right hash so that future calls to

  $ git submodule foreach 'git pull'

can use the branch's .git/modules/<name>/config settings.

But IMO thats the functionality which should be implemented in submodule update and not left to the user.

Quoted 15 lines

Quoted 14 lines

I would think more of some convention like:

	$ git checkout -t origin/$branch

when first initialising the submodule with e.g.

	$ git submodule update --init --branch

Then later calls of

	$ git submodule update --branch

would have a branch configured to pull from. I imagine that results in a similar behavior gerrit is doing on the server side?

Quoted 2 lines

That sounds like it's doing pretty much the same thing. Can you think of a test that would distinguish it from my current v4 implementation?

Well the main difference is that gerrit is automatically updating the superproject AFAIK. I would like it if we could implement the same workflow support in the submodule script. It seems to me that this is already proven to be useful workflow.

https://github.com/hvoigt/git/commits/hv/floating_submodules_draft

Quoted 8 lines

Quoted 3 lines

How about reusing the -b|--branch option for add? Since we only change the behavior when submodule.$name.update is set to branch it seems reasonable to me. Opinions?

That was the approach I used in v1, but people were concerned that we would be stomping on previously unclaimed config space. Since noone has pointed out other uses besides Gerrit's very similar case, I'm not sure if that is still an issue.

Could you point me to that mail? I cannot seem to find it in my archive.

Some projects now use the 'branch' config value to record the tracking branch for the submodule. Some ascribe different meaning to the configuration if the value is given vs. undefined. For example, see the Gerrit submodule-subscription mechanism. This change will cause those workflows to behave differently than they do now.

I do like the idea, but I wish it had a different name for the recording. Maybe --record-branch=${BRANCH} as an extra switch so the action is explicitly requested.

Quoted 22 lines · W. Trevor King

Quoted 20 lines · Heiko Voigt

Quoted 5 lines

Because you need to recurse through submodules for update --branch even if "$subsha1" == "$sha1", I had to amend the conditional controlling that block. This broke one of the existing tests, which I "fixed" in patch 4. I think a proper fix would involve rewriting

Quoted 8 lines

  (clear_local_git_env; cd "$sm_path" &&
   ( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) &&
    test -z "$rev") || git-fetch)) ||
  die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"

but I'm not familiar enough with rev-list to want to dig into that yet. If feedback for the earlier three patches is positive, I'll work up a clean fix and resubmit.

You probably need to separate your handling here. The comparison of the currently checked out sha1 and the recorded sha1 is an optimization which skips unnecessary fetching in case the submodules commits are already correct. This code snippet checks whether the to be checked out sha1 is already local and also skips the fetch if it is. We should not break that.

Quoted 2 lines

Agreed. However, determining if the target $sha1 is local should have nothing to do with the current checked out $subsha1.

See my draft or the diff below for an illustration of the splitup.

[snip diff]

Hi,

Quoted 7 lines · W. Trevor King

On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote:
The v4 series leaves the remote branch amigious, but it helps you point the local branch at the right hash so that future calls to

  $ git submodule foreach 'git pull'

can use the branch's .git/modules/<name>/config settings.

But IMO thats the functionality which should be implemented in submodule update and not left to the user.

Quoted 15 lines

Quoted 14 lines

I would think more of some convention like:

    $ git checkout -t origin/$branch

when first initialising the submodule with e.g.

    $ git submodule update --init --branch

Then later calls of

    $ git submodule update --branch

would have a branch configured to pull from. I imagine that results in a similar behavior gerrit is doing on the server side?

Quoted 2 lines

That sounds like it's doing pretty much the same thing. Can you think of a test that would distinguish it from my current v4 implementation?

Well the main difference is that gerrit is automatically updating the superproject AFAIK. I would like it if we could implement the same workflow support in the submodule script. It seems to me that this is already proven to be useful workflow.

Quoted 11 lines · Heiko Voigt

Quoted 7 lines · W. Trevor King

On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote:
The v4 series leaves the remote branch amigious, but it helps you point the local branch at the right hash so that future calls to

  $ git submodule foreach 'git pull'

can use the branch's .git/modules/<name>/config settings.

But IMO thats the functionality which should be implemented in submodule update and not left to the user.

Then you might need submodule.<name>.local-branch, submodule.<name>.remote-repository, and submodule.<name>.remote-branch to configure

  $ git checkout submodule.<name>.local-branch
  $ git pull submodule.<name>.remote-repository submodule.<name>.remote-branch

and this would ignore the $sha1 stored in the gitlink (which all of the other update commands use). This ignoring-the-$sha1 bit made me think that a built-in pull wasn't a good fit for 'submodule update'. Maybe if it went into a new 'submodule pull'? Then users have a clear distinction:

* 'update' to push superproject $sha1 changes into the submodules
* 'pull' to push upstream-branch changes into the submodules

Quoted 15 lines

Quoted 15 lines

Quoted 14 lines

I would think more of some convention like:

  $ git checkout -t origin/$branch

when first initialising the submodule with e.g.

  $ git submodule update --init --branch

Then later calls of

  $ git submodule update --branch

would have a branch configured to pull from. I imagine that results in a similar behavior gerrit is doing on the server side?

Quoted 7 lines

Quoted 2 lines

That sounds like it's doing pretty much the same thing. Can you think of a test that would distinguish it from my current v4 implementation?

Well the main difference is that gerrit is automatically updating the superproject AFAIK. I would like it if we could implement the same workflow support in the submodule script. It seems to me that this is already proven to be useful workflow.

Ah, sorry, I meant the configuring which remote branch you were pulling from happens at submodule initialization (via .git/modules/…) for both your workflow and my v4.

You're right that having a builtin pull is different from my v4.

Quoted 1 line

https://github.com/hvoigt/git/commits/hv/floating_submodules_draft

I looked over this before, but maybe not thoroughly enough ;).

Quoted 10 lines

Quoted 8 lines

Quoted 3 lines

How about reusing the -b|--branch option for add? Since we only change the behavior when submodule.$name.update is set to branch it seems reasonable to me. Opinions?

That was the approach I used in v1, but people were concerned that we would be stomping on previously unclaimed config space. Since noone has pointed out other uses besides Gerrit's very similar case, I'm not sure if that is still an issue.

Could you point me to that mail? I cannot seem to find it in my archive.

Hmm. It seems like Phil's initial response was (accidentally?) off list. The relevant portion was:

Quoted 9 lines · Phil Hord

Some projects now use the 'branch' config value to record the tracking branch for the submodule. Some ascribe different meaning to the configuration if the value is given vs. undefined. For example, see the Gerrit submodule-subscription mechanism. This change will cause those workflows to behave differently than they do now.

I do like the idea, but I wish it had a different name for the recording. Maybe --record-branch=${BRANCH} as an extra switch so the action is explicitly requested.

As I said, I'm happy to go back to --branch if opinions have changed.

Quoted 23 lines · Heiko Voigt

Quoted 22 lines · W. Trevor King

Quoted 20 lines · Heiko Voigt

Quoted 5 lines

Because you need to recurse through submodules for update --branch even if "$subsha1" == "$sha1", I had to amend the conditional controlling that block. This broke one of the existing tests, which I "fixed" in patch 4. I think a proper fix would involve rewriting

Quoted 8 lines

  (clear_local_git_env; cd "$sm_path" &&
   ( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) &&
    test -z "$rev") || git-fetch)) ||
  die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"

but I'm not familiar enough with rev-list to want to dig into that yet. If feedback for the earlier three patches is positive, I'll work up a clean fix and resubmit.

You probably need to separate your handling here. The comparison of the currently checked out sha1 and the recorded sha1 is an optimization which skips unnecessary fetching in case the submodules commits are already correct. This code snippet checks whether the to be checked out sha1 is already local and also skips the fetch if it is. We should not break that.

Quoted 6 lines

Quoted 2 lines

Agreed. However, determining if the target $sha1 is local should have nothing to do with the current checked out $subsha1.

See my draft or the diff below for an illustration of the splitup.

[snip diff]

This looks fine, but my current --branch implementation (which doesn't pull) is only a thin branch-checkout layer on top of the standard update functionality. I'm still unsure if built-in pulls are worth the configuration trouble. I'll sleep on it. Maybe I'll feel better about them tomorrow ;).

Cheers,
Trevor

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

I would prefer if we could squash all these commits together into one since it seems to me one logical step, using the new variable for update belongs together with its configuration on initialization.

	if test "$subsha1" != "$sha1"
	then
		handle_on_demand_fetch_update ...
	else
		handle_tracked_branch_update ...
	fi

From: "W. Trevor King" <[email protected]>

This makes it easy to access per-submodule variables. For example,

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

can now be reduced to

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

Quoted 3 lines

can now be reduced to

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

What other use cases are there? Would the need for this maybe go away once you had floating submodules following branches?

For completeness you should make the variables possible to override by repository from the local repository configuration like all other submodule options that are read directly from .gitmodules.

Should I rebase this so it lands cleanly atop 38ae92e4 in next?

commit 38ae92e4d027063b9b87e51a9bf12809d10066f6
Author: W. Trevor King <[email protected]>
Date: Tue Oct 23 17:00:21 2012 -0400

git-submodule: wrap branch option with "<>" in usage strings.

Quoted 1 line · Heiko Voigt

https://github.com/hvoigt/git/commits/hv/floating_submodules_draft

I looked over this before, but maybe not thoroughly enough ;).

https://github.com/hvoigt/git/commits/hv/floating_submodules

This looks fine, but my current --branch implementation (which doesn't pull) is only a thin branch-checkout layer on top of the standard update functionality. I'm still unsure if built-in pulls are worth the configuration trouble. I'll sleep on it. Maybe I'll feel better about them tomorrow ;).

* A new 'submodule pull' for tracking the submodule's remote, which is pulling --ff-only origin/$branch into a whatever state the submodule is currently in. If any changes were made to submodule $shas, optionally commit them with a shortlog-generated commit message.

I thought of a better idea on the train. How about adding --remote to submodule update that overrides the gitlinked SHA-1 with the SHA-1 for origin/$branch? All of the other checkout/merge/rebase functionality is untouched. The only thing that changes is where we look for the update. I'm working up the patch now, and will submit v5 shortly.

Quoted 1 line · W. Trevor King

[snip initial thoughts leading to the update --remote v5]

I was thinking the same thing, but reading this whole thread a couple of weeks late. Thanks for noticing.

Moreover, I think that 'git submodule update --pull' is also the wrong way
to spell this action. Maybe you are misled from the outset by your
current workflow:

For that reason, I don't like the --pull switch since it implies a fetch, but I will not always want to do a fetch.

I don't know which remote I should be tracking, though. I suppose it is 'origin' for now, but maybe it is just whatever $superproject's HEAD's remote-tracking branch indicates.

I am not sure I want the gitlinks in superproject to update automatically in the index, but I definitely do not want to automatically create a commit for them.

But I really don't want to figure out how to handle submodule collisions during a merge (or rebase!) of my superproject with changes that someone else auto-committed in his local $superproject as he and I arbitrarily floated up the upstream independently. There is nothing but loathing down that path.

Quoted 3 lines · Phil Hord

Quoted 1 line · W. Trevor King

[snip initial thoughts leading to the update --remote v5]

Quoted 6 lines

I was thinking the same thing, but reading this whole thread a couple of weeks late. Thanks for noticing.

Moreover, I think that 'git submodule update --pull' is also the wrong way
to spell this action. Maybe you are misled from the outset by your
current workflow:

Did you see my v5 (add --remote) series?

Quoted 2 lines

For that reason, I don't like the --pull switch since it implies a fetch, but I will not always want to do a fetch.

  $ git submodule update --remote --no-fetch

will not fetch the submodule remotes.

Quoted 3 lines

I don't know which remote I should be tracking, though. I suppose it is 'origin' for now, but maybe it is just whatever $superproject's HEAD's remote-tracking branch indicates.

With the --remote series, I always use "origin" because that's what submodule add should be setting up. If people want to change that up by hand, we may need a submodule.<name>.remote configuration option.

Quoted 3 lines

I am not sure I want the gitlinks in superproject to update automatically in the index, but I definitely do not want to automatically create a commit for them.

Commits are dissabled by default (see my recent --commit RFC for how they would be enabled).

Quoted 5 lines

But I really don't want to figure out how to handle submodule collisions during a merge (or rebase!) of my superproject with changes that someone else auto-committed in his local $superproject as he and I arbitrarily floated up the upstream independently. There is nothing but loathing down that path.

This is true. I'm not sure how gitlink collisions are currently handled…

Quoted 7 lines · W. Trevor King

Quoted 2 lines · Phil Hord

For that reason, I don't like the --pull switch since it implies a fetch, but I will not always want to do a fetch.

  $ git submodule update --remote --no-fetch

will not fetch the submodule remotes.

This seems precisely backwards to me. Why not use

  $ git submodule update --remote --fetch

to do your "default" behavior instead? I suppose I am arguing
against the tide of the dominant workflow, but the fetch-by-default idea needlessly conflates two primitive operations: "float" and "fetch".

Quoted 8 lines

Quoted 3 lines

I don't know which remote I should be tracking, though. I suppose it is 'origin' for now, but maybe it is just whatever $superproject's HEAD's remote-tracking branch indicates.

With the --remote series, I always use "origin" because that's what submodule add should be setting up. If people want to change that up by hand, we may need a submodule.<name>.remote configuration option.

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?

Quoted 13 lines

Quoted 3 lines

I am not sure I want the gitlinks in superproject to update automatically in the index, but I definitely do not want to automatically create a commit for them.

Commits are dissabled by default (see my recent --commit RFC for how they would be enabled).

Quoted 5 lines

But I really don't want to figure out how to handle submodule collisions during a merge (or rebase!) of my superproject with changes that someone else auto-committed in his local $superproject as he and I arbitrarily floated up the upstream independently. There is nothing but loathing down that path.

Quoted 2 lines

This is true. I'm not sure how gitlink collisions are currently handled…

They've always been trouble for me. But it may be that I am ignorant.

Am 26.11.2012 22:00, schrieb W. Trevor King:

Quoted 4 lines

From: "W. Trevor King" <[email protected]>

This allows users to override the .gitmodules value with a per-repository value.

Your intentions makes lots of sense, but your patch does more than that. Copying the branch setting into .git/config sets the initial branch setting into stone. That makes it impossible to have a branch "foo" in the superproject using a branch "bar" in a submodule and another superproject branch "frotz" using branch "nitfol" for the same submodule. You should use the branch setting from .git/config if present and fall back to the branch setting from .gitmodules if not, which would enable the user to have her own setting if she doesn't like what upstream provides but would still enable others to follow different submodule branches in different superproject branches.

Quoted 6 lines · Phil Hord

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.

Likewise for submodule sync, which seems to be
quite similar to init.

In my v5 patch, I check for submodule.<name>.remote first in the usual git config files. If I don't find what I'm looking for I fall back on .gitmodules (basically Jens' suggestion). However, my initial copying-to-.git/config approach was mostly done to mimic existing

configuration handling in git-submodule.sh. Since I agree with Jens on configuration precendence, and I now had two options to read (.branch and .remote), I thought I'd pull the logic out into its own function (code included at the end). While I was shifting the existing submodule config handling over to my new function, I noticed that with this logic, submodule init doesn't really do anything important anymore. Likewise for submodule sync, which seems to be quite similar to init.

What to do about this? init has been around for a while, so we can't just remove it (maybe in 2.0?). Leaving it in place is not really a problem though, it just means that the user is locking in the current .gitmodules configuration (as Jens pointed out with respect to .branch).

---
#
# Print a submodule configuration setting
#
# $1 = submodule name
# $2 = option name
# $3 = default value
#
# Checks in the usual git-config places first (for overrides),

# otherwise it falls back on .gitmodules.  This allows you to
# distribute project-wide defaults in .gitmodules, while still
# customizing individual repositories if necessary.  If the option is
# not in .gitmodules either, print a default value.
#
get_submodule_config()
{
	name="$1"
	option="$2"

	default="$3"
	value=$(git config submodule."$name"."$option")
	if test -z "$value"
	then
		value=$(git config -f .gitmodules submodule."$name"."$option")
	fi
	printf '%s' "${value:-$default}"
}

Quoted 6 lines · Jens Lehmann

Am 26.11.2012 22:00, schrieb W. Trevor King:

Quoted 4 lines

From: "W. Trevor King" <[email protected]>

This allows users to override the .gitmodules value with a per-repository value.

Quoted 11 lines

Your intentions makes lots of sense, but your patch does more than that. Copying the branch setting into .git/config sets the initial branch setting into stone. That makes it impossible to have a branch "foo" in the superproject using a branch "bar" in a submodule and another superproject branch "frotz" using branch "nitfol" for the same submodule. You should use the branch setting from .git/config if present and fall back to the branch setting from .gitmodules if not, which would enable the user to have her own setting if she doesn't like what upstream provides but would still enable others to follow different submodule branches in different superproject branches.

I've mulling this over, and when I started coding support for submodule.<name>.remote, I had an idea.

Quoted 11 lines · W. Trevor King

Quoted 6 lines · Phil Hord

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.

In my v5 patch, I check for submodule.<name>.remote first in the usual git config files. If I don't find what I'm looking for I fall back on .gitmodules (basically Jens' suggestion). However, my initial copying-to-.git/config approach was mostly done to mimic existing configuration handling in git-submodule.sh. Since I agree with Jens on configuration precendence, and I now had two options to read (.branch and .remote), I thought I'd pull the logic out into its own function (code included at the end). While I was shifting the existing submodule config handling over to my new function, I noticed that with this logic, submodule init doesn't really do anything important anymore.

Likewise for submodule sync, which seems to be
quite similar to init.

What to do about this? init has been around for a while, so we can't just remove it (maybe in 2.0?). Leaving it in place is not really a problem though, it just means that the user is locking in the current .gitmodules configuration (as Jens pointed out with respect to .branch).

I may be way off base here, as I'm fairly new to submodules in general and these two commands in particular, but I thought I'd float the idea.

Cheers,
Trevor

---
#
# Print a submodule configuration setting
#
# $1 = submodule name
# $2 = option name
# $3 = default value
#
# Checks in the usual git-config places first (for overrides),

# otherwise it falls back on .gitmodules.  This allows you to
# distribute project-wide defaults in .gitmodules, while still
# customizing individual repositories if necessary.  If the option is
# not in .gitmodules either, print a default value.
#
get_submodule_config()
{
        name="$1"
        option="$2"
        default="$3"
        value=$(git config submodule."$name"."$option")
        if test -z "$value"
        then
                value=$(git config -f .gitmodules submodule."$name"."$option")
        fi
        printf '%s' "${value:-$default}"
}

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

If I never 'submodule init' a submodule, it does not get visited by 'git submodule foreach', among others. I think some people use this behavior explicitly.

Quoted 1 line

If I never 'submodule init' a submodule, it does not get visited by 'git submodule foreach', among others. I think some people use this behavior explicitly.

This is something I'll fix while working up a trial patch. Currently cmd_update calls module_clone if the <submodule>/.git does not exist. This should probably happen in each command (in a wrapper around module_list?). It's possible that module_list itself would need some work, but I haven't absorbed its implementation yet [1].

Am 30.11.2012 18:53, schrieb W. Trevor King:

Quoted 4 lines

In my v5 patch, I check for submodule.<name>.remote first in the usual git config files. If I don't find what I'm looking for I fall back on .gitmodules (basically Jens' suggestion). However, my initial copying-to-.git/config approach was mostly done to mimic existing

Quoted 8 lines

configuration handling in git-submodule.sh. Since I agree with Jens on configuration precendence, and I now had two options to read (.branch and .remote), I thought I'd pull the logic out into its own function (code included at the end). While I was shifting the existing submodule config handling over to my new function, I noticed that with this logic, submodule init doesn't really do anything important anymore. Likewise for submodule sync, which seems to be quite similar to init.

You need to handle the 'url' setting differently. While I think the 'update' setting should not be copied into .git/config at all (because it makes it impossible for upstream to change that later without the user copying that himself as 'sync' doesn't do that) the 'url' setting in .git/config has two important implications:

1) It tells the submodule commands that the user wants to have that submodule populated (which is done in a subsequent "update" after "init" copied the url there).

2) It can be used to follow moving upstreams (think of checking out an earlier commit before the upstream was moved, you won't be able to clone it from there without having the new setting persist). And which repository you follow is a matter of trust, so the extra "git submodule sync" in that case is a good thing to have.

So I believe 'url' is the only setting that should be copied into .git/config while all the others shouldn't.

Quoted 1 line

[snip get_submodule_config()]

Something like that makes sense. You can use it for the settings you add first and we can then reuse that for 'update' in a separate patch later.

Quoted 3 lines

You need to handle the 'url' setting differently. While I think the 'update' setting should not be copied into .git/config at all (because it makes it impossible for upstream to change that later without the user copying that himself as 'sync' doesn't do that) the 'url' setting in .git/config has two important implications:

1) It tells the submodule commands that the user wants to have that submodule populated (which is done in a subsequent "update" after "init" copied the url there).

Good point, but this should depend on submodule.<name>.update; having it as a side effect of a local submodule.<name>.url makes no sense.

Perhaps submodule init should be reduced to just wrap:

$ git config submodule.<name>.update checkout

where the default update configuration would be 'none'.

Quoted 3 lines

2) It can be used to follow moving upstreams (think of checking out an earlier commit before the upstream was moved, you won't be able to clone it from there without having the new setting persist). And which repository you follow is a matter of trust, so the extra "git submodule sync" in that case is a good thing to have.

So I believe 'url' is the only setting that should be copied into .git/config while all the others shouldn't.

If you want to override the old repository location for an old commit, setting submodule.<name>.url makes sense. My rewritten sync updates the local submodule.<name>.url in the superproject if the configuration option is already set [1]. Perhaps a sync --local invocation should forcibly populate the local submodule.<name>.url to make this workflow easier. Bundling sugar for this special case should not happen under an extra command called init.

Quoted 3 lines

Quoted 1 line

[snip get_submodule_config()]

Something like that makes sense. You can use it for the settings you add first and we can then reuse that for 'update' in a separate patch later.

I'm currently working out the details independently against v1.8.0. This will be a fairly major shift, so I think it should stay independent of update --remote. The update --remote stuff should be easy to adjust/rebase if the init removal/adjustment develops into something acceptable.

Am 01.12.2012 17:30, schrieb W. Trevor King:

Quoted 5 lines · Jens Lehmann

Quoted 3 lines

1) It tells the submodule commands that the user wants to have that submodule populated (which is done in a subsequent "update" after "init" copied the url there).

Quoted 3 lines

Good point, but this should depend on submodule.<name>.update; having it as a side effect of a local submodule.<name>.url makes no sense.

Sorry, but that makes tons of sense: url controls if the submodule is to be populated and from where, update controls how (and can even veto populating it if set to "none"). We /could/ do it differently, but I can't see why we should (and risk severe compatibility issues).

Quoted 10 lines

Quoted 9 lines

2) It can be used to follow moving upstreams (think of checking out an earlier commit before the upstream was moved, you won't be able to clone it from there without having the new setting persist). And which repository you follow is a matter of trust, so the extra "git submodule sync" in that case is a good thing to have.

So I believe 'url' is the only setting that should be copied into .git/config while all the others shouldn't.

Quoted 8 lines

If you want to override the old repository location for an old commit, setting submodule.<name>.url makes sense. My rewritten sync updates the local submodule.<name>.url in the superproject if the configuration option is already set [1]. Perhaps a sync --local invocation should forcibly populate the local submodule.<name>.url to make this workflow easier. Bundling sugar for this special case should not happen under an extra command called init.

What real world problems do we have with the current init/sync that this approach would solve?

Quoted 6 lines

Quoted 5 lines

Quoted 1 line

[snip get_submodule_config()]

Something like that makes sense. You can use it for the settings you add first and we can then reuse that for 'update' in a separate patch later.

Quoted 5 lines

I'm currently working out the details independently against v1.8.0. This will be a fairly major shift, so I think it should stay independent of update --remote. The update --remote stuff should be easy to adjust/rebase if the init removal/adjustment develops into something acceptable.

I totally agree. Let's get the update --remote stuff ready first.

I think removing init will cause some compatibility issues anyway, so I was re-imaging how you do it. I don't think update='none' and "don't populate my submodule" are distinct ideas, while a locally configured url="somwhere" and "please populate my submodule" are (with the blank-url case defaulting to the superproject itself).

Quoted 2 lines

What real world problems do we have with the current init/sync that this approach would solve?

I don't have any, but in my update --remote series I'm adding two new config options that are handled differently (define in .gitmodules, override in superproject .git/config) than existing submodules options.

Am 01.12.2012 18:49, schrieb W. Trevor King:

Quoted 5 lines

I think removing init will cause some compatibility issues anyway, so I was re-imaging how you do it. I don't think update='none' and "don't populate my submodule" are distinct ideas, while a locally configured url="somwhere" and "please populate my submodule" are (with the blank-url case defaulting to the superproject itself).

Why would we want to remove "init"? It still has to copy the "url" setting (and it would be a compatibility nightmare if we would change that, imagine different git versions used on the same work tree).

Quoted 7 lines

Quoted 2 lines

What real world problems do we have with the current init/sync that this approach would solve?

I don't have any, but in my update --remote series I'm adding two new config options that are handled differently (define in .gitmodules, override in superproject .git/config) than existing submodules options.

No, they're not. They are just handled differently than "url" and "update", but will behave just like "fetchRecurseSubmodules" and "ignore" do since day one. And as I explained in another mail I think "url" is special and "update" should be change to behave like the other two some day.

If I never 'submodule init' a submodule, it does not get visited by 'git submodule foreach', among others. I think some people use this behavior explicitly.

On the other hand, I've also notice that a submodule which I have removed does not get de-inited later one. It causes my 'git submodule foreach' to emit errors. :-(

Am 01.12.2012 00:52, schrieb Phil Hord:

Quoted 7 lines

If I never 'submodule init' a submodule, it does not get visited by 'git submodule foreach', among others. I think some people use this behavior explicitly.

On the other hand, I've also notice that a submodule which I have removed does not get de-inited later one. It causes my 'git submodule foreach' to emit errors. :-(

I'm currently hacking on "git submodule deinit" which removes the 'url' setting from git/config. This should do the trick for you, right?

Just removing that submodule automagically would not work that well, as it would deinitialize a submodule when you switch to a branch where it isn't present and you'd have to reinitialize it when you come back.

Am 01.12.2012 00:52, schrieb Phil Hord:

Quoted 7 lines

If I never 'submodule init' a submodule, it does not get visited by 'git submodule foreach', among others. I think some people use this behavior explicitly.

On the other hand, I've also notice that a submodule which I have removed does not get de-inited later one. It causes my 'git submodule foreach' to emit errors. :-(

I'm currently hacking on "git submodule deinit" which removes the 'url' setting from git/config. This should do the trick for you, right?

With "git submodule init" the user is able to tell git he cares about one or more submodules and wants to have it populated on the next call to "git submodule update". But currently there is no easy way he could tell git he does not care about a submodule anymore and wants to get rid of his local work tree (except he knows a lot about submodule internals and removes the "submodule.$name.url" setting from .git/config himself).

Help those users by providing a 'deinit' command. This removes the url setting from .git/config either for the given submodule(s) or for all those which have been initialized if none were given. Complain only when for a submodule given on the command line the url setting can't be found in .git/config.

Add tests and link the man pages of "git submodule deinit" and "git rm" to assist the user in deciding whether removing or unregistering the submodule is the right thing to do for him.

Signed-off-by Jens Lehmann <[email protected]>

---

Am 01.12.2012 16:56, schrieb Jens Lehmann:

Quoted 11 lines

Am 01.12.2012 00:52, schrieb Phil Hord:

Quoted 7 lines

If I never 'submodule init' a submodule, it does not get visited by 'git submodule foreach', among others. I think some people use this behavior explicitly.

On the other hand, I've also notice that a submodule which I have removed does not get de-inited later one. It causes my 'git submodule foreach' to emit errors. :-(

I'm currently hacking on "git submodule deinit" which removes the 'url' setting from git/config. This should do the trick for you, right?

And here we go ...

 Documentation/git-rm.txt        |  4 ++++
 Documentation/git-submodule.txt | 11 +++++++++
 git-submodule.sh                | 50 ++++++++++++++++++++++++++++++++++++++++-
 t/t7400-submodule-basic.sh      | 11 +++++++++
 4 files changed, 75 insertions(+), 1 deletion(-)

Documentation/git-rm.txt +4 -0

index 262436b..ec42bf5 100644
@@ -149,6 +149,10 @@ files that aren't ignored are present in the submodules work tree.
 Ignored files are deemed expendable and won't stop a submodule's work
 tree from being removed.
 
+If you only want to remove the local checkout of a submodule from your
+work tree without committing that use `git submodule deinit` instead
+(see linkgit:git-submodule[1]).
+
 EXAMPLES
 --------
 `git rm Documentation/\*.txt`::

Documentation/git-submodule.txt +11 -0

index b1de3ba..fba77f6 100644
@@ -13,6 +13,7 @@ SYNOPSIS
 	      [--reference <repository>] [--] <repository> [<path>]
 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
 'git submodule' [--quiet] init [--] [<path>...]
+'git submodule' [--quiet] deinit [--] [<path>...]
 'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
 	      [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]

@@ -134,6 +135,16 @@ init::
 	the explicit 'init' step if you do not intend to customize
 	any submodule locations.
 
+deinit::
+	Unregister the submodules, i.e. remove the `submodule.$name.url`
+	setting from .git/config. Further calls to `git submodule update`,
+	`git submodule foreach` and `git submodule sync` will skip any
+	unregistered submodules until they are initialized again, so use
+	this command if you don't want to have a local checkout of the
+	submodule in your work tree anymore. If you really want to remove
+	a submodule from the repository and commit that use
+	linkgit:git-rm[1] instead.
+
 update::
 	Update the registered submodules, i.e. clone missing submodules and
 	checkout the commit specified in the index of the containing repository.

git-submodule.sh +49 -1

index 2365149..4059a2e 100755
@@ -8,6 +8,7 @@ dashless=$(basename "$0" | sed -e 's/-/ /')
 USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <repository>] [--] <repository> [<path>]
    or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] init [--] [<path>...]
+   or: $dashless [--quiet] deinit [--] [<path>...]
    or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
    or: $dashless [--quiet] foreach [--recursive] <command>

@@ -516,6 +517,53 @@ cmd_init()
 }
 
 #
+# Unregister submodules from .git/config
+#
+# $@ = requested paths (default to all)
+#
+cmd_deinit()
+{
+	# parse $args after "submodule ... init".
+	while test $# -ne 0
+	do
+		case "$1" in
+		-q|--quiet)
+			GIT_QUIET=1
+			;;
+		--)
+			shift
+			break
+			;;
+		-*)
+			usage
+			;;
+		*)
+			break
+			;;
+		esac
+		shift
+	done
+
+	module_list "$@" |
+	while read mode sha1 stage sm_path
+	do
+		die_if_unmatched "$mode"
+		name=$(module_name "$sm_path") || exit
+		url=$(git config submodule."$name".url)
+		if test -z "$url"
+		then
+			# Only mention uninitialized submodules when its
+			# path have been specified
+			test "$#" != "0" &&
+			say "$(eval_gettext "No url found for submodule path '\$sm_path' in .git/config")"
+			continue
+		fi
+		git config --unset submodule."$name".url &&
+		say "$(eval_gettext "Submodule '\$name' (\$url) unregistered")"
+	done
+}
+
+#
 # Update each submodule path to correct revision, using clone and checkout as needed
 #
 # $@ = requested paths (default to all)

@@ -1108,7 +1156,7 @@ cmd_sync()
 while test $# != 0 && test -z "$command"
 do
 	case "$1" in
-	add | foreach | init | update | status | summary | sync)
+	add | foreach | init | deinit | update | status | summary | sync)
 		command=$1
 		;;
 	-q|--quiet)

t/t7400-submodule-basic.sh +11 -0

index de7d453..803bda7 100755
@@ -756,4 +756,15 @@ test_expect_success 'submodule add with an existing name fails unless forced' '
 	)
 '
 
+test_expect_success 'submodule deinit should unregister submodule url from .git/config' '
+	url=$(git config submodule.example.url) &&
+	git submodule deinit &&
+	test -z "$(git config submodule.example.url)"
+'
+
+test_expect_success 'submodule deinit complains only when explicitly used on an uninitialized submodule' '
+	git submodule deinit &&
+	test_must_fail git submodule deinit example
+'
+
 test_done

Quoted 10 lines · Jens Lehmann

Am 01.12.2012 18:49, schrieb W. Trevor King:

Quoted 5 lines

I think removing init will cause some compatibility issues anyway, so I was re-imaging how you do it. I don't think update='none' and "don't populate my submodule" are distinct ideas, while a locally configured url="somwhere" and "please populate my submodule" are (with the blank-url case defaulting to the superproject itself).

Why would we want to remove "init"? It still has to copy the "url" setting (and it would be a compatibility nightmare if we would change that, imagine different git versions used on the same work tree).

In my init-less rewrite, it doesn't have to copy the url setting. People using older versions of Git would need to run init using their old version. Having the url defined in .git/config won't break my init-less submodule commands, it just means that the value in .gitmodules will be masked.

Quoted 1 line · Jens Lehmann

[snip v1 deinit commit message]

I fully agree with your analysis on the reason why the "url" element is special and has to be copied to $GIT_DIR/config, but when you deinit (or uninit) a submodule to say you are no longer interested in it and do not want it populated in the context of the superproject, I am not sure if removing only submodule.$name.url (so that when you later decide to "init" it again, you will keep the values for submodule.$name.update and other things from the previous life) is the sane thing to do, or it is better to remove submodule.$name.* altogether as if an earlier "init" has never happened. Would it be worth analyzing the pros-and-cons here?

Quoted 12 lines · Jens Lehmann

Am 01.12.2012 00:52, schrieb Phil Hord:

Quoted 7 lines

If I never 'submodule init' a submodule, it does not get visited by 'git submodule foreach', among others. I think some people use this behavior explicitly.

On the other hand, I've also notice that a submodule which I have removed does not get de-inited later one. It causes my 'git submodule foreach' to emit errors. :-(

I'm currently hacking on "git submodule deinit" which removes the 'url' setting from git/config. This should do the trick for you, right?

Quoted 3 lines

Just removing that submodule automagically would not work that well, as it would deinitialize a submodule when you switch to a branch where it isn't present and you'd have to reinitialize it when you come back.

I think this is another case where we should be looping through submodules based on the revision-specific .gitmodules content, and querying the local config only to determine if the user wants to update them (to drop into them with foreach, etc.).

Before I get into the details, I'd like to point out that I actually understand the purpose of submodule init now ;). To avoid further confusion, my current one-line command summaries would be:

init: mark a submodule as active for future submodule operation
deinit: mark a submodule as inactive for future submodule operation
sync: update remote.<name>.origin in submodules to reflect changes
in .gitmodules or the superproject's remote URL.

I don't think we disagree on that, we just don't agree on how to implement it.

Quoted 2 lines

What real world problems do we have with the current init/sync that this approach would solve?

I don't have any, ...

Am 02.12.2012 20:09, schrieb W. Trevor King:

Quoted 9 lines

Before I get into the details, I'd like to point out that I actually understand the purpose of submodule init now ;). To avoid further confusion, my current one-line command summaries would be:

init: mark a submodule as active for future submodule operation
deinit: mark a submodule as inactive for future submodule operation
sync: update remote.<name>.origin in submodules to reflect changes
in .gitmodules or the superproject's remote URL.

Quoted 2 lines

I don't think we disagree on that, we just don't agree on how to implement it.

Nope, it is already implemented and you are arguing to change the current implementation.

To quote from another mail:

Am 01.12.2012 18:49, schrieb W. Trevor King:

Quoted 5 lines · Jens Lehmann

Quoted 2 lines

What real world problems do we have with the current init/sync that this approach would solve?

I don't have any, ...

We don't want to change working code and cause compatibility issues just because we /could/ do things differently, no?

Sure. I was worried about throwing away other settings the user might have set in the submodule.$name section and the first reflex was to protect them. But thinking about that again I noticed we are already throwing away a possibly user customized "url" setting, so we already remove a possibly customized setting.

Quoted 9 lines · Jens Lehmann

Am 01.12.2012 18:49, schrieb W. Trevor King:

Quoted 5 lines · Jens Lehmann

Quoted 2 lines

What real world problems do we have with the current init/sync that this approach would solve?

I don't have any, ...

We don't want to change working code and cause compatibility issues just because we /could/ do things differently, no?

In principle, yes, but in this case I think changing the implementation does not risk much in the way of compatibility issues (it only hurts users who rely on submodule init setting submodule.<name>.url for reasons of their own. A few of the existing tests explictly check the url setting, so perhaps there are a number of users who do require this side effect?

I think this risk is outweighed by the benefits of having a clearer activation option.

I'm currently stuck with adding a commit-less existing repository as a submodule (which happens in t7400-submodule-basic.sh, ../bar/a/b/c works with relative local path):

  $ mkdir -p super/sub
  $ cd super
  $ git init
  $ (cd sub && git init)
  $ git submodule add ./ sub
  $ git status
  # On branch master
  #
  # Initial commit
  #
  # Changes to be committed:
  #   (use "git rm --cached <file>..." to unstage)
  #
  #       new file:   .gitmodules
  #

What I'm missing is a gitlink form sub for 'Subproject commit 00000...' or some such. When the subproject has an actual commit, things work as expected:

  $ mkdir -p super/sub
  $ cd super
  $ git init
  $ (cd sub && git init && echo line-1 > file && git add file && git commit -m file)
  $ git submodule add ./ sub
  $ git status
  # On branch master
  #
  # Initial commit
  #
  # Changes to be committed:
  #   (use "git rm --cached <file>..." to unstage)
  #
  #       new file:   .gitmodules
  #       new file:   sub
  #

This means that module_list isn't aware of the empty submodule, when the user has just explicitly added it. Fixing this would seem to need either 'Subproject commit 00000...' as I suggested earlier, or an adjustment to module_list that also spits out submodules that are in .gitmodules but not in the index.

Quoted 13 lines · Jens Lehmann

With "git submodule init" the user is able to tell git he cares about one or more submodules and wants to have it populated on the next call to "git submodule update". But currently there is no easy way he could tell git he does not care about a submodule anymore and wants to get rid of his local work tree (except he knows a lot about submodule internals and removes the "submodule.$name.url" setting from .git/config himself).

Help those users by providing a 'deinit' command. This removes the url setting from .git/config either for the given submodule(s) or for all those which have been initialized if none were given. Complain only when for a submodule given on the command line the url setting can't be found in .git/config.

Quoted 6 lines

Add tests and link the man pages of "git submodule deinit" and "git rm" to assist the user in deciding whether removing or unregistering the submodule is the right thing to do for him.

Signed-off-by Jens Lehmann <[email protected]>

---

I fully agree with your analysis on the reason why the "url" element is special and has to be copied to $GIT_DIR/config, but when you deinit (or uninit) a submodule to say you are no longer interested in it and do not want it populated in the context of the superproject, I am not sure if removing only submodule.$name.url (so that when you later decide to "init" it again, you will keep the values for submodule.$name.update and other things from the previous life) is the sane thing to do, or it is better to remove submodule.$name.* altogether as if an earlier "init" has never happened. Would it be worth analyzing the pros-and-cons here?

Maybe the principle of least surprise is better followed when we nuke the whole section, as it might surprise the user more to have a setting resurrected he customized in the last life cycle of the submodule than seeing that after an deinit followed by an init all former customizations are consistently gone. So I tend to think now that removing the whole section would be the better solution here.

Quoted 6 lines · Jens Lehmann

Maybe the principle of least surprise is better followed when we nuke the whole section, as it might surprise the user more to have a setting resurrected he customized in the last life cycle of the submodule than seeing that after an deinit followed by an init all former customizations are consistently gone. So I tend to think now that removing the whole section would be the better solution here.

I tend to agree; I suspect that a "deinit" would be mostly done either to

(1) correct mistakes the user made during a recent "init" and perhaps "sync"; or

(2) tell Git that the user has finished woing with this particular submodule and does not intend to use it for quite a while.

For both (1) and (2), I think it would be easier to users if we gave them a clean slate, the same state as the one the user who never had ran "init" on it would be in. A user in situation (1) is asking for a clean slate, and a user in situation (2) is better served if he does not have to worry about leftover entries in $GIT_DIR/config he has long forgotten from many months ago (during which time the way the project uses the particular submodule may well have changed) giving non-standard experience different from what other project participants would get.

With "git submodule init" the user is able to tell git he cares about one or more submodules and wants to have it populated on the next call to "git submodule update". But currently there is no easy way he could tell git he does not care about a submodule anymore and wants to get rid of his local work tree (except he knows a lot about submodule internals and removes the "submodule.$name.url" setting from .git/config himself).

Help those users by providing a 'deinit' command. This removes the whole submodule.<name> section from .git/config either for the given submodule(s) or for all those which have been initialized if none were given. Complain only when for a submodule given on the command line the url setting can't be found in .git/config.

Add tests and link the man pages of "git submodule deinit" and "git rm" to assist the user in deciding whether removing or unregistering the submodule is the right thing to do for him.

Signed-off-by Jens Lehmann <[email protected]>

---

Am 03.12.2012 08:58, schrieb Junio C Hamano:

Quoted 18 lines

Quoted 6 lines · Jens Lehmann

Maybe the principle of least surprise is better followed when we nuke the whole section, as it might surprise the user more to have a setting resurrected he customized in the last life cycle of the submodule than seeing that after an deinit followed by an init all former customizations are consistently gone. So I tend to think now that removing the whole section would be the better solution here.

I tend to agree; I suspect that a "deinit" would be mostly done either to

(1) correct mistakes the user made during a recent "init" and perhaps "sync"; or

(2) tell Git that the user has finished woing with this particular submodule and does not intend to use it for quite a while.

Quoted 9 lines

For both (1) and (2), I think it would be easier to users if we gave them a clean slate, the same state as the one the user who never had ran "init" on it would be in. A user in situation (1) is asking for a clean slate, and a user in situation (2) is better served if he does not have to worry about leftover entries in $GIT_DIR/config he has long forgotten from many months ago (during which time the way the project uses the particular submodule may well have changed) giving non-standard experience different from what other project participants would get.

Changes in v2:
- Remove the whole submodule section instead of only removing the "url" setting and explain why we do that in a comment
- Reworded commit message and git-submodule.txt to reflect that
- Extend the test to check that a custom settings are removed

 Documentation/git-rm.txt        |  4 ++++
 Documentation/git-submodule.txt | 12 ++++++++++
 git-submodule.sh                | 52 ++++++++++++++++++++++++++++++++++++++++-
 t/t7400-submodule-basic.sh      | 12 ++++++++++
 4 files changed, 79 insertions(+), 1 deletion(-)

Documentation/git-rm.txt +4 -0

index 262436b..ec42bf5 100644
@@ -149,6 +149,10 @@ files that aren't ignored are present in the submodules work tree.
 Ignored files are deemed expendable and won't stop a submodule's work
 tree from being removed.
 
+If you only want to remove the local checkout of a submodule from your
+work tree without committing that use `git submodule deinit` instead
+(see linkgit:git-submodule[1]).
+
 EXAMPLES
 --------
 `git rm Documentation/\*.txt`::

Documentation/git-submodule.txt +12 -0

index b1de3ba..08b55a7 100644
@@ -13,6 +13,7 @@ SYNOPSIS
 	      [--reference <repository>] [--] <repository> [<path>]
 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
 'git submodule' [--quiet] init [--] [<path>...]
+'git submodule' [--quiet] deinit [--] [<path>...]
 'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
 	      [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]

@@ -134,6 +135,17 @@ init::
 	the explicit 'init' step if you do not intend to customize
 	any submodule locations.
 
+deinit::
+	Unregister the submodules, i.e. remove the whole `submodule.$name`
+	section from .git/config. Further calls to `git submodule update`,
+	`git submodule foreach` and `git submodule sync` will skip any
+	unregistered submodules until they are initialized again, so use
+	this command if you don't want to have a local checkout of the
+	submodule in your work tree anymore (but note that this command
+	does not remove the submodule work tree). If you really want to
+	remove a submodule from the repository and commit that use
+	linkgit:git-rm[1] instead.
+
 update::
 	Update the registered submodules, i.e. clone missing submodules and
 	checkout the commit specified in the index of the containing repository.

git-submodule.sh +51 -1

index 2365149..3f558ed 100755
@@ -8,6 +8,7 @@ dashless=$(basename "$0" | sed -e 's/-/ /')
 USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <repository>] [--] <repository> [<path>]
    or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] init [--] [<path>...]
+   or: $dashless [--quiet] deinit [--] [<path>...]
    or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
    or: $dashless [--quiet] foreach [--recursive] <command>

@@ -516,6 +517,55 @@ cmd_init()
 }
 
 #
+# Unregister submodules from .git/config
+#
+# $@ = requested paths (default to all)
+#
+cmd_deinit()
+{
+	# parse $args after "submodule ... init".
+	while test $# -ne 0
+	do
+		case "$1" in
+		-q|--quiet)
+			GIT_QUIET=1
+			;;
+		--)
+			shift
+			break
+			;;
+		-*)
+			usage
+			;;
+		*)
+			break
+			;;
+		esac
+		shift
+	done
+
+	module_list "$@" |
+	while read mode sha1 stage sm_path
+	do
+		die_if_unmatched "$mode"
+		name=$(module_name "$sm_path") || exit
+		url=$(git config submodule."$name".url)
+		if test -z "$url"
+		then
+			# Only mention uninitialized submodules when its
+			# path have been specified
+			test "$#" != "0" &&
+			say "$(eval_gettext "No url found for submodule path '\$sm_path' in .git/config")"
+			continue
+		fi
+		# Remove the whole section so we have a clean state when the user
+		# later decides to init this submodule again
+		git config --remove-section submodule."$name" &&
+		say "$(eval_gettext "Submodule '\$name' (\$url) unregistered")"
+	done
+}
+
+#
 # Update each submodule path to correct revision, using clone and checkout as needed
 #
 # $@ = requested paths (default to all)

@@ -1108,7 +1158,7 @@ cmd_sync()
 while test $# != 0 && test -z "$command"
 do
 	case "$1" in
-	add | foreach | init | update | status | summary | sync)
+	add | foreach | init | deinit | update | status | summary | sync)
 		command=$1
 		;;
 	-q|--quiet)

t/t7400-submodule-basic.sh +12 -0

index de7d453..ee4f0ab 100755
@@ -756,4 +756,16 @@ test_expect_success 'submodule add with an existing name fails unless forced' '
 	)
 '
 
+test_expect_success 'submodule deinit should remove the whole submodule section from .git/config' '
+	git config submodule.example.foo bar &&
+	git submodule deinit &&
+	test -z "$(git config submodule.example.url)" &&
+	test -z "$(git config submodule.example.foo)"
+'
+
+test_expect_success 'submodule deinit complains only when explicitly used on an uninitialized submodule' '
+	git submodule deinit &&
+	test_must_fail git submodule deinit example
+'
+
 test_done

Jens Lehmann venit, vidit, dixit 04.12.2012 22:48:

Quoted 7 lines

With "git submodule init" the user is able to tell git he cares about one or more submodules and wants to have it populated on the next call to "git submodule update". But currently there is no easy way he could tell git he does not care about a submodule anymore and wants to get rid of his local work tree (except he knows a lot about submodule internals and removes the "submodule.$name.url" setting from .git/config himself).

Quoted 5 lines

Help those users by providing a 'deinit' command. This removes the whole submodule.<name> section from .git/config either for the given submodule(s) or for all those which have been initialized if none were given. Complain only when for a submodule given on the command line the url setting can't be found in .git/config.

Whoaaa, so why not have "git rm" remove everything unless I specify a file to be removed?

I know I'm exaggerating a bit, but defaulting to "--all" for a destructive operation seems to be a bit harsh, especially when the command is targeted at "those" users that you mention.

Especially as I suspect the number of submodule users having customized those in .git/config is not that high ...

Quoted 2 lines · Jens Lehmann

Especially as I suspect the number of submodule users having customized those in .git/config is not that high ...

I thought the point of "deinit" was to say "I am not interested in having a checkout of these submodules in my working tree anymore".

The user could do "rm -fr submodule && mkdir submodule" to remove it locally and keep "diff" and "status" from noticing the removal, but the primary reason the user needs an explicit "deinit" is because many subcommands of "git submodule" are documented to operate on all submodules that have been "init"ed when given no explicit submodule names [*1*].

Your "deinit" is documented not to actually remove the submodule checkout, but that very much goes against my intuition. What is the justification behind that choice?

"We'll remove the configuration,
you remove the tree yourself" will invite the mistake of running "git rm" on it, which you wanted to avoid with the addition to the "git rm" documentation, no?

[Footnote]

*1* In reality, the code looks at presense of .git in the submodule path to decide if it has been "init"ed (cf. cmd_update), but this implementation of "deinit" does not seem to cause that .git to be removed, leaving the submodule in "init"ed state from these other command's perspective.

So unless people agree that deinit should also remove the work tree I'll prepare some patches teaching all git commands to consistently ignore deinitialized submodules. Opinions?

So unless people agree that deinit should also remove the work tree I'll prepare some patches teaching all git commands to consistently ignore deinitialized submodules. Opinions?

Quoted 3 lines · Jens Lehmann

So unless people agree that deinit should also remove the work tree I'll prepare some patches teaching all git commands to consistently ignore deinitialized submodules. Opinions?

While I agree that consistency is good, "deinit" that does not remove the working tree of the submodule the user explicitly said he no longer wants to have the checkout for is a bug, and I think these two are orthogonal issues.

Should deinit remove the submodule checkout, replace it with the original gitlink, and clear the .git/config information then? That would restore the user to the state they'd be in if they were never interested in the submodule.

Quoted 4 lines · W. Trevor King

Should deinit remove the submodule checkout, replace it with the original gitlink, and clear the .git/config information then? That would restore the user to the state they'd be in if they were never interested in the submodule.

AFAIU, "restore the user to the state" is the goal. I am not sure what you meant by "replace it with the original gitlink", though. A checkout with a submodule that the user is not interested in would have an empty directory at that path, no?

Quoted 11 lines · W. Trevor King

Quoted 9 lines · Phil Hord

Quoted 5 lines

But I really don't want to figure out how to handle submodule collisions during a merge (or rebase!) of my superproject with changes that someone else auto-committed in his local $superproject as he and I arbitrarily floated up the upstream independently. There is nothing but loathing down that path.

This is true. I'm not sure how gitlink collisions are currently handled…

Quoted 1 line

They've always been trouble for me. But it may be that I am ignorant.

I haven't dealt with any gitlink merges, but I think that supporting easy gitlink merges is orthogonal to this --remote option. For simple cases like "autocommitted submodule floats", one of the conflicting gitlinks will be an ancestor of the other, so it should be easy to automate that merge.

Quoted 6 lines · Phil Hord

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.

From: "W. Trevor King" <[email protected]>

Several submodule configuration variables
(e.g. fetchRecurseSubmodules) are read from .gitmodules with local overrides from the usual git config files. This shell function mimics that logic to help initialize configuration variables in git-submodule.sh.

Signed-off-by W. Trevor King <[email protected]>

---
 git-submodule.sh | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

git-submodule.sh +4 -0

index ab6b110..97ce5e4 100755
@@ -152,6 +152,33 @@ die_if_unmatched ()
 }
 
 #
+# Print a submodule configuration setting
+#
+# $1 = submodule name
+# $2 = option name

git-submodule.sh +9 -0

+	if test -z "$value"
+	then
+		value=$(git config -f .gitmodules submodule."$name"."$option")
+	fi
+	printf '%s' "${value:-$default}"
+}
+
+
+#
 # Map submodule path to submodule name
 #
 # $1 = path