[PATCH v6 10/10] transport-helper: add support to delete branches

[PATCH v6 10/10] transport-helper: add support to delete branches

[Felipe Contreras]

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/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 454337e..c667965 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 'forced push' '
 	(cd local &&
 	git checkout -b force-test &&
diff --git a/transport-helper.c b/transport-helper.c
index 7411125..2257588 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -876,9 +876,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);
@@ -890,12 +887,16 @@ static int push_refs_with_export(struct transport *transport,
 		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.2+fc1

[PATCH v6 00/10] transport-helper: updates

[Felipe Contreras]

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.

Small changes since v5:

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 8ed41b4..4b76222 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -736,9 +736,10 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 		usage_with_options (fast_export_usage, options);
 
 	if (refspecs_list.nr) {
-		const char *refspecs_str[refspecs_list.nr];
+		const char **refspecs_str;
 		int i;
 
+		refspecs_str = xmalloc(sizeof(*refspecs_str) * refspecs_list.nr);
 		for (i = 0; i < refspecs_list.nr; i++)
 			refspecs_str[i] = refspecs_list.items[i].string;
 
@@ -746,6 +747,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 		refspecs = parse_fetch_refspec(refspecs_nr, refspecs_str);
 
 		string_list_clear(&refspecs_list, 1);
+		free(refspecs_str);
 	}
 
 	if (use_done_feature)
diff --git a/git-remote-testgit.sh b/git-remote-testgit.sh
index 716aa4c..1c006a0 100755
--- a/git-remote-testgit.sh
+++ b/git-remote-testgit.sh
@@ -15,6 +15,8 @@ test -z "$refspec" && prefix="refs"
 
 export GIT_DIR="$url/.git"
 
+force=
+
 mkdir -p "$dir"
 
 if test -z "$GIT_REMOTE_TESTGIT_NO_MARKS"

Felipe Contreras (10):
  transport-helper: fix extra lines
  transport-helper: don't update refs in dry-run
  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

 Documentation/git-fast-export.txt   |  4 +++
 Documentation/git-fast-import.txt   |  3 +++
 Documentation/gitremote-helpers.txt |  4 +++
 builtin/fast-export.c               | 49 ++++++++++++++++++++++++++++++++++++-
 fast-import.c                       | 13 +++++++---
 git-remote-testgit.sh               | 18 ++++++++++++++
 t/t5801-remote-helpers.sh           | 23 ++++++++++++++++-
 t/t9300-fast-import.sh              | 18 ++++++++++++++
 t/t9350-fast-export.sh              | 18 ++++++++++++++
 transport-helper.c                  | 47 +++++++++++++++++++++++------------
 10 files changed, 177 insertions(+), 20 deletions(-)

-- 
1.8.4.2+fc1

[PATCH v6 01/10] transport-helper: fix extra lines

[Felipe Contreras]

Commit 9c51558 (transport-helper: trivial code shuffle) moved these
lines above, but 99d9ec0 (Merge branch 'fc/transport-helper-no-refspec')
had a wrong merge conflict and readded them.

Reported-by: Richard Hansen <rhansen@bbn.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 transport-helper.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/transport-helper.c b/transport-helper.c
index 673b7c2..b66c7fd 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -875,9 +875,6 @@ 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->peer_ref->name, ref->name))
 				die("remote-helpers do not support old:new syntax");
-- 
1.8.4.2+fc1

[PATCH v6 08/10] fast-import: add support to delete refs

[Felipe Contreras]

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(-)

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 73f9806..2ffae42 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) specifies 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 f4d9969..fdce0b7 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];
 };
@@ -1690,10 +1691,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, NULL);
 	if (!lock)
 		return error("Unable to lock %s", b->name);
@@ -2620,8 +2624,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 27263df..5fc9ef2 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -2999,4 +2999,22 @@ test_expect_success 'T: ls root tree' '
 	test_cmp expect actual
 '
 
+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.2+fc1

[PATCH v6 05/10] fast-export: improve argument parsing

