[PATCH 0/5] Fix 'url.*.insteadOf' for submodule URLs

As suggested in a thread some time ago, one could redefine the URL used to 
fetch submodules by adding a 'url.*.insteadOf' rule prior to the first 
invocation of 'git submodule update'.

However, this does not work with current Git, because the super-repo config 
(which is home to the 'url.*.insteadOf' rule) is not consulted by the 'git 
clone' that is invoked by 'git submodule update'.

These patches fix this issue by making 'git submodule' explicitly rewrite 
the submodule URL according to the super-repo config, prior to calling 'git 
clone'.

In order for the 'git submodule' shell script to properly rewrite URLs, it 
must gain access to the URL rewriting functionality in remote.c. To expose 
the URL rewriting functionality to 'git submodule' (and others, if needed), 
a new option ('rewrite-url') has been added to 'git config'.

The patch series is based on master. Whether or not this should be 
considered for v1.6.0 is to be decided by Junio. (My personal opinion is 
that although we're late in the release cycle, the patches are fairly 
straightforward, and should not pose a great risk of regressions.)

Johan Herland (5):
  Add testcase for 'git submodule' with url.*.insteadOf set in the
    super-repo
  Teach 'git config' to rewrite URLs according to current
    url.*.insteadOf rules
  Add selftest for new option '--rewrite-url' to 'git config'
  Add documentation on the new --rewrite-url option to 'git config'
  Teach 'git submodule' to rewrite submodule URLs according to
    super-repo's rules

 Documentation/git-config.txt |   10 ++++++++++
 builtin-config.c             |   23 ++++++++++++++++++++++-
 git-submodule.sh             |    6 ++++++
 t/t1300-repo-config.sh       |   14 ++++++++++++++
 t/t7400-submodule-basic.sh   |   11 +++++++++++
 5 files changed, 63 insertions(+), 1 deletions(-)


Have fun!

...Johan

Currently, setting url.*.insteadOf in the super-repo in order to rewrite
submodule URLs, don't work. When cloning/fetching the submodule, the
super-repo config is never consulted, and thus the url.*.insteadOf rule
is never seen. This adds a testcase that confirms the current behaviour.

Signed-off-by: Johan Herland <redacted>
---
 t/t7400-submodule-basic.sh |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index cbc0c34..bafc46c 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh

@@ -209,4 +209,15 @@ test_expect_success 'update --init' '
 
 '
 
+test_expect_failure 'update --init with url.*.insteadOf' '
+
+	rm -rf init &&
+	git config -f .gitmodules submodule.example.url "http://example.com/init2" 

&&
+	git config --remove-section submodule.example
+	git config "url.$(pwd)/.insteadOf" "http://example.com/" &&
+	git submodule update --init init &&
+	test -d init/.git
+
+'
+
 test_done
-- 
1.6.0.rc1.34.g0fe8c

Simple check that URL rewriting functionality works as expected.

Signed-off-by: Johan Herland <redacted>
---
 t/t1300-repo-config.sh |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 64567fb..dcd04c2 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh

@@ -741,4 +741,18 @@ test_expect_success 'symlinked configuration' '
 
 '
 
