[RFC PATCH 0/1] Teach remote add a --prefix-tags option

[RFC PATCH 0/1] Teach remote add a --prefix-tags option

[Wink Saville]

Hello,

This patch was originally created as a pull request on github [1] and
then languisheh as I forgot about it. Recently I was asked to revive it
and have done so. I've rebased on top of master and validated it still
works.

Please review.

-- Wink

[1]: https://github.com/git/git/pull/486

Wink Saville (1):
  Teach remote add the --prefix-tags option

 Documentation/git-remote.txt |  8 +++++--
 builtin/remote.c             | 42 ++++++++++++++++++++++++++++++++----
 remote.c                     |  2 ++
 3 files changed, 46 insertions(+), 6 deletions(-)

-- 
2.16.2.7164.g7daebe18fb

[RFC PATCH 1/1] Teach remote add the --prefix-tags option

[Wink Saville]

When --prefix-tags is passed to `git remote add` the tagopt is set to
--prefix-tags and a second fetch line is added so tags are placed in
a separate hierarchy per remote.

For example:
  $ git remote add -f --prefix-tags gbenchmark git@github.com:google/benchmark
  Updating gbenchmark
  warning: no common commits
  remote: Counting objects: 4406, done.
  remote: Compressing objects: 100% (18/18), done.
  remote: Total 4406 (delta 7), reused 13 (delta 6), pack-reused 4382
  Receiving objects: 100% (4406/4406), 1.34 MiB | 7.58 MiB/s, done.
  Resolving deltas: 100% (2865/2865), done.
  From github.com:google/benchmark
   * [new branch]      clangtidy       -> gbenchmark/clangtidy
   * [new branch]      iter_report     -> gbenchmark/iter_report
   * [new branch]      master          -> gbenchmark/master
   * [new branch]      releasing       -> gbenchmark/releasing
   * [new branch]      reportercleanup -> gbenchmark/reportercleanup
   * [new branch]      rmheaders       -> gbenchmark/rmheaders
   * [new branch]      v2              -> gbenchmark/v2
   * [new tag]         v0.0.9          -> tags/gbenchmark/v0.0.9
   * [new tag]         v0.1.0          -> tags/gbenchmark/v0.1.0
   * [new tag]         v1.0.0          -> tags/gbenchmark/v1.0.0
   * [new tag]         v1.1.0          -> tags/gbenchmark/v1.1.0
   * [new tag]         v1.2.0          -> tags/gbenchmark/v1.2.0
   * [new tag]         v1.3.0          -> tags/gbenchmark/v1.3.0
   * [new tag]         v1.4.0          -> tags/gbenchmark/v1.4.0

