threads / patch / 34799

patch, 9 partstransport-helper: updates

Subject: [PATCH 0/9] transport-helper: updates

## tl;dr

20 messages between Aug 29, 2013 and Sep 23, 2013. Diffs are folded; open one to read it.

replies: 19people: 3as markdown or json

Felipe Contreras· Aug 29, 2013, 15:23 UTC · lore
Hi,

Here are the patches that allow transport helpers to be completely transparent; renaming branches, deleting them, custom refspecs, --force, --dry-run, reporting forced update, everything works.

Some of these were were sent before and rejected without a reason, but here they are again in case anybody is interested.

Felipe Contreras (9):
  transport-helper: add 'force' to 'export' helpers
  transport-helper: check for 'forced update' message
  fast-export: improve argument parsing
  fast-export: add new --refspec option
  transport-helper: add support for old:new refspec
  fast-import: add support to delete refs
  fast-export: add support to delete refs
  transport-helper: add support to delete branches
  transport-helper: don't update refs in dry-run
 Documentation/git-fast-export.txt |  4 ++++
 Documentation/git-fast-import.txt |  3 +++
 builtin/fast-export.c             | 47 ++++++++++++++++++++++++++++++++++++++-
 fast-import.c                     | 13 ++++++++---
 t/t5801-remote-helpers.sh         | 10 ++++++++-
 t/t9300-fast-import.sh            | 18 +++++++++++++++
 t/t9350-fast-export.sh            | 18 +++++++++++++++
 transport-helper.c                | 42 ++++++++++++++++++++++++----------
 8 files changed, 138 insertions(+), 17 deletions(-)
-- 
1.8.4-fc
Felipe Contreras· Aug 29, 2013, 15:23 UTC · re: Felipe Contreras · lore

[PATCH 1/9] transport-helper: add 'force' to 'export' helpers

Otherwise they cannot know when to force the push or not (other than hacks).

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 transport-helper.c | 3 +++
 1 file changed, 3 insertions(+)
Show changes to transport-helper.c +3 −0
diff --git a/transport-helper.c b/transport-helper.c
index 63cabc3..62051a6 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -814,6 +814,9 @@ static int push_refs_with_export(struct transport *transport,
 			die("helper %s does not support dry-run", data->name);
 	}
 
+	if (flags & TRANSPORT_PUSH_FORCE)
+		set_helper_option(transport, "force", "true");
+
 	helper = get_helper(transport);
 
 	write_constant(helper->in, "export\n");
-- 
1.8.4-fc
Richard Hansen· Sep 23, 2013, 02:41 UTC · re: Felipe Contreras · lore

Re: [PATCH 1/9] transport-helper: add 'force' to 'export' helpers

On 2013-08-29 11:23, Felipe Contreras wrote:
Show 18 quoted lines
> Otherwise they cannot know when to force the push or not (other than
> hacks).
> 
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  transport-helper.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/transport-helper.c b/transport-helper.c
> index 63cabc3..62051a6 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -814,6 +814,9 @@ static int push_refs_with_export(struct transport *transport,
>  			die("helper %s does not support dry-run", data->name);
>  	}
>  
> +	if (flags & TRANSPORT_PUSH_FORCE)
> +		set_helper_option(transport, "force", "true");
Should the return value of set_helper_option() be checked?

Also, should there be a #define TRANS_OPT_FORCE "force" with TRANS_OPT_FORCE added to boolean_options[]?

Thanks, Richard

Felipe Contreras· Sep 23, 2013, 02:39 UTC · re: Richard Hansen · lore

Re: [PATCH 1/9] transport-helper: add 'force' to 'export' helpers

Richard Hansen wrote:
Show 21 quoted lines
> On 2013-08-29 11:23, Felipe Contreras wrote:
> > Otherwise they cannot know when to force the push or not (other than
> > hacks).
> > 
> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> > ---
> >  transport-helper.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/transport-helper.c b/transport-helper.c
> > index 63cabc3..62051a6 100644
> > --- a/transport-helper.c
> > +++ b/transport-helper.c
> > @@ -814,6 +814,9 @@ static int push_refs_with_export(struct transport *transport,
> >  			die("helper %s does not support dry-run", data->name);
> >  	}
> >  
> > +	if (flags & TRANSPORT_PUSH_FORCE)
> > +		set_helper_option(transport, "force", "true");
> 
> Should the return value of set_helper_option() be checked?

