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

[PATCH v4 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.

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

This time rebased on top of the latest master, plus a few fixes.

Diff from v3:

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 9b728ca..60d4c80 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -686,7 +686,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 string_list refspecs_list = STRING_LIST_INIT_NODUP;
        struct option options[] = {
                OPT_INTEGER(0, "progress", &progress,
                            N_("show progress after <n> objects")),
diff --git a/transport-helper.c b/transport-helper.c
index d94eaf4..91636d5 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -836,9 +836,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 (10):
  transport-helper: add 'force' to 'export' helpers
  transport-helper: fix extra lines
  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                | 47 ++++++++++++++++++++++++++-------------
 8 files changed, 140 insertions(+), 20 deletions(-)

-- 
1.8.4-fc

[PATCH v4 03/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 707351d..f8eb143 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -643,7 +643,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;
@@ -701,6 +701,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)
@@ -722,6 +727,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

[PATCH v4 05/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             | 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.
+
 [<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..b6f623e 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,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");
 
@@ -741,5 +769,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

[PATCH v4 06/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 613f69a..407c18d 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 f8eb143..5454822 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -848,7 +848,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)
@@ -886,8 +886,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);
 		}
 	}
@@ -895,6 +900,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

[PATCH v4 02/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 408d596..707351d 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -879,9 +879,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-fc

[PATCH v4 09/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 407c18d..be543c0 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 5454822..4f47bdd 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);
@@ -888,12 +885,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-fc

[PATCH v4 01/10] transport-helper: add 'force' to 'export' helpers

[Felipe Contreras]

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 | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/transport-helper.c b/transport-helper.c
index b32e2d6..408d596 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -853,6 +853,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-fc

[PATCH v4 08/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 b6f623e..8ed41b4 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;
@@ -762,6 +775,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 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

[PATCH v4 10/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 4f47bdd..ef91882 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -733,7 +733,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;
@@ -747,7 +748,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 */
@@ -838,7 +839,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;
 }
 
@@ -905,7 +906,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

[PATCH v4 07/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..c49ede4 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 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 88fc407..f453388 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-fc

[PATCH v4 04/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-fc

Re: [PATCH v4 07/10] fast-import: add support to delete refs

[Eric Sunshine]

On Sun, Oct 27, 2013 at 3:05 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> 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..c49ede4 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

s/specifices/specifies/

> +  removed.
> +
>  The special case of restarting an incremental import from the
>  current branch value should be written as:
>  ----

[PATCH v4 11/10] fixup! transport-helper: add 'force' to 'export' helpers

[Richard Hansen]

document the new 'force' option

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 Documentation/gitremote-helpers.txt | 4 ++++
 1 file changed, 4 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]
-- 
1.8.4.1.610.g2fe5c0e

[PATCH v4 12/10] git-remote-testgit: support the new 'force' option

[Richard Hansen]

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 git-remote-testgit.sh | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/git-remote-testgit.sh b/git-remote-testgit.sh
index 6d2f282..80546c1 100755
--- a/git-remote-testgit.sh
+++ b/git-remote-testgit.sh
@@ -6,6 +6,7 @@ url=$2
 
 dir="$GIT_DIR/testgit/$alias"
 prefix="refs/testgit/$alias"
+forcearg=
 
 default_refspec="refs/heads/*:${prefix}/heads/*"
 
@@ -39,6 +40,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 +95,7 @@ do
 		before=$(git for-each-ref --format=' %(refname) %(objectname) ')
 
 		git fast-import \
+			${forcearg} \
 			${testgitmarks:+"--import-marks=$testgitmarks"} \
 			${testgitmarks:+"--export-marks=$testgitmarks"} \
 			--quiet
@@ -115,6 +118,21 @@ do
 
 		echo
 		;;
+	option\ *)
+		read cmd opt val <<EOF
+${line}
+EOF
+		case ${opt} in
+		    force)
+			case ${val} in
+			    true) forcearg=--force; echo 'ok';;
+			    false) forcearg=; echo 'ok';;
+			    *) printf %s\\n "error '${val}'\
+ is not a valid value for option ${opt}";;
+			esac;;
+		    *) echo "unsupported";;
+		esac
+		;;
 	'')
 		exit
 		;;
