[PATCH 1/3] git-submodule - Follow top-level remote on init/update/clone

[PATCH 1/3] git-submodule - Follow top-level remote on init/update/clone

[Mark Levedahl]

This change allows the remote used for relative submodules (those defined
using a url that is relative to their parent) to be defined by explicit
definition on the command line or through the top-level project's
branch.<name>.remote configuration.  This override is applied *only* to
submodules defined using a url relative to the top-level project's url,
under the assumption that such modules are logically part of the same
project and managed as a unit.  For relative submodules, the given remote
will be defined in each submodule using the relative url applied to the
top-level's url the first time the remote is used.  Any existing remote
definition is unaffected, so this can be changed manually by the user at
any time.

Submodules defined using an absolute url are unaffected.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 Documentation/git-submodule.txt |   16 +++++++-
 git-submodule.sh                |   76 ++++++++++++++++++++++++++++++++-------
 2 files changed, 77 insertions(+), 15 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index e818e6e..4fc17f6 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -9,9 +9,9 @@ git-submodule - Initialize, update or inspect submodules
 SYNOPSIS
 --------
 [verse]
-'git-submodule' [--quiet] add [-b branch] [--] <repository> [<path>]
+'git-submodule' [--quiet] add [-b branch] [-r <remote>] [--] <repository> [<path>]
 'git-submodule' [--quiet] status [--cached] [--] [<path>...]
-'git-submodule' [--quiet] [init|update] [--] [<path>...]
+'git-submodule' [--quiet] [init|update] [-r <remote>] [--] [<path>...]
 
 
 COMMANDS
@@ -55,6 +55,18 @@ OPTIONS
 -b, --branch::
 	Branch of repository to add as submodule.
 
+-r remote::
+	Name of remote to use or define when working with relative submodules
+	(i.e., submodules whose url is given relative to the top-level
+	project). If this value is undefined, the top-level project's
+	branch.<name>.remote is used, and if that is undefined the default
+	"origin" is used. The remote will be defined in each relative
+	submodule as needed by appending the relative url to the top level
+	project's url. This option has no effect upon submodules defined
+	using an absolute url: such project's are cloned using the default
+	"origin," and are updated using the submodule's branch.<name>.remote
+	machinery and defaulting to "origin."
+
 --cached::
 	Display the SHA-1 stored in the index, not the SHA-1 of the currently
 	checked out submodule commit. This option is only valid for the
diff --git a/git-submodule.sh b/git-submodule.sh
index a6aaf40..b97bf18 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -4,15 +4,17 @@
 #
 # Copyright (c) 2007 Lars Hjemli
 
-USAGE='[--quiet] [--cached] [add <repo> [-b branch]|status|init|update] [--] [<path>...]'
+USAGE='[--quiet] [--cached] [add [-r remote] <repo> [-b branch]|status|init [-r remote]|update [-r remote]] [--] [<path>...]'
 OPTIONS_SPEC=
 . git-sh-setup
+. git-parse-remote
 require_work_tree
 
 command=
 branch=
 quiet=
 cached=
+remote=
 
 #
 # print stuff on stdout unless -q was specified
@@ -40,11 +42,13 @@ get_repo_base() {
 # Resolve relative url by appending to parent's url
 resolve_relative_url ()
 {
-	branch="$(git symbolic-ref HEAD 2>/dev/null)"
-	remote="$(git config branch.${branch#refs/heads/}.remote)"
-	remote="${remote:-origin}"
-	remoteurl="$(git config remote.$remote.url)" ||
-		die "remote ($remote) does not have a url in .git/config"
+	remote="${remote:-$(get_default_remote)}"
+	remoteurl=$(git config remote.$remote.url)
+	if test -z "$remoteurl" ; then
+		echo >&2 "remote ($remote) does not have a url defined in .git/config"
+		return 1
+	fi
+	remoteurl="${remoteurl%/.git}"
 	url="$1"
 	while test -n "$url"
 	do
@@ -92,6 +96,7 @@ module_clone()
 {
 	path=$1
 	url=$2
+	remote=$3
 
 	# If there already is a directory at the submodule path,
 	# expect it to be empty (since that is the default checkout
@@ -107,7 +112,7 @@ module_clone()
 	test -e "$path" &&
 	die "A file already exist at path '$path'"
 
-	git-clone -n "$url" "$path" ||
+	git-clone -n -o "$remote" "$url" "$path" ||
 	die "Clone of '$url' into submodule path '$path' failed"
 }
 
@@ -129,6 +134,14 @@ cmd_add()
 			branch=$2
 			shift
 			;;
+		-r)
+			case "$2" in
+			'')
+				usage
+				;;
+			esac
+			remote="$2"; shift
+			;;
 		-q|--quiet)
 			quiet=1
 			;;
