[PATCH 0/3] submodule improvements

[PATCH 0/3] submodule improvements

[Lars Hjemli]

This is a series of somewhat unrelated changes, but all related to
git-submodule.

1/3 implements a mapping between submodule path and submodule name
2/3 adds a gitmodules(5) document
3/3 fixes a bug in testscript t7400, noticed when making 1/3

The stats:

Documentation/Makefile                             |    2 +-
Documentation/git-submodule.txt                    |   16 +++--
Documentation/gitmodules.txt                       |   63 ++++++++++++++++++++
git-submodule.sh                                   |   44 +++++++++----
t/t7400-submodule-basic.sh                         |    2 +-
...submodule-basic.sh => t7401-submodule-named.sh} |   27 ++++++---
6 files changed, 124 insertions(+), 30 deletions(-)

[PATCH 1/3] git-submodule: allow submodule name and path to differ

[Lars Hjemli]

This teaches git-submodule to check module.*.path when looking for the
config for a submodule path. If no match is found it falls back to the
current behaviour (module.$path).

With this change a submodule can be checked out at different paths in
different revisions of the superproject without changing the submodule
properties in .git/config.

While at it, add a new testscript for named submodules. This is basically
the same tests as can be found in t7400, but modified for a submodule
with name != path.

Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
 Documentation/git-submodule.txt                    |   16 ++++---
 git-submodule.sh                                   |   44 +++++++++++++------
 ...submodule-basic.sh => t7401-submodule-named.sh} |   27 ++++++++----
 3 files changed, 59 insertions(+), 28 deletions(-)
 copy t/{t7400-submodule-basic.sh => t7401-submodule-named.sh} (82%)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index f8fb80f..176e16d 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -24,8 +24,8 @@ status::
 
 init::
 	Initialize the submodules, i.e. register in .git/config each submodule
-	path and url found in .gitmodules. The key used in git/config is
-	`submodule.$path.url`. This command does not alter existing information
+	name and url found in .gitmodules. The key used in git/config is
+	`submodule.$name.url`. This command does not alter existing information
 	in .git/config.
 
 update::
@@ -50,11 +50,15 @@ OPTIONS
 
 FILES
 -----
-When initializing submodules, a .gitmodules file in the top-level directory
-of the containing repository is used to find the url of each submodule.
-This file should be formatted in the same way as $GIR_DIR/config. The key
-to each submodule url is "module.$path.url".
+The .gitmodules file in the top-level directory of the containing repository
+is used to map submodule path to submodule name. The map consist of keys
+named `module.$name.path` with a value matching the path found in the index of
+the containing repository. If no such key is found for a specific path, the
+submodule path is used as submodule name.
 
+During `git-submodule init` the url for each submodule is registered in
+.git/config of the containing repository. The url is located under the key
+`module.$name.url` in .gitmodules.
 
 AUTHOR
 ------
diff --git a/git-submodule.sh b/git-submodule.sh
index 8bdd99a..d9f0c91 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -25,6 +25,18 @@ say()
 	fi
 }
 
+#
+# Resolve submodule name from path, use path as name if no mapping exist
+#
+# $1 = path
+#
+module_name()
+{
+	name=$(GIT_CONFIG=.gitmodules git-config --get-regexp '^module\..*\.path$' "$1" |
+		sed -nre 's/^module\.(.+)\.path .+$/\1/p')
+	test -z "$name" && name="$1"
+	echo $name
+}
 
 #
 # Clone a submodule
@@ -62,18 +74,19 @@ modules_init()
 	git ls-files --stage -- "$@" | grep -e '^160000 ' |
 	while read mode sha1 stage path
 	do
-		# Skip already registered paths
-		url=$(git-config submodule."$path".url)
+		# Skip already registered submodules
+		name=$(module_name "$path")
+		url=$(git-config submodule."$name".url)
 		test -z "$url" || continue
 
-		url=$(GIT_CONFIG=.gitmodules git-config module."$path".url)
+		url=$(GIT_CONFIG=.gitmodules git-config module."$name".url)
 		test -z "$url" &&
-		die "No url found for submodule '$path' in .gitmodules"
+		die "No url found for submodule '$name', path '$path' in .gitmodules"
 
-		git-config submodule."$path".url "$url" ||
-		die "Failed to register url for submodule '$path'"
+		git-config submodule."$name".url "$url" ||
+		die "Failed to register url for submodule '$name', path '$path'"
 
-		say "Submodule '$path' registered with url '$url'"
+		say "Submodule '$name', path '$path' registered with url '$url'"
 	done
 }
 
@@ -87,13 +100,14 @@ modules_update()
 	git ls-files --stage -- "$@" | grep -e '^160000 ' |
 	while read mode sha1 stage path
 	do
-		url=$(git-config submodule."$path".url)
+		name=$(module_name "$path")
+		url=$(git-config submodule."$name".url)
 		if test -z "$url"
 		then
 			# Only mention uninitialized submodules when its
 			# path have been specified
 			test "$#" != "0" &&
-			say "Submodule '$path' not initialized"
+			say "Submodule '$name', path '$path' not initialized"
 			continue
 		fi
 
