threads / patch / 33758

patch, 3 partsSupport for old:new remote-helper push

Subject: [PATCH 0/3] Support for old:new remote-helper push

## tl;dr

19 messages between May 9, 2013 and May 16, 2013. Diffs are folded; open one to read it.

replies: 18people: 3as markdown or json

Felipe Contreras· May 9, 2013, 01:31 UTC · lore
Hi,

In order to support pushing old:new refspecs in remote-helpers, the best way to do what is to add an option to fast-export to rename the refs in the stream, so here it is:

Felipe Contreras (3):
  fast-export: improve argument parsing
  fast-export: add new --refspec option
  transport-helper: add support for old:new refspec
 Documentation/git-fast-export.txt |  4 ++++
 builtin/fast-export.c             | 33 ++++++++++++++++++++++++++++++++-
 t/t5801-remote-helpers.sh         |  2 +-
 t/t9350-fast-export.sh            |  7 +++++++
 transport-helper.c                | 14 ++++++++++++--
 5 files changed, 56 insertions(+), 4 deletions(-)
-- 
1.8.3.rc1.553.gac13664
Felipe Contreras· May 9, 2013, 01:31 UTC · re: Felipe Contreras · lore

[PATCH 1/3] fast-export: improve argument parsing

We don't want to pass arguments specific to fast-export to setup_revisions.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/fast-export.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Show changes to builtin/fast-export.c +2 −1
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index d60d675..6e46057 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -686,8 +686,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 	revs.topo_order = 1;
 	revs.show_source = 1;
 	revs.rewrite_parents = 1;
+	argc = parse_options(argc, argv, prefix, options, fast_export_usage,
+			PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN);
 	argc = setup_revisions(argc, argv, &revs, NULL);
-	argc = parse_options(argc, argv, prefix, options, fast_export_usage, 0);
 	if (argc > 1)
 		usage_with_options (fast_export_usage, options);
 
-- 
1.8.3.rc1.553.gac13664
Junio C Hamano· May 9, 2013, 22:17 UTC · re: Felipe Contreras · lore

Re: [PATCH 1/3] fast-export: improve argument parsing

Felipe Contreras <felipe.contreras@gmail.com> writes:
> We don't want to pass arguments specific to fast-export to
> setup_revisions.
Interesting.  What bad things happen with the current order?

Does "fast-export --export-marks=foo" causes setup_revisions() to mistakenly eat --export-marks=foo and barf?

Show 20 quoted lines
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  builtin/fast-export.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index d60d675..6e46057 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -686,8 +686,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
>  	revs.topo_order = 1;
>  	revs.show_source = 1;
>  	revs.rewrite_parents = 1;
> +	argc = parse_options(argc, argv, prefix, options, fast_export_usage,
> +			PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN);
>  	argc = setup_revisions(argc, argv, &revs, NULL);
> -	argc = parse_options(argc, argv, prefix, options, fast_export_usage, 0);
>  	if (argc > 1)
>  		usage_with_options (fast_export_usage, options);
There is a SP between the function name and its arguments here ;-)
Felipe Contreras· May 9, 2013, 23:02 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/3] fast-export: improve argument parsing

On Thu, May 9, 2013 at 5:17 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 9 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> We don't want to pass arguments specific to fast-export to
>> setup_revisions.
>
> Interesting.  What bad things happen with the current order?
>
> Does "fast-export --export-marks=foo" causes setup_revisions() to
> mistakenly eat --export-marks=foo and barf?

No, apparently it skips them. But try 'git fast-export --export-marks marks HEAD'.

