git-submodule: add "sync" command (v2)

git-submodule: add "sync" command (v2)

[David Aguilar]

The following patches rework the "sync" command in response to
Junio's input.

The first two patches are a simple refactor to move logic for
finding the name of the remote, which is not necessarily
"origin", into a standalone function.

The last patch is the reworked "git submodule sync" command.

[PATCH 1/3] git-submodule: add a get_remote function

[David Aguilar]

get_remote finds the remote that corresponds to HEAD.
This code is duplicated from resolve_relative_url but
will be refactored and reused in subsequent commits.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 git-submodule.sh |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 2a3a197..9d2bddb 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -27,6 +27,15 @@ say()
 	fi
 }
 
+# Gets the remote corresponding to HEAD
+get_remote()
+{
+	branch="$(git symbolic-ref HEAD 2>/dev/null)"
+	remote="$(git config branch.${branch#refs/heads/}.remote)"
+	remote="${remote:-origin}"
+	echo "$remote"
+}
+
 # Resolve relative url by appending to parent's url
 resolve_relative_url ()
 {
-- 
1.6.0.90.g436ed

[PATCH 2/3] git-submodule: use get_remote in resolve_relative_url

[David Aguilar]

This change removes replaces the remote finding logic in
resolve_relative_url with the new get_remote function.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 git-submodule.sh |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 9d2bddb..d2ae835 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -39,9 +39,7 @@ get_remote()
 # 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}"
+	remote=$(get_remote)
 	remoteurl=$(git config "remote.$remote.url") ||
 		die "remote ($remote) does not have a url defined in .git/config"
 	url="$1"
-- 
1.6.0.90.g436ed

Re: [PATCH 2/3] git-submodule: use get_remote in resolve_relative_url

[Junio C Hamano]

David Aguilar <davvid@gmail.com> writes:

> This change removes replaces the remote finding logic in
> resolve_relative_url with the new get_remote function.
>
> Signed-off-by: David Aguilar <davvid@gmail.com>

I think these first two patches should be squashed into one, with a title
"git-submodule: refactor logic to find the remote from the current branch".

If you agree, I'll do that myself -- no need to resend; please just say
the word.

Re: [PATCH 2/3] git-submodule: use get_remote in resolve_relative_url

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> David Aguilar <davvid@gmail.com> writes:
>
>> This change removes replaces the remote finding logic in
>> resolve_relative_url with the new get_remote function.
>>
>> Signed-off-by: David Aguilar <davvid@gmail.com>
>
> I think these first two patches should be squashed into one, with a title
> "git-submodule: refactor logic to find the remote from the current branch".
>
> If you agree, I'll do that myself -- no need to resend; please just say
> the word.

Actually, I took Mark's patch; you can do [3/3] with get_default_remote,
too, right?

[PATCH 3/3] git-submodule: add "sync" command

[David Aguilar]

When a submodule's URL changes upstream, existing submodules
will be out of sync since their remote."$origin".url will still
be set to the old value.

This change adds a "git submodule sync" command that reads the
submodule URLs from .gitmodules and updates any existing
submodules accordingly.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 Documentation/git-submodule.txt |    9 +++++++
 git-submodule.sh                |   51 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index abbd5b7..babaa9b 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -15,6 +15,7 @@ SYNOPSIS
 'git submodule' [--quiet] update [--init] [--] [<path>...]
 'git submodule' [--quiet] summary [--summary-limit <n>] [commit] [--] [<path>...]
 'git submodule' [--quiet] foreach <command>
+'git submodule' [--quiet] sync [--] [<path>...]
 
 
 DESCRIPTION
@@ -139,6 +140,14 @@ foreach::
 As an example, "git submodule foreach 'echo $path `git rev-parse HEAD`' will
 show the path and currently checked out commit for each submodule.
 
+sync::
+	Synchronizes submodules' remote URL configuration setting
+	to the value specified in .gitmodules.  This is useful when
+	submodule URLs change upstream and you need to update your local
+	repositories accordingly.
++
+"git submodule sync" synchronizes all submodules while
+"git submodule sync -- A" synchronizes submodule "A" only.
 
 OPTIONS
 -------
diff --git a/git-submodule.sh b/git-submodule.sh
index d2ae835..2af2ef4 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -6,7 +6,7 @@
 
 USAGE="[--quiet] [--cached] \
 [add <repo> [-b branch] <path>]|[status|init|update [-i|--init]|summary [-n|--summary-limit <n>] [<commit>]] \
-[--] [<path>...]|[foreach <command>]"
+[--] [<path>...]|[foreach <command>]|[sync [--] [<path>...]]"
 OPTIONS_SPEC=
 . git-sh-setup
 require_work_tree
@@ -609,6 +609,53 @@ cmd_status()
 		fi
 	done
 }