Yeah, it would make sense. I guess we want to die() if the user does 'git push -f' and the remote helper doesn't support that.

> Also, should there be a #define TRANS_OPT_FORCE "force" with

I don't see the point of that. Defines are useful when you want to change the value string, so you don't have to change the string everywhere, but we definitely don't want to do that, as that would break backwards compatibility, so TRANS_OPT_KEEP would always be "keep" so it's just a way to tire our fingers.

> TRANS_OPT_FORCE added to boolean_options[]?
I don't see how that would help us, the only thing that would achieve is to map:
  set_helper_option(transport, "force", 1);
to
  set_helper_option(transport, "force", "true");
But we are already doing that.
Moreover, the code is already doing something similar for all the other options.
  set_helper_option(t, "progress", t->progress ? "true" : "false");
  set_helper_option(t, "verbosity", buf);
  set_helper_option(transport, "servpath", exec);
  set_helper_option(transport, "dry-run", "true");
-- 
Felipe Contreras
Felipe Contreras· Aug 29, 2013, 15:23 UTC · re: Felipe Contreras · lore

[PATCH 2/9] transport-helper: check for 'forced update' message

So the remote-helpers can tell us when a forced push was needed.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 transport-helper.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
Show changes to transport-helper.c +7 −1
diff --git a/transport-helper.c b/transport-helper.c
index 62051a6..95dd72e 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -630,7 +630,7 @@ static int push_update_ref_status(struct strbuf *buf,
 				   struct ref *remote_refs)
 {
 	char *refname, *msg;
-	int status;
+	int status, forced = 0;
 
 	if (!prefixcmp(buf->buf, "ok ")) {
 		status = REF_STATUS_OK;
@@ -683,6 +683,11 @@ static int push_update_ref_status(struct strbuf *buf,
 			free(msg);
 			msg = NULL;
 		}
+		else if (!strcmp(msg, "forced update")) {
+			forced = 1;
+			free(msg);
+			msg = NULL;
+		}
 	}
 
 	if (*ref)
@@ -704,6 +709,7 @@ static int push_update_ref_status(struct strbuf *buf,
 	}
 
 	(*ref)->status = status;
+	(*ref)->forced_update = forced;
 	(*ref)->remote_status = msg;
 	return !(status == REF_STATUS_OK);
 }
-- 
1.8.4-fc
Felipe Contreras· Aug 29, 2013, 15:23 UTC · re: Felipe Contreras · lore

[PATCH 3/9] 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 8e19058..91114f4 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -694,8 +694,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.4-fc
Felipe Contreras· Aug 29, 2013, 15:23 UTC · re: Felipe Contreras · lore

[PATCH 4/9] 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 85f1f30..221506b 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 91114f4..7f314f0 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;
@@ -661,6 +673,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")),
@@ -681,6 +694,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()
 	};
 
@@ -700,6 +715,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");
 
@@ -734,5 +762,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 34c2d8f..dc6666f 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -504,4 +504,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.4-fc
Richard Hansen· Sep 23, 2013, 01:59 UTC · re: Felipe Contreras · lore

Re: [PATCH 4/9] fast-export: add new --refspec option

On 2013-08-29 11:23, Felipe Contreras wrote:
Show 21 quoted lines
> 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(+)
> 
> diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt
> index 85f1f30..221506b 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.
> +
Do you mean '--refspec=<refspec>' and/or '--refspec <refspec>'?

How are the multiple refspecs specified? Space/comma/colon separated list? Or multiple '--refspec' arguments with one refspec per '--refspec'?

Show 17 quoted lines
> diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
> index 34c2d8f..dc6666f 100755
> --- a/t/t9350-fast-export.sh
> +++ b/t/t9350-fast-export.sh
> @@ -504,4 +504,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
> 
I think it'd be good to add a test for multiple refspecs.
-Richard
Felipe Contreras· Sep 23, 2013, 02:35 UTC · re: Richard Hansen · lore

Re: [PATCH 4/9] fast-export: add new --refspec option

