All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Add an update=none option for 'loose' submodules
@ 2011-08-11 17:51 Heiko Voigt
  2011-08-11 17:51 ` [PATCH 1/2] submodule: move update configuration variable further up Heiko Voigt
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Heiko Voigt @ 2011-08-11 17:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jens Lehmann

If a submodule is used to seperate some bigger parts of a project into
an optional directory it is helpful to not clone/update them by default.

This series implements a new value 'none' for submodule.<name>.update.
If this option is set a submodule will not be updated or cloned by
default. If the user wants to work with the submodule he either needs
to explicitely configure the update option to 'checkout' or pass
--checkout as an option to the submodules. I chose this name to be
consistent with the existing --merge/--rebase options.

We have been talking about loose submodules for some time:

RFC patch for this series
http://thread.gmane.org/gmane.comp.version-control.git/175165

Using submodule groups/dependencies:
http://thread.gmane.org/gmane.comp.version-control.git/130928/focus=131050
http://thread.gmane.org/gmane.comp.version-control.git/176347/focus=178614

This lays the foundations for grouping of submodules. Once submodule
grouping will be implemented the value of submodule.$name.update
provides the default value when the user specifies no group. A group
specification could then be a layer on top which provides a shortcut to
choose other submodule.$name.update values to be registered in
.git/config.

Heiko Voigt (2):
  submodule: move update configuration variable further up
  add update 'none' flag to disable update of submodule by default

 Documentation/git-submodule.txt |    8 ++++-
 git-submodule.sh                |   22 ++++++++++----
 t/t7406-submodule-update.sh     |   62 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 85 insertions(+), 7 deletions(-)

-- 
1.7.6.435.g741d34

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/2] submodule: move update configuration variable further up
  2011-08-11 17:51 [PATCH 0/2] Add an update=none option for 'loose' submodules Heiko Voigt
@ 2011-08-11 17:51 ` Heiko Voigt
  2011-08-11 17:51 ` [PATCH 2/2] add update 'none' flag to disable update of submodule by default Heiko Voigt
  2011-08-11 18:28 ` [PATCH 0/2] Add an update=none option for 'loose' submodules Junio C Hamano
  2 siblings, 0 replies; 15+ messages in thread
From: Heiko Voigt @ 2011-08-11 17:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jens Lehmann

Lets always initialize the 'update_module' variable with the final
value. This way we allow code which wants to check this configuration
early to do so right in the beginning of cmd_update().

Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
---
 git-submodule.sh |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index f46862f..e544dbc 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -461,7 +461,13 @@ cmd_update()
 		fi
 		name=$(module_name "$path") || exit
 		url=$(git config submodule."$name".url)
-		update_module=$(git config submodule."$name".update)
+		if ! test -z "$update"
+		then
+			update_module=$update
+		else
+			update_module=$(git config submodule."$name".update)
+		fi
+
 		if test -z "$url"
 		then
 			# Only mention uninitialized submodules when its
@@ -483,11 +489,6 @@ Maybe you want to use 'update --init'?")"
 			die "$(eval_gettext "Unable to find current revision in submodule path '\$path'")"
 		fi
 
-		if ! test -z "$update"
-		then
-			update_module=$update
-		fi
-
 		if test "$subsha1" != "$sha1"
 		then
 			subforce=$force
-- 
1.7.6.435.g741d34

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/2] add update 'none' flag to disable update of submodule by default
  2011-08-11 17:51 [PATCH 0/2] Add an update=none option for 'loose' submodules Heiko Voigt
  2011-08-11 17:51 ` [PATCH 1/2] submodule: move update configuration variable further up Heiko Voigt
@ 2011-08-11 17:51 ` Heiko Voigt
  2011-08-11 18:28 ` [PATCH 0/2] Add an update=none option for 'loose' submodules Junio C Hamano
  2 siblings, 0 replies; 15+ messages in thread
From: Heiko Voigt @ 2011-08-11 17:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jens Lehmann

This is useful to mark a submodule as unneeded by default. When this
option is set and the user wants to work with such a submodule he
needs to configure 'submodule.<name>.update=checkout' or pass the
--checkout option. Then the submodule can be handled like a normal
submodule.

Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
---
 Documentation/git-submodule.txt |    8 ++++-
 git-submodule.sh                |    9 +++++
 t/t7406-submodule-update.sh     |   62 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 78 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 67cf5f0..6ec3fef 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -120,6 +120,8 @@ too (and can also report changes to a submodule's work tree).
 init::
 	Initialize the submodules, i.e. register each submodule name
 	and url found in .gitmodules into .git/config.
+	It will also copy the value of `submodule.$name.update` into
+	.git/config.
 	The key used in .git/config is `submodule.$name.url`.
 	This command does not alter existing information in .git/config.
 	You can then customize the submodule clone URLs in .git/config
@@ -133,7 +135,7 @@ update::
 	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` or `merge`.