+#
+# Sync remote urls for submodules
+# This makes the value for remote.$remote.url match the value
+# specified in .gitmodules.
+#
+cmd_sync()
+{
+	while test $# -ne 0
+	do
+		case "$1" in
+		-q|--quiet)
+			quiet=1
+			shift
+			;;
+		--)
+			shift
+			break
+			;;
+		-*)
+			usage
+			;;
+		*)
+			break
+			;;
+		esac
+	done
+
+	cd_to_toplevel
+	toplevel="$PWD"
+
+	module_list "$@" |
+	while read mode sha1 stage path
+	do
+		name=$(module_name "$path")
+		url=$(git config -f .gitmodules --get submodule."$name".url)
+		if test -d "$path"; then
+		(
+			unset GIT_DIR
+			cd "$path"
+			remote=$(get_remote)
+			say "Synchronizing submodule url for '$name'"
+			git config remote."$remote".url "$url"
+			cd "$toplevel"
+		)
+		fi
+	done
+}
 
 # This loop parses the command line arguments to find the
 # subcommand name to dispatch.  Parsing of the subcommand specific
@@ -619,7 +666,7 @@ cmd_status()
 while test $# != 0 && test -z "$command"
 do
 	case "$1" in
-	add | foreach | init | update | status | summary)
+	add | foreach | init | update | status | summary | sync)
 		command=$1
 		;;
 	-q|--quiet)
-- 
1.6.0.90.g436ed

Re: [PATCH 3/3] git-submodule: add "sync" command

[Junio C Hamano]

David Aguilar <davvid@gmail.com> writes:

> +#
> +# Sync remote urls for submodules
> +# This makes the value for remote.$remote.url match the value
> +# specified in .gitmodules.
> +#
> +cmd_sync()
> +{
> +...
> +	cd_to_toplevel
> +	toplevel="$PWD"

I do not think you need $toplevel, as you cd around inside a subshell.

> +	module_list "$@" |
> +	while read mode sha1 stage path
> +	do
> +		name=$(module_name "$path")
> +		url=$(git config -f .gitmodules --get submodule."$name".url)
> +		if test -d "$path"; then

I think this test is wrong.

Compare it with how cmd_foreach does this.  The difference is that you
force "sync" to every submodule that could be cloned and checked out,
while "foreach" skips submodules the user has not expressed any interest
in touching.

> +		(
> +			unset GIT_DIR
> +			cd "$path"
> +			remote=$(get_remote)
> +			say "Synchronizing submodule url for '$name'"
> +			git config remote."$remote".url "$url"

I am not sure about the way you determine $remote.  When the HEAD in the
submodule repository is detached by prior "git submodule update", this
will fall back to the default "origin" --- is it a good behaviour?

This is not an objection; I am merely wondering if that fallback is
sensible, or if people who are interested in submodules can suggest better
alternatives.

> +			cd "$toplevel"

Unneeded (in a subshell).

> +		)
> +		fi
> +	done
> +}

Other than the above comments, the patch looks sensible to me.

Re: [PATCH 3/3] git-submodule: add "sync" command

[David Aguilar]