+test_expect_success '--rewrite-url' '
+
+	git config url.ssh://example.com/foo/.insteadOf http://example.com/ &&
+	git config url.../.insteadOf git://example.com/ &&
+	a=$(git config --rewrite-url http://example.com/baz/xyzzy.git) &&
+	test "zssh://example.com/foo/baz/xyzzy.git" = "z$a" &&
+	b=$(git config --rewrite-url git://example.com/baz/xyzzy.git) &&
+	test "z../baz/xyzzy.git" = "z$b" &&
+	c=$(git config --rewrite-url ssh://example.com/foo/baz/xyzzy.git) &&
+	test "zssh://example.com/foo/baz/xyzzy.git" = "z$c" &&
+	d=$(git config --rewrite-url rsync://example.com/baz/xyzzy.git) &&
+	test "zrsync://example.com/baz/xyzzy.git" = "z$d"
+'
+
 test_done

-- 
1.6.0.rc1.34.g0fe8c

This patch adds the --rewrite-url option to 'git config'. The option takes
exactly one argument; a URL that is to be rewritten according to the longest
matching url.*.insteadOf rule in the current config. The resulting URL is
printed on stdout, and 0 is returned.

The rationale for this patch is to enable access to Git's URL rewriting
functionality from shell scripts.

The URL rewriting functionality is implemented by piggybacking on the
existing URL rewriting code in remote.c.

Signed-off-by: Johan Herland <redacted>
---
 builtin-config.c |   23 ++++++++++++++++++++++-
 1 files changed, 22 insertions(+), 1 deletions(-)

diff --git a/builtin-config.c b/builtin-config.c
index 91fdc49..977e6cb 100644
--- a/builtin-config.c
+++ b/builtin-config.c

@@ -1,9 +1,10 @@
 #include "builtin.h"
 #include "cache.h"
 #include "color.h"
+#include "remote.h"
 
 static const char git_config_set_usage[] =
-"git config [ --global | --system | [ -f | --file ] config-file ] [ --bool | --int | --bool-or-int ] [ -z | --null ] 

[--get | --get-all | --get-regexp | --replace-all | --add | --unset | --unset-all] name [value [value_regex]] | --rename-section old_name new_name | --remove-section name | --list | --get-color 
var [default] | --get-colorbool name [stdout-is-tty]";
+"git config [ --global | --system | [ -f | --file ] config-file ] [ --bool | --int | --bool-or-int ] [ -z | --null ] 
[--get | --get-all | --get-regexp | --replace-all | --add | --unset | --unset-all] name [value [value_regex]] | --rename-section old_name new_name | --remove-section name | --list | --get-color 
var [default] | --get-colorbool name [stdout-is-tty] | --rewrite-url url";
 
 static char *key;
 static regex_t *key_regexp;

@@ -281,6 +282,24 @@ static int get_colorbool(int argc, const char **argv)
 	}
 }
 
+static int rewrite_url(int argc, const char **argv)
+{
+	/*
+	 * git config --rewrite_url <url>
+	 *
+	 * returns <url> after rewriting it according to url.*.insteadOf rules.
+	 */
+
+	if (argc > 1)
+		usage(git_config_set_usage);
+
+	struct remote *remote = remote_get(argv[0]);
+	if (remote->url_nr != 1)
+		die("Expected exactly one URL from remote_get()!");
+	printf("%s\n", remote->url[0]);
+	return 0;
+}
+
 int cmd_config(int argc, const char **argv, const char *prefix)
 {
 	int nongit;

@@ -362,6 +381,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 			return get_color(argc-2, argv+2);
 		} else if (!strcmp(argv[1], "--get-colorbool")) {
 			return get_colorbool(argc-2, argv+2);
+		} else if (!strcmp(argv[1], "--rewrite-url")) {
+			return rewrite_url(argc-2, argv+2);
 		} else
 			break;
 		argc--;

-- 
1.6.0.rc1.34.g0fe8c

Signed-off-by: Johan Herland <redacted>
---
 Documentation/git-config.txt |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 28e1861..154c80b 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt

@@ -22,6 +22,7 @@ SYNOPSIS
 'git config' [<file-option>] [-z|--null] -l | --list
 'git config' [<file-option>] --get-color name [default]
 'git config' [<file-option>] --get-colorbool name [stdout-is-tty]
+'git config' [<file-option>] --rewrite-url url
 
 DESCRIPTION
 -----------

@@ -157,6 +158,15 @@ See also <<FILES>>.
 	output.  The optional `default` parameter is used instead, if
 	there is no color configured for `name`.
 
+--rewrite-url url::
+
+	Rewrite `url` according to the longest matching URL rewriting rule
+	(see documentation on `url.<base>.insteadOf` for more information
+	on URL rewriting rules), and output the resulting URL to the
+	standard output. If there is no matching URL rewriting rule, the
+	original `url` is printed on the standard output. In either case,
+	the exit code is 0.
+
 [[FILES]]
 FILES
 -----

-- 
1.6.0.rc1.34.g0fe8c

When a 'url.*.insteadOf' rule in the superrepo matches a submodule URL, we
should rewrite the submodule URL accordingly, so that we don't request the
submodule from location that is inaccessible (or unwanted by the user for
some other reason).

Signed-off-by: Johan Herland <redacted>
---
 git-submodule.sh           |    6 ++++++
 t/t7400-submodule-basic.sh |    2 +-
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index b40f876..ea2aa3b 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh

@@ -96,6 +96,12 @@ module_clone()
 	test -e "$path" &&
 	die "A file already exist at path '$path'"
 
+	# The user may have added url.*.insteadOf rules intending to rewrite
+	# submodule URLs. We must explicitly do this rewrite before calling
+	# 'git clone', since 'git clone' will not consult the super-repo
+	# config and thus never see any url.*.insteadOf rules placed therein.
+	url=$(git config --rewrite-url "$url")
+
 	git-clone -n "$url" "$path" ||
 	die "Clone of '$url' into submodule path '$path' failed"
 }

diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index bafc46c..9c56de2 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh

@@ -209,7 +209,7 @@ test_expect_success 'update --init' '
 
 '
 
-test_expect_failure 'update --init with url.*.insteadOf' '
+test_expect_success 'update --init with url.*.insteadOf' '
 
 	rm -rf init &&
 	git config -f .gitmodules submodule.example.url "http://example.com/init2" &&

-- 
1.6.0.rc1.34.g0fe8c

Currently, setting url.*.insteadOf in the super-repo in order to rewrite
submodule URLs, don't work. When cloning/fetching the submodule, the
super-repo config is never consulted, and thus the url.*.insteadOf rule
is never seen. This adds a testcase that confirms the current behaviour.

Signed-off-by: Johan Herland <redacted>
---
 t/t7400-submodule-basic.sh |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)