@@ -156,13 +169,16 @@ cmd_add()
 	case "$repo" in
 	./*|../*)
 		# dereference source url relative to parent's url
-		realrepo="$(resolve_relative_url $repo)" ;;
+		realremote=${remote:-$(get_default_remote)}
+		realrepo=$(resolve_relative_url $repo) || exit 1
+		;;
 	*)
 		# Turn the source into an absolute path if
 		# it is local
 		if base=$(get_repo_base "$repo"); then
 			repo="$base"
 		fi
+		realremote=origin
 		realrepo=$repo
 		;;
 	esac
@@ -180,8 +196,8 @@ cmd_add()
 	git ls-files --error-unmatch "$path" > /dev/null 2>&1 &&
 	die "'$path' already exists in the index"
 
-	module_clone "$path" "$realrepo" || exit
-	(unset GIT_DIR; cd "$path" && git checkout -q ${branch:+-b "$branch" "origin/$branch"}) ||
+	module_clone "$path" "$realrepo" "$realremote" || exit
+	(unset GIT_DIR; cd "$path" && git checkout -q ${branch:+-b "$branch" "$realremote/$branch"}) ||
 	die "Unable to checkout submodule '$path'"
 	git add "$path" ||
 	die "Failed to add submodule '$path'"
@@ -206,6 +222,14 @@ cmd_init()
 		-q|--quiet)
 			quiet=1
 			;;
+		-r)
+			case "$2" in
+			'')
+				usage
+				;;
+			esac
+			remote="$2"; shift
+			;;
 		--)
 			shift
 			break
@@ -235,7 +259,7 @@ cmd_init()
 		# Possibly a url relative to parent
 		case "$url" in
 		./*|../*)
-			url="$(resolve_relative_url "$url")"
+			url=$(resolve_relative_url "$url") || exit 1
 			;;
 		esac
 
@@ -248,6 +272,8 @@ cmd_init()
 
 #
 # Update each submodule path to correct revision, using clone and checkout as needed
+# For relative submodules (defined using relative url), we use master project's remote
+# and define that in each submodule if not already there
 #
 # $@ = requested paths (default to all)
 #
@@ -260,6 +286,14 @@ cmd_update()
 		-q|--quiet)
 			quiet=1
 			;;
+		-r)
+			case "$2" in
+			'')
+				usage
+				;;
+			esac
+			remote="$2"; shift
+			;;
 		--)
 			shift
 			break
@@ -274,6 +308,7 @@ cmd_update()
 		shift
 	done
 
+	remote=${remote:-$(get_default_remote)}
 	git ls-files --stage -- "$@" | grep -e '^160000 ' |
 	while read mode sha1 stage path
 	do
@@ -290,7 +325,7 @@ cmd_update()
 
 		if ! test -d "$path"/.git
 		then
-			module_clone "$path" "$url" || exit
+			module_clone "$path" "$url" "$remote" || exit
 			subsha1=
 		else
 			subsha1=$(unset GIT_DIR; cd "$path" &&
@@ -298,9 +333,24 @@ cmd_update()
 			die "Unable to find current revision in submodule path '$path'"
 		fi
 
+		baseurl="$(GIT_CONFIG=.gitmodules git config submodule."$name".url)"
+		case "$baseurl" in
+		./*|../*)
+			fetch_remote=$remote
+			absurl=$(resolve_relative_url $baseurl) || exit 1
+			(unset GIT_DIR ; cd "$path" && git config remote."$fetch_remote".url > /dev/null) ||
+			(
+				unset GIT_DIR; cd "$path" && git remote add "$fetch_remote" "$absurl"
+			) || die "Unable to define remote '$fetch_remote' in submodule path '$path'"
+			;;
+		*)
+			fetch_remote=
+			;;
+		esac
+
 		if test "$subsha1" != "$sha1"
 		then
-			(unset GIT_DIR; cd "$path" && git-fetch &&
+			(unset GIT_DIR; cd "$path" && git-fetch $fetch_remote &&
 				git-checkout -q "$sha1") ||
 			die "Unable to checkout '$sha1' in submodule path '$path'"
 
-- 
1.5.4.47.gcca7b3

[PATCH 2/3] git-submodule - Allow adding a submodule in-place

[Mark Levedahl]

When working in top-level project, it is useful to create a new submodule
as a git repo in a subdirectory, then add that submodule to top-level in
place.  This allows "git submodule add <intended url> subdir" to add the
existing subdir to the current project.  The presumption is the user will
later push / clone the subdir to the <intended url> so that future
submodule init / updates will work.

Absent this patch, "git submodule add" insists upon cloning the subdir
from a repository at the given url, which is fine for adding an existing
project in but less useful when adding a new submodule from scratch to an
existing project.  The former functionality remains, and the clone is
attempted if the subdir does not already exist as a valid git repo.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 Documentation/git-submodule.txt |    5 ++-
 git-submodule.sh                |   57 +++++++++++++++++++++++---------------
 2 files changed, 37 insertions(+), 25 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 4fc17f6..85d7dd3 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -18,8 +18,9 @@ COMMANDS
 --------
 add::
 	Add the given repository as a submodule at the given path
-	to the changeset to be committed next.  In particular, the
-	repository is cloned at the specified path, added to the
+	to the changeset to be committed next.  If path is a valid
+	repository within the project, it is added as is. Otherwise,
+	repository is cloned at the specified path. path is added to the
 	changeset and registered in .gitmodules.   If no path is
 	specified, the path is deduced from the repository specification.
 	If the repository url begins with ./ or ../, it is stored as
diff --git a/git-submodule.sh b/git-submodule.sh
index b97bf18..4c86a3c 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -166,23 +166,6 @@ cmd_add()
 		usage
 	fi
 
-	case "$repo" in
-	./*|../*)
-		# dereference source url relative to parent's url
-		realremote=${remote:-$(get_default_remote)}
-		realrepo=$(resolve_relative_url $repo) || exit 1
-		;;
-	*)
-		# Turn the source into an absolute path if
-		# it is local
-		if base=$(get_repo_base "$repo"); then
-			repo="$base"
-		fi
-		realremote=origin
-		realrepo=$repo
-		;;
-	esac
-
 	# Guess path from repo if not specified or strip trailing slashes
 	if test -z "$path"; then
 		path=$(echo "$repo" | sed -e 's|/*$||' -e 's|:*/*\.git$||' -e 's|.*[/:]||g')