Richard Hansen wrote:
Show 24 quoted lines
> On 2013-08-29 11:23, Felipe Contreras wrote:
> > 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(+)
> > 
> > diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt
> > index 85f1f30..221506b 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.
> > +
> 
> Do you mean '--refspec=<refspec>' and/or '--refspec <refspec>'?
I mean --refspec refspec1 --refspec refspec2.
Show 19 quoted lines
> > diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
> > index 34c2d8f..dc6666f 100755
> > --- a/t/t9350-fast-export.sh
> > +++ b/t/t9350-fast-export.sh
> > @@ -504,4 +504,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
> > 
> 
> I think it'd be good to add a test for multiple refspecs.
Maybe. If these patches were to actually be applied.
-- 
Felipe Contreras
Felipe Contreras· Aug 29, 2013, 15:23 UTC · re: Felipe Contreras · lore

[PATCH 5/9] 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        | 13 ++++++++++---
 2 files changed, 11 insertions(+), 4 deletions(-)
Show changes to 2 files +11 −4

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

diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 8c4c539..8e2dd9f 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -87,7 +87,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 95dd72e..c7135ef 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -809,7 +809,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;
 
 	if (!data->refspecs)
@@ -848,8 +848,13 @@ static int push_refs_with_export(struct transport *transport,
 			die("remote-helpers do not support ref deletion");
 
 		if (ref->peer_ref) {
-			if (strcmp(ref->peer_ref->name, ref->name))
-				die("remote-helpers do not support old:new syntax");
+			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);
 		}
 	}
@@ -857,6 +862,8 @@ static int push_refs_with_export(struct transport *transport,
 	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.4-fc
Felipe Contreras· Aug 29, 2013, 15:23 UTC · re: Felipe Contreras · lore

[PATCH 6/9] fast-import: add support to delete refs

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-fast-import.txt |  3 +++
 fast-import.c                     | 13 ++++++++++---
 t/t9300-fast-import.sh            | 18 ++++++++++++++++++
 3 files changed, 31 insertions(+), 3 deletions(-)
Show changes to 3 files +31 −3

Documentation/git-fast-import.txt, fast-import.c, t/t9300-fast-import.sh

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index bf1a02a..fe5c952 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -483,6 +483,9 @@ Marks must be declared (via `mark`) before they can be used.
 * Any valid Git SHA-1 expression that resolves to a commit.  See
   ``SPECIFYING REVISIONS'' in linkgit:gitrevisions[7] for details.
 
+* The special null SHA-1 (40 zeros) specifices that the branch is to be
+  removed.
+
 The special case of restarting an incremental import from the
 current branch value should be written as:
 ----
diff --git a/fast-import.c b/fast-import.c
index 23f625f..b6be7a7 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -248,6 +248,7 @@ struct branch {
 	uintmax_t last_commit;
 	uintmax_t num_notes;
 	unsigned active : 1;
+	unsigned delete : 1;
 	unsigned pack_id : PACK_ID_BITS;
 	unsigned char sha1[20];
 };
@@ -1674,10 +1675,13 @@ static int update_branch(struct branch *b)
 	struct ref_lock *lock;
 	unsigned char old_sha1[20];
 
-	if (is_null_sha1(b->sha1))
-		return 0;
 	if (read_ref(b->name, old_sha1))
 		hashclr(old_sha1);
+	if (is_null_sha1(b->sha1)) {
+		if (b->delete)
+			delete_ref(b->name, old_sha1, 0);
+		return 0;
+	}
 	lock = lock_any_ref_for_update(b->name, old_sha1, 0);
 	if (!lock)
 		return error("Unable to lock %s", b->name);
@@ -2604,8 +2608,11 @@ static int parse_from(struct branch *b)
 			free(buf);
 		} else
 			parse_from_existing(b);
-	} else if (!get_sha1(from, b->sha1))
+	} else if (!get_sha1(from, b->sha1)) {
 		parse_from_existing(b);
+		if (is_null_sha1(b->sha1))
+			b->delete = 1;
+	}
 	else
 		die("Invalid ref name or SHA1 expression: %s", from);
 
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index ac6f3b6..0150aa6 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -2934,4 +2934,22 @@ test_expect_success 'S: ls with garbage after sha1 must fail' '
 	test_i18ngrep "space after tree-ish" err
 '
 