@@ -104,15 +118,15 @@ modules_update()
 
 		subsha1=$(unset GIT_DIR && cd "$path" &&
 			git-rev-parse --verify HEAD) ||
-		die "Unable to find current revision of submodule '$path'"
+		die "Unable to find current revision of submodule '$name', path '$path'"
 
 		if test "$subsha1" != "$sha1"
 		then
 			(unset GIT_DIR && cd "$path" && git-fetch &&
 				git-checkout -q "$sha1") ||
-			die "Unable to checkout '$sha1' in submodule '$path'"
+			die "Unable to checkout '$sha1' in submodule '$name', path '$path'"
 
-			say "Submodule '$path': checked out '$sha1'"
+			say "Submodule '$name', path '$path': checked out '$sha1'"
 		fi
 	done
 }
@@ -132,10 +146,12 @@ modules_list()
 	git ls-files --stage -- "$@" | grep -e '^160000 ' |
 	while read mode sha1 stage path
 	do
-		if ! test -d "$path"/.git
+		name=$(module_name "$path")
+		url=$(git-config submodule."$name".url)
+		if test -z "$url" || ! test -d "$path"/.git
 		then
 			say "-$sha1 $path"
-			continue;
+			continue
 		fi
 		revname=$(unset GIT_DIR && cd "$path" && git-describe $sha1)
 		if git diff-files --quiet -- "$path"
diff --git a/t/t7400-submodule-basic.sh b/t/t7401-submodule-named.sh
similarity index 82%
copy from t/t7400-submodule-basic.sh
copy to t/t7401-submodule-named.sh
index 3940433..3c3d195 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7401-submodule-named.sh
@@ -3,10 +3,10 @@
 # Copyright (c) 2007 Lars Hjemli
 #
 
-test_description='Basic porcelain support for submodules
+test_description='Basic porcelain support for named submodules
 
 This test tries to verify basic sanity of the init, update and status
-subcommands of git-submodule.
+subcommands of git-submodule when a submodule name differs from its path.
 '
 
 . ./test-lib.sh
@@ -18,7 +18,7 @@ subcommands of git-submodule.
 #  -add directory lib to 'superproject', this creates a DIRLINK entry
 #  -add a couple of regular files to enable testing of submodule filtering
 #  -mv lib subrepo
-#  -add an entry to .gitmodules for path 'lib'
+#  -add an entry to .gitmodules for submodule 'foo'
 #
 test_expect_success 'Prepare submodule testing' '
 	mkdir lib &&
@@ -40,7 +40,7 @@ test_expect_success 'Prepare submodule testing' '
 	git-add a lib z &&
 	git-commit -m "super commit 1" &&
 	mv lib .subrepo &&
-	GIT_CONFIG=.gitmodules git-config module.lib.url git://example.com/lib.git
+	GIT_CONFIG=.gitmodules git-config module.foo.url git://example.com/libfoo.git
 '
 
 test_expect_success 'status should only print one line' '
@@ -52,14 +52,25 @@ test_expect_success 'status should initially be "missing"' '
 	git-submodule status | grep "^-$rev1"
 '
 
+test_expect_success 'init should fail when map from path to name is missing' '
+	if git-submodule init
+	then
+		echo "[OOPS] init should have failed"
+		false
+	elif ! GIT_CONFIG=.gitmodules git-config module.foo.path lib
+	then
+		echo "[OOPS] init failed but so did git-config"
+	fi
+'
+
 test_expect_success 'init should register submodule url in .git/config' '
 	git-submodule init &&
-	url=$(git-config submodule.lib.url) &&
-	if test "$url" != "git://example.com/lib.git"
+	url=$(git-config submodule.foo.url) &&
+	if test "$url" != "git://example.com/libfoo.git"
 	then
 		echo "[OOPS] init succeeded but submodule url is wrong"
 		false
-	elif ! git-config submodule.lib.url ./.subrepo
+	elif ! git-config submodule.foo.url ./.subrepo
 	then
 		echo "[OOPS] init succeeded but update of url failed"
 		false
@@ -72,7 +83,7 @@ test_expect_success 'update should fail when path is used by a file' '
 	then
 		echo "[OOPS] update should have failed"
 		false
-	elif test -f lib && test "$(cat lib)" != "hello"
+	elif test "$(cat lib)" != "hello"
 	then
 		echo "[OOPS] update failed but lib file was molested"
 		false
-- 
1.5.2.1.914.gbd3a7

Re: [PATCH 1/3] git-submodule: allow submodule name and path to differ

[Junio C Hamano]

Lars Hjemli <hjemli@gmail.com> writes:

> This teaches git-submodule to check module.*.path when looking for the
> config for a submodule path. If no match is found it falls back to the
> current behaviour (module.$path).

I have a feeling that it might be much less troublesome in the
longer term to admit that module.$path was a mistake and support
only one format; wouldn't trying to support both leave ambiguity
and confusion?

Re: [PATCH 1/3] git-submodule: allow submodule name and path to differ

[Johannes Schindelin]

Hi,