+	`rebase`, `merge` or `none`.
 +
 If the submodule is not yet initialized, and you just want to use the
 setting as stored in .gitmodules, you can automatically initialize the
@@ -141,6 +143,10 @@ submodule with the `--init` option.
 +
 If `--recursive` is specified, this command will recurse into the
 registered submodules, and update any nested submodules within.
++
+If the configuration key `submodule.$name.update` is set to `none` the
+submodule with name `$name` will not be updated by default. This can be
+overriden by adding `--checkout` to the command.
 
 summary::
 	Show commit summary between the given commit (defaults to HEAD) and
diff --git a/git-submodule.sh b/git-submodule.sh
index e544dbc..34d2be6 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -429,6 +429,9 @@ cmd_update()
 		--recursive)
 			recursive=1
 			;;
+		--checkout)
+			update="checkout"
+			;;
 		--)
 			shift
 			break
@@ -468,6 +471,12 @@ cmd_update()
 			update_module=$(git config submodule."$name".update)
 		fi
 
+		if test "$update_module" = "none"
+		then
+			echo "Skipping submodule '$path'"
+			continue
+		fi
+
 		if test -z "$url"
 		then
 			# Only mention uninitialized submodules when its
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index c679f36..58877d3 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -30,6 +30,7 @@ test_expect_success 'setup a submodule tree' '
 	git clone super submodule &&
 	git clone super rebasing &&
 	git clone super merging &&
+	git clone super none &&
 	(cd super &&
 	 git submodule add ../submodule submodule &&
 	 test_tick &&
@@ -58,6 +59,11 @@ test_expect_success 'setup a submodule tree' '
 	 test_tick &&
 	 git commit -m "rebasing"
 	)
+	(cd super &&
+	 git submodule add ../none none &&
+	 test_tick &&
+	 git commit -m "none"
+	)
 '
 
 test_expect_success 'submodule update detaching the HEAD ' '
@@ -298,6 +304,62 @@ test_expect_success 'submodule update ignores update=rebase config for new submo
 	)
 '
 
+test_expect_success 'submodule init picks up update=none' '
+	(cd super &&
+	 git config -f .gitmodules submodule.none.update none &&
+	 git submodule init none &&
+	 test "none" = "$(git config submodule.none.update)"
+	)
+'
+
+test_expect_success 'submodule update - update=none in .git/config' '
+	(cd super &&
+	 git config submodule.submodule.update none &&
+	 (cd submodule &&
+	  git checkout master &&
+	  compare_head
+	 ) &&
+	 git diff --raw | grep "	submodule" &&
+	 git submodule update &&
+	 git diff --raw | grep "	submodule" &&
+	 (cd submodule &&
+	  compare_head
+	 ) &&
+	 git config --unset submodule.submodule.update &&
+	 git submodule update submodule
+	)
+'
+
+test_expect_success 'submodule update - update=none in .git/config but --checkout given' '
+	(cd super &&
+	 git config submodule.submodule.update none &&
+	 (cd submodule &&
+	  git checkout master &&
+	  compare_head
+	 ) &&
+	 git diff --raw | grep "	submodule" &&
+	 git submodule update --checkout &&
+	 test_must_fail git diff --raw \| grep "	submodule" &&
+	 (cd submodule &&
+	  test_must_fail compare_head
+	 ) &&
+	 git config --unset submodule.submodule.update
+	)
+'
+
+test_expect_success 'submodule update --init skips submodule with update=none' '
+	(cd super &&
+	 git add .gitmodules &&
+	 git commit -m ".gitmodules"
+	) &&
+	git clone super cloned &&
+	(cd cloned &&
+	 git submodule update --init &&
+	 test -e submodule/.git &&
+	 test_must_fail test -e none/.git
+	)
+'
+
 test_expect_success 'submodule update continues after checkout error' '
 	(cd super &&
 	 git reset --hard HEAD &&
-- 
1.7.6.435.g741d34

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] Add an update=none option for 'loose' submodules
  2011-08-11 17:51 [PATCH 0/2] Add an update=none option for 'loose' submodules Heiko Voigt
  2011-08-11 17:51 ` [PATCH 1/2] submodule: move update configuration variable further up Heiko Voigt
  2011-08-11 17:51 ` [PATCH 2/2] add update 'none' flag to disable update of submodule by default Heiko Voigt
@ 2011-08-11 18:28 ` Junio C Hamano
  2011-08-11 20:00   ` Heiko Voigt
  2 siblings, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2011-08-11 18:28 UTC (permalink / raw)
  To: Heiko Voigt; +Cc: git, Jens Lehmann

Heiko Voigt <hvoigt@hvoigt.net> writes:

> If a submodule is used to seperate some bigger parts of a project into
> an optional directory it is helpful to not clone/update them by default.

Sorry if I am slow, but I do not get this.

I thought unless you say "submodule init" once, a submodule you are not
interested in should not be cloned nor updated at all. If that is not the
case, isn't it a bug to be fixed without a new configuration variable that
fixes it only when it is set?

> We have been talking about loose submodules for some time:

Also before introducing a new terminology "loose submodule", please define
it somewhere. It feels confusing to me that a normal submodule, which
shouldn't be auto-cloned nor auto-updated without "submodule init", needs
to be called by a name other than simply a "submodule" but with an
adjuctive "loose submodule".

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] Add an update=none option for 'loose' submodules
  2011-08-11 18:28 ` [PATCH 0/2] Add an update=none option for 'loose' submodules Junio C Hamano
@ 2011-08-11 20:00   ` Heiko Voigt
  2011-08-15 20:37     ` Junio C Hamano
  0 siblings, 1 reply; 15+ messages in thread
From: Heiko Voigt @ 2011-08-11 20:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jens Lehmann

Hi Junio,

On Thu, Aug 11, 2011 at 11:28:31AM -0700, Junio C Hamano wrote:
> Heiko Voigt <hvoigt@hvoigt.net> writes:
> 
> > If a submodule is used to seperate some bigger parts of a project into
> > an optional directory it is helpful to not clone/update them by default.
> 
> Sorry if I am slow, but I do not get this.
> 
> I thought unless you say "submodule init" once, a submodule you are not
> interested in should not be cloned nor updated at all. If that is not the
> case, isn't it a bug to be fixed without a new configuration variable that
> fixes it only when it is set?

What I usually do is say "submodule init" without any extra option once.
That will register all submodules from .gitmodules in the config. Now
when I say "submodule update" all submodules would be cloned. In the
case of recursive submodules actually

	git submodule update --init --recursive

is the only command which can get you really everything in one go.

Do you think the "submodule init" behavior is wrong? If so I think its a
bit late to change this since people using submodules (me included)
already have got used to it.

With this config variable all submodules will still be registered to
.git/config on "submodule init" but "submodule update" will skip those
submodules. Since we already have merge and rebase as alternate options
to update a submodule it just sounds logical to me to have an additional
option to disable updating.

> > We have been talking about loose submodules for some time:
> 
> Also before introducing a new terminology "loose submodule", please define
> it somewhere. It feels confusing to me that a normal submodule, which
> shouldn't be auto-cloned nor auto-updated without "submodule init", needs
> to be called by a name other than simply a "submodule" but with an
> adjuctive "loose submodule".

Thats why I avoided talking about it in the docs. For the commit message
I thought it would be kind of intuitive but I can update the commit
message so that it becomes more clear.

Cheers Heiko

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] Add an update=none option for 'loose' submodules
  2011-08-11 20:00   ` Heiko Voigt
@ 2011-08-15 20:37     ` Junio C Hamano
  2011-08-22 20:00       ` Heiko Voigt
  0 siblings, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2011-08-15 20:37 UTC (permalink / raw)
  To: Heiko Voigt; +Cc: git, Jens Lehmann

Heiko Voigt <hvoigt@hvoigt.net> writes:

> On Thu, Aug 11, 2011 at 11:28:31AM -0700, Junio C Hamano wrote:
>> Heiko Voigt <hvoigt@hvoigt.net> writes:
>> 
>> > If a submodule is used to seperate some bigger parts of a project into
>> > an optional directory it is helpful to not clone/update them by default.
>> 
>> Sorry if I am slow, but I do not get this.
>> 
>> I thought unless you say "submodule init" once, a submodule you are not
>> interested in should not be cloned nor updated at all. If that is not the
>> case, isn't it a bug to be fixed without a new configuration variable that
>> fixes it only when it is set?
>
> What I usually do is say "submodule init" without any extra option once.
> That will register all submodules from .gitmodules in the config. Now
> when I say "submodule update" all submodules would be cloned. In the
> case of recursive submodules actually
>
> 	git submodule update --init --recursive
>
> is the only command which can get you really everything in one go.
>
> Do you think the "submodule init" behavior is wrong? If so I think its a
> bit late to change this since people using submodules (me included)
> already have got used to it.
>
> With this config variable all submodules will still be registered to
> .git/config on "submodule init" but "submodule update" will skip those
> submodules.

Ok, that sort-of makes sense, but we have been using "do we have the
submodule registered in the .git/config of the superproject?" to decide
"does the user interested in having a checkout of the submodule?" (I think
in the ancient days it was "do we have .git in that submodule directory?"
that decided it, but we dropped that because it won't work when switching
branches that has and does not have the submodule in superproject).