+test_expect_success 'T: delete branch' '
+	git branch to-delete &&
+	git fast-import <<-EOF &&
+	reset refs/heads/to-delete
+	from 0000000000000000000000000000000000000000
+	EOF
+	test_must_fail git rev-parse --verify refs/heads/to-delete
+'
+
+test_expect_success 'T: empty reset doesnt delete branch' '
+	git branch not-to-delete &&
+	git fast-import <<-EOF &&
+	reset refs/heads/not-to-delete
+	EOF
+	git show-ref &&
+	git rev-parse --verify refs/heads/not-to-delete
+'
+
 test_done
-- 
1.8.4-fc
Felipe Contreras· Aug 29, 2013, 15:23 UTC · re: Felipe Contreras · lore

[PATCH 7/9] fast-export: add support to delete refs

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/fast-export.c  | 14 ++++++++++++++
 t/t9350-fast-export.sh | 11 +++++++++++
 2 files changed, 25 insertions(+)
Show changes to 2 files +25 −0

builtin/fast-export.c, t/t9350-fast-export.sh

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 7f314f0..9b728ca 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -665,6 +665,19 @@ static void import_marks(char *input_file)
 	fclose(f);
 }
 
+static void handle_deletes(void)
+{
+	int i;
+	for (i = 0; i < refspecs_nr; i++) {
+		struct refspec *refspec = &refspecs[i];
+		if (*refspec->src)
+			continue;
+
+		printf("reset %s\nfrom %s\n\n",
+			refspec->dst, sha1_to_hex(null_sha1));
+	}
+}
+
 int cmd_fast_export(int argc, const char **argv, const char *prefix)
 {
 	struct rev_info revs;
@@ -755,6 +768,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 	}
 
 	handle_tags_and_duplicates(&extra_refs);
+	handle_deletes();
 
 	if (export_filename && lastimportid != last_idnum)
 		export_marks(export_filename);
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index dc6666f..ea6c96c 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -511,4 +511,15 @@ test_expect_success 'use refspec' '
 	test_cmp expected actual
 '
 
+test_expect_success 'delete refspec' '
+	git branch to-delete &&
+	git fast-export --refspec :refs/heads/to-delete to-delete ^to-delete > actual &&
+	cat > expected <<-EOF &&
+	reset refs/heads/to-delete
+	from 0000000000000000000000000000000000000000
+
+	EOF
+	test_cmp expected actual
+'
+
 test_done
-- 
1.8.4-fc
Felipe Contreras· Aug 29, 2013, 15:23 UTC · re: Felipe Contreras · lore

[PATCH 8/9] transport-helper: add support to delete branches

For remote-helpers that use 'export' to push.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t5801-remote-helpers.sh |  8 ++++++++
 transport-helper.c        | 11 ++++++-----
 2 files changed, 14 insertions(+), 5 deletions(-)
Show changes to 2 files +14 −5

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

diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 8e2dd9f..a66a4e3 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -94,6 +94,14 @@ test_expect_success 'push new branch with old:new refspec' '
 	compare_refs local HEAD server refs/heads/new-refspec
 '
 
+test_expect_success 'push delete branch' '
+	(cd local &&
+	 git push origin :new-name
+	) &&
+	test_must_fail git --git-dir="server/.git" \
+	 rev-parse --verify refs/heads/new-name
+'
+
 test_expect_success 'cloning without refspec' '
 	GIT_REMOTE_TESTGIT_REFSPEC="" \
 	git clone "testgit::${PWD}/server" local2 2>error &&
diff --git a/transport-helper.c b/transport-helper.c
index c7135ef..5490796 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -844,18 +844,19 @@ static int push_refs_with_export(struct transport *transport,
 		}
 		free(private);
 
-		if (ref->deletion)
-			die("remote-helpers do not support ref deletion");
-
 		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);
+				if (!ref->deletion)
+					strbuf_addf(&buf, "%s:%s", ref->peer_ref->name, ref->name);
+				else
+					strbuf_addf(&buf, ":%s", 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 (!ref->deletion)
+				string_list_append(&revlist_args, ref->peer_ref->name);
 		}
 	}
 
-- 
1.8.4-fc
Richard Hansen· Sep 23, 2013, 01:50 UTC · re: Felipe Contreras · lore

Re: [PATCH 8/9] transport-helper: add support to delete branches

