Add url.<base>.pushInsteadOf: URL rewriting for push only

PATCH In review Josh Triplett 2 participants 7 messages Jun 15, 2016 Documentation

Share & tools

Josh Triplett

author

2016-06-15 22:47 UTC #0 lore raw

v2 cover Add url.<base>.pushInsteadOf: URL rewriting for push only

Many sites host repositories via both git:// for fetch-only anonymous access and ssh:// for push-capable access. The "insteadOf" mechanism makes it straightforward to substitute the push-capable URLs for the pull-only URLs, which proves convenient when the site hosts many repositories using the same URL scheme. However, if you use such a substitution and you cannot use the ssh:// URLs (either because you don't have SSH access or you don't have permission to a particular repository), you cannot clone or fetch either, even though you could do so via the git:// URLs. A situation like this arises when sharing git configuration files between systems, of which only a few have SSH access to repositories.

"pushurl" provides a way to specify URLs used only for push, but this requires configuring a pushurl for each such repository. As in the rationale for insteadOf, it makes sense to configure this for all repositories hosted on a given system at once.

This patch series adds a new "pushInsteadOf" option to go with "insteadOf". pushInsteadOf allows systematically rewriting fetch-only URLs to push-capable URLs when used with push. For instance:

[url "ssh://example.org/"]
pushInsteadOf = "git://example.org/"

This will allow clones of "git://example.org/path/to/repo" to subsequently push to "ssh://example.org/path/to/repo", without manually configuring pushurl for that remote.

Includes documentation for the new option, bash completion updates, and test cases (both that pushInsteadOf applies to push and that it does *not* apply to fetch).

Changes in v2:
- Don't add implicit pushurls from pushInsteadOf to remotes with explicit pushurls (suggested by Junio).
- Fix pushInsteadOf rewrites to occur before insteadOf rewrites, to avoid basing the pushInsteadOf rewrites on the rewritten URLs.

Josh Triplett (2):
Wrap rewrite globals in a struct in preparation for adding another set Add url.<base>.pushInsteadOf: URL rewriting for push only

 Documentation/config.txt               |   13 +++++
 Documentation/urls.txt                 |   18 +++++++
 contrib/completion/git-completion.bash |    2 +-
 remote.c                               |   83 ++++++++++++++++++++------------
 t/t5516-fetch-push.sh                  |   31 ++++++++++++
 5 files changed, 115 insertions(+), 32 deletions(-)

Josh Triplett

author

2016-06-15 22:47 UTC #1 lore raw

PATCH v2 1/2 Wrap rewrite globals in a struct in preparation for adding another set

remote.c has a global set of URL rewrites, accessed by alias_url and make_rewrite. Wrap them in a new "struct rewrites", passed to alias_url and make_rewrite. This allows adding other sets of rewrites.

Signed-off-by Josh Triplett <[email protected]>

---
 remote.c |   53 ++++++++++++++++++++++++++++-------------------------
 1 files changed, 28 insertions(+), 25 deletions(-)

remote.c +28 ~~-25~~

index 4b5b905..ff8e71f 100644
@@ -28,6 +28,11 @@ struct rewrite {
 	int instead_of_nr;
 	int instead_of_alloc;
 };
+struct rewrites {
+	struct rewrite **rewrite;
+	int rewrite_alloc;
+	int rewrite_nr;
+};
 
 static struct remote **remotes;
 static int remotes_alloc;

@@ -41,14 +46,12 @@ static struct branch *current_branch;
 static const char *default_remote_name;
 static int explicit_default_remote_name;
 
-static struct rewrite **rewrite;
-static int rewrite_alloc;
-static int rewrite_nr;
+static struct rewrites rewrites;
 
 #define BUF_SIZE (2048)
 static char buffer[BUF_SIZE];
 