It is somewhat worrying that some parts of the system may still be using
that old criteria "do we have it in .git/config of the superproject?" to
decide if the user is interested in the submodule. If so they need to be
updated to take this new semantics "do we have it in .git/config without
its submodule.$name.update set to none" into account. We would probably
need to have a paragraph in the release notes to warn about the semantics
change (which I tend to agree with you that it is a good one).

>> > We have been talking about loose submodules for some time:
>> 
>> Also before introducing a new terminology "loose submodule", please define
>> it somewhere. It feels confusing to me that a normal submodule, which
>> shouldn't be auto-cloned nor auto-updated without "submodule init", needs
>> to be called by a name other than simply a "submodule" but with an
>> adjuctive "loose submodule".
>
> Thats why I avoided talking about it in the docs. For the commit message
> I thought it would be kind of intuitive but I can update the commit
> message so that it becomes more clear.

That sounds like a good thing to do.

Thanks.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Re: [PATCH 0/2] Add an update=none option for 'loose' submodules
  2011-08-15 20:37     ` Junio C Hamano
@ 2011-08-22 20:00       ` Heiko Voigt
  2011-08-22 22:42         ` Junio C Hamano
  0 siblings, 1 reply; 15+ messages in thread
From: Heiko Voigt @ 2011-08-22 20:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jens Lehmann

Hi,

On Mon, Aug 15, 2011 at 01:37:53PM -0700, Junio C Hamano wrote:
> Heiko Voigt <hvoigt@hvoigt.net> writes:
> 
> > On Thu, Aug 11, 2011 at 11:28:31AM -0700, Junio C Hamano wrote:
> >> Heiko Voigt <hvoigt@hvoigt.net> writes:
> >> > We have been talking about loose submodules for some time:
> >> 
> >> Also before introducing a new terminology "loose submodule", please define
> >> it somewhere. It feels confusing to me that a normal submodule, which
> >> shouldn't be auto-cloned nor auto-updated without "submodule init", needs
> >> to be called by a name other than simply a "submodule" but with an
> >> adjuctive "loose submodule".
> >
> > Thats why I avoided talking about it in the docs. For the commit message
> > I thought it would be kind of intuitive but I can update the commit
> > message so that it becomes more clear.
> 
> That sounds like a good thing to do.

I discovered that I only talked in the cover letter about the term
'loose'. Since that will not go into any commit I guess we can keep the
series this way?

Cheers Heiko

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] Add an update=none option for 'loose' submodules
  2011-08-22 20:00       ` Heiko Voigt
@ 2011-08-22 22:42         ` Junio C Hamano
  2011-08-23 19:43           ` Heiko Voigt
  0 siblings, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2011-08-22 22:42 UTC (permalink / raw)
  To: Heiko Voigt; +Cc: git, Jens Lehmann

Heiko Voigt <hvoigt@hvoigt.net> writes:

> On Mon, Aug 15, 2011 at 01:37:53PM -0700, Junio C Hamano wrote:
>> Heiko Voigt <hvoigt@hvoigt.net> writes:
>> 
>> > On Thu, Aug 11, 2011 at 11:28:31AM -0700, Junio C Hamano wrote:
>> >> Heiko Voigt <hvoigt@hvoigt.net> writes:
>> >> > We have been talking about loose submodules for some time:
>> >> 
>> >> Also before introducing a new terminology "loose submodule", please define
>> ... 
>> That sounds like a good thing to do.
> 
> I discovered that I only talked in the cover letter about the term
> 'loose'. Since that will not go into any commit I guess we can keep the
> series this way?

Yes, except that I do not have a clear answer to my other half of the
question in the same message you omitted from your quote.