Show 21 quoted lines
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>>  builtin/fast-export.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
>> index d60d675..6e46057 100644
>> --- a/builtin/fast-export.c
>> +++ b/builtin/fast-export.c
>> @@ -686,8 +686,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
>>       revs.topo_order = 1;
>>       revs.show_source = 1;
>>       revs.rewrite_parents = 1;
>> +     argc = parse_options(argc, argv, prefix, options, fast_export_usage,
>> +                     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN);
>>       argc = setup_revisions(argc, argv, &revs, NULL);
>> -     argc = parse_options(argc, argv, prefix, options, fast_export_usage, 0);
>>       if (argc > 1)
>>               usage_with_options (fast_export_usage, options);
>
> There is a SP between the function name and its arguments here ;-)

Yeah, and I already did my part: I sent a patch to fix this style. Not that it has anything to do with this patch.

-- 
Felipe Contreras
Junio C Hamano· May 9, 2013, 23:27 UTC · re: Felipe Contreras · lore

Re: [PATCH 1/3] fast-export: improve argument parsing

Felipe Contreras <felipe.contreras@gmail.com> writes:
Show 13 quoted lines
> On Thu, May 9, 2013 at 5:17 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>> We don't want to pass arguments specific to fast-export to
>>> setup_revisions.
>>
>> Interesting.  What bad things happen with the current order?
>>
>> Does "fast-export --export-marks=foo" causes setup_revisions() to
>> mistakenly eat --export-marks=foo and barf?
>
> No, apparently it skips them. But try 'git fast-export --export-marks
> marks HEAD'.

That is the kind of thing that needs to be said, not in the discussion but in the history, either in the log or in a new test, or both.

Felipe Contreras· May 9, 2013, 23:33 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/3] fast-export: improve argument parsing

On Thu, May 9, 2013 at 6:27 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 19 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Thu, May 9, 2013 at 5:17 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>>
>>>> We don't want to pass arguments specific to fast-export to
>>>> setup_revisions.
>>>
>>> Interesting.  What bad things happen with the current order?
>>>
>>> Does "fast-export --export-marks=foo" causes setup_revisions() to
>>> mistakenly eat --export-marks=foo and barf?
>>
>> No, apparently it skips them. But try 'git fast-export --export-marks
>> marks HEAD'.
>
> That is the kind of thing that needs to be said, not in the
> discussion but in the history, either in the log or in a new test,
> or both.
If only I had known that when I wrote the commit message.
-- 
Felipe Contreras
Junio C Hamano· May 10, 2013, 04:49 UTC · re: Felipe Contreras · lore

Re: [PATCH 1/3] fast-export: improve argument parsing

Felipe Contreras <felipe.contreras@gmail.com> writes:
Show 7 quoted lines
> On Thu, May 9, 2013 at 6:27 PM, Junio C Hamano <gitster@pobox.com> wrote:
> ...
>> That is the kind of thing that needs to be said, not in the
>> discussion but in the history, either in the log or in a new test,
>> or both.
>
> If only I had known that when I wrote the commit message.

This is exactly what we review patches on the list for. There is no need to feel bad. Nobody expects anybody to be perfect on the first try.

Other people who may not know what the thought process of the author was when the patch was written (or those who may not be as familiar with the area in question as the author of the patch) can sometimes spot a gap in the thought that is recorded in the patch.

Regarding the text of the patch in question, I am of two minds. It may solve the "--import-marks foo" to swap the orders, but I suspect that may merely be shifting the problem and break rev-list options (e.g. "--since 3.days") for the same reason.

In the longer term, setup_revisions() may need to allow its callers to tell it what to do when it sees a parameter that it does not understand.

There already is a similar cascading mechanism in place from revision argument parsing to diff argument parsing (look for the place where diff_opt_parse() is called in revision.c). It may be just the matter of adding a "struct option *" to rev_info and calling parse_options_step() after diff_opt_parse() says "I dunno about this option".

For the mechanism to be more flexible, we may want to give the custom option table the caller of setup_revisions() gives us precedence over revision/diff options, though.

Felipe Contreras· May 9, 2013, 01:31 UTC · re: Felipe Contreras · lore

[PATCH 2/3] fast-export: add new --refspec option