On  0, Junio C Hamano <gitster@pobox.com> wrote:
> David Aguilar <davvid@gmail.com> writes:
> > +		(
> > +			unset GIT_DIR
> > +			cd "$path"
> > +			remote=$(get_remote)
> > +			say "Synchronizing submodule url for '$name'"
> > +			git config remote."$remote".url "$url"
> 
> I am not sure about the way you determine $remote.  When the HEAD in the
> submodule repository is detached by prior "git submodule update", this
> will fall back to the default "origin" --- is it a good behaviour?
> 
> This is not an objection; I am merely wondering if that fallback is
> sensible, or if people who are interested in submodules can suggest better
> alternatives.


This is true.  I recall there was a lengthy thread a while
back about how "origin" might not always be what you'd want
when cloning a repo.  I'll address your comments, use
Mark's get_default_remote, and resend.  I have a feeling that
addressing the use of remote names in submodules is beyond
the scope of this patch for now since it seems like we'd need
that info stored somewhere in a superproject gitfile.

-- 

	David

[PATCH v3] git-submodule: add "sync" command

[David Aguilar]

When a submodule's URL changes upstream, existing submodules
will be out of sync since their remote."$origin".url will still
be set to the old value.

This adds a "git submodule sync" command that reads submodules'
URLs from .gitmodules and updates them accordingly.

Signed-off-by: David Aguilar <davvid@gmail.com>
---

This uses get_default_remote() per Mark's latest update.

 Documentation/git-submodule.txt |    9 +++++++
 git-submodule.sh                |   48 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index abbd5b7..babaa9b 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -15,6 +15,7 @@ SYNOPSIS
 'git submodule' [--quiet] update [--init] [--] [<path>...]
 'git submodule' [--quiet] summary [--summary-limit <n>] [commit] [--] [<path>...]
 'git submodule' [--quiet] foreach <command>
+'git submodule' [--quiet] sync [--] [<path>...]
 
 
 DESCRIPTION
@@ -139,6 +140,14 @@ foreach::
 As an example, "git submodule foreach 'echo $path `git rev-parse HEAD`' will
 show the path and currently checked out commit for each submodule.
 
+sync::
+	Synchronizes submodules' remote URL configuration setting
+	to the value specified in .gitmodules.  This is useful when
+	submodule URLs change upstream and you need to update your local
+	repositories accordingly.
++
+"git submodule sync" synchronizes all submodules while
+"git submodule sync -- A" synchronizes submodule "A" only.
 
 OPTIONS
 -------
diff --git a/git-submodule.sh b/git-submodule.sh
index 59fe7b3..4a95035 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -6,7 +6,7 @@
 
 USAGE="[--quiet] [--cached] \
 [add <repo> [-b branch] <path>]|[status|init|update [-i|--init]|summary [-n|--summary-limit <n>] [<commit>]] \
-[--] [<path>...]|[foreach <command>]"
+[--] [<path>...]|[foreach <command>]|[sync [--] [<path>...]]"
 OPTIONS_SPEC=
 . git-sh-setup
 . git-parse-remote
@@ -601,6 +601,50 @@ cmd_status()
 		fi
 	done
 }
+#
+# Sync remote urls for submodules
+# This makes the value for remote.$remote.url match the value
+# specified in .gitmodules.
+#
+cmd_sync()
+{
+	while test $# -ne 0
+	do
+		case "$1" in
+		-q|--quiet)
+			quiet=1
+			shift
+			;;
+		--)
+			shift
+			break
+			;;
+		-*)
+			usage
+			;;
+		*)
+			break
+			;;
+		esac
+	done
+	cd_to_toplevel
+	module_list "$@" |
+	while read mode sha1 stage path
+	do
+		name=$(module_name "$path")
+		url=$(git config -f .gitmodules --get submodule."$name".url)
+		if test -e "$path"/.git
+		then
+		(
+			unset GIT_DIR
+			cd "$path"
+			remote=$(get_default_remote)
+			say "Synchronizing submodule url for '$name'"
+			git config remote."$remote".url "$url"
+		)
+		fi
+	done
+}
 
 # This loop parses the command line arguments to find the
 # subcommand name to dispatch.  Parsing of the subcommand specific