[Felipe Contreras]

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(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 78250ea..eea5b8c 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -701,8 +701,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.2+fc1

[PATCH v6 06/10] fast-export: add new --refspec option

[Felipe Contreras]

So that we can convert the exported ref names.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-fast-export.txt |  4 ++++
 builtin/fast-export.c             | 32 ++++++++++++++++++++++++++++++++
 t/t9350-fast-export.sh            |  7 +++++++
 3 files changed, 43 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.
+
 [<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 eea5b8c..cf745ec 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]"),
@@ -31,6 +32,8 @@ static int use_done_feature;
 static int no_data;
 static int full_tree;
 static struct string_list extra_refs = STRING_LIST_INIT_NODUP;
+static struct refspec *refspecs;
+static int refspecs_nr;
 
 static int parse_opt_signed_tag_mode(const struct option *opt,
 				     const char *arg, int unset)
@@ -525,6 +528,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;
+			}
+		}
+
 		commit = get_commit(e, full_name);
 		if (!commit) {
 			warning("%s: Unexpected object of type %s, skipping.",
@@ -668,6 +680,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 = STRING_LIST_INIT_NODUP;
 	struct option options[] = {
 		OPT_INTEGER(0, "progress", &progress,
 			    N_("show progress after <n> objects")),
@@ -688,6 +701,8 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 		OPT_BOOL(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()
 	};
 
@@ -707,6 +722,21 @@ 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;
+		int i;
+
+		refspecs_str = xmalloc(sizeof(*refspecs_str) * refspecs_list.nr);
+		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);
+		free(refspecs_str);
+	}
+
 	if (use_done_feature)
 		printf("feature done\n");
 
@@ -741,5 +771,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 2312dec..3d475af 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.2+fc1

[PATCH v6 03/10] transport-helper: add 'force' to 'export' helpers

[Felipe Contreras]

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

Tests-by: Richard Hansen <rhansen@bbn.com>
Documentation-by: Richard Hansen <rhansen@bbn.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/gitremote-helpers.txt |  4 ++++
 git-remote-testgit.sh               | 18 ++++++++++++++++++
 t/t5801-remote-helpers.sh           | 13 +++++++++++++
 transport-helper.c                  |  5 +++++
 4 files changed, 40 insertions(+)

diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt
index f1f4ca9..e75699c 100644
--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt
@@ -437,6 +437,10 @@ set by Git if the remote helper has the 'option' capability.
 'option check-connectivity' \{'true'|'false'\}::
 	Request the helper to check connectivity of a clone.
 
+'option force' \{'true'|'false'\}::
+	Request the helper to perform a force update.  Defaults to
+	'false'.
+
 SEE ALSO
 --------
 linkgit:git-remote[1]
diff --git a/git-remote-testgit.sh b/git-remote-testgit.sh
index 6d2f282..1c006a0 100755
--- a/git-remote-testgit.sh
+++ b/git-remote-testgit.sh
@@ -15,6 +15,8 @@ test -z "$refspec" && prefix="refs"
 
 export GIT_DIR="$url/.git"
 
+force=
+
 mkdir -p "$dir"
 
 if test -z "$GIT_REMOTE_TESTGIT_NO_MARKS"
@@ -39,6 +41,7 @@ do
 		fi
 		test -n "$GIT_REMOTE_TESTGIT_SIGNED_TAGS" && echo "signed-tags"
 		test -n "$GIT_REMOTE_TESTGIT_NO_PRIVATE_UPDATE" && echo "no-private-update"
+		echo 'option'
 		echo
 		;;
 	list)
@@ -93,6 +96,7 @@ do
 		before=$(git for-each-ref --format=' %(refname) %(objectname) ')
 
 		git fast-import \
+			${force:+--force} \
 			${testgitmarks:+"--import-marks=$testgitmarks"} \
 			${testgitmarks:+"--export-marks=$testgitmarks"} \
 			--quiet
@@ -115,6 +119,20 @@ do
 
 		echo
 		;;
+	option\ *)
+		read cmd opt val <<-EOF
+		$line
+		EOF
+		case $opt in
+		force)
+			test $val = "true" && force="true" || force=
+			echo "ok"
+			;;
+		*)
+			echo "unsupported"
+			;;
+		esac
+		;;
 	'')
 		exit
 		;;
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 613f69a..c33cc25 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -94,6 +94,19 @@ test_expect_failure 'push new branch with old:new refspec' '
 	compare_refs local HEAD server refs/heads/new-refspec
 '
 
+test_expect_success 'forced push' '
+	(cd local &&
+	git checkout -b force-test &&
+	echo content >> file &&
+	git commit -a -m eight &&
+	git push origin force-test &&
+	echo content >> file &&
+	git commit -a --amend -m eight-modified &&
+	git push --force origin force-test
+	) &&
+	compare_refs local refs/heads/force-test server refs/heads/force-test
+'
+
 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 9558a0d..bead9b9 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -855,6 +855,11 @@ static int push_refs_with_export(struct transport *transport,
 			die("helper %s does not support dry-run", data->name);
 	}
 
+	if (flags & TRANSPORT_PUSH_FORCE) {
+		if (set_helper_option(transport, "force", "true") != 0)
+			die("helper %s does not support 'force'", data->name);
+	}
+
 	helper = get_helper(transport);
 
 	write_constant(helper->in, "export\n");
-- 
1.8.4.2+fc1

[PATCH v6 09/10] fast-export: add support to delete refs

[Felipe Contreras]

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(+)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index cf745ec..4b76222 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -673,6 +673,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;
@@ -764,6 +777,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 	}
 
 	handle_tags_and_duplicates();
+	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 3d475af..66c8b0a 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.2+fc1

[PATCH v6 07/10] transport-helper: add support for old:new refspec

[Felipe Contreras]

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(-)

diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index c33cc25..454337e 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 9a5814d..7411125 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -850,7 +850,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)
@@ -888,8 +888,13 @@ static int push_refs_with_export(struct transport *transport,
 		free(private);
 
 		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);
 		}
 	}
@@ -897,6 +902,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, flags);
-- 
1.8.4.2+fc1

[PATCH v6 04/10] transport-helper: check for 'forced update' message

[Felipe Contreras]

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(-)

diff --git a/transport-helper.c b/transport-helper.c
index bead9b9..9a5814d 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -644,7 +644,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;
@@ -702,6 +702,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)
@@ -723,6 +728,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.2+fc1

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

[Felipe Contreras]

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(-)

diff --git a/transport-helper.c b/transport-helper.c
index b66c7fd..9558a0d 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -728,7 +728,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;
@@ -742,7 +743,7 @@ static void push_update_refs_status(struct helper_data *data,
 		if (push_update_ref_status(&buf, &ref, remote_refs))
 			continue;
 
-		if (!data->refspecs || data->no_private_update)
+		if (flags & TRANSPORT_PUSH_DRY_RUN || !data->refspecs || data->no_private_update)
 			continue;
 
 		/* propagate back the update to the remote namespace */
@@ -833,7 +834,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;
 }
 
@@ -887,7 +888,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.2+fc1

Re: [PATCH v6 06/10] fast-export: add new --refspec option

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> So that we can convert the exported ref names.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---