On 2013-08-29 11:23, Felipe Contreras wrote:
Show 8 quoted lines
> For remote-helpers that use 'export' to push.
> 
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  t/t5801-remote-helpers.sh |  8 ++++++++
>  transport-helper.c        | 11 ++++++-----
>  2 files changed, 14 insertions(+), 5 deletions(-)
> 
[...]
Show 11 quoted lines
> diff --git a/transport-helper.c b/transport-helper.c
> index c7135ef..5490796 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -844,18 +844,19 @@ static int push_refs_with_export(struct transport *transport,
>  		}
>  		free(private);
>  
> -		if (ref->deletion)
> -			die("remote-helpers do not support ref deletion");
> -

The above deleted lines actually appear twice in transport-helper.c due to an incorrect merge conflict resolution in 99d9ec090677c925c534001f01cbaf303a31cb82. The other copy of those lines should also be deleted:

Show changes to transport-helper.c +0 −3
diff --git a/transport-helper.c b/transport-helper.c
index 859131f..bbf4e7c 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -874,9 +874,6 @@ static int push_refs_with_export(struct transport *transport,
 		char *private;
 		unsigned char sha1[20];

-		if (ref->deletion)
-			die("remote-helpers do not support ref deletion");
-
 		private = apply_refspecs(data->refspecs, data->refspec_nr, ref->name);
 		if (private && !get_sha1(private, sha1)) {
 			strbuf_addf(&buf, "^%s", private);
Felipe Contreras· Sep 23, 2013, 02:34 UTC · re: Richard Hansen · lore

Re: [PATCH 8/9] transport-helper: add support to delete branches

Richard Hansen wrote:
Show 41 quoted lines
> On 2013-08-29 11:23, Felipe Contreras wrote:
> > For remote-helpers that use 'export' to push.
> > 
> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> > ---
> >  t/t5801-remote-helpers.sh |  8 ++++++++
> >  transport-helper.c        | 11 ++++++-----
> >  2 files changed, 14 insertions(+), 5 deletions(-)
> > 
> [...]
> > diff --git a/transport-helper.c b/transport-helper.c
> > index c7135ef..5490796 100644
> > --- a/transport-helper.c
> > +++ b/transport-helper.c
> > @@ -844,18 +844,19 @@ static int push_refs_with_export(struct transport *transport,
> >  		}
> >  		free(private);
> >  
> > -		if (ref->deletion)
> > -			die("remote-helpers do not support ref deletion");
> > -
> 
> The above deleted lines actually appear twice in transport-helper.c due
> to an incorrect merge conflict resolution in
> 99d9ec090677c925c534001f01cbaf303a31cb82.  The other copy of those lines should
> also be deleted:
> 
> diff --git a/transport-helper.c b/transport-helper.c
> index 859131f..bbf4e7c 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -874,9 +874,6 @@ static int push_refs_with_export(struct transport *transport,
>  		char *private;
>  		unsigned char sha1[20];
> 
> -		if (ref->deletion)
> -			die("remote-helpers do not support ref deletion");
> -
>  		private = apply_refspecs(data->refspecs, data->refspec_nr, ref->name);
>  		if (private && !get_sha1(private, sha1)) {
>  			strbuf_addf(&buf, "^%s", private);
Right.
-- 
Felipe Contreras
Felipe Contreras· Aug 29, 2013, 15:23 UTC · re: Felipe Contreras · lore

[PATCH 9/9] transport-helper: don't update refs in dry-run

The remote helper namespace should not be updated.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 transport-helper.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
Show changes to transport-helper.c +5 −4
diff --git a/transport-helper.c b/transport-helper.c
index 5490796..9bbf209 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -715,7 +715,8 @@ static int push_update_ref_status(struct strbuf *buf,
 }
 
 static void push_update_refs_status(struct helper_data *data,
-				    struct ref *remote_refs)
+				    struct ref *remote_refs,
+				    int flags)
 {
 	struct strbuf buf = STRBUF_INIT;
 	struct ref *ref = remote_refs;
@@ -729,7 +730,7 @@ static void push_update_refs_status(struct helper_data *data,
 		if (push_update_ref_status(&buf, &ref, remote_refs))
 			continue;
 
-		if (!data->refspecs)
+		if (flags & TRANSPORT_PUSH_DRY_RUN || !data->refspecs)
 			continue;
 
 		/* propagate back the update to the remote namespace */
@@ -799,7 +800,7 @@ static int push_refs_with_push(struct transport *transport,
 	sendline(data, &buf);
 	strbuf_release(&buf);
 
-	push_update_refs_status(data, remote_refs);
+	push_update_refs_status(data, remote_refs, flags);
 	return 0;
 }
 
@@ -867,7 +868,7 @@ static int push_refs_with_export(struct transport *transport,
 
 	if (finish_command(&exporter))
 		die("Error while running fast-export");
-	push_update_refs_status(data, remote_refs);
+	push_update_refs_status(data, remote_refs, flags);
 	return 0;
 }
 
-- 
1.8.4-fc
Junio C Hamano· Aug 29, 2013, 19:22 UTC · re: Felipe Contreras · lore

Re: [PATCH 0/9] transport-helper: updates

Felipe Contreras <felipe.contreras@gmail.com> writes:
> Some of these were were sent before and rejected without a
> reason,...

Earlier, you were asked to leave because many constructive criticism or suggestion for improvement to your patches in reviews led to stubborn resistance, ended up wasting everybody's time and left a bad atmosphere on the list.

It seems that Matthew is trying to see if you can work better with others than before after a break, but I personally am not hopeful yet and do not want to waste my/our time on flamewars like we saw in the past.

So I'll look at these and may pick patches that are obviously and trivially good, but will refrain from asking clarifications and suggesting improvements, etc. on other patches that I suspect will end up wasting the time of people again. Do not take silence from me as a silent agreement.

Felipe Contreras· Aug 29, 2013, 19:26 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/9] transport-helper: updates

On Thu, Aug 29, 2013 at 2:22 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 20 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> Some of these were were sent before and rejected without a
>> reason,...
>
> Earlier, you were asked to leave because many constructive criticism
> or suggestion for improvement to your patches in reviews led to
> stubborn resistance, ended up wasting everybody's time and left a
> bad atmosphere on the list.
>
> It seems that Matthew is trying to see if you can work better with
> others than before after a break, but I personally am not hopeful
> yet and do not want to waste my/our time on flamewars like we saw in
> the past.
>
> So I'll look at these and may pick patches that are obviously and
> trivially good, but will refrain from asking clarifications and
> suggesting improvements, etc. on other patches that I suspect will
> end up wasting the time of people again.  Do not take silence from
> me as a silent agreement.

If you do not pick these patches, the only one that looses is the Git project and its users.

-- 
Felipe Contreras
Richard Hansen· Sep 23, 2013, 01:49 UTC · re: Felipe Contreras · lore

Re: [PATCH 0/9] transport-helper: updates

On 2013-08-29 11:23, Felipe Contreras wrote:
> Here are the patches that allow transport helpers to be completely transparent;
> renaming branches, deleting them, custom refspecs, --force, --dry-run,
> reporting forced update, everything works.
What is the status of these patches?

I would like to be able to 'git push old:new' and 'git push -f' to a bzr repo, and these are a prerequisite. I imagine changes to git-remote-bzr will also be required; I'm willing write up such changes to git-remote-bzr, but only if there's interest in picking up this patch series.

Thanks, Richard

Felipe Contreras· Sep 23, 2013, 02:32 UTC · re: Richard Hansen · lore

Re: [PATCH 0/9] transport-helper: updates

Richard Hansen wrote:
Show 6 quoted lines
> On 2013-08-29 11:23, Felipe Contreras wrote:
> > Here are the patches that allow transport helpers to be completely transparent;
> > renaming branches, deleting them, custom refspecs, --force, --dry-run,
> > reporting forced update, everything works.
> 
> What is the status of these patches?

Ask Junio. All the valid feedback has been addressed, there's no reason not to apply them.

Show 5 quoted lines
> I would like to be able to 'git push old:new' and 'git push -f' to a
> bzr repo, and these are a prerequisite.  I imagine changes to
> git-remote-bzr will also be required; I'm willing write up such
> changes to git-remote-bzr, but only if there's interest in picking up
> this patch series.

There's no need to modify git-remote-bzr, if theese patches are applied, old:new would work for all remote-helpers.

-- 
Felipe Contreras

← back to recent threads