-static const char *alias_url(const char *url)
+static const char *alias_url(const char *url, struct rewrites *r)
 {
 	int i, j;
 	char *ret;

@@ -57,14 +60,14 @@ static const char *alias_url(const char *url)
 
 	longest = NULL;
 	longest_i = -1;
-	for (i = 0; i < rewrite_nr; i++) {
-		if (!rewrite[i])
+	for (i = 0; i < r->rewrite_nr; i++) {
+		if (!r->rewrite[i])
 			continue;
-		for (j = 0; j < rewrite[i]->instead_of_nr; j++) {
-			if (!prefixcmp(url, rewrite[i]->instead_of[j].s) &&
+		for (j = 0; j < r->rewrite[i]->instead_of_nr; j++) {
+			if (!prefixcmp(url, r->rewrite[i]->instead_of[j].s) &&
 			    (!longest ||
-			     longest->len < rewrite[i]->instead_of[j].len)) {
-				longest = &(rewrite[i]->instead_of[j]);
+			     longest->len < r->rewrite[i]->instead_of[j].len)) {
+				longest = &(r->rewrite[i]->instead_of[j]);
 				longest_i = i;
 			}
 		}

@@ -72,10 +75,10 @@ static const char *alias_url(const char *url)
 	if (!longest)
 		return url;
 
-	ret = xmalloc(rewrite[longest_i]->baselen +
+	ret = xmalloc(r->rewrite[longest_i]->baselen +
 		     (strlen(url) - longest->len) + 1);
-	strcpy(ret, rewrite[longest_i]->base);
-	strcpy(ret + rewrite[longest_i]->baselen, url + longest->len);
+	strcpy(ret, r->rewrite[longest_i]->base);
+	strcpy(ret + r->rewrite[longest_i]->baselen, url + longest->len);
 	return ret;
 }

@@ -103,7 +106,7 @@ static void add_url(struct remote *remote, const char *url)
 
 static void add_url_alias(struct remote *remote, const char *url)
 {
-	add_url(remote, alias_url(url));
+	add_url(remote, alias_url(url, &rewrites));
 }
 
 static void add_pushurl(struct remote *remote, const char *pushurl)

@@ -169,22 +172,22 @@ static struct branch *make_branch(const char *name, int len)
 	return ret;
 }
 
-static struct rewrite *make_rewrite(const char *base, int len)
+static struct rewrite *make_rewrite(struct rewrites *r, const char *base, int len)
 {
 	struct rewrite *ret;
 	int i;
 
-	for (i = 0; i < rewrite_nr; i++) {
+	for (i = 0; i < r->rewrite_nr; i++) {
 		if (len
-		    ? (len == rewrite[i]->baselen &&
-		       !strncmp(base, rewrite[i]->base, len))
-		    : !strcmp(base, rewrite[i]->base))
-			return rewrite[i];
+		    ? (len == r->rewrite[i]->baselen &&
+		       !strncmp(base, r->rewrite[i]->base, len))
+		    : !strcmp(base, r->rewrite[i]->base))
+			return r->rewrite[i];
 	}
 
-	ALLOC_GROW(rewrite, rewrite_nr + 1, rewrite_alloc);
+	ALLOC_GROW(r->rewrite, r->rewrite_nr + 1, r->rewrite_alloc);
 	ret = xcalloc(1, sizeof(struct rewrite));
-	rewrite[rewrite_nr++] = ret;
+	r->rewrite[r->rewrite_nr++] = ret;
 	if (len) {
 		ret->base = xstrndup(base, len);
 		ret->baselen = len;

@@ -355,7 +358,7 @@ static int handle_config(const char *key, const char *value, void *cb)
 		subkey = strrchr(name, '.');
 		if (!subkey)
 			return 0;
-		rewrite = make_rewrite(name, subkey - name);
+		rewrite = make_rewrite(&rewrites, name, subkey - name);
 		if (!strcmp(subkey, ".insteadof")) {
 			if (!value)
 				return config_error_nonbool(key);

@@ -433,10 +436,10 @@ static void alias_all_urls(void)
 		if (!remotes[i])
 			continue;
 		for (j = 0; j < remotes[i]->url_nr; j++) {
-			remotes[i]->url[j] = alias_url(remotes[i]->url[j]);
+			remotes[i]->url[j] = alias_url(remotes[i]->url[j], &rewrites);
 		}
 		for (j = 0; j < remotes[i]->pushurl_nr; j++) {
-			remotes[i]->pushurl[j] = alias_url(remotes[i]->pushurl[j]);
+			remotes[i]->pushurl[j] = alias_url(remotes[i]->pushurl[j], &rewrites);
 		}
 	}
 }

Signature · 1 line

1.6.3.3

Josh Triplett

author

2016-06-15 22:47 UTC #2 lore raw

PATCH v2 2/2 Add url.<base>.pushInsteadOf: URL rewriting for push only

In reply to Josh Triplett

This configuration option allows systematically rewriting fetch-only URLs to push-capable URLs when used with push. For instance:

[url "ssh://example.org/"]
pushInsteadOf = "git://example.org/"

This will allow clones of "git://example.org/path/to/repo" to subsequently push to "ssh://example.org/path/to/repo", without manually configuring pushurl for that remote.

Includes documentation for the new option, bash completion updates, and test cases (both that pushInsteadOf applies to push and that it does *not* apply to fetch).

Signed-off-by Josh Triplett <[email protected]>

---
 Documentation/config.txt               |   13 +++++++++++
 Documentation/urls.txt                 |   18 ++++++++++++++++
 contrib/completion/git-completion.bash |    2 +-
 remote.c                               |   36 ++++++++++++++++++++++++--------
 t/t5516-fetch-push.sh                  |   31 +++++++++++++++++++++++++++
 5 files changed, 90 insertions(+), 10 deletions(-)

Documentation/config.txt +13 -0

index 5256c7f..38c7086 100644
@@ -1500,6 +1500,19 @@ url.<base>.insteadOf::
 	never-before-seen repository on the site.  When more than one
 	insteadOf strings match a given URL, the longest match is used.
 
+url.<base>.pushInsteadOf::
+	Any URL that starts with this value will not be pushed to;
+	instead, it will be rewritten to start with <base>, and the
+	resulting URL will be pushed to. In cases where some site serves
+	a large number of repositories, and serves them with multiple
+	access methods, some of which do not allow push, this feature
+	allows people to specify a pull-only URL and have git
+	automatically use an appropriate URL to push, even for a
+	never-before-seen repository on the site.  When more than one
+	pushInsteadOf strings match a given URL, the longest match is
+	used.  If a remote has an explicit pushurl, git will ignore this
+	setting for that remote.
+
 user.email::
 	Your email address to be recorded in any newly created commits.
 	Can be overridden by the 'GIT_AUTHOR_EMAIL', 'GIT_COMMITTER_EMAIL', and

Documentation/urls.txt +18 -0

index 5355ebc..d813ceb 100644
@@ -67,3 +67,21 @@ For example, with this:
 a URL like "work:repo.git" or like "host.xz:/path/to/repo.git" will be
 rewritten in any context that takes a URL to be "git://git.host.xz/repo.git".
 
+If you want to rewrite URLs for push only, you can create a
+configuration section of the form:
+
+------------
+	[url "<actual url base>"]
+		pushInsteadOf = <other url base>
+------------
+
+For example, with this:
+
+------------
+	[url "ssh://example.org/"]
+		pushInsteadOf = git://example.org/
+------------
+
+a URL like "git://example.org/path/to/repo.git" will be rewritten to
+"ssh://example.org/path/to/repo.git" for pushes, but pulls will still
+use the original URL.

contrib/completion/git-completion.bash +1 -1

index bf688e1..9859204 100755
@@ -1532,7 +1532,7 @@ _git_config ()
 	url.*.*)
 		local pfx="${cur%.*}."
 		cur="${cur##*.}"
-		__gitcomp "insteadof" "$pfx" "$cur"
+		__gitcomp "insteadOf pushInsteadOf" "$pfx" "$cur"
 		return
 		;;
 	esac

remote.c +27 -9

index ff8e71f..73d33f2 100644
@@ -47,6 +47,7 @@ static const char *default_remote_name;
 static int explicit_default_remote_name;
 
 static struct rewrites rewrites;
+static struct rewrites rewrites_push;
 
 #define BUF_SIZE (2048)
 static char buffer[BUF_SIZE];

@@ -104,17 +105,25 @@ static void add_url(struct remote *remote, const char *url)
 	remote->url[remote->url_nr++] = url;
 }
 
-static void add_url_alias(struct remote *remote, const char *url)
-{
-	add_url(remote, alias_url(url, &rewrites));
-}
-
 static void add_pushurl(struct remote *remote, const char *pushurl)
 {
 	ALLOC_GROW(remote->pushurl, remote->pushurl_nr + 1, remote->pushurl_alloc);
 	remote->pushurl[remote->pushurl_nr++] = pushurl;
 }
 
+static void add_pushurl_alias(struct remote *remote, const char *url)
+{
+	const char *pushurl = alias_url(url, &rewrites_push);
+	if (pushurl != url)
+		add_pushurl(remote, pushurl);
+}
+
+static void add_url_alias(struct remote *remote, const char *url)
+{
+	add_url(remote, alias_url(url, &rewrites));
+	add_pushurl_alias(remote, url);
+}
+
 static struct remote *make_remote(const char *name, int len)
 {
 	struct remote *ret;

@@ -358,8 +367,13 @@ static int handle_config(const char *key, const char *value, void *cb)
 		subkey = strrchr(name, '.');
 		if (!subkey)
 			return 0;
-		rewrite = make_rewrite(&rewrites, name, subkey - name);
 		if (!strcmp(subkey, ".insteadof")) {
+			rewrite = make_rewrite(&rewrites, name, subkey - name);
+			if (!value)
+				return config_error_nonbool(key);
+			add_instead_of(rewrite, xstrdup(value));
+		} else if (!strcmp(subkey, ".pushinsteadof")) {
+			rewrite = make_rewrite(&rewrites_push, name, subkey - name);
 			if (!value)
 				return config_error_nonbool(key);
 			add_instead_of(rewrite, xstrdup(value));

@@ -433,14 +447,18 @@ static void alias_all_urls(void)
 {
 	int i, j;
 	for (i = 0; i < remotes_nr; i++) {
+		int add_pushurl_aliases;
 		if (!remotes[i])
 			continue;
-		for (j = 0; j < remotes[i]->url_nr; j++) {
-			remotes[i]->url[j] = alias_url(remotes[i]->url[j], &rewrites);
-		}
 		for (j = 0; j < remotes[i]->pushurl_nr; j++) {
 			remotes[i]->pushurl[j] = alias_url(remotes[i]->pushurl[j], &rewrites);
 		}
+		add_pushurl_aliases = remotes[i]->pushurl_nr == 0;
+		for (j = 0; j < remotes[i]->url_nr; j++) {
+			if (add_pushurl_aliases)
+				add_pushurl_alias(remotes[i], remotes[i]->url[j]);
+			remotes[i]->url[j] = alias_url(remotes[i]->url[j], &rewrites);
+		}
 	}
 }

t/t5516-fetch-push.sh +31 -0

index 2d2633f..8f455c7 100755
@@ -122,6 +122,23 @@ test_expect_success 'fetch with insteadOf' '
 	)
 '
 
+test_expect_success 'fetch with pushInsteadOf (should not rewrite)' '
+	mk_empty &&
+	(
+		TRASH=$(pwd)/ &&
+		cd testrepo &&
+		git config "url.trash/.pushInsteadOf" "$TRASH" &&
+		git config remote.up.url "$TRASH." &&
+		git config remote.up.fetch "refs/heads/*:refs/remotes/origin/*" &&
+		git fetch up &&
+
+		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
+		test "z$r" = "z$the_commit" &&
+
+		test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
+	)
+'
+
 test_expect_success 'push without wildcard' '
 	mk_empty &&

@@ -162,6 +179,20 @@ test_expect_success 'push with insteadOf' '
 	)
 '
 
+test_expect_success 'push with pushInsteadOf' '
+	mk_empty &&
+	TRASH="$(pwd)/" &&
+	git config "url.$TRASH.pushInsteadOf" trash/ &&
+	git push trash/testrepo refs/heads/master:refs/remotes/origin/master &&
+	(
+		cd testrepo &&
+		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
+		test "z$r" = "z$the_commit" &&
+
+		test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
+	)
+'
+
 test_expect_success 'push with matching heads' '
 
 	mk_test heads/master &&

Signature · 1 line

1.6.3.3

Junio C Hamano

2016-06-15 22:47 UTC #3 lore raw

Quoted 6 lines · Josh Triplett

This configuration option allows systematically rewriting fetch-only URLs to push-capable URLs when used with push. For instance:
[url "ssh://example.org/"]
pushInsteadOf = "git://example.org/"

Quoted 7 lines

This will allow clones of "git://example.org/path/to/repo" to subsequently push to "ssh://example.org/path/to/repo", without manually configuring pushurl for that remote.
Includes documentation for the new option, bash completion updates, and test cases (both that pushInsteadOf applies to push and that it does *not* apply to fetch).

Thanks.

I will queue these patches, but I presume you would also want to add a test that pushInsteadOf is ignored for remotes with an explicit pushURL?

Josh Triplett

author

2016-06-15 22:47 UTC #4 lore raw

Quoted 12 lines · Junio C Hamano

Quoted 10 lines · Josh Triplett
This configuration option allows systematically rewriting fetch-only URLs to push-capable URLs when used with push. For instance:
[url "ssh://example.org/"]
pushInsteadOf = "git://example.org/"
This will allow clones of "git://example.org/path/to/repo" to subsequently push to "ssh://example.org/path/to/repo", without manually configuring pushurl for that remote.

Quoted 8 lines

Quoted 3 lines
Includes documentation for the new option, bash completion updates, and test cases (both that pushInsteadOf applies to push and that it does *not* apply to fetch).
Thanks.
I will queue these patches, but I presume you would also want to add a test that pushInsteadOf is ignored for remotes with an explicit pushURL?

Will do.

- Josh Triplett

Josh Triplett

author

2016-06-15 22:47 UTC #5 lore raw

PATCH 1/1 Add test for ignoring pushInsteadOf when remote has explicit pushurl

Signed-off-by Josh Triplett <[email protected]>

---

Quoted 17 lines · Josh Triplett

Quoted 16 lines · Junio C Hamano
Quoted 13 lines · Josh Triplett
This configuration option allows systematically rewriting fetch-only URLs to push-capable URLs when used with push. For instance:
[url "ssh://example.org/"]
pushInsteadOf = "git://example.org/"
This will allow clones of "git://example.org/path/to/repo" to subsequently push to "ssh://example.org/path/to/repo", without manually configuring pushurl for that remote.
Includes documentation for the new option, bash completion updates, and test cases (both that pushInsteadOf applies to push and that it does *not* apply to fetch).

Quoted 6 lines

Quoted 4 lines
Thanks.
I will queue these patches, but I presume you would also want to add a test that pushInsteadOf is ignored for remotes with an explicit pushURL?
Will do.

Done. Please add this to the queue, optionally squashing it into patch 2/2 if you prefer.

 t/t5516-fetch-push.sh |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

t/t5516-fetch-push.sh +16 -0

index 8f455c7..6889a53 100755
@@ -193,6 +193,22 @@ test_expect_success 'push with pushInsteadOf' '
 	)
 '
 
+test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf should not rewrite)' '
+	mk_empty &&
+	TRASH="$(pwd)/" &&
+	git config "url.trash2/.pushInsteadOf" trash/ &&
+	git config remote.r.url trash/wrong &&
+	git config remote.r.pushurl "$TRASH/testrepo" &&
+	git push r refs/heads/master:refs/remotes/origin/master &&
+	(
+		cd testrepo &&
+		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
+		test "z$r" = "z$the_commit" &&
+
+		test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
+	)
+'
+
 test_expect_success 'push with matching heads' '
 
 	mk_test heads/master &&

Signature · 1 line

1.6.3.3

Junio C Hamano

2016-06-15 22:47 UTC #6 lore raw

Re: [PATCH] Add test for ignoring pushInsteadOf when remote has explicit pushurl

Quoted 2 lines · Josh Triplett

Done. Please add this to the queue, optionally squashing it into patch 2/2 if you prefer.

Thanks.