I thought that the discussion agreed this option should not be
called --refspec but something like --refmap?

Re: [PATCH v6 03/10] transport-helper: add 'force' to 'export' helpers

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Otherwise they cannot know when to force the push or not (other than
> hacks).
>
> Tests-by: Richard Hansen <rhansen@bbn.com>
> Documentation-by: Richard Hansen <rhansen@bbn.com>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---

Didn't we agree that this should be warn, not die?

> +	if (flags & TRANSPORT_PUSH_FORCE) {
> +		if (set_helper_option(transport, "force", "true") != 0)
> +			die("helper %s does not support 'force'", data->name);
> +	}
> +
>  	helper = get_helper(transport);
>  
>  	write_constant(helper->in, "export\n");

Re: [PATCH v6 00/10] transport-helper: updates

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> 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.

How are you sending your patches?  The Message-ID's suggest that
git-send-email is being used, but when the patches are ordered by
the sender datestamp, they seem to come out in a random order,
unlike the patches other people send with git-send-email.

git-send-email gives a timestamp to the first message it sends out
and then gives consecutive timestamps, one second apart, to
subsequent messages, in order to make this "sorting by sender
timestamp on Date: field" work. I am wondering if there is something
you are doing differently, and/or if there is something that needs
to be fixed in the version of git-send-email you are using.

Thanks.

Re: [PATCH v6 00/10] transport-helper: updates

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Small changes since v5:
>
> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index 8ed41b4..4b76222 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -736,9 +736,10 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
>  		usage_with_options (fast_export_usage, options);
>  
>  	if (refspecs_list.nr) {
> -		const char *refspecs_str[refspecs_list.nr];
> +		const char **refspecs_str;
>  		int i;
>  
> +		refspecs_str = xmalloc(sizeof(*refspecs_str) * refspecs_list.nr);
>  		for (i = 0; i < refspecs_list.nr; i++)
>  			refspecs_str[i] = refspecs_list.items[i].string;
>  
> @@ -746,6 +747,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
>  		refspecs = parse_fetch_refspec(refspecs_nr, refspecs_str);
>  
>  		string_list_clear(&refspecs_list, 1);
> +		free(refspecs_str);
>  	}
>  
>  	if (use_done_feature)
> diff --git a/git-remote-testgit.sh b/git-remote-testgit.sh
> index 716aa4c..1c006a0 100755
> --- a/git-remote-testgit.sh
> +++ b/git-remote-testgit.sh
> @@ -15,6 +15,8 @@ test -z "$refspec" && prefix="refs"
>  
>  export GIT_DIR="$url/.git"
>  
> +force=
> +
>  mkdir -p "$dir"
>  
>  if test -z "$GIT_REMOTE_TESTGIT_NO_MARKS"

Looking good, modulo a few minor nits I noticed while comparing with
the one that has been queued in 'pu', which I commented separately.

Will re-queue.  Thanks.

Re: [PATCH v6 00/10] transport-helper: updates

[Felipe Contreras]

On Mon, Nov 11, 2013 at 5:33 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> 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.
>
> How are you sending your patches?

% git-send-email --no-annotate list-of-patches

However, I just noticed that the list-of-patches is in the wrong order.

-- 
Felipe Contreras

Re: [PATCH v6 03/10] transport-helper: add 'force' to 'export' helpers

[Felipe Contreras]

On Mon, Nov 11, 2013 at 5:28 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> Otherwise they cannot know when to force the push or not (other than
>> hacks).
>>
>> Tests-by: Richard Hansen <rhansen@bbn.com>
>> Documentation-by: Richard Hansen <rhansen@bbn.com>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>
> Didn't we agree that this should be warn, not die?

Yes, and I assumed you would do it without a reroll, because no reroll
was needed. I've updated my side as well now, for the next reroll.

-- 
Felipe Contreras

Re: [PATCH v6 06/10] fast-export: add new --refspec option

[Felipe Contreras]

On Mon, Nov 11, 2013 at 5:25 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> So that we can convert the exported ref names.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>
> I thought that the discussion agreed this option should not be
> called --refspec but something like --refmap?

I don't know what you agreed to, but I didn't agree to anything. What
you pass to this option is a refspec, so it makes sense to name the
option --refspec.

-- 
Felipe Contreras

Re: [PATCH v6 00/10] transport-helper: updates

[Richard Hansen]

On 2013-11-11 17:54, Felipe Contreras wrote:
> 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.
> 
> Small changes since v5:
> 
> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index 8ed41b4..4b76222 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -736,9 +736,10 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
>  		usage_with_options (fast_export_usage, options);
>  
>  	if (refspecs_list.nr) {
> -		const char *refspecs_str[refspecs_list.nr];
> +		const char **refspecs_str;
>  		int i;
>  
> +		refspecs_str = xmalloc(sizeof(*refspecs_str) * refspecs_list.nr);
>  		for (i = 0; i < refspecs_list.nr; i++)
>  			refspecs_str[i] = refspecs_list.items[i].string;
>  
> @@ -746,6 +747,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
>  		refspecs = parse_fetch_refspec(refspecs_nr, refspecs_str);
>  
>  		string_list_clear(&refspecs_list, 1);
> +		free(refspecs_str);
>  	}
>  
>  	if (use_done_feature)
> diff --git a/git-remote-testgit.sh b/git-remote-testgit.sh
> index 716aa4c..1c006a0 100755
> --- a/git-remote-testgit.sh
> +++ b/git-remote-testgit.sh
> @@ -15,6 +15,8 @@ test -z "$refspec" && prefix="refs"
>  
>  export GIT_DIR="$url/.git"
>  
> +force=
> +
>  mkdir -p "$dir"
>  
>  if test -z "$GIT_REMOTE_TESTGIT_NO_MARKS"