On Sat, 9 Jun 2007, Junio C Hamano wrote:

> Lars Hjemli <hjemli@gmail.com> writes:
> 
> > This teaches git-submodule to check module.*.path when looking for the 
> > config for a submodule path. If no match is found it falls back to the 
> > current behaviour (module.$path).
> 
> I have a feeling that it might be much less troublesome in the longer 
> term to admit that module.$path was a mistake and support only one 
> format; wouldn't trying to support both leave ambiguity and confusion?

Just my 2cents: git-submodule is not yet in any released version. So let's 
fix things early. In our world, it's not like you lose face when you have 
to admit mistakes. (Instead, you lose face when you refuse to fix them.) 
Ah, if only politics learnt from our world...

Ciao,
Dscho

Re: [PATCH 1/3] git-submodule: allow submodule name and path to differ

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Sat, 9 Jun 2007, Junio C Hamano wrote:
>
>> Lars Hjemli <hjemli@gmail.com> writes:
>> 
>> > This teaches git-submodule to check module.*.path when looking for the 
>> > config for a submodule path. If no match is found it falls back to the 
>> > current behaviour (module.$path).
>> 
>> I have a feeling that it might be much less troublesome in the longer 
>> term to admit that module.$path was a mistake and support only one 
>> format; wouldn't trying to support both leave ambiguity and confusion?
>
> Just my 2cents: git-submodule is not yet in any released version. So let's 
> fix things early. In our world, it's not like you lose face when you have 
> to admit mistakes. (Instead, you lose face when you refuse to fix them.) 
> Ah, if only politics learnt from our world...

Well, I completely agree with what you said up to "So let's fix
things early.", but when I re-read what I wrote, I realize that
my wording was bad --- I did not mean to drive the discussion in
that direction.

It was not Lars's *mistake* to admit to begin with.  His was one
valid design that was consistent within his patch series.  The
thing is that there just was a better alternative suggested
later; it does not make the first iteration a mistake in any
way.

So, Lars, my apologies if I offended you -- I did not mean it
that way.

Re: [PATCH 1/3] git-submodule: allow submodule name and path to differ

[Lars Hjemli]

On 6/10/07, Junio C Hamano <gitster@pobox.com> wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > On Sat, 9 Jun 2007, Junio C Hamano wrote:
> >
> >> Lars Hjemli <hjemli@gmail.com> writes:
> >>
> >> > This teaches git-submodule to check module.*.path when looking for the
> >> > config for a submodule path. If no match is found it falls back to the
> >> > current behaviour (module.$path).
> >>
> >> I have a feeling that it might be much less troublesome in the longer
> >> term to admit that module.$path was a mistake and support only one
> >> format; wouldn't trying to support both leave ambiguity and confusion?
> >
> > Just my 2cents: git-submodule is not yet in any released version. So let's
> > fix things early. In our world, it's not like you lose face when you have
> > to admit mistakes. (Instead, you lose face when you refuse to fix them.)
> > Ah, if only politics learnt from our world...
>
> Well, I completely agree with what you said up to "So let's fix
> things early.", but when I re-read what I wrote, I realize that
> my wording was bad --- I did not mean to drive the discussion in
> that direction.
>
> It was not Lars's *mistake* to admit to begin with.  His was one
> valid design that was consistent within his patch series.  The
> thing is that there just was a better alternative suggested
> later; it does not make the first iteration a mistake in any
> way.
>
> So, Lars, my apologies if I offended you -- I did not mean it
> that way.

No need for apologies, no offence taken :)

-- 
larsh

Re: [PATCH 1/3] git-submodule: allow submodule name and path to differ

[Lars Hjemli]

On 6/10/07, Junio C Hamano <gitster@pobox.com> wrote:
> Lars Hjemli <hjemli@gmail.com> writes:
>
> > This teaches git-submodule to check module.*.path when looking for the
> > config for a submodule path. If no match is found it falls back to the
> > current behaviour (module.$path).
>
> I have a feeling that it might be much less troublesome in the
> longer term to admit that module.$path was a mistake and support
> only one format;

Hmm, what the patch does should match what is described in

  http://article.gmane.org/gmane.comp.version-control.git/48879

except that I've kept [module] instead of renaming it to [submodule].
But my wording ("module.$path") might be disconcerting :)

Fwiw: I do admit that the [path "path"] construct was a mistake.

-- 
larsh

[PATCH 2/3] Add gitmodules(5)

[Lars Hjemli]

This adds documentation for the .gitmodules file.

Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
 Documentation/Makefile       |    2 +-
 Documentation/gitmodules.txt |   63 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/gitmodules.txt

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 9cef480..2ad18e0 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -2,7 +2,7 @@ MAN1_TXT= \
 	$(filter-out $(addsuffix .txt, $(ARTICLES) $(SP_ARTICLES)), \
 		$(wildcard git-*.txt)) \
 	gitk.txt
-MAN5_TXT=gitattributes.txt gitignore.txt
+MAN5_TXT=gitattributes.txt gitignore.txt gitmodules.txt
 MAN7_TXT=git.txt
 
 DOC_HTML=$(patsubst %.txt,%.html,$(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT))
diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
new file mode 100644
index 0000000..37888c9
--- /dev/null
+++ b/Documentation/gitmodules.txt
@@ -0,0 +1,63 @@
+gitmodules(5)
+=============
+
+NAME
+----
+gitmodules - defining submodule properties
+
+SYNOPSIS
+--------
+.gitmodules
+
+
+DESCRIPTION
+-----------
+
+The `.gitmodules` file, located in the top-level directory of a
+gitlink:git[7] working tree, is a text file with a layout matching the
+requirements of gitlink:git-config[1].
+
+The file consists of sections named `module`, divided into one subsection
+per submodule. The subsections are named with the logical name of the
+submodule it describes.
+
+Each submodule can contain the following keys.
+
+module.$name.path::
+	Define a path, relative to the top-level directory of the git
+	working tree, where the submodule is expected to be checked out.
+
+module.$name.url::
+	Define a url from where the submodule repository can be cloned.
+
+
+EXAMPLES
+--------
+
+Consider the following .gitmodules file:
+
+	[module 'libfoo']
+		path = include/foo
+		url = git://example1.com/git/libfoo.git
+
+	[module 'libbar']
+		url = git://example2.com/pub/git/libbar.git
+
+
+This defines two submodules, `libfoo` and `libbar`. The former specifies
+both a checkout path and a suggested url, while the latter only specifies
+a url. This file would make gitlink:git-submodule[1] map the path
+`include/foo` to the submodule `libfoo` and the path `libbar` to the
+submodule `libbar`.
+
+SEE ALSO
+--------
+gitlink:git-submodule[1] gitlink:git-config[1]
+
+DOCUMENTATION
+-------------
+Documentation by Lars Hjemli <hjemli@gmail.com>
+
+GIT
+---
+Part of the gitlink:git[7] suite
-- 
1.5.2.1.914.gbd3a7

Re: [PATCH 2/3] Add gitmodules(5)

[Frank Lichtenheld]

The unofficial git documentation nitpicker at work ;)

On Sat, Jun 09, 2007 at 11:38:51PM +0200, Lars Hjemli wrote:
> +The `.gitmodules` file, located in the top-level directory of a
> +gitlink:git[7] working tree, is a text file with a layout matching the

That link seems superfluous to me.
I would have used "syntax" instead of "layout".

> +requirements of gitlink:git-config[1].
> +
> +The file consists of sections named `module`, divided into one subsection
> +per submodule. The subsections are named with the logical name of the
> +submodule it describes.

"sections named module" sounds confusing to me. Why are there multiple
sections named module? (for the record: I know what you mean, I just
don't know if it couldn't be said simpler)
Maybe better "subsections of section `module`, one per submodule"?
Hmm, sounds ugly too.

> +Each submodule can contain the following keys.
> +
> +module.$name.path::
> +	Define a path, relative to the top-level directory of the git
> +	working tree, where the submodule is expected to be checked out.
> +
> +module.$name.url::
> +	Define a url from where the submodule repository can be cloned.

For .path a "Defaults to name of submodule" probably wouldn't hurt.

