Re: [PATCH] Fix submodule sync with relative submodule URLs

On  0, "Shawn O. Pearce" [off-list ref] wrote:

Johan Herland [off-list ref] wrote:

quoted

On Wednesday 24 September 2008, David Aguilar wrote:

quoted

Instead of just doing an "|| exit" shouldn't it report an explanation
of the error?
Other than that, it looks good to me.

Fixed. Thanks.

OK, time for the drive-by patch commenting.  I've largely stayed
out of git-submodule related code, but I just looked at in the
context of applying this patch.

There are three callers to resolve_relative_url in master and next.
All three callers just "|| exit" when resolve_relative_url fails.

The only reason resolve_relative_url can fail is when there is no
remote.$remote.url configuration option set for the current default
remote ("origin"?).

I guess I'm unclear about why cmd_sync is different from the
existing callers.

Right, it's not.  resolve_relative_url() is already calling
die() when that error condition is met, so the "|| exit" thing
can be removed entirely.  I sent a patch on top of Johan's
first patch to remove the "|| exit" calls in all callers of
resolve_relative_url.  That seems like the right thing to do;
in the very least it makes it easier to read.  What do you
think?

quoted

diff --git a/git-submodule.sh b/git-submodule.sh
index 1c39b59..f89bdbe 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh

@@ -634,6 +634,15 @@ cmd_sync()
 	do
 		name=$(module_name "$path")
 		url=$(git config -f .gitmodules --get submodule."$name".url)
+
+		# Possibly a url relative to parent
+		case "$url" in
+		./*|../*)
+			url=$(resolve_relative_url "$url") ||
+				die "failed to resolve relative submodule url for '$name'"
+			;;
+		esac
+
 		if test -e "$path"/.git
 		then
 		(

-- 
Shawn.

-- 

	David