-- 
1.8.4.1.614.ga09cf56

[PATCH v4 13/10] test: remote-helper: add test for force pushes

[Richard Hansen]

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 t/t5801-remote-helpers.sh | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index be543c0..93a7d34 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -102,6 +102,19 @@ test_expect_success 'push delete branch' '
 	 rev-parse --verify refs/heads/new-name
 '
 
+test_expect_success 'force 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 &&
-- 
1.8.4.1.614.ga09cf56

RE: [PATCH v4 12/10] git-remote-testgit: support the new 'force' option

[Felipe Contreras]

Richard Hansen wrote:
> Signed-off-by: Richard Hansen <rhansen@bbn.com>
> ---
>  git-remote-testgit.sh | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/git-remote-testgit.sh b/git-remote-testgit.sh
> index 6d2f282..80546c1 100755
> --- a/git-remote-testgit.sh
> +++ b/git-remote-testgit.sh
> @@ -6,6 +6,7 @@ url=$2
>  
>  dir="$GIT_DIR/testgit/$alias"
>  prefix="refs/testgit/$alias"
> +forcearg=
>  
>  default_refspec="refs/heads/*:${prefix}/heads/*"
>  
> @@ -39,6 +40,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 +95,7 @@ do
>  		before=$(git for-each-ref --format=' %(refname) %(objectname) ')
>  
>  		git fast-import \
> +			${forcearg} \
>  			${testgitmarks:+"--import-marks=$testgitmarks"} \
>  			${testgitmarks:+"--export-marks=$testgitmarks"} \
>  			--quiet
> @@ -115,6 +118,21 @@ do
>  
>  		echo
>  		;;
> +	option\ *)
> +		read cmd opt val <<EOF
> +${line}
> +EOF

We can do <<-EOF to align this properly.

Also, I don't see why all the variables are ${foo} instead of $foo.

> +		case ${opt} in
> +		    force)

I think the convention is to align these:

case $opt in
force)

> +			case ${val} in
> +			    true) forcearg=--force; echo 'ok';;
> +			    false) forcearg=; echo 'ok';;
> +			    *) printf %s\\n "error '${val}'\
> + is not a valid value for option ${opt}";;

I think this is packing a lot of stuff and it's not that readable.

Moreover, this is not for production purposes, it's for testing purposes and a
guideline, I think this suffices.


	option\ *)
		read cmd opt val <<-EOF
		$line
		EOF
		case $opt in
		force)
			test $val = "true" && force="true" || force=
			echo "ok"
			;;
		*)
			echo "unsupported"
			;;
		esac
		;;

But this is definetly good to have, will merge.

-- 
Felipe Contreras

Re: [PATCH v4 12/10] git-remote-testgit: support the new 'force' option

[Richard Hansen]

On 2013-10-29 04:41, Felipe Contreras wrote:
> Richard Hansen wrote:
>> Signed-off-by: Richard Hansen <rhansen@bbn.com>
>> ---
>>  git-remote-testgit.sh | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/git-remote-testgit.sh b/git-remote-testgit.sh
>> index 6d2f282..80546c1 100755
>> --- a/git-remote-testgit.sh
>> +++ b/git-remote-testgit.sh
>> @@ -6,6 +6,7 @@ url=$2
>>  
>>  dir="$GIT_DIR/testgit/$alias"
>>  prefix="refs/testgit/$alias"
>> +forcearg=
>>  
>>  default_refspec="refs/heads/*:${prefix}/heads/*"
>>  
>> @@ -39,6 +40,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 +95,7 @@ do
>>  		before=$(git for-each-ref --format=' %(refname) %(objectname) ')
>>  
>>  		git fast-import \
>> +			${forcearg} \
>>  			${testgitmarks:+"--import-marks=$testgitmarks"} \
>>  			${testgitmarks:+"--export-marks=$testgitmarks"} \
>>  			--quiet
>> @@ -115,6 +118,21 @@ do
>>  
>>  		echo
>>  		;;
>> +	option\ *)
>> +		read cmd opt val <<EOF
>> +${line}
>> +EOF
> 
> We can do <<-EOF to align this properly.