So that we can covert the exported ref names.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-fast-export.txt |  4 ++++
 builtin/fast-export.c             | 30 ++++++++++++++++++++++++++++++
 t/t9350-fast-export.sh            |  7 +++++++
 3 files changed, 41 insertions(+)
Show changes to 3 files +41 −0

Documentation/git-fast-export.txt, builtin/fast-export.c, t/t9350-fast-export.sh

diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt
index 03fc8c3..d1985d3 100644
--- a/Documentation/git-fast-export.txt
+++ b/Documentation/git-fast-export.txt
@@ -105,6 +105,10 @@ marks the same across runs.
 	in the commit (as opposed to just listing the files which are
 	different from the commit's first parent).
 
+--refspec::
+	Apply the specified refspec to each ref exported. Multiple of them can
+	be specified.
+
 [<git-rev-list-args>...]::
        A list of arguments, acceptable to 'git rev-parse' and
        'git rev-list', that specifies the specific objects and references
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 6e46057..0097103 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -17,6 +17,7 @@
 #include "utf8.h"
 #include "parse-options.h"
 #include "quote.h"
+#include "remote.h"
 
 static const char *fast_export_usage[] = {
 	N_("git fast-export [rev-list-opts]"),
@@ -30,6 +31,8 @@ static int fake_missing_tagger;
 static int use_done_feature;
 static int no_data;
 static int full_tree;
+static struct refspec *refspecs;
+static int refspecs_nr;
 
 static int parse_opt_signed_tag_mode(const struct option *opt,
 				     const char *arg, int unset)
@@ -502,6 +505,15 @@ static void get_tags_and_duplicates(struct rev_cmdline_info *info,
 		if (dwim_ref(e->name, strlen(e->name), sha1, &full_name) != 1)
 			continue;
 
+		if (refspecs) {
+			char *private;
+			private = apply_refspecs(refspecs, refspecs_nr, full_name);
+			if (private) {
+				free(full_name);
+				full_name = private;
+			}
+		}
+
 		switch (e->item->type) {
 		case OBJ_COMMIT:
 			commit = (struct commit *)e->item;
@@ -653,6 +665,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 	struct commit *commit;
 	char *export_filename = NULL, *import_filename = NULL;
 	uint32_t lastimportid;
+	struct string_list refspecs_list;
 	struct option options[] = {
 		OPT_INTEGER(0, "progress", &progress,
 			    N_("show progress after <n> objects")),
@@ -673,6 +686,8 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN(0, "use-done-feature", &use_done_feature,
 			     N_("Use the done feature to terminate the stream")),
 		OPT_BOOL(0, "no-data", &no_data, N_("Skip output of blob data")),
+		OPT_STRING_LIST(0, "refspec", &refspecs_list, N_("refspec"),
+			     N_("Apply refspec to exported refs")),
 		OPT_END()
 	};
 
@@ -692,6 +707,19 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 	if (argc > 1)
 		usage_with_options (fast_export_usage, options);
 
+	if (refspecs_list.nr) {
+		const char *refspecs_str[refspecs_list.nr];
+		int i;
+
+		for (i = 0; i < refspecs_list.nr; i++)
+			refspecs_str[i] = refspecs_list.items[i].string;
+
+		refspecs_nr = refspecs_list.nr;
+		refspecs = parse_fetch_refspec(refspecs_nr, refspecs_str);
+
+		string_list_clear(&refspecs_list, 1);
+	}
+
 	if (use_done_feature)
 		printf("feature done\n");
 
@@ -726,5 +754,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 	if (use_done_feature)
 		printf("done\n");
 
+	free_refspec(refspecs_nr, refspecs);
+
 	return 0;
 }
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 2471bc6..ef2d76e 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -505,4 +505,11 @@ test_expect_success 'refs are updated even if no commits need to be exported' '
 	test_cmp expected actual
 '
 
+test_expect_success 'use refspec' '
+	git fast-export --refspec refs/heads/master:refs/heads/foobar master | \
+		grep "^commit " | sort | uniq > actual &&
+	echo "commit refs/heads/foobar" > expected &&
+	test_cmp expected actual
+'
+
 test_done
-- 
1.8.3.rc1.553.gac13664
Junio C Hamano· May 9, 2013, 22:32 UTC · re: Felipe Contreras · lore

Re: [PATCH 2/3] fast-export: add new --refspec option

Felipe Contreras <felipe.contreras@gmail.com> writes:
> +test_expect_success 'use refspec' '
> +	git fast-export --refspec refs/heads/master:refs/heads/foobar master | \
> +		grep "^commit " | sort | uniq > actual &&

You do not need backslash after the pipe symbol at the end of line; the shell knows you haven't finished speaking at that point.

The usual "pipe hides the error status of upstream commands" applies here. The command may die after writing enough to fill the pipe buffer and showing the lines that begin with "commit".

Also it makes it harder to debug the test when something goes wrong.

By the way, don't you find that something does not feel quite right with this command line?

    git fast-export --refspec=refs/heads/master:refs/heads/foobar master 

Why do we even have to say "master" at the end, when the other option makes it clear that we are shipping "master" out?

Without thinking ramifications through, my gut feeling is that it would feel more natural if we took:

    git fast-export master:foobar

to mean the same thing (which is what happens to the users of "git push"). Is there a case where you have some ref on the left hand side of the --refspec but you do not push out the history leading to it?

With such an update, this part of the test would of course look like:

	git fast-export master:foobar >actual.dump &&
        grep "^commit " actual.dump |
        sort -u >actual &&
        ...

and we do not need a new option. Just a new extension to express what gets pushed out.

But of course I may be missing some cases why there need to be a separate option.

Felipe Contreras· May 9, 2013, 22:53 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/3] fast-export: add new --refspec option

On Thu, May 9, 2013 at 5:32 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 46 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> +test_expect_success 'use refspec' '
>> +     git fast-export --refspec refs/heads/master:refs/heads/foobar master | \
>> +             grep "^commit " | sort | uniq > actual &&
>
> You do not need backslash after the pipe symbol at the end of line;
> the shell knows you haven't finished speaking at that point.
>
> The usual "pipe hides the error status of upstream commands" applies
> here.  The command may die after writing enough to fill the pipe
> buffer and showing the lines that begin with "commit".
>
> Also it makes it harder to debug the test when something goes wrong.
>
> By the way, don't you find that something does not feel quite right
> with this command line?
>
>     git fast-export --refspec=refs/heads/master:refs/heads/foobar master
>
> Why do we even have to say "master" at the end, when the other
> option makes it clear that we are shipping "master" out?
>
> Without thinking ramifications through, my gut feeling is that it
> would feel more natural if we took:
>
>     git fast-export master:foobar
>
> to mean the same thing (which is what happens to the users of "git
> push").  Is there a case where you have some ref on the left hand
> side of the --refspec but you do not push out the history leading to
> it?
>
> With such an update, this part of the test would of course look
> like:
>
>         git fast-export master:foobar >actual.dump &&
>         grep "^commit " actual.dump |
>         sort -u >actual &&
>         ...
>
> and we do not need a new option.  Just a new extension to express
> what gets pushed out.
>
> But of course I may be missing some cases why there need to be a
> separate option.

Of course, but how do you implement that? That's mixing refspecs and revlist arguments, which AFAIK don't mix:

% git fast-export ^next:new-next master:new-master --not refs/tags/*:refs/tags/backup/*

-- 
Felipe Contreras
Junio C Hamano· May 9, 2013, 23:23 UTC · re: Felipe Contreras · lore

Re: [PATCH 2/3] fast-export: add new --refspec option

Felipe Contreras <felipe.contreras@gmail.com> writes:
> Of course, but how do you implement that? That's mixing refspecs and
> revlist arguments, which AFAIK don't mix:

Simple. You treat everything as refspecs and form revision ranges out of them. Note that that is exactly the reason why "git push" can take "master" as a short-hand for "master:master" [*1*].

> % git fast-export ^next:new-next master:new-master --not
> refs/tags/*:refs/tags/backup/*

I thought you stopped mentioning the bottom of the range (e.g. ^next) in the output from export stream at around 49266e8a11cf (fast-export: don't handle uninteresting refs, 2012-11-28).

What does ^next:new-next (or mapping after "--not" in general) even mean? They would not make sense, would they?

So I would imagine you would be spelling that as:
    git fast-export master:new-master --not next refs/tags/*
or something, no?
[Footnote]
*1* Of course, unlike "git push", but similar to "git bundle",
    export does not know who the "receiving side" is and what they
    have, so in addition to the positives, you would need to tell
    the command where the bottoms of the range you are exporting
    are, so there needs to be some difference between the way "git
    push" and export/bundle specify the ranges.
    But that does not affect what should happen on the positive end
    of the ranges, which both "git push" and export/bundle need to
    specify anyway.
Felipe Contreras· May 9, 2013, 23:32 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/3] fast-export: add new --refspec option

On Thu, May 9, 2013 at 6:23 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 8 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> Of course, but how do you implement that? That's mixing refspecs and
>> revlist arguments, which AFAIK don't mix:
>
> Simple.  You treat everything as refspecs and form revision ranges
> out of them.  Note that that is exactly the reason why "git push"
> can take "master" as a short-hand for "master:master" [*1*].
And how do you implement that?
Show 6 quoted lines
>> % git fast-export ^next:new-next master:new-master --not
>> refs/tags/*:refs/tags/backup/*
>
> I thought you stopped mentioning the bottom of the range
> (e.g. ^next) in the output from export stream at around 49266e8a11cf
> (fast-export: don't handle uninteresting refs, 2012-11-28).
That doesn't prevent the rev-list parsing from working.
> What does ^next:new-next (or mapping after "--not" in general) even
> mean?  They would not make sense, would they?
They don't, which is precisely my point.
Show 5 quoted lines
> So I would imagine you would be spelling that as:
>
>     git fast-export master:new-master --not next refs/tags/*
>
> or something, no?
rev-list doesn't accept 'refs/tags/*'.
-- 
Felipe Contreras
Junio C Hamano· May 10, 2013, 00:21 UTC · re: Felipe Contreras · lore

Re: [PATCH 2/3] fast-export: add new --refspec option

Felipe Contreras <felipe.contreras@gmail.com> writes:
Show 12 quoted lines
> On Thu, May 9, 2013 at 6:23 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Simple.  You treat everything as refspecs and form revision ranges
>> out of them.  Note that that is exactly the reason why "git push"
>> can take "master" as a short-hand for "master:master" [*1*].
> ...
>> So I would imagine you would be spelling that as:
>>
>>     git fast-export master:new-master --not next refs/tags/*
>>
>> or something, no?
>
> rev-list doesn't accept 'refs/tags/*'.

I think you misunderstood. That is meant to illustrate what your end users feed "fast-export". "fast-export" in turn expands that into a revision range, which needs to happen anyway when it strips :new-master from the positive end of the range to make the range into

	master ^next ^refs/tags/v1.0 ^refs/tags/v1.1 ...
Felipe Contreras· May 10, 2013, 00:44 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/3] fast-export: add new --refspec option

On Thu, May 9, 2013 at 7:21 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 22 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Thu, May 9, 2013 at 6:23 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Simple.  You treat everything as refspecs and form revision ranges
>>> out of them.  Note that that is exactly the reason why "git push"
>>> can take "master" as a short-hand for "master:master" [*1*].
>> ...
>>> So I would imagine you would be spelling that as:
>>>
>>>     git fast-export master:new-master --not next refs/tags/*
>>>
>>> or something, no?
>>
>> rev-list doesn't accept 'refs/tags/*'.
>
> I think you misunderstood.  That is meant to illustrate what your
> end users feed "fast-export". "fast-export" in turn expands that
> into a revision range, which needs to happen anyway when it strips
> :new-master from the positive end of the range to make the range
> into
>
>         master ^next ^refs/tags/v1.0 ^refs/tags/v1.1 ...

'git fast-export' accepts rev-list arguments, to make it do something else would not only break existing users, but would require massive work. I'm not interested in doing that.

Cheers.
-- 
Felipe Contreras
Junio C Hamano· May 10, 2013, 01:13 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/3] fast-export: add new --refspec option

Junio C Hamano <gitster@pobox.com> writes:
Show 22 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Thu, May 9, 2013 at 6:23 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Simple.  You treat everything as refspecs and form revision ranges
>>> out of them.  Note that that is exactly the reason why "git push"
>>> can take "master" as a short-hand for "master:master" [*1*].
>> ...
>>> So I would imagine you would be spelling that as:
>>>
>>>     git fast-export master:new-master --not next refs/tags/*
>>>
>>> or something, no?
>>
>> rev-list doesn't accept 'refs/tags/*'.
>
> I think you misunderstood.  That is meant to illustrate what your
> end users feed "fast-export". "fast-export" in turn expands that
> into a revision range, which needs to happen anyway when it strips
> :new-master from the positive end of the range to make the range
> into
>
> 	master ^next ^refs/tags/v1.0 ^refs/tags/v1.1 ...
A few corrections and a related random thought.
 * The original --not "refs/tags/*:refs/tags/backup/*" fooled me. As
   my primary point was that an earlier change to fast-export made
   the uninteresting bottom not appear as reset in the output
   stream, I omitted everything after colon without much thought.
   If the current fast-export does not let you say "refs/tags/*" to
   export all the tags, we do not have to extend it to allow it to
   do so.
 * On the other hand, "git log 'fc/*'" might be a handy thing for
   any command that wants to have multiple starting points for
   revision traversal, so in principle I would not mind such an
   enhancement to rev-list machinery.
   But that does not have to be part of this series.
 * I alluded to the similarity between fast-export and bundle,
   meaning to hint that "git bundle" might also benefit from being
   able to say "record these refs under different name", but after
   thinking about it a bit more, I think it is unnecessary.
   The user of the bundle's output, either fetch or clone, can use
   the usual refspec mapping to use the data in any way it wants.
   If the bundle says refs/heads/master is at one commit, the _user_
   of that bundle can decide to map it to refs/remotes/origin/master
   (or refs/heads/origin, for that matter).
   It is a very powerful concept that we can generate data once,
   cast it in stone, and delay the decision on _how_ it is used
   until the last minute, much better than mapping at export time.
   So bundle does not need a similar refspec mechanism to map what
   it exports.  I haven't thought things through to see if the same
   logic applies to fast-export output (if so, it would mean it is
   better to allow the consumer of the stream take the refspec
   parameter and map the tips it finds in its input), though.
Johannes Sixt· May 10, 2013, 06:42 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/3] fast-export: add new --refspec option

Am 5/10/2013 3:13, schrieb Junio C Hamano:
>  * On the other hand, "git log 'fc/*'" might be a handy thing for
>    any command that wants to have multiple starting points for
>    revision traversal, so in principle I would not mind such an
>    enhancement to rev-list machinery.

Currently, we spell this as "git log --branches=fc" or "git log --branches='*export*'". But I do not mean to say that "git log fc/\*" would be a bad thing to have.

-- Hannes
Felipe Contreras· May 16, 2013, 09:15 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/3] fast-export: add new --refspec option

On Thu, May 9, 2013 at 8:13 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 8 quoted lines
>    It is a very powerful concept that we can generate data once,
>    cast it in stone, and delay the decision on _how_ it is used
>    until the last minute, much better than mapping at export time.
>    So bundle does not need a similar refspec mechanism to map what
>    it exports.  I haven't thought things through to see if the same
>    logic applies to fast-export output (if so, it would mean it is
>    better to allow the consumer of the stream take the refspec
>    parameter and map the tips it finds in its input), though.

It's possible to delay the renaming of refspecs to leave that duty to the remote-helper, however, each and every remote-helper would need advertise a new capability, and then implement the renaming. I think it makes much more sense to implement something that just works for free in all remote-helpers.

But I wasn't even that interested in this patch, I implemented it because while I was helping the emacs folks I had to explain to them that old:new didn't work, and I thought it would be trivial to make it work.

I'm dropping this series. Somebody should at least make it so a proper error is displayed so that the users are not left in the dark wondering what the hell is going on.

-- 
Felipe Contreras
Felipe Contreras· May 9, 2013, 01:31 UTC · re: Felipe Contreras · lore

[PATCH 3/3] transport-helper: add support for old:new refspec

By using fast-export's new --refspec option.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t5801-remote-helpers.sh |  2 +-
 transport-helper.c        | 14 ++++++++++++--
 2 files changed, 13 insertions(+), 3 deletions(-)
Show changes to 2 files +13 −3

t/t5801-remote-helpers.sh, transport-helper.c

diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index dbb02e2..d15f794 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -92,7 +92,7 @@ test_expect_success 'push new branch by name' '
 	compare_refs local HEAD server refs/heads/new-name
 '
 
-test_expect_failure 'push new branch with old:new refspec' '
+test_expect_success 'push new branch with old:new refspec' '
 	(cd local &&
 	 git push origin new-name:new-refspec
 	) &&
diff --git a/transport-helper.c b/transport-helper.c
index 835815f..b1fdd39 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -787,7 +787,7 @@ static int push_refs_with_export(struct transport *transport,
 	struct ref *ref;
 	struct child_process *helper, exporter;
 	struct helper_data *data = transport->data;
-	struct string_list revlist_args = STRING_LIST_INIT_NODUP;
+	struct string_list revlist_args = STRING_LIST_INIT_DUP;
 	struct strbuf buf = STRBUF_INIT;
 
 	helper = get_helper(transport);
@@ -814,14 +814,24 @@ static int push_refs_with_export(struct transport *transport,
 			die("remote-helpers do not support ref deletion");
 		}
 
-		if (ref->peer_ref)
+		if (ref->peer_ref) {
+			if (strcmp(ref->name, ref->peer_ref->name)) {
+				struct strbuf buf = STRBUF_INIT;
+				strbuf_addf(&buf, "%s:%s", ref->peer_ref->name, ref->name);
+				string_list_append(&revlist_args, "--refspec");
+				string_list_append(&revlist_args, buf.buf);
+				strbuf_release(&buf);
+			}
 			string_list_append(&revlist_args, ref->peer_ref->name);
+		}
 
 	}
 
 	if (get_exporter(transport, &exporter, &revlist_args))
 		die("Couldn't run fast-export");
 
+	string_list_clear(&revlist_args, 1);
+
 	if (finish_command(&exporter))
 		die("Error while running fast-export");
 	push_update_refs_status(data, remote_refs);
-- 
1.8.3.rc1.553.gac13664
Felipe Contreras· May 16, 2013, 09:15 UTC · re: Felipe Contreras · lore

Re: [PATCH 0/3] Support for old:new remote-helper push

On Wed, May 8, 2013 at 8:31 PM, Felipe Contreras <felipe.contreras@gmail.com> wrote:

> In order to support pushing old:new refspecs in remote-helpers, the best way to
> do what is to add an option to fast-export to rename the refs in the stream, so
> here it is:
I'm not going to work on this series any more.
-- 
Felipe Contreras

← back to recent threads