threads / patch / 11992

patch, 3 partsgit-submodule - Follow top-level remote on init/update/clone

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

## tl;dr

8 messages between Feb 9, 2008 and Mar 3, 2008. Diffs are folded; open one to read it.

replies: 7people: 3as markdown or json

Mark Levedahl· Feb 9, 2008, 16:57 UTC · lore

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(-)
Show changes to 2 files +77 −15

Documentation/git-submodule.txt, git-submodule.sh

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
Mark Levedahl· Feb 9, 2008, 16:57 UTC · re: Mark Levedahl · lore

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

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(-)
Show changes to 2 files +37 −25

Documentation/git-submodule.txt, git-submodule.sh

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
Mark Levedahl· Feb 9, 2008, 16:57 UTC · re: Mark Levedahl · lore

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

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
Show changes to t/t7401-submodule-remote.sh +118 −0
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
Ping Yin· Mar 3, 2008, 13:14 UTC · re: Mark Levedahl · lore

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

On Sun, Feb 10, 2008 at 12:57 AM, Mark Levedahl <mlevedahl@gmail.com> wrote:
Show 114 quoted lines
> 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
Johannes Schindelin· Feb 11, 2008, 22:09 UTC · re: Mark Levedahl · lore

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

Hi,
On Sat, 9 Feb 2008, Mark Levedahl wrote:
Show 7 quoted lines
> 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.
Show 9 quoted lines
> @@ -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.

Show 9 quoted lines
> @@ -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.

Show 8 quoted lines
> @@ -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?
Show 6 quoted lines
> @@ -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.

Show 5 quoted lines
> @@ -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'"
Show 21 quoted lines
>  		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

Mark Levedahl· Feb 12, 2008, 01:00 UTC · re: Johannes Schindelin · lore

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

Johannes Schindelin wrote:
Show 23 quoted lines
>> @@ -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.
Show 16 quoted lines
>> @@ -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.

Show 13 quoted lines
>   
>> @@ -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.

Show 12 quoted lines
>> @@ -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.
Show 12 quoted lines
>> @@ -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
Johannes Schindelin· Feb 12, 2008, 01:10 UTC · re: Mark Levedahl · lore

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

Hi,
On Mon, 11 Feb 2008, Mark Levedahl wrote:
Show 18 quoted lines
> 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

Mark Levedahl· Feb 12, 2008, 01:34 UTC · re: Johannes Schindelin · lore

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

Johannes Schindelin wrote:
Show 30 quoted lines
> 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

← back to recent threads