@@ -611,7 +655,7 @@ cmd_status()
 while test $# != 0 && test -z "$command"
 do
 	case "$1" in
-	add | foreach | init | update | status | summary)
+	add | foreach | init | update | status | summary | sync)
 		command=$1
 		;;
 	-q|--quiet)
-- 
1.6.0.106.gd6096

Re: [PATCH v3] git-submodule: add "sync" command

[David Aguilar]

Hi Junio
I'm just checking up on the status of this patch:
http://thread.gmane.org/gmane.comp.version-control.git/93535/focus=93557

I believe v3 addressed all of your comments from that thread.
Let me know if there's anything else I should consider.

Thanks,

-- 

	David

Re: [PATCH v3] git-submodule: add "sync" command

[Junio C Hamano]

David Aguilar <davvid@gmail.com> writes:

> I'm just checking up on the status of this patch:
> http://thread.gmane.org/gmane.comp.version-control.git/93535/focus=93557
>
> I believe v3 addressed all of your comments from that thread.
> Let me know if there's anything else I should consider.

I didn't find anything wrong in there and already have a topic for it, but
ran out of time merging it to even 'pu' last night which was not my git
day.

Will most likely appear in 'next' by the end of today, but no promises.

Re: [PATCH 3/3] git-submodule: add "sync" command

[Mark Levedahl]

Junio C Hamano wrote:
> I am not sure about the way you determine $remote.  When the HEAD in the
> submodule repository is detached by prior "git submodule update", this
> will fall back to the default "origin" --- is it a good behaviour?
>
> This is not an objection; I am merely wondering if that fallback is
> sensible, or if people who are interested in submodules can suggest better
> alternatives.
>
>   
I think it makes sense to split submodules into two categories, name 
them what you will, but the discernible difference is whether they are 
defined using relative or absolute urls.

1) relative url - this should *always* fetch / pull from a url relative 
to the superproject's url. Overriding not possible (except by recording 
an absolute url). The idea is these submodules are closely related to 
the superproject, and the use of a relative url is a positive 
declaration that "I will maintain this 'forest' of git trees as a unit".
2) absolute url - use .gitmodules entry as the initial hint, allow 
overrides in .git/config, such a submodule is maintained separately.

If we could reach some such agreement as above, we might begin to make 
some progress on what role the porcelain has in maintaining a 'forest' 
of git projects as a superproject and submodules.

Mark

[PATCH] git-submodule - Use "get_default_remote" from git-parse-remote

[Mark Levedahl]

Resolve_relative_url was using its own code for this function, but
this is duplication with the best result that this continues to work.
Replace with the common function provided by git-parse-remote.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---

I think it makes more sense to use the facility provided elsewhere rather
than duplicate. (I've had this patch in my tree for quite a while, never
really had a reason to submit it.)


 git-submodule.sh |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 2a3a197..59fe7b3 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -9,6 +9,7 @@ USAGE="[--quiet] [--cached] \
 [--] [<path>...]|[foreach <command>]"
 OPTIONS_SPEC=
 . git-sh-setup
+. git-parse-remote
 require_work_tree

 command=
@@ -30,9 +31,7 @@ say()
 # 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}"
+	remote=$(get_default_remote)
 	remoteurl=$(git config "remote.$remote.url") ||
 		die "remote ($remote) does not have a url defined in .git/config"
 	url="$1"
--
1.6.0.127.gf1d7c

Re: [PATCH] git-submodule - Use "get_default_remote" from git-parse-remote

[Junio C Hamano]

Mark Levedahl <mlevedahl@gmail.com> writes:

> Resolve_relative_url was using its own code for this function, but
> this is duplication with the best result that this continues to work.
> Replace with the common function provided by git-parse-remote.
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
>
> I think it makes more sense to use the facility provided elsewhere rather
> than duplicate.

Ah, that's true.  Will apply, thanks.