@@ -190,15 +173,43 @@ cmd_add()
 		path=$(echo "$path" | sed -e 's|/*$||')
 	fi
 
-	test -e "$path" &&
-	die "'$path' already exists"
-
 	git ls-files --error-unmatch "$path" > /dev/null 2>&1 &&
 	die "'$path' already exists in the index"
 
-	module_clone "$path" "$realrepo" "$realremote" || exit
-	(unset GIT_DIR; cd "$path" && git checkout -q ${branch:+-b "$branch" "$realremote/$branch"}) ||
-	die "Unable to checkout submodule '$path'"
+	# perhaps the path exists and is already a git repo, else clone it
+	if test -e "$path"
+	then
+		if test -d "$path/.git" &&
+		test "$(unset GIT_DIR; cd $path; git rev-parse --git-dir)" = ".git"
+		then
+			echo "Adding existing repo at '$path' to the index"
+		else
+			die "'$path' already exists and is not a valid git repo"
+		fi
+	else
+		case "$repo" in
+		./*|../*)
+			# dereference source url relative to parent's url
+			realremote=${remote:-$(get_default_remote)}
+			realrepo=$(resolve_relative_url $repo) || exit 1
+			;;
+		*)
+			# Turn the source into an absolute path if
+			# it is local
+			if base=$(get_repo_base "$repo")
+			then
+				repo="$base"
+			fi
+			realremote=origin
+			realrepo=$repo
+			;;
+		esac
+
+		module_clone "$path" "$realrepo" "$realremote" || exit
+		(unset GIT_DIR; cd "$path" && git checkout -q ${branch:+-b "$branch" "$realremote/$branch"}) ||
+		die "Unable to checkout submodule '$path'"
+	fi
+
 	git add "$path" ||
 	die "Failed to add submodule '$path'"
 
-- 
1.5.4.47.gcca7b3

[PATCH 3/3] Add t/t7401 - test submodule interaction with remotes machinery

[Mark Levedahl]

This adds a sequence of tests to assure that the following two sequences
work:

	git clone -o frotz <someurl> foo
	cd foo
	git submodule init
	git submodule update

This should result in the top-level project and subproject having "frotz"
as the only defined remote, and should succeed with the
branch.<name>.remote mechanism supplying frotz as the remote.

Then, in the same working directory
	git remote add fork <some url>
	git fetch fork
	git checkout --track -b fork fork/<somebranch>
	git submodule init
	git submodule update

will retrive new submodules from remote "fork", and define fork in the
existing modules.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 t/t7401-submodule-remote.sh |  118 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 118 insertions(+), 0 deletions(-)
 create mode 100755 t/t7401-submodule-remote.sh

diff --git a/t/t7401-submodule-remote.sh b/t/t7401-submodule-remote.sh
new file mode 100755
index 0000000..e5bef27
--- /dev/null
+++ b/t/t7401-submodule-remote.sh
@@ -0,0 +1,118 @@
+#!/bin/sh
+
+test_description='Porcelain support for submodules with multiple remotes
+
+This test verifies operation of submodules using multiple remotes and
+differing remote per top-level branch.  This includes ability to follow
+the top-level remote.<branch>, and to propagate definition of new remotes
+down to submodules as needed.'
+
+. ./test-lib.sh
+
+# the standard tests all work with one repo, but we need several..
+rm -rf .git
+
+test_expect_success 'Prepare master repository with 1 submodule' '
+	(
+	mkdir master &&
+	cd master &&
+	git init &&
+	echo "on master" > master.txt &&
+	git add master.txt &&
+	git commit -m "Add master.txt" &&
+	mkdir submod1 &&
+	cd submod1 &&
+	git init &&
+	echo "submod1" > submod1.txt &&
+	git add submod1.txt &&
+	git commit -m "Added submod1.txt" &&
+	cd .. &&
+	git submodule add ./submod1 submod1 &&
+	git commit -m "Added submodule submod1"
+	)
+'
+
+test_expect_success 'Clone master as fork' '
+	(
+	git clone master fork &&
+	cd fork &&
+	test "$(git remote)" = "origin" &&
+	git submodule init &&
+	git submodule update &&
+	test -e submod1/.git
+	)
+'
+
+test_expect_success 'Add second submodule in fork' '
+	(
+	cd fork &&
+	mkdir submod2 &&
+	cd submod2 &&
+	git init &&
+	echo "submod2" > submod2.txt &&
+	git add submod2.txt &&
+	git commit -m "Added submod2.txt" &&
+	cd .. &&
+	git submodule add ./submod2 submod2 &&
+	git commit -m "Added submodule submod2 on fork"
+	)
+'
+
+test_expect_success 'Clone master using frotz instead of origin' '
+	(
+	git clone -o frotz master worker &&
+	cd worker &&
+	test "$(git remote)" = "frotz"
+	)
+'
+
+test_expect_success 'Get submodules using frotz instead of origin' '
+	(
+	cd worker &&
+	git submodule init &&
+	git submodule update &&
+	test -e submod1/.git &&
+	cd submod1 &&
+	test "$(git remote)" = "frotz"
+	)
+'
+
+test_expect_success 'submodule update fails on detached head' '
+	(
+	cd worker &&
+	git checkout $(git rev-parse HEAD) &&
+	! git submodule update
+	)
+'
+
+test_expect_success 'submodule update -r remote succeeds on detached head' '
+	(
+	cd worker &&
+	git checkout $(git rev-parse HEAD) &&
+	git submodule update -r frotz
+	)
+'
+
+test_expect_success 'Update using fork to get additional submodule' '
+	(
+	cd worker &&
+	git remote add fork $(pwd)/../fork &&
+	git fetch fork &&
+	git checkout --track -b fork_master fork/master &&
+	git submodule init &&
+	git submodule update &&
+	test -e submod2/.git &&
+	cd submod2 &&
+	test "$(git remote)" = "fork" &&
+	cd ../submod1 &&
+	remotes1=$(git remote) &&
+	case $remotes1 in
+		fork*frotz|frotz*fork)
+			true ;;
+		*)
+			false ;;
+	esac
+	)
+'
+
+test_done
-- 
1.5.4.47.gcca7b3

Re: [PATCH 1/3] git-submodule - Follow top-level remote on init/update/clone

[Johannes Schindelin]

Hi,

On Sat, 9 Feb 2008, Mark Levedahl wrote:

> This change allows the remote used for relative submodules (those defined
> using a url that is relative to their parent) to be defined by explicit
> definition on the command line or through the top-level project's
> branch.<name>.remote configuration.  This override is applied *only* to
> submodules defined using a url relative to the top-level project's url,
> under the assumption that such modules are logically part of the same
> project and managed as a unit.

That makes sense.

> @@ -40,11 +42,13 @@ get_repo_base() {
>  # Resolve relative url by appending to parent's url
>  resolve_relative_url ()
>  {
> -	branch="$(git symbolic-ref HEAD 2>/dev/null)"
> -	remote="$(git config branch.${branch#refs/heads/}.remote)"
> -	remote="${remote:-origin}"
> -	remoteurl="$(git config remote.$remote.url)" ||
> -		die "remote ($remote) does not have a url in .git/config"

I did not really look at this code before, but does that not mean that 
git-submodule does already what you want?

Because usually, you clone the superproject from the URL that you actually 
want to use, and in that case, the initial branch's default remote will 
have exactly that URL.

So I have to admit that I do not see the reason why you remove that code, 
replace it with another (that I think does the same), and claim that you 
introduce that behaviour.

Your patch seems to change only one thing: you can specify "-r <remote>" 
with "git submodule add/init/update" -- except for some peculiarity; see 
below.

> @@ -107,7 +112,7 @@ module_clone()
>  	test -e "$path" &&
>  	die "A file already exist at path '$path'"
>  
> -	git-clone -n "$url" "$path" ||
> +	git-clone -n -o "$remote" "$url" "$path" ||
>  	die "Clone of '$url' into submodule path '$path' failed"
>  }
>  

If you do _that_, you will _force_ the submodule to have no "origin" 
remote.  As discussed _at length_, this is not what you should do.  The 
only reason to use "-o <other-nick-name>" is if you plan _not_ to use the 
same URL for the default remote.

IOW I do not like this hunk _at_ _all_.  Because I get the impression that 
I really wasted my time explaining the same issue over and over and over 
again.

> @@ -156,13 +169,16 @@ cmd_add()
>  	case "$repo" in
>  	./*|../*)
>  		# dereference source url relative to parent's url
> -		realrepo="$(resolve_relative_url $repo)" ;;
> +		realremote=${remote:-$(get_default_remote)}
> +		realrepo=$(resolve_relative_url $repo) || exit 1
> +		;;

Why do you need the "realremote" here?  Why is "$remote" not enough?

> @@ -235,7 +259,7 @@ cmd_init()
>  		# Possibly a url relative to parent
>  		case "$url" in
>  		./*|../*)
> -			url="$(resolve_relative_url "$url")"
> +			url=$(resolve_relative_url "$url") || exit 1

Yes for the "|| exit 1".  No for the removal of the quotes: keep in mind: 
you are possibly getting a url from the _config_, which is supposed to be 
user-editable.

> @@ -274,6 +308,7 @@ cmd_update()
>  		shift
>  	done
>  
> +	remote=${remote:-$(get_default_remote)}

You have this paradigm so often, but AFAICS you only use it for the call 
to module_clone.  Why not let module_clone figure it out, if $remote is 
empty?

> @@ -298,9 +333,24 @@ cmd_update()
>  			die "Unable to find current revision in submodule path '$path'"

>  		fi
>  
> +		baseurl="$(GIT_CONFIG=.gitmodules git config submodule."$name".url)"
> +		case "$baseurl" in
> +		./*|../*)
> +			fetch_remote=$remote
> +			absurl=$(resolve_relative_url $baseurl) || exit 1
> +			(unset GIT_DIR ; cd "$path" && git config remote."$fetch_remote".url > /dev/null) ||
> +			(
> +				unset GIT_DIR; cd "$path" && git remote add "$fetch_remote" "$absurl"
> +			) || die "Unable to define remote '$fetch_remote' in submodule path '$path'"
> +			;;
> +		*)
> +			fetch_remote=
> +			;;
> +		esac
> +
>  		if test "$subsha1" != "$sha1"
>  		then
> -			(unset GIT_DIR; cd "$path" && git-fetch &&
> +			(unset GIT_DIR; cd "$path" && git-fetch $fetch_remote &&

Wasn't the whole _point_ of having a two-stage init/update that you could 
_change_ the remote in the config?

Now you override those settings if .gitmodules says that the path is 
relative?  Shouldn't you respect the setting in the config (which the user 
can change freely), rather than .gitmodules (which the user cannot change 
without either committing it or having a permanently dirty working 
directory)?

Ciao,
Dscho

Re: [PATCH 1/3] git-submodule - Follow top-level remote on init/update/clone

[Mark Levedahl]

Johannes Schindelin wrote:
>> @@ -40,11 +42,13 @@ get_repo_base() {
>>  # Resolve relative url by appending to parent's url
>>  resolve_relative_url ()
>>  {
>> -	branch="$(git symbolic-ref HEAD 2>/dev/null)"
>> -	remote="$(git config branch.${branch#refs/heads/}.remote)"
>> -	remote="${remote:-origin}"
>> -	remoteurl="$(git config remote.$remote.url)" ||
>> -		die "remote ($remote) does not have a url in .git/config"
>>     
>
> I did not really look at this code before, but does that not mean that 
> git-submodule does already what you want?
>
> Because usually, you clone the superproject from the URL that you actually 
> want to use, and in that case, the initial branch's default remote will 
> have exactly that URL.
>
> So I have to admit that I do not see the reason why you remove that code, 
> replace it with another (that I think does the same), and claim that you 
> introduce that behaviour.
>
>   
There were three changes:
1) Eliminate die() in a subshell - it doesn't work. Instead, must have a 
return code and pass that up to top-level shell before exit can be done.
2) Eliminated duplication of get_default_remote(), instead using the 
definition in git-parse-remote.
Suggestions? (Separate patches, don't do 2)?
3) Use the user specified $remote if given.

>> @@ -107,7 +112,7 @@ module_clone()
>>  	test -e "$path" &&
>>  	die "A file already exist at path '$path'"
>>  
>> -	git-clone -n "$url" "$path" ||
>> +	git-clone -n -o "$remote" "$url" "$path" ||
>>  	die "Clone of '$url' into submodule path '$path' failed"
>>  }
>>  
>>     
>
> If you do _that_, you will _force_ the submodule to have no "origin" 
> remote.  As discussed _at length_, this is not what you should do.  The 
> only reason to use "-o <other-nick-name>" is if you plan _not_ to use the 
> same URL for the default remote.
>   
This *must* define the remote using the same name as flowed down from 
top-level, whatever that name is. Otherwise, subsequent fetch / update 
following top-level just won't work. Consider if top-level 
branch.<name>.remote = frotz and we define not frotz but origin in the 
submodule. Later, on different branch, top-level branch.<name>.remote  = 
origin. The submodule has origin defined but it points to a server 
different than top-level's origin. Now what?

The same holds true if user gave -r frotz: ignoring that and defining 
origin instead is an outright bug.

>
> Why do you need the "realremote" here?  Why is "$remote" not enough?
>   
Good catch - there was a reason in an earlier version of this, no longer 
relevant.
>   
>> @@ -235,7 +259,7 @@ cmd_init()
>>  		# Possibly a url relative to parent
>>  		case "$url" in
>>  		./*|../*)
>> -			url="$(resolve_relative_url "$url")"
>> +			url=$(resolve_relative_url "$url") || exit 1
>>     
>
> Yes for the "|| exit 1".  No for the removal of the quotes: keep in mind: 
> you are possibly getting a url from the _config_, which is supposed to be 
> user-editable.
>   
Works fine as is (You need the quotes when using the variable, not when 
defining it):

git>git config foo.name "some string
 > with cr"
git>z=$(git config foo.name)
git>echo "$z"
some string
with cr

As written, the old code had the perverse effect of *not* quoting $url, 
and that was the part that needed to be quoted.

>> @@ -274,6 +308,7 @@ cmd_update()
>>  		shift
>>  	done
>>  
>> +	remote=${remote:-$(get_default_remote)}
>>     
>
> You have this paradigm so often, but AFAICS you only use it for the call 
> to module_clone.  Why not let module_clone figure it out, if $remote is 
> empty?
>
>   
Will do.
>> @@ -298,9 +333,24 @@ cmd_update()
>>  			die "Unable to find current revision in submodule path '$path'"
>>     
> Wasn't the whole _point_ of having a two-stage init/update that you could 
> _change_ the remote in the config?
>
> Now you override those settings if .gitmodules says that the path is 
> relative?  Shouldn't you respect the setting in the config (which the user 
> can change freely), rather than .gitmodules (which the user cannot change 
> without either committing it or having a permanently dirty working 
> directory)?
>   
Ok, I was trying to avoid defining another config variable. The 
"relativeness" of a submodule is not knowable from  submodule.<path>.url 
as that is always absolute (if relative in .gitmodules, it is resolved 
into an absolute url during git init). I see two choices:
    1) define variable  submodule.<path>.isrelative during init, and use 
that instead. The user would have to modify that and the url to override.
    2) always resolve the relative url from .gitmodules, compare with 
submodule.<path>.url and decide it was relative if those two agree. More 
overhead, enough complexity that I fear for strange failure modes later on.

Any preferences or other suggestions?

Mark

Re: [PATCH 1/3] git-submodule - Follow top-level remote on init/update/clone

[Johannes Schindelin]

Hi,

On Mon, 11 Feb 2008, Mark Levedahl wrote:

> Johannes Schindelin wrote:
> 
> > > @@ -107,7 +112,7 @@ module_clone()
> > >  	test -e "$path" &&
> > >  	die "A file already exist at path '$path'"
> > >  -	git-clone -n "$url" "$path" ||
> > > +	git-clone -n -o "$remote" "$url" "$path" ||
> > >  	die "Clone of '$url' into submodule path '$path' failed"
> > >  }
> > >      
> > 
> > If you do _that_, you will _force_ the submodule to have no "origin" 
> > remote.  As discussed _at length_, this is not what you should do.  
> > The only reason to use "-o <other-nick-name>" is if you plan _not_ to 
> > use the same URL for the default remote.
>
> This *must* define the remote using the same name as flowed down from 
> top-level, whatever that name is.

At this point, I give up my review in despair,
Dscho

Re: [PATCH 1/3] git-submodule - Follow top-level remote on init/update/clone

[Mark Levedahl]

Johannes Schindelin wrote:
> Hi,
>
> On Mon, 11 Feb 2008, Mark Levedahl wrote:
>
>   
>> Johannes Schindelin wrote:
>>
>>     
>>>> @@ -107,7 +112,7 @@ module_clone()
>>>>  	test -e "$path" &&
>>>>  	die "A file already exist at path '$path'"
>>>>  -	git-clone -n "$url" "$path" ||
>>>> +	git-clone -n -o "$remote" "$url" "$path" ||
>>>>  	die "Clone of '$url' into submodule path '$path' failed"
>>>>  }
>>>>      
>>>>         
>>> If you do _that_, you will _force_ the submodule to have no "origin" 
>>> remote.  As discussed _at length_, this is not what you should do.  
>>> The only reason to use "-o <other-nick-name>" is if you plan _not_ to 
>>> use the same URL for the default remote.
>>>       
>> This *must* define the remote using the same name as flowed down from 
>> top-level, whatever that name is.
>>     
>
> At this point, I give up my review in despair,
> Dscho
>
>   
The submodules must use the same remote name to refer to the same 
server/repo-tree as top-level, or the coordinated fetch / update driven 
by top-level's branch.<name>.remote cannot work. It is always origin, or 
always frotz, not mix and match. There are two ways to achieve this:

1) Use the name given by top-level, as I did.
2) Restrict git to only allow one remote name, *origin*, ever (or at 
least if submodules are used). Not just the default remote, *any* 
remote. This removes branch.<name>.remote as that is defined to be origin.

I infer you choose option 2? It is certainly simpler, though 
significantly more restrictive.

Mark

Re: [PATCH 2/3] git-submodule - Allow adding a submodule in-place

[Ping Yin]

On Sun, Feb 10, 2008 at 12:57 AM, Mark Levedahl <mlevedahl@gmail.com> wrote:
> When working in top-level project, it is useful to create a new submodule
>  as a git repo in a subdirectory, then add that submodule to top-level in
>  place.  This allows "git submodule add <intended url> subdir" to add the
>  existing subdir to the current project.  The presumption is the user will
>  later push / clone the subdir to the <intended url> so that future
>  submodule init / updates will work.
>
>  Absent this patch, "git submodule add" insists upon cloning the subdir
>  from a repository at the given url, which is fine for adding an existing
>  project in but less useful when adding a new submodule from scratch to an
>  existing project.  The former functionality remains, and the clone is
>  attempted if the subdir does not already exist as a valid git repo.
>
>  Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
>  ---
>   Documentation/git-submodule.txt |    5 ++-
>   git-submodule.sh                |   57 +++++++++++++++++++++++---------------
>   2 files changed, 37 insertions(+), 25 deletions(-)
>
>  diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
>  index 4fc17f6..85d7dd3 100644
>  --- a/Documentation/git-submodule.txt
>  +++ b/Documentation/git-submodule.txt
>  @@ -18,8 +18,9 @@ COMMANDS
>   --------
>   add::
>         Add the given repository as a submodule at the given path
>  -       to the changeset to be committed next.  In particular, the
>  -       repository is cloned at the specified path, added to the
>  +       to the changeset to be committed next.  If path is a valid
>  +       repository within the project, it is added as is. Otherwise,
>  +       repository is cloned at the specified path. path is added to the
>         changeset and registered in .gitmodules.   If no path is
>         specified, the path is deduced from the repository specification.
>         If the repository url begins with ./ or ../, it is stored as
>  diff --git a/git-submodule.sh b/git-submodule.sh
>  index b97bf18..4c86a3c 100755
>  --- a/git-submodule.sh
>  +++ b/git-submodule.sh
>  @@ -166,23 +166,6 @@ cmd_add()
>                 usage
>         fi
>
>  -       case "$repo" in
>  -       ./*|../*)
>  -               # dereference source url relative to parent's url
>  -               realremote=${remote:-$(get_default_remote)}
>  -               realrepo=$(resolve_relative_url $repo) || exit 1
>  -               ;;
>  -       *)
>  -               # Turn the source into an absolute path if
>  -               # it is local
>  -               if base=$(get_repo_base "$repo"); then
>  -                       repo="$base"
>  -               fi
>  -               realremote=origin
>  -               realrepo=$repo
>  -               ;;
>  -       esac
>  -
>         # Guess path from repo if not specified or strip trailing slashes
>         if test -z "$path"; then
>                 path=$(echo "$repo" | sed -e 's|/*$||' -e 's|:*/*\.git$||' -e 's|.*[/:]||g')
>  @@ -190,15 +173,43 @@ cmd_add()
>                 path=$(echo "$path" | sed -e 's|/*$||')
>         fi
>
>  -       test -e "$path" &&
>  -       die "'$path' already exists"
>  -
>         git ls-files --error-unmatch "$path" > /dev/null 2>&1 &&
>         die "'$path' already exists in the index"
>
>  -       module_clone "$path" "$realrepo" "$realremote" || exit
>  -       (unset GIT_DIR; cd "$path" && git checkout -q ${branch:+-b "$branch" "$realremote/$branch"}) ||
>  -       die "Unable to checkout submodule '$path'"
>  +       # perhaps the path exists and is already a git repo, else clone it
>  +       if test -e "$path"
>  +       then
>  +               if test -d "$path/.git" &&
>  +               test "$(unset GIT_DIR; cd $path; git rev-parse --git-dir)" = ".git"
>  +               then
>  +                       echo "Adding existing repo at '$path' to the index"
>  +               else
>  +                       die "'$path' already exists and is not a valid git repo"
>  +               fi
>  +       else
>  +               case "$repo" in
>  +               ./*|../*)
>  +                       # dereference source url relative to parent's url
>  +                       realremote=${remote:-$(get_default_remote)}
>  +                       realrepo=$(resolve_relative_url $repo) || exit 1
>  +                       ;;
>  +               *)
>  +                       # Turn the source into an absolute path if
>  +                       # it is local
>  +                       if base=$(get_repo_base "$repo")
>  +                       then
>  +                               repo="$base"
>  +                       fi
>  +                       realremote=origin
>  +                       realrepo=$repo
>  +                       ;;
>  +               esac
>  +
>  +               module_clone "$path" "$realrepo" "$realremote" || exit
>  +               (unset GIT_DIR; cd "$path" && git checkout -q ${branch:+-b "$branch" "$realremote/$branch"}) ||
>  +               die "Unable to checkout submodule '$path'"
>  +       fi
>  +
>         git add "$path" ||
>         die "Failed to add submodule '$path'"
>
>  --

I think it's a very useful patch. But why has it sleeped for over one month?



-- 
Ping Yin