>> What I usually do is say "submodule init" without any extra option once.
>> That will register all submodules from .gitmodules in the config. Now
>> when I say "submodule update" all submodules would be cloned. In the
>> case of recursive submodules actually
>>
>> 	git submodule update --init --recursive
>>
>> is the only command which can get you really everything in one go.
>>
>> Do you think the "submodule init" behavior is wrong? If so I think its a
>> bit late to change this since people using submodules (me included)
>> already have got used to it.
>>
>> With this config variable all submodules will still be registered to
>> .git/config on "submodule init" but "submodule update" will skip those
>> submodules.
>
> Ok, that sort-of makes sense, but we have been using "do we have the
> submodule registered in the .git/config of the superproject?" to decide
> "does the user interested in having a checkout of the submodule?" (I think
> in the ancient days it was "do we have .git in that submodule directory?"
> that decided it, but we dropped that because it won't work when switching
> branches that has and does not have the submodule in superproject).
>
> It is somewhat worrying that some parts of the system may still be using
> that old criteria "do we have it in .git/config of the superproject?" to
> decide if the user is interested in the submodule. If so they need to be
> updated to take this new semantics "do we have it in .git/config without
> its submodule.$name.update set to none" into account. We would probably
> need to have a paragraph in the release notes to warn about the semantics
> change (which I tend to agree with you that it is a good one).

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] Add an update=none option for 'loose' submodules
  2011-08-22 22:42         ` Junio C Hamano
@ 2011-08-23 19:43           ` Heiko Voigt
  2011-08-23 20:18             ` Jens Lehmann
  0 siblings, 1 reply; 15+ messages in thread
From: Heiko Voigt @ 2011-08-23 19:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jens Lehmann

Hi,

On Mon, Aug 22, 2011 at 03:42:55PM -0700, Junio C Hamano wrote:
> > On Mon, Aug 15, 2011 at 01:37:53PM -0700, Junio C Hamano wrote:
> >> Heiko Voigt <hvoigt@hvoigt.net> writes:
> >> What I usually do is say "submodule init" without any extra option once.
> >> That will register all submodules from .gitmodules in the config. Now
> >> when I say "submodule update" all submodules would be cloned. In the
> >> case of recursive submodules actually
> >>
> >> 	git submodule update --init --recursive
> >>
> >> is the only command which can get you really everything in one go.
> >>
> >> Do you think the "submodule init" behavior is wrong? If so I think its a
> >> bit late to change this since people using submodules (me included)
> >> already have got used to it.
> >>
> >> With this config variable all submodules will still be registered to
> >> .git/config on "submodule init" but "submodule update" will skip those
> >> submodules.
> >
> > Ok, that sort-of makes sense, but we have been using "do we have the
> > submodule registered in the .git/config of the superproject?" to decide
> > "does the user interested in having a checkout of the submodule?" (I think
> > in the ancient days it was "do we have .git in that submodule directory?"
> > that decided it, but we dropped that because it won't work when switching
> > branches that has and does not have the submodule in superproject).
> >
> > It is somewhat worrying that some parts of the system may still be using
> > that old criteria "do we have it in .git/config of the superproject?" to
> > decide if the user is interested in the submodule. If so they need to be
> > updated to take this new semantics "do we have it in .git/config without
> > its submodule.$name.update set to none" into account. We would probably
> > need to have a paragraph in the release notes to warn about the semantics
> > change (which I tend to agree with you that it is a good one).

Sorry that I forgot to answer to this. I am not sure what you mean by
"the semantics change". This patch does not change any existing
behavior. I rather see this as an extra way to specify the default
behavior of what happens on submodule update. If people do not use it
there will be no expectations broken.

Another change I am thinking of (which would definitely need an entry in
the release notes) is to change submodule foreach to iterate over all
gitmodule entries in the index/HEAD/worktree (not sure yet) instead of
"just entries that are in .git/config".

What do you think?

Cheers Heiko

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] Add an update=none option for 'loose' submodules
  2011-08-23 19:43           ` Heiko Voigt
@ 2011-08-23 20:18             ` Jens Lehmann
  2011-08-23 21:46               ` Junio C Hamano
  2011-08-24 20:38               ` Heiko Voigt
  0 siblings, 2 replies; 15+ messages in thread
From: Jens Lehmann @ 2011-08-23 20:18 UTC (permalink / raw)
  To: Heiko Voigt; +Cc: Junio C Hamano, git

Am 23.08.2011 21:43, schrieb Heiko Voigt:
> On Mon, Aug 22, 2011 at 03:42:55PM -0700, Junio C Hamano wrote:
>>> It is somewhat worrying that some parts of the system may still be using
>>> that old criteria "do we have it in .git/config of the superproject?" to
>>> decide if the user is interested in the submodule. If so they need to be
>>> updated to take this new semantics "do we have it in .git/config without
>>> its submodule.$name.update set to none" into account. We would probably
>>> need to have a paragraph in the release notes to warn about the semantics
>>> change (which I tend to agree with you that it is a good one).
> 
> Sorry that I forgot to answer to this. I am not sure what you mean by
> "the semantics change". This patch does not change any existing
> behavior. I rather see this as an extra way to specify the default
> behavior of what happens on submodule update. If people do not use it
> there will be no expectations broken.

It might surprise people. E.g. when their old scripts don't work anymore as
they did before because a submodule won't be populated or updated in the work
tree even though it is present in .git/config. So I agree that this should be
documented in the release notes so people can check if their expectations are
still met.

> Another change I am thinking of (which would definitely need an entry in
> the release notes) is to change submodule foreach to iterate over all
> gitmodule entries in the index/HEAD/worktree (not sure yet) instead of
> "just entries that are in .git/config".

When changing the default I think we'll surprise a lot of users (imagine
someone running a "git submodule foreach pwd" when some submodules aren't
populated). But adding an option to "git submodule foreach" (and maybe others)
to get the list of submodules from the index or HEAD might make sense (while
I'm not sure parsing the work tree does, as you'll basically have to pick up
any .git you find. AFAICS a submodule is defined either by an entry in the
.gitmodules file, in .git/config or through a gitlink entry in a commit or the
index. So maybe the third alternative to index and HEAD is to use those found
in .gitmodules?).

Could you describe a use case for that?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] Add an update=none option for 'loose' submodules
  2011-08-23 20:18             ` Jens Lehmann