Good point.  I personally avoid tabs whenever possible, and <<- only
works with tabs, so I'm in the habit of doing <<EOF.

> 
> Also, I don't see why all the variables are ${foo} instead of $foo.

I'm in the habit of doing ${foo} because I like the consistency --
sometimes you need them to disambiguate, and sometimes you need special
expansions like ${foo##bar} or ${foo:-bar}.

In this case it's actually less consistent to do ${foo} because the rest
of the file doesn't use {} when not needed, so I agree with your change.

> 
>> +		case ${opt} in
>> +		    force)
> 
> I think the convention is to align these:
> 
> case $opt in
> force)

The existing case statement in this file indents the patterns the same
amount as the case statement, so this should be aligned to match.

In general I rarely see the case patterns indented at the same level as
the case statement, possibly because Emacs shell-mode indents the
patterns more than the case statement (by default).  The POSIX spec
contains a mix of styles:
  * the normative text documenting the format of a 'case' construct
    indents the patterns more than the 'case' statement
  * two of the four non-normative examples indent the patterns
    more than the 'case' statements; the other two do not

> 
>> +			case ${val} in
>> +			    true) forcearg=--force; echo 'ok';;
>> +			    false) forcearg=; echo 'ok';;
>> +			    *) printf %s\\n "error '${val}'\
>> + is not a valid value for option ${opt}";;
> 
> I think this is packing a lot of stuff and it's not that readable.
> 
> Moreover, this is not for production purposes, it's for testing purposes and a
> guideline, I think this suffices.
> 
> 
> 	option\ *)
> 		read cmd opt val <<-EOF
> 		$line
> 		EOF
> 		case $opt in
> 		force)
> 			test $val = "true" && force="true" || force=
> 			echo "ok"
> 			;;
> 		*)
> 			echo "unsupported"
> 			;;
> 		esac
> 		;;

Works for me.

> 
> But this is definetly good to have, will merge.

Thanks,
Richard

Re: [PATCH v4 12/10] git-remote-testgit: support the new 'force' option

[Felipe Contreras]

On Sun, Nov 10, 2013 at 4:46 PM, Richard Hansen <rhansen@bbn.com> wrote:
> On 2013-10-29 04:41, Felipe Contreras wrote:
>> Richard Hansen wrote:
>>> Signed-off-by: Richard Hansen <rhansen@bbn.com>
>>> ---
>>>  git-remote-testgit.sh | 18 ++++++++++++++++++
>>>  1 file changed, 18 insertions(+)
>>>
>>> diff --git a/git-remote-testgit.sh b/git-remote-testgit.sh
>>> index 6d2f282..80546c1 100755
>>> --- a/git-remote-testgit.sh
>>> +++ b/git-remote-testgit.sh
>>> @@ -6,6 +6,7 @@ url=$2
>>>
>>>  dir="$GIT_DIR/testgit/$alias"
>>>  prefix="refs/testgit/$alias"
>>> +forcearg=
>>>
>>>  default_refspec="refs/heads/*:${prefix}/heads/*"
>>>
>>> @@ -39,6 +40,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 +95,7 @@ do
>>>              before=$(git for-each-ref --format=' %(refname) %(objectname) ')
>>>
>>>              git fast-import \
>>> +                    ${forcearg} \
>>>                      ${testgitmarks:+"--import-marks=$testgitmarks"} \
>>>                      ${testgitmarks:+"--export-marks=$testgitmarks"} \
>>>                      --quiet
>>> @@ -115,6 +118,21 @@ do
>>>
>>>              echo
>>>              ;;
>>> +    option\ *)
>>> +            read cmd opt val <<EOF
>>> +${line}
>>> +EOF
>>
>> We can do <<-EOF to align this properly.
>
> Good point.  I personally avoid tabs whenever possible, and <<- only
> works with tabs, so I'm in the habit of doing <<EOF.