What about changing those two test-hg.sh tests to test_expect_success?

  http://article.gmane.org/gmane.comp.version-control.git/237606

Should those changes be squashed into the "transport-helper: don't
update refs in dry-run" and "transport-helper: add 'force' to 'export'
helpers" commits?  Or are those commits not really the appropriate place?

Thanks,
Richard

[PATCH v2] remote-bzr: support the new 'force' option

[Richard Hansen]

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---

This is a reroll of:
  http://article.gmane.org/gmane.comp.version-control.git/237607
based on feedback from Felipe:
  http://article.gmane.org/gmane.comp.version-control.git/237615

This patch is an optional extension to Felipe's "transport-helper:
updates" patch series:
  http://thread.gmane.org/gmane.comp.version-control.git/237663
and it requires those changes to work.

 contrib/remote-helpers/git-remote-bzr | 32 +++++++++++++++++++++++++++++++-
 contrib/remote-helpers/test-bzr.sh    | 22 +++++++++++++++++++++-
 2 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
index 7e34532..2f481e9 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -42,6 +42,7 @@ import json
 import re
 import StringIO
 import atexit, shutil, hashlib, urlparse, subprocess
+import types
 
 NAME_RE = re.compile('^([^<>]+)')
 AUTHOR_RE = re.compile('^([^<>]+?)? ?[<>]([^<>]*)(?:$|>)')
@@ -684,7 +685,8 @@ def do_export(parser):
                 peer = bzrlib.branch.Branch.open(peers[name],
                                                  possible_transports=transports)
                 try:
-                    peer.bzrdir.push_branch(branch, revision_id=revid)
+                    peer.bzrdir.push_branch(branch, revision_id=revid,
+                                            overwrite=force)
                 except bzrlib.errors.DivergedBranches:
                     print "error %s non-fast forward" % ref
                     continue
@@ -718,8 +720,32 @@ def do_capabilities(parser):
         print "*import-marks %s" % path
     print "*export-marks %s" % path
 
+    print "option"
     print
 
+class InvalidOptionValue(Exception):
+    pass
+
+def get_bool_option(val):
+    if val == 'true':
+        return True
+    elif val == 'false':
+        return False
+    else:
+        raise InvalidOptionValue()
+
+def do_option(parser):
+    global force
+    (opt, val) = parser[1:3]
+    try:
+        if opt == 'force':
+            force = get_bool_option(val)
+            print 'ok'
+        else:
+            print 'unsupported'
+    except InvalidOptionValue:
+        print "error '%s' is not a valid value for option '%s'" % (val, opt)
+
 def ref_is_valid(name):
     return not True in [c in name for c in '~^: \\']
 
@@ -882,6 +908,7 @@ def main(args):
     global is_tmp
     global branches, peers
     global transports
+    global force
 
     alias = args[1]
     url = args[2]
@@ -895,6 +922,7 @@ def main(args):
     branches = {}
     peers = {}
     transports = []
+    force = False
 
     if alias[5:] == url:
         is_tmp = True
@@ -930,6 +958,8 @@ def main(args):
             do_import(parser)
         elif parser.check('export'):
             do_export(parser)
+        elif parser.check('option'):
+            do_option(parser)
         else:
             die('unhandled command: %s' % line)
         sys.stdout.flush()
diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh
index 1e53ff9..4f379c2 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -66,13 +66,33 @@ test_expect_success 'pushing' '
 	test_cmp expected actual
 '
 
+test_expect_success 'forced pushing' '
+	(
+	cd gitrepo &&
+	echo three-new >content &&
+	git commit -a --amend -m three-new &&
+	git push -f
+	) &&
+
+	(
+	cd bzrrepo &&
+	# the forced update overwrites the bzr branch but not the bzr
+	# working directory (it tries to merge instead)
+	bzr revert
+	) &&
+
+	echo three-new >expected &&
+	cat bzrrepo/content >actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'roundtrip' '
 	(
 	cd gitrepo &&
 	git pull &&
 	git log --format="%s" -1 origin/master >actual
 	) &&
-	echo three >expected &&
+	echo three-new >expected &&
 	test_cmp expected actual &&
 
 	(cd gitrepo && git push && git pull) &&
-- 
1.8.5.rc1.208.g8ff7964

Re: [PATCH v6 06/10] fast-export: add new --refspec option

[Richard Hansen]

On 2013-11-11 18:50, Felipe Contreras wrote:
> On Mon, Nov 11, 2013 at 5:25 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>> So that we can convert the exported ref names.
>>>
>>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>>> ---
>>
>> I thought that the discussion agreed this option should not be
>> called --refspec but something like --refmap?
> 
> I don't know what you agreed to,

http://article.gmane.org/gmane.comp.version-control.git/237473

> but I didn't agree to anything.

Based on your silence I too thought that you had agreed.

> What you pass to this option is a refspec, so it makes sense to name
> the option --refspec.

As discussed in that thread, it's not really the same thing as a refspec
used in push or fetch.  In those commands, the refspec specifies two
separable things:  what to transfer, and how to translate refs names
between the remote and local repositories.  IIUC, the fast-export
--refspec argument only specifies how to translate ref names, not what
gets transferred.

If my understanding is correct, then I agree with Junio and Peff that
--refmap is a better name.

-Richard

Re: [PATCH v6 06/10] fast-export: add new --refspec option

[Junio C Hamano]

Richard Hansen <rhansen@bbn.com> writes:

>>> I thought that the discussion agreed this option should not be
>>> called --refspec but something like --refmap?
>> 
>> I don't know what you agreed to,
>
> http://article.gmane.org/gmane.comp.version-control.git/237473

Yup, that was what I had in mind.

>> but I didn't agree to anything.
>
> Based on your silence I too thought that you had agreed.

Careful.  Silence does not mean agreement, at least around here.  It
may be just the person was busy and hasn't got around to it, was not
paying attention and missed the discussion, or was not as interested
in the topic as his/her other activities.

That, especially the last possibility among the three example
reasons above, was why I said "the discussion agreed", not "you
agreed".

>> What you pass to this option is a refspec, so it makes sense to name
>> the option --refspec.
>
> As discussed in that thread, it's not really the same thing as a refspec
> used in push or fetch.  In those commands, the refspec specifies two
> separable things:  what to transfer, and how to translate refs names
> between the remote and local repositories.  IIUC, the fast-export
> --refspec argument only specifies how to translate ref names, not what
> gets transferred.
>
> If my understanding is correct, then I agree with Junio and Peff that
> --refmap is a better name.

I know from one of the tests that the option Felipe added is
implemented in such a way that allows:

    git fast-export --option refs/heads/master:refs/heads/next master

to rename the destination, but I didn't check, when I wrote the
message to envision how a similar mechanism could be used to enhance
push/fetch in the future, if it can be actually used as a mapping

    git fast-export --option refs/heads/*:refs/remotes/mine/* master

Being able to do so was the only reason why I agree with the patch
in question (not my toy patch, but [6/10] that started this thread)
that it is a good idea in the longer term, as the other approach I
suggested to teach revision command line parser to optionally take
refspecs as if they specify LHS of the colon as object name with
rev_cmdline annotations would not work well for such a purpose.

Thanks.

Re: [PATCH v6 00/10] transport-helper: updates

[Felipe Contreras]

Richard Hansen wrote:
> On 2013-11-11 17:54, Felipe Contreras wrote:
> > 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.
> > 
> > Small changes since v5:
> > 
> > diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> > index 8ed41b4..4b76222 100644
> > --- a/builtin/fast-export.c
> > +++ b/builtin/fast-export.c
> > @@ -736,9 +736,10 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
> >  		usage_with_options (fast_export_usage, options);
> >  
> >  	if (refspecs_list.nr) {
> > -		const char *refspecs_str[refspecs_list.nr];
> > +		const char **refspecs_str;
> >  		int i;
> >  
> > +		refspecs_str = xmalloc(sizeof(*refspecs_str) * refspecs_list.nr);
> >  		for (i = 0; i < refspecs_list.nr; i++)
> >  			refspecs_str[i] = refspecs_list.items[i].string;
> >  
> > @@ -746,6 +747,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
> >  		refspecs = parse_fetch_refspec(refspecs_nr, refspecs_str);
> >  
> >  		string_list_clear(&refspecs_list, 1);
> > +		free(refspecs_str);
> >  	}
> >  
> >  	if (use_done_feature)
> > diff --git a/git-remote-testgit.sh b/git-remote-testgit.sh
> > index 716aa4c..1c006a0 100755
> > --- a/git-remote-testgit.sh
> > +++ b/git-remote-testgit.sh
> > @@ -15,6 +15,8 @@ test -z "$refspec" && prefix="refs"
> >  
> >  export GIT_DIR="$url/.git"
> >  
> > +force=
> > +
> >  mkdir -p "$dir"
> >  
> >  if test -z "$GIT_REMOTE_TESTGIT_NO_MARKS"
> 
> What about changing those two test-hg.sh tests to test_expect_success?
> 
>   http://article.gmane.org/gmane.comp.version-control.git/237606

I can do that.

> Should those changes be squashed into the "transport-helper: don't
> update refs in dry-run" and "transport-helper: add 'force' to 'export'
> helpers" commits?  Or are those commits not really the appropriate place?

I think it should be a separate commit, since it's not part of the core, but
contrib area.

-- 
Felipe Contreras

RE: [PATCH v2] remote-bzr: support the new 'force' option

[Felipe Contreras]

Richard Hansen wrote:
> Signed-off-by: Richard Hansen <rhansen@bbn.com>
> ---
> 
> This is a reroll of:
>   http://article.gmane.org/gmane.comp.version-control.git/237607
> based on feedback from Felipe:
>   http://article.gmane.org/gmane.comp.version-control.git/237615
> 
> This patch is an optional extension to Felipe's "transport-helper:
> updates" patch series:
>   http://thread.gmane.org/gmane.comp.version-control.git/237663
> and it requires those changes to work.
> 
>  contrib/remote-helpers/git-remote-bzr | 32 +++++++++++++++++++++++++++++++-
>  contrib/remote-helpers/test-bzr.sh    | 22 +++++++++++++++++++++-
>  2 files changed, 52 insertions(+), 2 deletions(-)
> 
> diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
> index 7e34532..2f481e9 100755
> --- a/contrib/remote-helpers/git-remote-bzr
> +++ b/contrib/remote-helpers/git-remote-bzr
> @@ -42,6 +42,7 @@ import json
>  import re
>  import StringIO
>  import atexit, shutil, hashlib, urlparse, subprocess
> +import types

No need for this any more.

>  NAME_RE = re.compile('^([^<>]+)')
>  AUTHOR_RE = re.compile('^([^<>]+?)? ?[<>]([^<>]*)(?:$|>)')
> @@ -684,7 +685,8 @@ def do_export(parser):
>                  peer = bzrlib.branch.Branch.open(peers[name],
>                                                   possible_transports=transports)
>                  try:
> -                    peer.bzrdir.push_branch(branch, revision_id=revid)
> +                    peer.bzrdir.push_branch(branch, revision_id=revid,
> +                                            overwrite=force)
>                  except bzrlib.errors.DivergedBranches:
>                      print "error %s non-fast forward" % ref
>                      continue
> @@ -718,8 +720,32 @@ def do_capabilities(parser):
>          print "*import-marks %s" % path
>      print "*export-marks %s" % path
>  
> +    print "option"
>      print
>  
> +class InvalidOptionValue(Exception):
> +    pass
> +
> +def get_bool_option(val):
> +    if val == 'true':
> +        return True
> +    elif val == 'false':
> +        return False
> +    else:
> +        raise InvalidOptionValue()
> +
> +def do_option(parser):
> +    global force
> +    (opt, val) = parser[1:3]

I prefer:

  opt, val = parser[1:3]

But not a big deal.

Otherwise the patch looks OK to me.

-- 
Felipe Contreras

Re: [PATCH v6 06/10] fast-export: add new --refspec option

[Felipe Contreras]

Richard Hansen wrote:
> On 2013-11-11 18:50, Felipe Contreras wrote:
> > On Mon, Nov 11, 2013 at 5:25 PM, Junio C Hamano <gitster@pobox.com> wrote:
> >> Felipe Contreras <felipe.contreras@gmail.com> writes:
> >>
> >>> So that we can convert the exported ref names.
> >>>
> >>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> >>> ---
> >>
> >> I thought that the discussion agreed this option should not be
> >> called --refspec but something like --refmap?
> > 
> > I don't know what you agreed to,
> 
> http://article.gmane.org/gmane.comp.version-control.git/237473
> 
> > but I didn't agree to anything.
> 
> Based on your silence I too thought that you had agreed.

Given that my opinion is regarded as inferior by those in the discussion, I
don't see why I should share it, specially since when I do, it's considered
toxic if I disagree.

> > What you pass to this option is a refspec, so it makes sense to name
> > the option --refspec.
> 
> As discussed in that thread, it's not really the same thing as a refspec
> used in push or fetch.  In those commands, the refspec specifies two
> separable things:  what to transfer, and how to translate refs names
> between the remote and local repositories.  IIUC, the fast-export
> --refspec argument only specifies how to translate ref names, not what
> gets transferred.

Does it?

 % git config remote.origin.fetch '+refs/heads/*:refs/remotes-test/origin/*'
 % git fetch origin master
 From /home/felipec/dev/git
  * branch            master     -> FETCH_HEAD
  * [new branch]      master     -> refs/remotes-test/origin/master

In this case remote.origin.fetch is determining how to translate ref names, not
what gets transferred, *exactly* the same as we are doing with --refspec. And
as far as I know, remote.origin.fetch is a refspec.

-- 
Felipe Contreras

Re: [PATCH v6 06/10] fast-export: add new --refspec option

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Does it?
>
>  % git config remote.origin.fetch '+refs/heads/*:refs/remotes-test/origin/*'
>  % git fetch origin master
>  From /home/felipec/dev/git
>   * branch            master     -> FETCH_HEAD
>   * [new branch]      master     -> refs/remotes-test/origin/master
>
> In this case remote.origin.fetch is determining how to translate ref names, not
> what gets transferred, *exactly* the same as we are doing with --refspec. And
> as far as I know, remote.origin.fetch is a refspec.

If you had 'next' and 'pu' branches at the remote, do they get
fetched with that command line?

Re: [PATCH v6 06/10] fast-export: add new --refspec option

[Felipe Contreras]

Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > Does it?
> >
> >  % git config remote.origin.fetch '+refs/heads/*:refs/remotes-test/origin/*'
> >  % git fetch origin master
> >  From /home/felipec/dev/git
> >   * branch            master     -> FETCH_HEAD
> >   * [new branch]      master     -> refs/remotes-test/origin/master
> >
> > In this case remote.origin.fetch is determining how to translate ref names, not
> > what gets transferred, *exactly* the same as we are doing with --refspec. And
> > as far as I know, remote.origin.fetch is a refspec.
> 
> If you had 'next' and 'pu' branches at the remote, do they get
> fetched with that command line?

No, why would they? You specified a single branch to fetch. Try it yourself.

 % git clone git://git.kernel.org/pub/scm/git/git.git
 % git config remote.origin.fetch '+refs/heads/*:refs/remotes-test/origin/*'
 % git fetch origin master

-- 
Felipe Contreras

Re: [PATCH v6 06/10] fast-export: add new --refspec option

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> Does it?
>>
>>  % git config remote.origin.fetch '+refs/heads/*:refs/remotes-test/origin/*'
>>  % git fetch origin master
>>  From /home/felipec/dev/git
>>   * branch            master     -> FETCH_HEAD
>>   * [new branch]      master     -> refs/remotes-test/origin/master
>>
>> In this case remote.origin.fetch is determining how to translate ref names, not
>> what gets transferred, *exactly* the same as we are doing with --refspec. And
>> as far as I know, remote.origin.fetch is a refspec.
>
> If you had 'next' and 'pu' branches at the remote, do they get
> fetched with that command line?

More interestingly, if you have

	[remote "mothership"]
		push = +refs/heads/*:refs/remotes/satellite/*

in your configuration and then said

	$ git push mothership master

then configured remote.mothership.push is not even used (I bring
this up because "export" is more closely related to "push" than
"fetch").

This (and why 'next' and 'pu' are not fetched in the "fetch"
example) is because traditionally, refspecs that are explicitly
given on the command line overrides configured ones (in other words,
configured ones are used as a fallback default).

This is a bit of tangent, but since the recent discussion on the
triangular workflows, I've been wondering if we may want to have a
new way to configure things so that we can say "When I push to
mothership any one of my local branches, I want it to go to the ref
with the same name at the mothership under refs/remotes/satellite/
hierarchy (because I am emulating 'git fetch' that is run on the
mothership to get updates from this satellite)", somewhat similar to
what you added to "fast-export" via the option in question.

But we cannot achieve that mapping by changing the meaning of the
configured refspecs remote.mothership.push without having to worry
about breaking existing practices of people.  When they say "git
push mothership master", they do mean to push refs/heads/master to
refs/heads/master, and it will break the expectation in their
existing repositories if we change the semantics under them.

A possible way to achieve that mapping "When I push to mothership
any one of my local branches,..." could be to introduce a _new_
configuration (so that existing repositories will not suddenly
change their behaviour):

	[remote "mothership"]
		pushmap = +refs/heads/*:refs/remotes/satellite/*

And then we can allow this command line

	$ git push mothership master

to be affected by the mapping when expanding the short-hand refspecs
given on the command line.  Traditionally, anything without colon
stood as a short-hand for "push to the same name", e.g. 'master' is
for 'refs/heads/master:refs/heads/master' in this example, and
'v1.0' would be for 'refs/tags/v1.0:refs/tags/v1.0'.

But it does not have to stay that way.  In order to move things
forward in that direction, this new configuration has to be
distinguishable from the traditional "refspec", as it embodies a
different concept.

Re: [PATCH v6 06/10] fast-export: add new --refspec option

[Felipe Contreras]

Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> > Felipe Contreras <felipe.contreras@gmail.com> writes:

> But it does not have to stay that way.  In order to move things
> forward in that direction, this new configuration has to be
> distinguishable from the traditional "refspec", as it embodies a
> different concept.

Is it a different concept?

 % git config remote.origin.fetch '+refs/heads/*:refs/remotes-test/origin/*'
 % git fetch origin master

What do you call this thing? ------^

-- 
Felipe Contreras

Re: [PATCH v6 06/10] fast-export: add new --refspec option

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Junio C Hamano wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>> > Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> But it does not have to stay that way.  In order to move things
>> forward in that direction, this new configuration has to be
>> distinguishable from the traditional "refspec", as it embodies a
>> different concept.
>
> Is it a different concept?
>
>  % git config remote.origin.fetch '+refs/heads/*:refs/remotes-test/origin/*'
>  % git fetch origin master
>
> What do you call this thing? ------^

The answer to that question is the "value of the 'remote.*.fetch'
configuration variable".

The refspec mechanism and syntax used in "fetch" and "push" were
invented and designed to express two things [*1*]:

 1) what is the set of objects that are necessary and sufficient to
    complete some histories need to be transferred; what are the
    tips of these histories that are being completed?

 2) after the object transfer, some refs on the side that receive
    the objects are optionally to be updated;

    2-a) which refs are they?
    2-b) what objects are they updated to point at?

A refspec consists of one or more elements, each of which has
right and left hand side separated by a colon, i.e. RHS:LHS, and

 1) is determined by the RHS
 2-a) is determined by the LHS
 2-b) is determined by the correspondence between RHS-to-LHS.

A wildcarded "refs/heads/*:refs/remotes/origin/*" dynamically
expands to what the side that sends objects have, e.g. if fetching
from a repository with 'master' and 'next' branches, it becomes

	refs/heads/master:refs/remotes/origin/master
        refs/heads/next:refs/remotes/origin/next

So with

	$ refspec='refs/heads/master:refs/remotes/origin/master
	        refs/heads/next:refs/remotes/origin/next'
	$ git fetch origin $refspec

we transfer objects to allow us to have complete histories leading
to 'master' and 'next' from the origin repository.  And we update
our refs/remotes/origin/{next,master} refs.

Traditionally, when there is _no_ refspec on the command line, the
value of 'remote.*.fetch' configuration variable is used as the
fallback default, and that usage is still true.  When used in that
way, the value of the variable _is taken as_ a refspec.

However, we can no longer say that the variable _is_ a refspec with
the modern Git, and here is why.

"git fetch origin master" used to ignore the 'remote.*.fetch'
configuration variable completely, but since f2690487 (which is a
fairly recent invention), the variable participates in the "fetch"
process in a different way [*2*].  With this in the config:

    [remote "origin"]
    	fetch = refs/heads/master:refs/remotes/origin/master
        fetch = refs/heads/next:refs/remotes/origin/next

the command with 'master' refspec on the command line transfers the
objects required to complete the history only for the 'master', and
not 'next':

	$ git fetch origin master

In this usage, 'master' on the command line is the only thing that
determines what histories are completed (because it does not say
'next', the objects necessary to complete its history are not
transferred unless they are needed to complete 'master').  The value
of the 'remote.*.fetch' configuration does not participate in the
determination of the history being transferred at all.  It is not
used as a refspec.

But unlike Git of the last year, we do map this 'master' using the
'remote.*.fetch' configuration variable, in order to decide 2)
above.  We find that the given remote ref, 'master', has an element
that corresopnds to it in the 'remote.*.fetch' configuration, and
that element tells us to update refs/remotes/origin/master to point
at the object they call 'master', effectively turning the above
command line into this form (using "refspec" that has only one
element, refs/heads/master:refs/remotes/origin/master):

	$ git fetch origin refs/heads/master:refs/remotes/origin/master

There is no "refs/heads/next:refs/remotes/origin/next" here, because
the 'fetch' configuration is not used as a refspec, but as something
else.

My understanding of the added option parameter to "git fast-export"
is that it is not about specifying the history being transferred,
but is about mapping the name of the destination.  For example, does
object between 'master' and 'next' participate in the datastream
produced with this?

	fast-export \
            --refspec=refs/heads/master:refs/remotes/origin/master \
            --refspec=refs/heads/next:refs/remotes/origin/next \
            master

If this parameter were a refspec, as we have discussed already in
previous rounds [*3*], we should be able to give it on the command line,
like any normal refspec, instead of repeating the same thing
(i.e. up to what commit should the history be transported) as in:

	fast-export --refspec=refs/heads/master:refs/remotes/origin/master master

but just

	fast-export refs/heads/master:refs/remotes/origin/master


[Footnote]

 *1* Note that readers are hearing the authoritative definition
     given by the person who invented and designed it back in August
     2005.

 *2* And a recent $gmane/238832 moves "push" in the direction to be
     in line with "fetch" in this regard.

 *3* And I think I even outlined the code to do so.

Re: [PATCH v6 06/10] fast-export: add new --refspec option

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> A refspec consists of one or more elements, each of which has
> right and left hand side separated by a colon, i.e. RHS:LHS, and
>
>  1) is determined by the RHS
>  2-a) is determined by the LHS
>  2-b) is determined by the correspondence between RHS-to-LHS.

Heh, I got my right and left the other way around ;-)

Re: [PATCH v6 06/10] fast-export: add new --refspec option

[Felipe Contreras]

Hi,

Sorry it took too long to reply to this.

Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > Junio C Hamano wrote:
> >> Junio C Hamano <gitster@pobox.com> writes:
> >> > Felipe Contreras <felipe.contreras@gmail.com> writes:
> >
> >> But it does not have to stay that way.  In order to move things
> >> forward in that direction, this new configuration has to be
> >> distinguishable from the traditional "refspec", as it embodies a
> >> different concept.
> >
> > Is it a different concept?
> >
> >  % git config remote.origin.fetch '+refs/heads/*:refs/remotes-test/origin/*'
> >  % git fetch origin master
> >
> > What do you call this thing? ------^
> 
> The answer to that question is the "value of the 'remote.*.fetch'
> configuration variable".

You are avoiding the question: it's a refspec.

> There is no "refs/heads/next:refs/remotes/origin/next" here, because
> the 'fetch' configuration is not used as a refspec, but as something
> else.

Yet both remote.fetch and remote.push are a 'struct refspec', and the
documentation says they are a "refspec".

> My understanding of the added option parameter to "git fast-export"
> is that it is not about specifying the history being transferred,
> but is about mapping the name of the destination.  For example, does
> object between 'master' and 'next' participate in the datastream
> produced with this?
> 
> 	fast-export \
>             --refspec=refs/heads/master:refs/remotes/origin/master \
>             --refspec=refs/heads/next:refs/remotes/origin/next \
>             master
> 
> If this parameter were a refspec, as we have discussed already in
> previous rounds [*3*], we should be able to give it on the command line,
> like any normal refspec, instead of repeating the same thing
> (i.e. up to what commit should the history be transported) as in:
> 
> 	fast-export --refspec=refs/heads/master:refs/remotes/origin/master master
> 
> but just
> 
> 	fast-export refs/heads/master:refs/remotes/origin/master

Maybe in an ideal world, which we don't live in.

My guess is that trying to accomplish such a goal would result in an
unbelievable mess of the code that I wouldn't even want to think about where
to start to code this.

Moreover, `git grep refmap` returns me the following:

  t/t5516-fetch-push.sh:test_expect_success 'pushing a specific ref applies remote.$name.push as refmap' '
  t/t5516-fetch-push.sh:test_expect_success 'with no remote.$name.push, it is not used as refmap' '

I'd say with --refspec is the simplest and most sensible way this can be
implemented.

-- 
Felipe Contreras

[PATCH v6 06/10] fast-export: add new --refspec option

[Felipe Contreras]

So that we can convert the exported ref names.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-fast-export.txt |  4 ++++
 builtin/fast-export.c             | 32 ++++++++++++++++++++++++++++++++
 t/t9350-fast-export.sh            |  7 +++++++
 3 files changed, 43 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.
+
 [<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 eea5b8c..cf745ec 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]"),
@@ -31,6 +32,8 @@ static int use_done_feature;
 static int no_data;
 static int full_tree;
 static struct string_list extra_refs = STRING_LIST_INIT_NODUP;
+static struct refspec *refspecs;
+static int refspecs_nr;
 
 static int parse_opt_signed_tag_mode(const struct option *opt,
 				     const char *arg, int unset)
@@ -525,6 +528,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;
+			}
+		}
+
 		commit = get_commit(e, full_name);
 		if (!commit) {
 			warning("%s: Unexpected object of type %s, skipping.",
@@ -668,6 +680,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 = STRING_LIST_INIT_NODUP;
 	struct option options[] = {
 		OPT_INTEGER(0, "progress", &progress,
 			    N_("show progress after <n> objects")),
@@ -688,6 +701,8 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 		OPT_BOOL(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()
 	};
 
@@ -707,6 +722,21 @@ 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;
+		int i;
+
+		refspecs_str = xmalloc(sizeof(*refspecs_str) * refspecs_list.nr);
+		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);
+		free(refspecs_str);
+	}
+
 	if (use_done_feature)
 		printf("feature done\n");
 
@@ -741,5 +771,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 2312dec..3d475af 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.2+fc1