For the sake of documentation consistency I would suggest
module.<name>.path. You can compare the output of
$ grep "\.<[a-z]" Documentation/*.txt
with
$ grep "\.\$[a-z]" Documentation/*.txt
to see what I mean.

> +	[module 'libfoo']
> +		path = include/foo
> +		url = git://example1.com/git/libfoo.git
> +
> +	[module 'libbar']
> +		url = git://example2.com/pub/git/libbar.git

This would actually be a syntax error in a git config file
(subsection names can be enclosed in "" but not '').

Gruesse,
-- 
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/

Re: [PATCH 2/3] Add gitmodules(5)

[Lars Hjemli]

On 6/10/07, Frank Lichtenheld <frank@lichtenheld.de> wrote:
> The unofficial git documentation nitpicker at work ;)

That's good, keep it up :)

>
> On Sat, Jun 09, 2007 at 11:38:51PM +0200, Lars Hjemli wrote:
> > +The `.gitmodules` file, located in the top-level directory of a
> > +gitlink:git[7] working tree, is a text file with a layout matching the
>
> That link seems superfluous to me.

Yeah, what I really wanted here was a link to the definition of "git
working tree". I'll drop it.

> I would have used "syntax" instead of "layout".

Agreed

>
> > +requirements of gitlink:git-config[1].
> > +
> > +The file consists of sections named `module`, divided into one subsection
> > +per submodule. The subsections are named with the logical name of the
> > +submodule it describes.
>
> "sections named module" sounds confusing to me. Why are there multiple
> sections named module? (for the record: I know what you mean, I just
> don't know if it couldn't be said simpler)
> Maybe better "subsections of section `module`, one per submodule"?
> Hmm, sounds ugly too.

Good documentation is hard, so I'll work on it some more...

>
> > +Each submodule can contain the following keys.
> > +
> > +module.$name.path::
> > +     Define a path, relative to the top-level directory of the git
> > +     working tree, where the submodule is expected to be checked out.
> > +
> > +module.$name.url::
> > +     Define a url from where the submodule repository can be cloned.
>
> For .path a "Defaults to name of submodule" probably wouldn't hurt.

True. But there might be some issues with this rule, so I'll leave it
as is for now.

>
> For the sake of documentation consistency I would suggest
> module.<name>.path. You can compare the output of
> $ grep "\.<[a-z]" Documentation/*.txt
> with
> $ grep "\.\$[a-z]" Documentation/*.txt
> to see what I mean.

That was very descriptive, thanks!

>
> > +     [module 'libfoo']
> > +             path = include/foo
> > +             url = git://example1.com/git/libfoo.git
> > +
> > +     [module 'libbar']
> > +             url = git://example2.com/pub/git/libbar.git
>
> This would actually be a syntax error in a git config file
> (subsection names can be enclosed in "" but not '').

Shame on me for drinking while documenting ;-)

Thanks for the review, I'll try to send a fixed-up patch later today.

--
larsh

Re: [PATCH 2/3] Add gitmodules(5)

[Johannes Schindelin]

Hi,

On Sun, 10 Jun 2007, Lars Hjemli wrote:

> Shame on me for drinking while documenting ;-)

So I'm not the only one trying to cheer me up with some ethanol-containing 
beverage, when writing documentation?

Ciao,
Dscho

Re: [PATCH 2/3] Add gitmodules(5)

[Lars Hjemli]

On 6/10/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Sun, 10 Jun 2007, Lars Hjemli wrote:
>
> > Shame on me for drinking while documenting ;-)
>
> So I'm not the only one trying to cheer me up with some ethanol-containing
> beverage, when writing documentation?
>

Nope, you're not alone. Certain activities do require lots of stimuli
(also known as "Gourmet Stout" :)

-- 
larsh

Re: [PATCH 2/3] Add gitmodules(5)

[Sven Verdoolaege]

On Sun, Jun 10, 2007 at 10:58:29AM +0200, Lars Hjemli wrote:
> On 6/10/07, Frank Lichtenheld <frank@lichtenheld.de> wrote:
> >> +Each submodule can contain the following keys.
> >> +
> >> +module.$name.path::
> >> +     Define a path, relative to the top-level directory of the git
> >> +     working tree, where the submodule is expected to be checked out.
> >> +
> >> +module.$name.url::
> >> +     Define a url from where the submodule repository can be cloned.
> >
> >For .path a "Defaults to name of submodule" probably wouldn't hurt.
> 
> True. But there might be some issues with this rule, so I'll leave it
> as is for now.

And what might those issues be?

How about adding something like

	For a given path relative to the top-level directory of the
	git working tree, the logical name of the submodule at that
	path (if any) is the submodule <name> with a module.<name>.path
	value equal to that path, or, if no such submodule exists, the
	submodule <path>.  It is an error for different submodules to have
	identical path values.

For uniformity, you may also want to specify that a path value
must (or must not) end with a slash.

Other than that, I like it.
It doesn't have the extensions proposed by Linus, but it seems
to be forward compatible with them.

skimo

Re: [PATCH 2/3] Add gitmodules(5)

[Lars Hjemli]

On 6/10/07, Sven Verdoolaege <skimo@kotnet.org> wrote:
> On Sun, Jun 10, 2007 at 10:58:29AM +0200, Lars Hjemli wrote:
> > On 6/10/07, Frank Lichtenheld <frank@lichtenheld.de> wrote:
> > >> +Each submodule can contain the following keys.
> > >> +
> > >> +module.$name.path::
> > >> +     Define a path, relative to the top-level directory of the git
> > >> +     working tree, where the submodule is expected to be checked out.
> > >> +
> > >> +module.$name.url::
> > >> +     Define a url from where the submodule repository can be cloned.
> > >
> > >For .path a "Defaults to name of submodule" probably wouldn't hurt.
> >
> > True. But there might be some issues with this rule, so I'll leave it
> > as is for now.
>
> And what might those issues be?

There's been some discussion about allowing a default value for path, see
  http://comments.gmane.org/gmane.comp.version-control.git/49620

>
> How about adding something like
>
>         For a given path relative to the top-level directory of the
>         git working tree, the logical name of the submodule at that
>         path (if any) is the submodule <name> with a module.<name>.path
>         value equal to that path, or, if no such submodule exists, the
>         submodule <path>.  It is an error for different submodules to have
>         identical path values.
>
> For uniformity, you may also want to specify that a path value
> must (or must not) end with a slash.

Nice suggestions, I'll try to come up with a new patch later today

> Other than that, I like it.

Thanks :)

> It doesn't have the extensions proposed by Linus, but it seems
> to be forward compatible with them.

I'm trying to take baby-steps with the submodule support, so the
module/submodule extension Linus talked about would possibly come at a
later stage.

Hmm, maybe I should just rename [module] to [submodule] right now? It
would be better forward compatible with the proposed extension, it
would 'harmonize' the section names used in .gitmodules and
.git/config, and it would offer a clean break from what's currently
supported in 'master'.

--
larsh

Re: [PATCH 2/3] Add gitmodules(5)

[Sven Verdoolaege]

On Sun, Jun 10, 2007 at 02:30:58PM +0200, Lars Hjemli wrote:
> On 6/10/07, Sven Verdoolaege <skimo@kotnet.org> wrote:
> >On Sun, Jun 10, 2007 at 10:58:29AM +0200, Lars Hjemli wrote:
> >> >For .path a "Defaults to name of submodule" probably wouldn't hurt.
> >>
> >> True. But there might be some issues with this rule, so I'll leave it
> >> as is for now.
> >
> >And what might those issues be?
> 
> There's been some discussion about allowing a default value for path, see
>  http://comments.gmane.org/gmane.comp.version-control.git/49620

Euhm... this just points back to this thread...
or are you referring to Junio's misinterpretation of the patch 1/3 
commit message?

> >It doesn't have the extensions proposed by Linus, but it seems
> >to be forward compatible with them.
> 
> I'm trying to take baby-steps with the submodule support, so the
> module/submodule extension Linus talked about would possibly come at a
> later stage.
> 
> Hmm, maybe I should just rename [module] to [submodule] right now?

I don't see the need, but if you _really_ can't resist then please do it
sooner rather than later.

skimo

Re: [PATCH 2/3] Add gitmodules(5)

[Lars Hjemli]

On 6/10/07, Sven Verdoolaege <skimo@kotnet.org> wrote:
> On Sun, Jun 10, 2007 at 02:30:58PM +0200, Lars Hjemli wrote:
> > On 6/10/07, Sven Verdoolaege <skimo@kotnet.org> wrote:
> > >On Sun, Jun 10, 2007 at 10:58:29AM +0200, Lars Hjemli wrote:
> > >> >For .path a "Defaults to name of submodule" probably wouldn't hurt.
> > >>
> > >> True. But there might be some issues with this rule, so I'll leave it
> > >> as is for now.
> > >
> > >And what might those issues be?
> >
> > There's been some discussion about allowing a default value for path, see
> >  http://comments.gmane.org/gmane.comp.version-control.git/49620
>
> Euhm... this just points back to this thread...

I know ;-)

> or are you referring to Junio's misinterpretation of the patch 1/3
> commit message?

Well, yes, I _think_ it's a misinterpretation but there is always the
possibility of me being exceptionally slow and not getting his point.

>
> > >It doesn't have the extensions proposed by Linus, but it seems
> > >to be forward compatible with them.
> >
> > I'm trying to take baby-steps with the submodule support, so the
> > module/submodule extension Linus talked about would possibly come at a
> > later stage.
> >
> > Hmm, maybe I should just rename [module] to [submodule] right now?
>
> I don't see the need, but if you _really_ can't resist then please do it
> sooner rather than later.

Ok

--
larsh

Re: [PATCH 2/3] Add gitmodules(5)

[Junio C Hamano]

"Lars Hjemli" <hjemli@gmail.com> writes:

> Hmm, maybe I should just rename [module] to [submodule] right now? It
> would be better forward compatible with the proposed extension, it
> would 'harmonize' the section names used in .gitmodules and
> .git/config, and it would offer a clean break from what's currently
> supported in 'master'.

Yes, the difference between '[submodule]' vs '[module]' in
.git/config and .gitmodules confused me while looking at your
latest patch series.  I am in favor of unifying them.  We would
not be breaking any released version if we harmonize them now.

Re: [PATCH 2/3] Add gitmodules(5)

[Lars Hjemli]

On 6/10/07, Junio C Hamano <gitster@pobox.com> wrote:
> "Lars Hjemli" <hjemli@gmail.com> writes:
>
> > Hmm, maybe I should just rename [module] to [submodule] right now? It
> > would be better forward compatible with the proposed extension, it
> > would 'harmonize' the section names used in .gitmodules and
> > .git/config, and it would offer a clean break from what's currently
> > supported in 'master'.
>
> Yes, the difference between '[submodule]' vs '[module]' in
> .git/config and .gitmodules confused me while looking at your
> latest patch series.  I am in favor of unifying them.  We would
> not be breaking any released version if we harmonize them now.
>

Good, then I'll do it that way. And I'll drop the "unless otherwise
noted, name=path" behaviour. If we later decide it would be useful we
can always "re-add" it.

--
larsh

Re: [PATCH 2/3] Add gitmodules(5)

[Sven Verdoolaege]

On Sun, Jun 10, 2007 at 11:14:27PM +0200, Lars Hjemli wrote:
> And I'll drop the "unless otherwise
> noted, name=path" behaviour. If we later decide it would be useful we
> can always "re-add" it.

So every (sub)module subsection will have to have a path element?
Why?

skimo

Re: [PATCH 2/3] Add gitmodules(5)

[Lars Hjemli]

On 6/11/07, Sven Verdoolaege <skimo@kotnet.org> wrote:
> On Sun, Jun 10, 2007 at 11:14:27PM +0200, Lars Hjemli wrote:
> > And I'll drop the "unless otherwise
> > noted, name=path" behaviour. If we later decide it would be useful we
> > can always "re-add" it.
>
> So every (sub)module subsection will have to have a path element?

Yes

> Why?

It addresses Junio's concern about ambiguity and confusion. And it
felt like the 'safe choice', i.e. starting out with a strict rule and
possibly loosen it later on is a lot easier than starting loose and
then trying to make it stricter.

Btw: adding support for the optional path specification on top of my
latest patch-series is trivial:

diff --git a/git-submodule.sh b/git-submodule.sh
index 89a3885..297272b 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -34,8 +34,7 @@ module_name()
 {
        name=$(GIT_CONFIG=.gitmodules git-config --get-regexp
'^submodule\..*\.path$' "$1" |
        sed -nre 's/^submodule\.(.+)\.path .+$/\1/p')
-       test -z "$name" &&
-       die "No submodule mapping found in .gitmodules for path '$path'"
+       test -z "$name" && name="$path"
        echo "$name"
 }

--
larsh

Re: [PATCH 2/3] Add gitmodules(5)

[Sven Verdoolaege]

On Mon, Jun 11, 2007 at 12:47:43PM +0200, Lars Hjemli wrote:
> On 6/11/07, Sven Verdoolaege <skimo@kotnet.org> wrote:
> >On Sun, Jun 10, 2007 at 11:14:27PM +0200, Lars Hjemli wrote:
> >> And I'll drop the "unless otherwise
> >> noted, name=path" behaviour. If we later decide it would be useful we
> >> can always "re-add" it.
> >
> >So every (sub)module subsection will have to have a path element?
> 
> Yes
> 
> >Why?
> 
> It addresses Junio's concern about ambiguity and confusion.

Junio, is this really what you meant?

Linus even advocated removing the mapping completely
(http://article.gmane.org/gmane.comp.version-control.git/48871),
but I wouldn't go that far.

skimo

Re: [PATCH 2/3] Add gitmodules(5)

[Junio C Hamano]

Sven Verdoolaege <skimo@kotnet.org> writes:

>> >Why?
>> 
>> It addresses Junio's concern about ambiguity and confusion.
>
> Junio, is this really what you meant?

I think I initially misread Lars's patch and log message that
said:

    This teaches git-submodule to check module.*.path when looking for the
    config for a submodule path. If no match is found it falls back to the
    current behaviour (module.$path).

Somehow I thought that it would make it ambiguous to have
module.$name.path and module.$path when answering the question:
"given a name of a module how would you find which path to bind
it to".

But I do not think it is a problem; at least "ambiguity" worry
is unfounded.  This section, without "path = " line:

	[module "foo"]
        	url = ...

is equivalent to have "path = foo" in it.  Finding path from
module name is well defined.

The other, finding what module should go to a given path, is not
well defined.  IOW, you would not know which module you want at
the path bar/ when you have:

	[module "foo"]
        	url = ...
                path = bar
	[module "bar"]
        	url = ...

But that problem exists with or without these "module.*.path
and module.$path", so it cannot be an argument against the
definition of the fallback behaviour.

Re: [PATCH 2/3] Add gitmodules(5)

[Matthias Lederhofer]

Lars Hjemli <hjemli@gmail.com> wrote:
> +SYNOPSIS
> +--------
> +.gitmodules

My asciidoc (7.1.2 and 8.2.1) generates an empty refsynopsisdiv from
this.  xmlto refuses to convert this into anything else:

xmlto -m callouts.xsl man gitmodules.xml
xmlto: input does not validate (status 3)
/path/to/git/Documentation/gitmodules.xml:15: element refsynopsisdiv: validity error : Element refsynopsisdiv content does not follow the DTD, expecting (refsynopsisdivinfo? , (title , subtitle? , titleabbrev?)? , (((calloutlist | glosslist | itemizedlist | orderedlist | segmentedlist | simplelist | variablelist | caution | important | note | tip | warning | literallayout | programlisting | programlistingco | screen | screenco | screenshot | synopsis | cmdsynopsis | funcsynopsis | classsynopsis | fieldsynopsis | constructorsynopsis | destructorsynopsis | methodsynopsis | formalpara | para | simpara | address | blockquote | graphic | graphicco | mediaobject | mediaobjectco | informalequation | informalexample | informalfigure | informaltable | equation | example | figure | table | msgset | procedure | sidebar | qandaset | anchor | bridgehead | remark | highlights | abstract | authorblurb 
 | epigraph | indexterm | beginpage)+ , refsect2*) | refsect2+)), got ()
Document /path/to/git/Documentation/gitmodules.xml does not validate

This breaks the build process for the documentation.  I could not find
out how to fix this.  Probably just needs the right quoting for the
dot at the beginning of the line.

Re: [PATCH 2/3] Add gitmodules(5)

[Lars Hjemli]

On 6/13/07, Matthias Lederhofer <matled@gmx.net> wrote:
> Lars Hjemli <hjemli@gmail.com> wrote:
> > +SYNOPSIS
> > +--------
> > +.gitmodules
>
> My asciidoc (7.1.2 and 8.2.1) generates an empty refsynopsisdiv from
> this.  xmlto refuses to convert this into anything else:

Sorry about that, I don't have a working asciidoc :-(

But looking through the latest asciidoc-doc suggests that .gitmodules
is treated like a blocktitle:
http://www.methods.co.nz/asciidoc/userguide.html#toc33

Could you please try one or both of the following escape-sequences?

\.gitmodules
#.gitmodules#


Thanks

--
larsh

Re: [PATCH 2/3] Add gitmodules(5)

[Matthias Lederhofer]

Lars Hjemli <hjemli@gmail.com> wrote:
> Could you please try one or both of the following escape-sequences?
> 
> \.gitmodules
> #.gitmodules#

#.gitmodules# seems to produce a good xml file, html is fine too.  But
the conversion from xml to a manpage seems to be broken:

    $ xmlto -m callouts.xsl man gitmodules.xml

transforms

    <refsynopsisdiv>
    <simpara>.gitmodules</simpara>

    </refsynopsisdiv>

to

    .SH "SYNOPSIS"
    .gitmodules

".gitmodules" is not shown in the manpage and vim highlights the ".gi"
just like ".SH" in a special color.  Perhaps we should just highlight
.gitmodules like commands with single quotes.  This solves both
problems :)

Re: [PATCH 2/3] Add gitmodules(5)

[Lars Hjemli]

On 6/13/07, Matthias Lederhofer <matled@gmx.net> wrote:
> Lars Hjemli <hjemli@gmail.com> wrote:
> > Could you please try one or both of the following escape-sequences?
> >
> > \.gitmodules
> > #.gitmodules#
>
> #.gitmodules# seems to produce a good xml file, html is fine too.  But
> the conversion from xml to a manpage seems to be broken:
>
>     $ xmlto -m callouts.xsl man gitmodules.xml
>
> transforms
>
>     <refsynopsisdiv>
>     <simpara>.gitmodules</simpara>
>
>     </refsynopsisdiv>
>
> to
>
>     .SH "SYNOPSIS"
>     .gitmodules
>
> ".gitmodules" is not shown in the manpage and vim highlights the ".gi"
> just like ".SH" in a special color.  Perhaps we should just highlight
> .gitmodules like commands with single quotes.  This solves both
> problems :)
>

Ok, thanks for trying to clean up my mess. I guess there's a good
reason why Junios examples drop the leading dot (cat
/usr/share/man/man1/git-submodule.1 was an eyeopener for me right now
;-)

-- 
larsh

Re: [PATCH 2/3] Add gitmodules(5)

[Junio C Hamano]

Matthias Lederhofer <matled@gmx.net> writes:

> Lars Hjemli <hjemli@gmail.com> wrote:
>> +SYNOPSIS
>> +--------
>> +.gitmodules
>
> My asciidoc (7.1.2 and 8.2.1) generates an empty refsynopsisdiv from
> this.  xmlto refuses to convert this into anything else:
> ...
> This breaks the build process for the documentation.  I could not find
> out how to fix this.  Probably just needs the right quoting for the
> dot at the beginning of the line.

Sorry, I should have caught this much earlier, as already knew
about this issue when I did gitattributes(5).

As it seems to be customary not to even say 'dot' at the
beginning for man(5) section (existing examples I found are
netrc, postgresrc, procmailrc, ...), I think the best fix is
just to do s/\.gitmodules/gitmodules/ there.

[PATCH] gitmodules(5): remove leading period from synopsis

[Lars Hjemli]

Asciidoc treats a line starting with a period followed by a title as a
blocktitle element. My introduction of gitmodules(5) unfortunatly broke
the documentation build process due to this processing, since it made
asciidoc generate an illegal (empty) synopsis element. Removing the leading
period fixes the problem and also makes gitmodules(5) use the same synopsis
notation as gitattributes(5).

Noticed-by: Matthias Lederhofer <matled@gmx.net>
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---

Sorry for the breakage, I'll try to get asciidoc working before submitting
any more patches to Documentation/.

 Documentation/gitmodules.txt |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index 7814b6a..035294e 100644
--- a/Documentation/gitmodules.txt
+++ b/Documentation/gitmodules.txt
@@ -7,7 +7,7 @@ gitmodules - defining submodule properties
 
 SYNOPSIS
 --------
-.gitmodules
+gitmodules
 
 
 DESCRIPTION
-- 
1.5.2.1.914.gbd3a7

[PATCH 3/3] t7400: barf if git-submodule removes or replaces a file

[Lars Hjemli]

The test for an unmolested file wouldn't fail properly if the file had been
removed or replaced by something other than a regular file. This fixes it.

Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
 t/t7400-submodule-basic.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 3940433..74fafce 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -72,7 +72,7 @@ test_expect_success 'update should fail when path is used by a file' '
 	then
 		echo "[OOPS] update should have failed"
 		false
-	elif test -f lib && test "$(cat lib)" != "hello"
+	elif test "$(cat lib)" != "hello"
 	then
 		echo "[OOPS] update failed but lib file was molested"
 		false
-- 
1.5.2.1.914.gbd3a7