And the .git/config remote "gbenchmark" section looks like:
  [remote "gbenchmark"]
    url = git@github.com:google/benchmark
    fetch = +refs/heads/*:refs/remotes/gbenchmark/*
    fetch = +refs/tags/*:refs/remotes/tags/gbenchmark/*
    tagopt = --prefix-tags

Based on a solution proposed by Junio on the email list [1]

[1]: https://public-inbox.org/git/xmqqbme51rgn.fsf@gitster-ct.c.googlers.com/T/#me7f7f153b8ba742c0dc48d8ec79c280c9682d32e

Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Jacob Keller <jacob.keller@gmail.com>
Signed-off-by: Wink Saville <wink@saville.com>
---
 Documentation/git-remote.txt |  8 +++++--
 builtin/remote.c             | 42 ++++++++++++++++++++++++++++++++----
 remote.c                     |  2 ++
 3 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
index 9659abbf8e..db0238e8bd 100644
--- a/Documentation/git-remote.txt
+++ b/Documentation/git-remote.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git remote' [-v | --verbose]
-'git remote add' [-t <branch>] [-m <master>] [-f] [--[no-]tags] [--mirror=<fetch|push>] <name> <url>
+'git remote add' [-t <branch>] [-m <master>] [-f] [--tags | --prefix-tags | --no-tags] [--mirror=<fetch|push>] <name> <url>
 'git remote rename' <old> <new>
 'git remote remove' <name>
 'git remote set-head' <name> (-a | --auto | -d | --delete | <branch>)
@@ -54,7 +54,11 @@ With `-f` option, `git fetch <name>` is run immediately after
 the remote information is set up.
 +
 With `--tags` option, `git fetch <name>` imports every tag from the
-remote repository.
+remote repository to refs/tags, use --prefix-tags to import them
+to refs/remotes/tags/<name>/<tag>.
++
+With `--prefix-tags` option, `git fetch <name>` imports every tag from the
+remote repository to refs/remotes/tags/<name>/<tag>.
 +
 With `--no-tags` option, `git fetch <name>` does not import tags from
 the remote repository.
diff --git a/builtin/remote.c b/builtin/remote.c
index 5591cef775..88991f9fbe 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -14,7 +14,7 @@
 
 static const char * const builtin_remote_usage[] = {
 	N_("git remote [-v | --verbose]"),
-	N_("git remote add [-t <branch>] [-m <master>] [-f] [--tags | --no-tags] [--mirror=<fetch|push>] <name> <url>"),
+	N_("git remote add [-t <branch>] [-m <master>] [-f] [--prefix-tags | --tags | --no-tags] [--mirror=<fetch|push>] <name> <url>"),
 	N_("git remote rename <old> <new>"),
 	N_("git remote remove <name>"),
 	N_("git remote set-head <name> (-a | --auto | -d | --delete | <branch>)"),
@@ -104,7 +104,8 @@ static int fetch_remote(const char *name)
 enum {
 	TAGS_UNSET = 0,
 	TAGS_DEFAULT = 1,
-	TAGS_SET = 2
+	TAGS_SET = 2,
+	TAGS_SET_PREFIX = 3
 };
 
 #define MIRROR_NONE 0
@@ -126,6 +127,14 @@ static void add_branch(const char *key, const char *branchname,
 	git_config_set_multivar(key, tmp->buf, "^$", 0);
 }
 
+static void add_remote_tags(const char *key, const char *remotename,
+			    struct strbuf *tmp)
+{
+	strbuf_reset(tmp);
+	strbuf_addf(tmp, "+refs/tags/*:refs/remotes/tags/%s/*", remotename);
+	git_config_set_multivar(key, tmp->buf, "^$", 0);
+}
+
 static const char mirror_advice[] =
 N_("--mirror is dangerous and deprecated; please\n"
    "\t use --mirror=fetch or --mirror=push instead");
@@ -164,6 +173,9 @@ static int add(int argc, const char **argv)
 		OPT_SET_INT(0, "tags", &fetch_tags,
 			    N_("import all tags and associated objects when fetching"),
 			    TAGS_SET),
+		OPT_SET_INT(0, "prefix-tags", &fetch_tags,
+			    N_("import all tags and associated objects when fetching and prefix with <name>"),
+			    TAGS_SET_PREFIX),
 		OPT_SET_INT(0, NULL, &fetch_tags,
 			    N_("or do not fetch any tag at all (--no-tags)"), TAGS_UNSET),
 		OPT_STRING_LIST('t', "track", &track, N_("branch"),
@@ -185,6 +197,8 @@ static int add(int argc, const char **argv)
 		die(_("specifying a master branch makes no sense with --mirror"));
 	if (mirror && !(mirror & MIRROR_FETCH) && track.nr)
 		die(_("specifying branches to track makes sense only with fetch mirrors"));
+	if (mirror && (fetch_tags == TAGS_SET_PREFIX))
+		die(_("specifying a --prefix-tags makes no sense with --mirror"));
 
 	name = argv[0];
 	url = argv[1];
@@ -218,10 +232,30 @@ static int add(int argc, const char **argv)
 	}
 
 	if (fetch_tags != TAGS_DEFAULT) {
+		if (fetch_tags == TAGS_SET_PREFIX) {
+			strbuf_reset(&buf);
+			strbuf_addf(&buf, "remote.%s.fetch", name);
+			add_remote_tags(buf.buf, name, &buf2);
+		}
+
 		strbuf_reset(&buf);
 		strbuf_addf(&buf, "remote.%s.tagopt", name);
-		git_config_set(buf.buf,
-			       fetch_tags == TAGS_SET ? "--tags" : "--no-tags");
+		char *config_val = NULL;
+		switch (fetch_tags) {
+		case TAGS_UNSET:
+			config_val = "--no-tags";
+			break;
+		case TAGS_SET:
+			config_val = "--tags";
+			break;
+		case TAGS_SET_PREFIX:
+			config_val = "--prefix-tags";
+			break;
+		default:
+			die(_("Unexpected TAGS enum %d"), fetch_tags);
+			break;
+		}
+		git_config_set(buf.buf, config_val);
 	}
 
 	if (fetch && fetch_remote(name))
diff --git a/remote.c b/remote.c
index e50f7602ed..d916fda029 100644
--- a/remote.c
+++ b/remote.c
@@ -421,6 +421,8 @@ static int handle_config(const char *key, const char *value, void *cb)
 			remote->fetch_tags = -1;
 		else if (!strcmp(value, "--tags"))
 			remote->fetch_tags = 2;
+		else if (!strcmp(value, "--prefix-tags"))
+			remote->fetch_tags = -1;
 	} else if (!strcmp(subkey, "proxy")) {
 		return git_config_string((const char **)&remote->http_proxy,
 					 key, value);
-- 
2.16.2.7164.g7daebe18fb

Re: [RFC PATCH 1/1] Teach remote add the --prefix-tags option

[Junio C Hamano]

Wink Saville <wink@saville.com> writes:

> When --prefix-tags is passed to `git remote add` the tagopt is set to
> --prefix-tags and a second fetch line is added so tags are placed in
> a separate hierarchy per remote.


In the olden days, there was no refs/remotes/$remoteName/ hiearchy,
and until we made it the default at around Git 1.5.0, such a modern
layout for the branches were called the "separate remote" layout,
and can be opted into with "clone --use-separate-remote" by early
adopters.

I doubt that use of refs/tags/$remoteName/ is a good design if we
want to achieve similar isolation between local tags and and tags
obtained from each remote.

An obvious alternative, refs/remotes/$remoteName/tags/, is not a
good design for exactly the same reason.  You cannot tell between a
local tag foo/bar and a tag bar obtained from remote foo when you
see refs/tags/foo/bar, and you cannot tell between a branch tag/bar
obtained from remote foo and a tag bar obtained from remote foo when
you see refs/remotes/foo/tags/bar.  In the past, people suggested to
use refs/remoteTags/$remoteName/ for proper isolation, and it might
be a better middle-ground than either of the two, at least in the
shorter term, but not ideal.

In short, if you truly want to see "separate hierarchy per remote",
you should consider how you can reliably implement an equivalent of
"git branch --list --remote"; a design that does not allow it is a
failure.

A better solution with longer lifetime would probably be to use

	refs/remotes/$remoteName/{heads,tags,...}/

when core.useTotallySeparateRemote configuration exists (and
eventually at Git 3.0 make the layout the default).  It would
involve changes in the refname look-up rules, but it would not have
to pollute refs/ namespace like the refs/remoteTags/ half-ground
design, which would require us to add refs/remoteNotes/ and friends,
who knows how many more we would end up having to support if we go
that route.

Thanks.

Re: [RFC PATCH 1/1] Teach remote add the --prefix-tags option

[Wink Saville]

> In short, if you truly want to see "separate hierarchy per remote",
> you should consider how you can reliably implement an equivalent of
> "git branch --list --remote"; a design that does not allow it is a
> failure.
>
> A better solution with longer lifetime would probably be to use
>
>         refs/remotes/$remoteName/{heads,tags,...}/
>
> when core.useTotallySeparateRemote configuration exists (and
> eventually at Git 3.0 make the layout the default).  It would
> involve changes in the refname look-up rules, but it would not have
> to pollute refs/ namespace like the refs/remoteTags/ half-ground
> design, which would require us to add refs/remoteNotes/ and friends,
> who knows how many more we would end up having to support if we go
> that route.

I've used submodules a little bit and in some sense it seems to
already implement
core.useTotallySeparateRemote. So from my perspective trying to make this change
significantly different this isn't worth the effort and I'm not sure I
have the expertise
to do it well.

Is there a roadmap and or timeline for Git 3.0?

Re: [RFC PATCH 1/1] Teach remote add the --prefix-tags option

[Jacob Keller]

On Mon, Oct 14, 2019 at 8:07 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Wink Saville <wink@saville.com> writes:
>
> > When --prefix-tags is passed to `git remote add` the tagopt is set to
> > --prefix-tags and a second fetch line is added so tags are placed in
> > a separate hierarchy per remote.
>
>
> In the olden days, there was no refs/remotes/$remoteName/ hiearchy,
> and until we made it the default at around Git 1.5.0, such a modern
> layout for the branches were called the "separate remote" layout,
> and can be opted into with "clone --use-separate-remote" by early
> adopters.
>
> I doubt that use of refs/tags/$remoteName/ is a good design if we
> want to achieve similar isolation between local tags and and tags
> obtained from each remote.
>
> An obvious alternative, refs/remotes/$remoteName/tags/, is not a
> good design for exactly the same reason.  You cannot tell between a
> local tag foo/bar and a tag bar obtained from remote foo when you
> see refs/tags/foo/bar, and you cannot tell between a branch tag/bar
> obtained from remote foo and a tag bar obtained from remote foo when
> you see refs/remotes/foo/tags/bar.  In the past, people suggested to
> use refs/remoteTags/$remoteName/ for proper isolation, and it might
> be a better middle-ground than either of the two, at least in the
> shorter term, but not ideal.
>
> In short, if you truly want to see "separate hierarchy per remote",
> you should consider how you can reliably implement an equivalent of
> "git branch --list --remote"; a design that does not allow it is a
> failure.
>
> A better solution with longer lifetime would probably be to use
>
>         refs/remotes/$remoteName/{heads,tags,...}/
>
> when core.useTotallySeparateRemote configuration exists (and
> eventually at Git 3.0 make the layout the default).  It would
> involve changes in the refname look-up rules, but it would not have
> to pollute refs/ namespace like the refs/remoteTags/ half-ground
> design, which would require us to add refs/remoteNotes/ and friends,
> who knows how many more we would end up having to support if we go
> that route.
>
> Thanks.
>

Something like this makes sense and I've thought about the problem for
a long time. Unfortunately it's quite a bit trickier to do this.

It would solve the problem more generally though, and definitely seems
like the right approach.. but at least for me, every time I looked at
trying this I got lost. I haven't had time to investigate it recently
:(

Thanks,
Jake

Re: [RFC PATCH 1/1] Teach remote add the --prefix-tags option

[Jacob Keller]

On Tue, Oct 15, 2019 at 2:13 PM Wink Saville <wink@saville.com> wrote:
>>
>> Something like this makes sense and I've thought about the problem for
>> a long time. Unfortunately it's quite a bit trickier to do this.
>>
>> It would solve the problem more generally though, and definitely seems
>> like the right approach.. but at least for me, every time I looked at
>> trying this I got lost. I haven't had time to investigate it recently
>> :(
>>
>> Thanks,
>> Jake
>
>
> Give it a go, you'll learn something at a minimum :)

I've started a couple of times, but mostly it's lack of time to
invest, since $DAYJOB hasn't given me cycles to try at the moment.

Thanks,
Jake