@ 2011-08-23 21:46               ` Junio C Hamano
  2011-08-24 19:30                 ` Heiko Voigt
  2011-08-24 20:38               ` Heiko Voigt
  1 sibling, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2011-08-23 21:46 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: Heiko Voigt, Junio C Hamano, git

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

> It might surprise people. E.g. when their old scripts don't work anymore as
> they did before because a submodule won't be populated or updated in the work
> tree even though it is present in .git/config. So I agree that this should be
> documented in the release notes so people can check if their expectations are
> still met.

Worse yet, their custom old scripts that they use to update submodules in
their repository, if properly written, assume that anything registered in
the .git/config file as [submodule "foo"] _must_ be populated, but they
can no longer assume that and now has to look at submodule.foo.update and
if it notices the variable is set to "none" leave the submodule repository
alone. Having "submodule.foo" registered in the .git/config file alone
used to mean the user is interested in "foo" submodule and wants to have a
checkout for it, now it does not necessarily mean that.

That is definitely a huge semantics change.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] Add an update=none option for 'loose' submodules
  2011-08-23 21:46               ` Junio C Hamano
@ 2011-08-24 19:30                 ` Heiko Voigt
  2011-08-26  6:27                   ` Junio C Hamano
  0 siblings, 1 reply; 15+ messages in thread
From: Heiko Voigt @ 2011-08-24 19:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jens Lehmann, git

Hi,

On Tue, Aug 23, 2011 at 02:46:39PM -0700, Junio C Hamano wrote:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
> > It might surprise people. E.g. when their old scripts don't work anymore as
> > they did before because a submodule won't be populated or updated in the work
> > tree even though it is present in .git/config. So I agree that this should be
> > documented in the release notes so people can check if their expectations are
> > still met.
> 
> Worse yet, their custom old scripts that they use to update submodules in
> their repository, if properly written, assume that anything registered in
> the .git/config file as [submodule "foo"] _must_ be populated, but they
> can no longer assume that and now has to look at submodule.foo.update and
> if it notices the variable is set to "none" leave the submodule repository
> alone. Having "submodule.foo" registered in the .git/config file alone
> used to mean the user is interested in "foo" submodule and wants to have a
> checkout for it, now it does not necessarily mean that.
> 
> That is definitely a huge semantics change.

Ok seeing it that way. You are right. How about this?

-8<---
From: Heiko Voigt <hvoigt@hvoigt.net>
Subject: [PATCH] mention the new submodule.$name.update=none flag in the
 ReleaseNotes

---
 Documentation/RelNotes/1.7.7.txt |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/Documentation/RelNotes/1.7.7.txt b/Documentation/RelNotes/1.7.7.txt
index 8de880f..b8edcf1 100644
--- a/Documentation/RelNotes/1.7.7.txt
+++ b/Documentation/RelNotes/1.7.7.txt
@@ -71,6 +71,10 @@ Updates since v1.7.6
    submodule; it now goes on to update other submodules that can be
    updated, and reports the ones with errors at the end.
 
+ * "git submodule update" does not clone/update a submodule when
+   submodule.$name.update is set to 'none'. This option is copied from
+   .gitmodules when a submodule is initialized.
+
  * "git upload-pack" and "git receive-pack" learned to pretend only a
    subset of the refs exist in a repository. This may help a site to
    put many tiny repositories into one repository (this would not be
-- 
1.7.6.551.g4266ca

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] Add an update=none option for 'loose' submodules
  2011-08-23 20:18             ` Jens Lehmann
  2011-08-23 21:46               ` Junio C Hamano