That looks very weird to me, plus <<-EOF is often used already in git tests.

>> Also, I don't see why all the variables are ${foo} instead of $foo.
>
> I'm in the habit of doing ${foo} because I like the consistency --

Sure, but with the price of less readibility. If consistency was the
priority, we would be doing the follwoing in C:

if (foo) {
  # single line
}

Since the if might contain multiple lines, but we don't do that,
because readibility is more important than consistency. So sometimes
it's with braces, sometimes without.

>>> +            case ${opt} in
>>> +                force)
>>
>> I think the convention is to align these:
>>
>> case $opt in
>> force)
>
> The existing case statement in this file indents the patterns the same
> amount as the case statement, so this should be aligned to match.
>
> In general I rarely see the case patterns indented at the same level as
> the case statement, possibly because Emacs shell-mode indents the
> patterns more than the case statement (by default).  The POSIX spec
> contains a mix of styles:
>   * the normative text documenting the format of a 'case' construct
>     indents the patterns more than the 'case' statement
>   * two of the four non-normative examples indent the patterns
>     more than the 'case' statements; the other two do not

The style in C has the cases at the same level, so I think it makes
sense to do the same in shell, but I'm not sure if that's followed
already.

>>> +                    case ${val} in
>>> +                        true) forcearg=--force; echo 'ok';;
>>> +                        false) forcearg=; echo 'ok';;
>>> +                        *) printf %s\\n "error '${val}'\
>>> + is not a valid value for option ${opt}";;
>>
>> I think this is packing a lot of stuff and it's not that readable.
>>
>> Moreover, this is not for production purposes, it's for testing purposes and a
>> guideline, I think this suffices.
>>
>>
>>       option\ *)
>>               read cmd opt val <<-EOF
>>               $line
>>               EOF
>>               case $opt in
>>               force)
>>                       test $val = "true" && force="true" || force=
>>                       echo "ok"
>>                       ;;
>>
>>                       echo "unsupported"
>>                       ;;
>>               esac
>>               ;;
>
> Works for me.

Good, the final code style can be decided later on, and perhaps update
Documentation/CodingGuidelines, but it's good the rest is more or less
settled.

Cheers.

-- 
Felipe Contreras

Re: [PATCH v4 12/10] git-remote-testgit: support the new 'force' option

[Junio C Hamano]

Richard Hansen <rhansen@bbn.com> writes:

>> I think the convention is to align these:
>> 
>> case $opt in
>> force)
>
> The existing case statement in this file indents the patterns the same
> amount as the case statement, so this should be aligned to match.
>
> In general I rarely see the case patterns indented at the same level as
> the case statement,

What you see does not matter in the context of this project ;-)
This is what we have in Documentation/CodingGuidelines:

    For shell scripts specifically (not exhaustive):

     - Case arms are indented at the same depth as case and esac lines.

Thanks.

Re: [PATCH v4 12/10] git-remote-testgit: support the new 'force' option

[Richard Hansen]

On 2013-11-11 13:28, Junio C Hamano wrote:
> Richard Hansen <rhansen@bbn.com> writes:
> 
>>> I think the convention is to align these:
>>>
>>> case $opt in
>>> force)
>>
>> The existing case statement in this file indents the patterns the same
>> amount as the case statement, so this should be aligned to match.
>>
>> In general I rarely see the case patterns indented at the same level as
>> the case statement,
> 
> What you see does not matter in the context of this project ;-)
> This is what we have in Documentation/CodingGuidelines:
> 
>     For shell scripts specifically (not exhaustive):
> 
>      - Case arms are indented at the same depth as case and esac lines.

Doh!  I missed that.  Thanks for pointing it out.

Thanks,
Richard