The previous patch was whitespace-damaged. Sorry. Trying again.


...Johan

diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index cbc0c34..bafc46c 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh

@@ -209,4 +209,15 @@ test_expect_success 'update --init' '
 
 '
 
+test_expect_failure 'update --init with url.*.insteadOf' '
+
+	rm -rf init &&
+	git config -f .gitmodules submodule.example.url "http://example.com/init2" &&
+	git config --remove-section submodule.example
+	git config "url.$(pwd)/.insteadOf" "http://example.com/" &&
+	git submodule update --init init &&
+	test -d init/.git
+
+'
+
 test_done

-- 
1.6.0.rc1.34.g0fe8c

Hi,

On Mon, 4 Aug 2008, Johan Herland wrote:

As suggested in a thread some time ago, one could redefine the URL used 
to fetch submodules by adding a 'url.*.insteadOf' rule prior to the 
first invocation of 'git submodule update'.

If I suggested it, but forgot the "--global" flag to "git config", I 
apologize.

Ciao,
Dscho

On Monday 04 August 2008, Johannes Schindelin wrote:

On Mon, 4 Aug 2008, Johan Herland wrote:

quoted

As suggested in a thread some time ago, one could redefine the URL used
to fetch submodules by adding a 'url.*.insteadOf' rule prior to the
first invocation of 'git submodule update'.

If I suggested it, but forgot the "--global" flag to "git config", I
apologize.

Does this mean that you don't agree with the rationale for this patch? I.e. 
that submodule URLs should not be rewritten according to the rules in the 
super-repo (but instead require such rules to be set in the user's global 
config)?

There are (at least) two reasons for why I think this should work without 
having to use '--global':

1. Consistency: Other git commands in the supermodule does _not_ require the 
URL rewriting rule to reside in the global config. Why should 'git 
submodule' be different.

2. I believe there are valid use cases for adding URL rewriting rules to the 
repo config instead of the global config. You may want to check out Fred's 
version of project X (including submodules), without making your other 
clones of project X start cloning/fetching from Fred.


Puzzled,
...Johan


-- 
Johan Herland, [off-list ref]
www.herland.net