@ 2011-08-24 20:38               ` Heiko Voigt
  1 sibling, 0 replies; 15+ messages in thread
From: Heiko Voigt @ 2011-08-24 20:38 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: Junio C Hamano, git

On Tue, Aug 23, 2011 at 10:18:11PM +0200, Jens Lehmann wrote:
> Am 23.08.2011 21:43, schrieb Heiko Voigt:
> > Another change I am thinking of (which would definitely need an entry in
> > the release notes) is to change submodule foreach to iterate over all
> > gitmodule entries in the index/HEAD/worktree (not sure yet) instead of
> > "just entries that are in .git/config".
> 
> When changing the default I think we'll surprise a lot of users (imagine
> someone running a "git submodule foreach pwd" when some submodules aren't
> populated). But adding an option to "git submodule foreach" (and maybe others)
> to get the list of submodules from the index or HEAD might make sense (while
> I'm not sure parsing the work tree does, as you'll basically have to pick up
> any .git you find. AFAICS a submodule is defined either by an entry in the
> .gitmodules file, in .git/config or through a gitlink entry in a commit or the
> index. So maybe the third alternative to index and HEAD is to use those found
> in .gitmodules?).
> 
> Could you describe a use case for that?

Yes, a repository using the submodule.$name.checkout=none config.

Currently its hard to iterate over all submodules to set this config to
'checkout' locally. You can not use submodule foreach for that since it
will skip submodule directories that do not have .git in them.

But you are right this should obviously be done using an option like

	git submodule foreach --head ...

or similar.

Cheers Heiko

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] Add an update=none option for 'loose' submodules
  2011-08-24 19:30                 ` Heiko Voigt
@ 2011-08-26  6:27                   ` Junio C Hamano
  2011-08-26 16:14                     ` Jens Lehmann
  0 siblings, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2011-08-26  6:27 UTC (permalink / raw)
  To: Heiko Voigt; +Cc: Jens Lehmann, git

Heiko Voigt <hvoigt@hvoigt.net> writes:

>> That is definitely a huge semantics change.
>
> Ok seeing it that way. You are right. How about this?

Actually I had an update to Documentation/git-submodule.txt in mind. For
example, we say this for "update":

  Update the registered submodules, i.e. clone missing submodules and
  checkout the commit specified in the index of the containing repository.

I know you added "yes, we earlier said this will clone missing ones and
checkout, but if this configuration is set to none then none of that
happens" much later in the section, but that feels backwards.

Thinking about it more, I am starting to think that this backwardness may
be an indication that we are describing a wrong solution to a wrong
problem.

Isn't the root cause of the issue that a "submodule init" without pathspec
limit adds everything to .git/config, ending up with all submodules fully
instantiated, and it is too easy to run such a lazy "submodule init"?

If we allowed the project managers (i.e. the ones who write .gitmodules)
to specify the default set of submodules to be initialized with such a
"submodule init", omitting some submodules from even getting registered to
the recipients' .git/config in the first place, wouldn't that solve the
issue you are trying to address equally well, without anything to worry
about this semantic change at all? I am trying to see if we can come up
with a solution with which we do not even have to add any entry for a
submodule to .git/config of the superproject, if it is "don't care" kind
of submodule for the copy of the superproject repository the user has.

The way in which the project managers specify that a module is not meant
to be "init"ed by default may be to have "submodule.$name.update = none"
in the .gitmodules file they ship, so externally there may not be huge
difference from the behaviour (but not the implementation) of your patch,
even though submodule.$name.update probably is not a good variable name to
be used for this purpose.

Another thing we may want to consider is to make .gitmodules describe
submodule dependencies. If your hypothetical superproject is about a
library, which consists of doc/, include/, and libsrc/ submodules, with
pre-built binary perhaps shipped as part of the superproject itself, those
who work on documentation may want to populate only doc/, those who are
interested in using the library may want to populate only include/ and
possibly doc/, and those who work on the library itself would populate
include/ and libsrc/, possibly with doc/ submodules. It wouldn't make any
sense to populate libsrc/ without populating include/ submodule, as the
source would not be buildable without the includes.

Now if we imagine that much more people are interested in using the
library than working on it, it is plausible that the project may want to
suggest:

 - Majority of people may want to omit libsrc/ submodule; and

 - If you populate libsrc/, then you would definitely want to populate
   include/ submodule.

Your "submodule.libsrc.update = none" in .gitmodules can express the
former, and I think it is natural to express the latter (i.e. submodule
dependency) in the same file, to be propagated in the same way to the
consumers.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] Add an update=none option for 'loose' submodules
  2011-08-26  6:27                   ` Junio C Hamano
@ 2011-08-26 16:14                     ` Jens Lehmann
  0 siblings, 0 replies; 15+ messages in thread
From: Jens Lehmann @ 2011-08-26 16:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Heiko Voigt, git

Am 26.08.2011 08:27, schrieb Junio C Hamano:
> Thinking about it more, I am starting to think that this backwardness may
> be an indication that we are describing a wrong solution to a wrong
> problem.

I Agree, I see two related topics mingled here.

> Isn't the root cause of the issue that a "submodule init" without pathspec
> limit adds everything to .git/config, ending up with all submodules fully
> instantiated, and it is too easy to run such a lazy "submodule init"?

Yes. And that upstream has no option (yet) to tell which submodules should
be automatically initialized on clone, which would get rid of the need to
run "git submodule init" in the first place for the average user.

To me Heiko's change fixes that issue `accidentally` because "update=none"
implies "even when it is initialized I never want to update its work tree,
so don't even clone it". Which is correct but not quite the meaning of the
update config.

> If we allowed the project managers (i.e. the ones who write .gitmodules)
> to specify the default set of submodules to be initialized with such a
> "submodule init", omitting some submodules from even getting registered to
> the recipients' .git/config in the first place, wouldn't that solve the
> issue you are trying to address equally well, without anything to worry
> about this semantic change at all? I am trying to see if we can come up
> with a solution with which we do not even have to add any entry for a
> submodule to .git/config of the superproject, if it is "don't care" kind
> of submodule for the copy of the superproject repository the user has.

I really think we need such an option. I imagine enabling it to even
initialize the submodules on clone or when checking out a branch
introducing a new submodule (and fetch should proactively fetch such
submodules so they can be checked out when disconnected later). Working
title: "init=[auto|no]", with "auto" initializing it on clone and "none"
never initializing it unless the submodule is given on the command line.

> The way in which the project managers specify that a module is not meant
> to be "init"ed by default may be to have "submodule.$name.update = none"
> in the .gitmodules file they ship, so externally there may not be huge
> difference from the behaviour (but not the implementation) of your patch,
> even though submodule.$name.update probably is not a good variable name to
> be used for this purpose.

IMO the update option is about what should happen to an initialized
submodule when a the commit recorded in the superproject changes: Shall it
be checked out, rebased, merged or left alone. Heiko's proposal adds the
missing fourth case, so I think his patch is going in the right direction.
But we should have another config option to state "not to be initialized".

> Another thing we may want to consider is to make .gitmodules describe
> submodule dependencies. If your hypothetical superproject is about a
> library, which consists of doc/, include/, and libsrc/ submodules, with
> pre-built binary perhaps shipped as part of the superproject itself, those
> who work on documentation may want to populate only doc/, those who are
> interested in using the library may want to populate only include/ and
> possibly doc/, and those who work on the library itself would populate
> include/ and libsrc/, possibly with doc/ submodules. It wouldn't make any
> sense to populate libsrc/ without populating include/ submodule, as the
> source would not be buildable without the includes.

Yes, it would be cool having a "git submodule init libsrc"  initializing
submodule "include" too.

> Now if we imagine that much more people are interested in using the
> library than working on it, it is plausible that the project may want to
> suggest:
> 
>  - Majority of people may want to omit libsrc/ submodule; and
> 
>  - If you populate libsrc/, then you would definitely want to populate
>    include/ submodule.
> 
> Your "submodule.libsrc.update = none" in .gitmodules can express the
> former, and I think it is natural to express the latter (i.e. submodule
> dependency) in the same file, to be propagated in the same way to the
> consumers.

As I said above, I think "submodule.libsrc.update = none" only does the
trick because "git submodule update" is used to both clone an update a
submodule. I vote for a separate option to express the first condition.

For me this is thread covers three different issues:

1) What submodules should be initialized?
   I think we need to add a new option ("init"?) to control that.

2) How should initialized submodules be updated when the SHA1 changes?
   This is what "update" should control, and Heiko's patch adds the
   missing "none" case.

3) What dependencies do submodules have?
   This is slightly orthogonal but should honor the settings in 1) and 2).

And I believe there are use cases where you want the submodule to be
initialized but never updated (and I expect "ignore=all" to be set in
that case too, as the user should not be bothered if the submodule is out
of sync). Imagine large intro movies of a gaming project separated into a
submodule which each developer should have to start the application but
doesn't care if he doesn't have the most up-to-date version.

So before people start (mis-)using the "update=none" option because they
can't use "init=none" yet, I vote for an additional patch implementing
that config option (per submodule and globally). Then we can document each
option properly.

Does that make sense?

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2011-08-26 16:14 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-11 17:51 [PATCH 0/2] Add an update=none option for 'loose' submodules Heiko Voigt
2011-08-11 17:51 ` [PATCH 1/2] submodule: move update configuration variable further up Heiko Voigt
2011-08-11 17:51 ` [PATCH 2/2] add update 'none' flag to disable update of submodule by default Heiko Voigt
2011-08-11 18:28 ` [PATCH 0/2] Add an update=none option for 'loose' submodules Junio C Hamano
2011-08-11 20:00   ` Heiko Voigt
2011-08-15 20:37     ` Junio C Hamano
2011-08-22 20:00       ` Heiko Voigt
2011-08-22 22:42         ` Junio C Hamano
2011-08-23 19:43           ` Heiko Voigt
2011-08-23 20:18             ` Jens Lehmann
2011-08-23 21:46               ` Junio C Hamano
2011-08-24 19:30                 ` Heiko Voigt
2011-08-26  6:27                   ` Junio C Hamano
2011-08-26 16:14                     ` Jens Lehmann
2011-08-24 20:38               ` Heiko Voigt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.