Git development
 help / color / mirror / Atom feed
* [PATCH 3/7] Move print_commit_list to libgit.a
From: Nguyễn Thái Ngọc Duy @ 2012-10-26 15:53 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1351266835-22610-1-git-send-email-pclouds@gmail.com>

This is used by bisect.c, part of libgit.a while it stays in
builtin/rev-list.c. Move it to commit.c so that we won't get undefined
reference if a program that uses libgit.a happens to pull it in.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 bisect.h           |  4 ----
 builtin/rev-list.c | 10 ----------
 commit.c           | 10 ++++++++++
 commit.h           |  4 ++++
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/bisect.h b/bisect.h
index ec3c3ff..2a6c831 100644
--- a/bisect.h
+++ b/bisect.h
@@ -11,10 +11,6 @@ extern struct commit_list *filter_skipped(struct commit_list *list,
 					  int *count,
 					  int *skipped_first);
 
-extern void print_commit_list(struct commit_list *list,
-			      const char *format_cur,
-			      const char *format_last);
-
 #define BISECT_SHOW_ALL		(1<<0)
 #define REV_LIST_QUIET		(1<<1)
 
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index a0ec84e..67701be 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -201,16 +201,6 @@ static void show_edge(struct commit *commit)
 	printf("-%s\n", sha1_to_hex(commit->object.sha1));
 }
 
-void print_commit_list(struct commit_list *list,
-		       const char *format_cur,
-		       const char *format_last)
-{
-	for ( ; list; list = list->next) {
-		const char *format = list->next ? format_cur : format_last;
-		printf(format, sha1_to_hex(list->item->object.sha1));
-	}
-}
-
 static void print_var_str(const char *var, const char *val)
 {
 	printf("%s='%s'\n", var, val);
diff --git a/commit.c b/commit.c
index 213bc98..e8eb0ae 100644
--- a/commit.c
+++ b/commit.c
@@ -1347,3 +1347,13 @@ struct commit_list **commit_list_append(struct commit *commit,
 	new->next = NULL;
 	return &new->next;
 }
+
+void print_commit_list(struct commit_list *list,
+		       const char *format_cur,
+		       const char *format_last)
+{
+	for ( ; list; list = list->next) {
+		const char *format = list->next ? format_cur : format_last;
+		printf(format, sha1_to_hex(list->item->object.sha1));
+	}
+}
diff --git a/commit.h b/commit.h
index 9f21313..c4cd046 100644
--- a/commit.h
+++ b/commit.h
@@ -222,4 +222,8 @@ struct commit *get_merge_parent(const char *name);
 
 extern int parse_signed_commit(const unsigned char *sha1,
 			       struct strbuf *message, struct strbuf *signature);
+extern void print_commit_list(struct commit_list *list,
+			      const char *format_cur,
+			      const char *format_last);
+
 #endif /* COMMIT_H */
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH 4/7] Move setup_diff_pager to libgit.a
From: Nguyễn Thái Ngọc Duy @ 2012-10-26 15:53 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1351266835-22610-1-git-send-email-pclouds@gmail.com>

This is used by diff-no-index.c, part of libgit.a while it stays in
builtin/diff.c. Move it to diff.c so that we won't get undefined
reference if a program that uses libgit.a happens to pull it in.

While at it, move check_pager from git.c to pager.c. It makes more
sense there and pager.c is also part of libgit.a

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin.h      |  4 ----
 builtin/diff.c | 16 ----------------
 cache.h        |  1 +
 diff.c         | 16 ++++++++++++++++
 diff.h         |  1 +
 git.c          | 33 ---------------------------------
 pager.c        | 34 ++++++++++++++++++++++++++++++++++
 7 files changed, 52 insertions(+), 53 deletions(-)

diff --git a/builtin.h b/builtin.h
index 95116b8..3faf9d6 100644
--- a/builtin.h
+++ b/builtin.h
@@ -37,10 +37,6 @@ int copy_note_for_rewrite(struct notes_rewrite_cfg *c,
 			  const unsigned char *from_obj, const unsigned char *to_obj);
 void finish_copy_notes_for_rewrite(struct notes_rewrite_cfg *c);
 
-extern int check_pager_config(const char *cmd);
-struct diff_options;
-extern void setup_diff_pager(struct diff_options *);
-
 extern int textconv_object(const char *path, unsigned mode, const unsigned char *sha1, int sha1_valid, char **buf, unsigned long *buf_size);
 
 extern int cmd_add(int argc, const char **argv, const char *prefix);
diff --git a/builtin/diff.c b/builtin/diff.c
index 9650be2..9c70e40 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -418,19 +418,3 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 		refresh_index_quietly();
 	return result;
 }
-
-void setup_diff_pager(struct diff_options *opt)
-{
-	/*
-	 * If the user asked for our exit code, then either they want --quiet
-	 * or --exit-code. We should definitely not bother with a pager in the
-	 * former case, as we will generate no output. Since we still properly
-	 * report our exit code even when a pager is run, we _could_ run a
-	 * pager with --exit-code. But since we have not done so historically,
-	 * and because it is easy to find people oneline advising "git diff
-	 * --exit-code" in hooks and other scripts, we do not do so.
-	 */
-	if (!DIFF_OPT_TST(opt, EXIT_WITH_STATUS) &&
-	    check_pager_config("diff") != 0)
-		setup_pager();
-}
diff --git a/cache.h b/cache.h
index 2dc4dec..dbd8018 100644
--- a/cache.h
+++ b/cache.h
@@ -1183,6 +1183,7 @@ extern int pager_in_use(void);
 extern int pager_use_color;
 extern int term_columns(void);
 extern int decimal_width(int);
+extern int check_pager_config(const char *cmd);
 
 extern const char *editor_program;
 extern const char *askpass_program;
diff --git a/diff.c b/diff.c
index 35d3f07..1b5727d 100644
--- a/diff.c
+++ b/diff.c
@@ -4871,3 +4871,19 @@ size_t fill_textconv(struct userdiff_driver *driver,
 
 	return size;
 }
+
+void setup_diff_pager(struct diff_options *opt)
+{
+	/*
+	 * If the user asked for our exit code, then either they want --quiet
+	 * or --exit-code. We should definitely not bother with a pager in the
+	 * former case, as we will generate no output. Since we still properly
+	 * report our exit code even when a pager is run, we _could_ run a
+	 * pager with --exit-code. But since we have not done so historically,
+	 * and because it is easy to find people oneline advising "git diff
+	 * --exit-code" in hooks and other scripts, we do not do so.
+	 */
+	if (!DIFF_OPT_TST(opt, EXIT_WITH_STATUS) &&
+	    check_pager_config("diff") != 0)
+		setup_pager();
+}
diff --git a/diff.h b/diff.h
index a658f85..a47bae4 100644
--- a/diff.h
+++ b/diff.h
@@ -335,5 +335,6 @@ extern int parse_rename_score(const char **cp_p);
 
 extern int print_stat_summary(FILE *fp, int files,
 			      int insertions, int deletions);
+extern void setup_diff_pager(struct diff_options *);
 
 #endif /* DIFF_H */
diff --git a/git.c b/git.c
index 8788b32..d33f9b3 100644
--- a/git.c
+++ b/git.c
@@ -17,39 +17,6 @@ const char git_more_info_string[] =
 
 static struct startup_info git_startup_info;
 static int use_pager = -1;
-struct pager_config {
-	const char *cmd;
-	int want;
-	char *value;
-};
-
-static int pager_command_config(const char *var, const char *value, void *data)
-{
-	struct pager_config *c = data;
-	if (!prefixcmp(var, "pager.") && !strcmp(var + 6, c->cmd)) {
-		int b = git_config_maybe_bool(var, value);
-		if (b >= 0)
-			c->want = b;
-		else {
-			c->want = 1;
-			c->value = xstrdup(value);
-		}
-	}
-	return 0;
-}
-
-/* returns 0 for "no pager", 1 for "use pager", and -1 for "not specified" */
-int check_pager_config(const char *cmd)
-{
-	struct pager_config c;
-	c.cmd = cmd;
-	c.want = -1;
-	c.value = NULL;
-	git_config(pager_command_config, &c);
-	if (c.value)
-		pager_program = c.value;
-	return c.want;
-}
 
 static void commit_pager_choice(void) {
 	switch (use_pager) {
diff --git a/pager.c b/pager.c
index 4dcb08d..060fa88 100644
--- a/pager.c
+++ b/pager.c
@@ -6,6 +6,12 @@
 #define DEFAULT_PAGER "less"
 #endif
 
+struct pager_config {
+	const char *cmd;
+	int want;
+	char *value;
+};
+
 /*
  * This is split up from the rest of git so that we can do
  * something different on Windows.
@@ -159,3 +165,31 @@ int decimal_width(int number)
 		i *= 10;
 	return width;
 }
+
+static int pager_command_config(const char *var, const char *value, void *data)
+{
+	struct pager_config *c = data;
+	if (!prefixcmp(var, "pager.") && !strcmp(var + 6, c->cmd)) {
+		int b = git_config_maybe_bool(var, value);
+		if (b >= 0)
+			c->want = b;
+		else {
+			c->want = 1;
+			c->value = xstrdup(value);
+		}
+	}
+	return 0;
+}
+
+/* returns 0 for "no pager", 1 for "use pager", and -1 for "not specified" */
+int check_pager_config(const char *cmd)
+{
+	struct pager_config c;
+	c.cmd = cmd;
+	c.want = -1;
+	c.value = NULL;
+	git_config(pager_command_config, &c);
+	if (c.value)
+		pager_program = c.value;
+	return c.want;
+}
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH 5/7] send-pack: move core code to libgit.a
From: Nguyễn Thái Ngọc Duy @ 2012-10-26 15:53 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1351266835-22610-1-git-send-email-pclouds@gmail.com>

send_pack() is used by transport.c, part of libgit.a while it stays in
builtin/send-pack.c. Move it to send-pack.c so that we won't get
undefined reference if a program that uses libgit.a happens to pull it
in.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Makefile            |   1 +
 builtin/send-pack.c | 333 --------------------------------------------------
 send-pack.c         | 344 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 transport.h         |   5 +
 4 files changed, 350 insertions(+), 333 deletions(-)
 create mode 100644 send-pack.c

diff --git a/Makefile b/Makefile
index 2c333dc..414f15f 100644
--- a/Makefile
+++ b/Makefile
@@ -797,6 +797,7 @@ LIB_OBJS += rerere.o
 LIB_OBJS += resolve-undo.o
 LIB_OBJS += revision.o
 LIB_OBJS += run-command.o
+LIB_OBJS += send-pack.o
 LIB_OBJS += sequencer.o
 LIB_OBJS += server-info.o
 LIB_OBJS += setup.o
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 7d05064..d342013 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -16,164 +16,6 @@ static const char send_pack_usage[] =
 
 static struct send_pack_args args;
 
-static int feed_object(const unsigned char *sha1, int fd, int negative)
-{
-	char buf[42];
-
-	if (negative && !has_sha1_file(sha1))
-		return 1;
-
-	memcpy(buf + negative, sha1_to_hex(sha1), 40);
-	if (negative)
-		buf[0] = '^';
-	buf[40 + negative] = '\n';
-	return write_or_whine(fd, buf, 41 + negative, "send-pack: send refs");
-}
-
-/*
- * Make a pack stream and spit it out into file descriptor fd
- */
-static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *extra, struct send_pack_args *args)
-{
-	/*
-	 * The child becomes pack-objects --revs; we feed
-	 * the revision parameters to it via its stdin and
-	 * let its stdout go back to the other end.
-	 */
-	const char *argv[] = {
-		"pack-objects",
-		"--all-progress-implied",
-		"--revs",
-		"--stdout",
-		NULL,
-		NULL,
-		NULL,
-		NULL,
-		NULL,
-	};
-	struct child_process po;
-	int i;
-
-	i = 4;
-	if (args->use_thin_pack)
-		argv[i++] = "--thin";
-	if (args->use_ofs_delta)
-		argv[i++] = "--delta-base-offset";
-	if (args->quiet || !args->progress)
-		argv[i++] = "-q";
-	if (args->progress)
-		argv[i++] = "--progress";
-	memset(&po, 0, sizeof(po));
-	po.argv = argv;
-	po.in = -1;
-	po.out = args->stateless_rpc ? -1 : fd;
-	po.git_cmd = 1;
-	if (start_command(&po))
-		die_errno("git pack-objects failed");
-
-	/*
-	 * We feed the pack-objects we just spawned with revision
-	 * parameters by writing to the pipe.
-	 */
-	for (i = 0; i < extra->nr; i++)
-		if (!feed_object(extra->array[i], po.in, 1))
-			break;
-
-	while (refs) {
-		if (!is_null_sha1(refs->old_sha1) &&
-		    !feed_object(refs->old_sha1, po.in, 1))
-			break;
-		if (!is_null_sha1(refs->new_sha1) &&
-		    !feed_object(refs->new_sha1, po.in, 0))
-			break;
-		refs = refs->next;
-	}
-
-	close(po.in);
-
-	if (args->stateless_rpc) {
-		char *buf = xmalloc(LARGE_PACKET_MAX);
-		while (1) {
-			ssize_t n = xread(po.out, buf, LARGE_PACKET_MAX);
-			if (n <= 0)
-				break;
-			send_sideband(fd, -1, buf, n, LARGE_PACKET_MAX);
-		}
-		free(buf);
-		close(po.out);
-		po.out = -1;
-	}
-
-	if (finish_command(&po))
-		return -1;
-	return 0;
-}
-
-static int receive_status(int in, struct ref *refs)
-{
-	struct ref *hint;
-	char line[1000];
-	int ret = 0;
-	int len = packet_read_line(in, line, sizeof(line));
-	if (len < 10 || memcmp(line, "unpack ", 7))
-		return error("did not receive remote status");
-	if (memcmp(line, "unpack ok\n", 10)) {
-		char *p = line + strlen(line) - 1;
-		if (*p == '\n')
-			*p = '\0';
-		error("unpack failed: %s", line + 7);
-		ret = -1;
-	}
-	hint = NULL;
-	while (1) {
-		char *refname;
-		char *msg;
-		len = packet_read_line(in, line, sizeof(line));
-		if (!len)
-			break;
-		if (len < 3 ||
-		    (memcmp(line, "ok ", 3) && memcmp(line, "ng ", 3))) {
-			fprintf(stderr, "protocol error: %s\n", line);
-			ret = -1;
-			break;
-		}
-
-		line[strlen(line)-1] = '\0';
-		refname = line + 3;
-		msg = strchr(refname, ' ');
-		if (msg)
-			*msg++ = '\0';
-
-		/* first try searching at our hint, falling back to all refs */
-		if (hint)
-			hint = find_ref_by_name(hint, refname);
-		if (!hint)
-			hint = find_ref_by_name(refs, refname);
-		if (!hint) {
-			warning("remote reported status on unknown ref: %s",
-					refname);
-			continue;
-		}
-		if (hint->status != REF_STATUS_EXPECTING_REPORT) {
-			warning("remote reported status on unexpected ref: %s",
-					refname);
-			continue;
-		}
-
-		if (line[0] == 'o' && line[1] == 'k')
-			hint->status = REF_STATUS_OK;
-		else {
-			hint->status = REF_STATUS_REMOTE_REJECT;
-			ret = -1;
-		}
-		if (msg)
-			hint->remote_status = xstrdup(msg);
-		/* start our next search from the next ref */
-		hint = hint->next;
-	}
-	return ret;
-}
-
 static void print_helper_status(struct ref *ref)
 {
 	struct strbuf buf = STRBUF_INIT;
@@ -227,181 +69,6 @@ static void print_helper_status(struct ref *ref)
 	strbuf_release(&buf);
 }
 
-static int sideband_demux(int in, int out, void *data)
-{
-	int *fd = data, ret;
-#ifdef NO_PTHREADS
-	close(fd[1]);
-#endif
-	ret = recv_sideband("send-pack", fd[0], out);
-	close(out);
-	return ret;
-}
-
-int send_pack(struct send_pack_args *args,
-	      int fd[], struct child_process *conn,
-	      struct ref *remote_refs,
-	      struct extra_have_objects *extra_have)
-{
-	int in = fd[0];
-	int out = fd[1];
-	struct strbuf req_buf = STRBUF_INIT;
-	struct ref *ref;
-	int new_refs;
-	int allow_deleting_refs = 0;
-	int status_report = 0;
-	int use_sideband = 0;
-	int quiet_supported = 0;
-	int agent_supported = 0;
-	unsigned cmds_sent = 0;
-	int ret;
-	struct async demux;
-
-	/* Does the other end support the reporting? */
-	if (server_supports("report-status"))
-		status_report = 1;
-	if (server_supports("delete-refs"))
-		allow_deleting_refs = 1;
-	if (server_supports("ofs-delta"))
-		args->use_ofs_delta = 1;
-	if (server_supports("side-band-64k"))
-		use_sideband = 1;
-	if (server_supports("quiet"))
-		quiet_supported = 1;
-	if (server_supports("agent"))
-		agent_supported = 1;
-
-	if (!remote_refs) {
-		fprintf(stderr, "No refs in common and none specified; doing nothing.\n"
-			"Perhaps you should specify a branch such as 'master'.\n");
-		return 0;
-	}
-
-	/*
-	 * Finally, tell the other end!
-	 */
-	new_refs = 0;
-	for (ref = remote_refs; ref; ref = ref->next) {
-		if (!ref->peer_ref && !args->send_mirror)
-			continue;
-
-		/* Check for statuses set by set_ref_status_for_push() */
-		switch (ref->status) {
-		case REF_STATUS_REJECT_NONFASTFORWARD:
-		case REF_STATUS_UPTODATE:
-			continue;
-		default:
-			; /* do nothing */
-		}
-
-		if (ref->deletion && !allow_deleting_refs) {
-			ref->status = REF_STATUS_REJECT_NODELETE;
-			continue;
-		}
-
-		if (!ref->deletion)
-			new_refs++;
-
-		if (args->dry_run) {
-			ref->status = REF_STATUS_OK;
-		} else {
-			char *old_hex = sha1_to_hex(ref->old_sha1);
-			char *new_hex = sha1_to_hex(ref->new_sha1);
-			int quiet = quiet_supported && (args->quiet || !args->progress);
-
-			if (!cmds_sent && (status_report || use_sideband ||
-					   quiet || agent_supported)) {
-				packet_buf_write(&req_buf,
-						 "%s %s %s%c%s%s%s%s%s",
-						 old_hex, new_hex, ref->name, 0,
-						 status_report ? " report-status" : "",
-						 use_sideband ? " side-band-64k" : "",
-						 quiet ? " quiet" : "",
-						 agent_supported ? " agent=" : "",
-						 agent_supported ? git_user_agent_sanitized() : ""
-						);
-			}
-			else
-				packet_buf_write(&req_buf, "%s %s %s",
-						 old_hex, new_hex, ref->name);
-			ref->status = status_report ?
-				REF_STATUS_EXPECTING_REPORT :
-				REF_STATUS_OK;
-			cmds_sent++;
-		}
-	}
-
-	if (args->stateless_rpc) {
-		if (!args->dry_run && cmds_sent) {
-			packet_buf_flush(&req_buf);
-			send_sideband(out, -1, req_buf.buf, req_buf.len, LARGE_PACKET_MAX);
-		}
-	} else {
-		safe_write(out, req_buf.buf, req_buf.len);
-		packet_flush(out);
-	}
-	strbuf_release(&req_buf);
-
-	if (use_sideband && cmds_sent) {
-		memset(&demux, 0, sizeof(demux));
-		demux.proc = sideband_demux;
-		demux.data = fd;
-		demux.out = -1;
-		if (start_async(&demux))
-			die("send-pack: unable to fork off sideband demultiplexer");
-		in = demux.out;
-	}
-
-	if (new_refs && cmds_sent) {
-		if (pack_objects(out, remote_refs, extra_have, args) < 0) {
-			for (ref = remote_refs; ref; ref = ref->next)
-				ref->status = REF_STATUS_NONE;
-			if (args->stateless_rpc)
-				close(out);
-			if (git_connection_is_socket(conn))
-				shutdown(fd[0], SHUT_WR);
-			if (use_sideband)
-				finish_async(&demux);
-			return -1;
-		}
-	}
-	if (args->stateless_rpc && cmds_sent)
-		packet_flush(out);
-
-	if (status_report && cmds_sent)
-		ret = receive_status(in, remote_refs);
-	else
-		ret = 0;
-	if (args->stateless_rpc)
-		packet_flush(out);
-
-	if (use_sideband && cmds_sent) {
-		if (finish_async(&demux)) {
-			error("error in sideband demultiplexer");
-			ret = -1;
-		}
-		close(demux.out);
-	}
-
-	if (ret < 0)
-		return ret;
-
-	if (args->porcelain)
-		return 0;
-
-	for (ref = remote_refs; ref; ref = ref->next) {
-		switch (ref->status) {
-		case REF_STATUS_NONE:
-		case REF_STATUS_UPTODATE:
-		case REF_STATUS_OK:
-			break;
-		default:
-			return -1;
-		}
-	}
-	return 0;
-}
-
 int cmd_send_pack(int argc, const char **argv, const char *prefix)
 {
 	int i, nr_refspecs = 0;
diff --git a/send-pack.c b/send-pack.c
new file mode 100644
index 0000000..f50dfd9
--- /dev/null
+++ b/send-pack.c
@@ -0,0 +1,344 @@
+#include "builtin.h"
+#include "commit.h"
+#include "refs.h"
+#include "pkt-line.h"
+#include "sideband.h"
+#include "run-command.h"
+#include "remote.h"
+#include "send-pack.h"
+#include "quote.h"
+#include "transport.h"
+#include "version.h"
+
+static int feed_object(const unsigned char *sha1, int fd, int negative)
+{
+	char buf[42];
+
+	if (negative && !has_sha1_file(sha1))
+		return 1;
+
+	memcpy(buf + negative, sha1_to_hex(sha1), 40);
+	if (negative)
+		buf[0] = '^';
+	buf[40 + negative] = '\n';
+	return write_or_whine(fd, buf, 41 + negative, "send-pack: send refs");
+}
+
+/*
+ * Make a pack stream and spit it out into file descriptor fd
+ */
+static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *extra, struct send_pack_args *args)
+{
+	/*
+	 * The child becomes pack-objects --revs; we feed
+	 * the revision parameters to it via its stdin and
+	 * let its stdout go back to the other end.
+	 */
+	const char *argv[] = {
+		"pack-objects",
+		"--all-progress-implied",
+		"--revs",
+		"--stdout",
+		NULL,
+		NULL,
+		NULL,
+		NULL,
+		NULL,
+	};
+	struct child_process po;
+	int i;
+
+	i = 4;
+	if (args->use_thin_pack)
+		argv[i++] = "--thin";
+	if (args->use_ofs_delta)
+		argv[i++] = "--delta-base-offset";
+	if (args->quiet || !args->progress)
+		argv[i++] = "-q";
+	if (args->progress)
+		argv[i++] = "--progress";
+	memset(&po, 0, sizeof(po));
+	po.argv = argv;
+	po.in = -1;
+	po.out = args->stateless_rpc ? -1 : fd;
+	po.git_cmd = 1;
+	if (start_command(&po))
+		die_errno("git pack-objects failed");
+
+	/*
+	 * We feed the pack-objects we just spawned with revision
+	 * parameters by writing to the pipe.
+	 */
+	for (i = 0; i < extra->nr; i++)
+		if (!feed_object(extra->array[i], po.in, 1))
+			break;
+
+	while (refs) {
+		if (!is_null_sha1(refs->old_sha1) &&
+		    !feed_object(refs->old_sha1, po.in, 1))
+			break;
+		if (!is_null_sha1(refs->new_sha1) &&
+		    !feed_object(refs->new_sha1, po.in, 0))
+			break;
+		refs = refs->next;
+	}
+
+	close(po.in);
+
+	if (args->stateless_rpc) {
+		char *buf = xmalloc(LARGE_PACKET_MAX);
+		while (1) {
+			ssize_t n = xread(po.out, buf, LARGE_PACKET_MAX);
+			if (n <= 0)
+				break;
+			send_sideband(fd, -1, buf, n, LARGE_PACKET_MAX);
+		}
+		free(buf);
+		close(po.out);
+		po.out = -1;
+	}
+
+	if (finish_command(&po))
+		return -1;
+	return 0;
+}
+
+static int receive_status(int in, struct ref *refs)
+{
+	struct ref *hint;
+	char line[1000];
+	int ret = 0;
+	int len = packet_read_line(in, line, sizeof(line));
+	if (len < 10 || memcmp(line, "unpack ", 7))
+		return error("did not receive remote status");
+	if (memcmp(line, "unpack ok\n", 10)) {
+		char *p = line + strlen(line) - 1;
+		if (*p == '\n')
+			*p = '\0';
+		error("unpack failed: %s", line + 7);
+		ret = -1;
+	}
+	hint = NULL;
+	while (1) {
+		char *refname;
+		char *msg;
+		len = packet_read_line(in, line, sizeof(line));
+		if (!len)
+			break;
+		if (len < 3 ||
+		    (memcmp(line, "ok ", 3) && memcmp(line, "ng ", 3))) {
+			fprintf(stderr, "protocol error: %s\n", line);
+			ret = -1;
+			break;
+		}
+
+		line[strlen(line)-1] = '\0';
+		refname = line + 3;
+		msg = strchr(refname, ' ');
+		if (msg)
+			*msg++ = '\0';
+
+		/* first try searching at our hint, falling back to all refs */
+		if (hint)
+			hint = find_ref_by_name(hint, refname);
+		if (!hint)
+			hint = find_ref_by_name(refs, refname);
+		if (!hint) {
+			warning("remote reported status on unknown ref: %s",
+					refname);
+			continue;
+		}
+		if (hint->status != REF_STATUS_EXPECTING_REPORT) {
+			warning("remote reported status on unexpected ref: %s",
+					refname);
+			continue;
+		}
+
+		if (line[0] == 'o' && line[1] == 'k')
+			hint->status = REF_STATUS_OK;
+		else {
+			hint->status = REF_STATUS_REMOTE_REJECT;
+			ret = -1;
+		}
+		if (msg)
+			hint->remote_status = xstrdup(msg);
+		/* start our next search from the next ref */
+		hint = hint->next;
+	}
+	return ret;
+}
+
+static int sideband_demux(int in, int out, void *data)
+{
+	int *fd = data, ret;
+#ifdef NO_PTHREADS
+	close(fd[1]);
+#endif
+	ret = recv_sideband("send-pack", fd[0], out);
+	close(out);
+	return ret;
+}
+
+int send_pack(struct send_pack_args *args,
+	      int fd[], struct child_process *conn,
+	      struct ref *remote_refs,
+	      struct extra_have_objects *extra_have)
+{
+	int in = fd[0];
+	int out = fd[1];
+	struct strbuf req_buf = STRBUF_INIT;
+	struct ref *ref;
+	int new_refs;
+	int allow_deleting_refs = 0;
+	int status_report = 0;
+	int use_sideband = 0;
+	int quiet_supported = 0;
+	int agent_supported = 0;
+	unsigned cmds_sent = 0;
+	int ret;
+	struct async demux;
+
+	/* Does the other end support the reporting? */
+	if (server_supports("report-status"))
+		status_report = 1;
+	if (server_supports("delete-refs"))
+		allow_deleting_refs = 1;
+	if (server_supports("ofs-delta"))
+		args->use_ofs_delta = 1;
+	if (server_supports("side-band-64k"))
+		use_sideband = 1;
+	if (server_supports("quiet"))
+		quiet_supported = 1;
+	if (server_supports("agent"))
+		agent_supported = 1;
+
+	if (!remote_refs) {
+		fprintf(stderr, "No refs in common and none specified; doing nothing.\n"
+			"Perhaps you should specify a branch such as 'master'.\n");
+		return 0;
+	}
+
+	/*
+	 * Finally, tell the other end!
+	 */
+	new_refs = 0;
+	for (ref = remote_refs; ref; ref = ref->next) {
+		if (!ref->peer_ref && !args->send_mirror)
+			continue;
+
+		/* Check for statuses set by set_ref_status_for_push() */
+		switch (ref->status) {
+		case REF_STATUS_REJECT_NONFASTFORWARD:
+		case REF_STATUS_UPTODATE:
+			continue;
+		default:
+			; /* do nothing */
+		}
+
+		if (ref->deletion && !allow_deleting_refs) {
+			ref->status = REF_STATUS_REJECT_NODELETE;
+			continue;
+		}
+
+		if (!ref->deletion)
+			new_refs++;
+
+		if (args->dry_run) {
+			ref->status = REF_STATUS_OK;
+		} else {
+			char *old_hex = sha1_to_hex(ref->old_sha1);
+			char *new_hex = sha1_to_hex(ref->new_sha1);
+			int quiet = quiet_supported && (args->quiet || !args->progress);
+
+			if (!cmds_sent && (status_report || use_sideband ||
+					   quiet || agent_supported)) {
+				packet_buf_write(&req_buf,
+						 "%s %s %s%c%s%s%s%s%s",
+						 old_hex, new_hex, ref->name, 0,
+						 status_report ? " report-status" : "",
+						 use_sideband ? " side-band-64k" : "",
+						 quiet ? " quiet" : "",
+						 agent_supported ? " agent=" : "",
+						 agent_supported ? git_user_agent_sanitized() : ""
+						);
+			}
+			else
+				packet_buf_write(&req_buf, "%s %s %s",
+						 old_hex, new_hex, ref->name);
+			ref->status = status_report ?
+				REF_STATUS_EXPECTING_REPORT :
+				REF_STATUS_OK;
+			cmds_sent++;
+		}
+	}
+
+	if (args->stateless_rpc) {
+		if (!args->dry_run && cmds_sent) {
+			packet_buf_flush(&req_buf);
+			send_sideband(out, -1, req_buf.buf, req_buf.len, LARGE_PACKET_MAX);
+		}
+	} else {
+		safe_write(out, req_buf.buf, req_buf.len);
+		packet_flush(out);
+	}
+	strbuf_release(&req_buf);
+
+	if (use_sideband && cmds_sent) {
+		memset(&demux, 0, sizeof(demux));
+		demux.proc = sideband_demux;
+		demux.data = fd;
+		demux.out = -1;
+		if (start_async(&demux))
+			die("send-pack: unable to fork off sideband demultiplexer");
+		in = demux.out;
+	}
+
+	if (new_refs && cmds_sent) {
+		if (pack_objects(out, remote_refs, extra_have, args) < 0) {
+			for (ref = remote_refs; ref; ref = ref->next)
+				ref->status = REF_STATUS_NONE;
+			if (args->stateless_rpc)
+				close(out);
+			if (git_connection_is_socket(conn))
+				shutdown(fd[0], SHUT_WR);
+			if (use_sideband)
+				finish_async(&demux);
+			return -1;
+		}
+	}
+	if (args->stateless_rpc && cmds_sent)
+		packet_flush(out);
+
+	if (status_report && cmds_sent)
+		ret = receive_status(in, remote_refs);
+	else
+		ret = 0;
+	if (args->stateless_rpc)
+		packet_flush(out);
+
+	if (use_sideband && cmds_sent) {
+		if (finish_async(&demux)) {
+			error("error in sideband demultiplexer");
+			ret = -1;
+		}
+		close(demux.out);
+	}
+
+	if (ret < 0)
+		return ret;
+
+	if (args->porcelain)
+		return 0;
+
+	for (ref = remote_refs; ref; ref = ref->next) {
+		switch (ref->status) {
+		case REF_STATUS_NONE:
+		case REF_STATUS_UPTODATE:
+		case REF_STATUS_OK:
+			break;
+		default:
+			return -1;
+		}
+	}
+	return 0;
+}
diff --git a/transport.h b/transport.h
index 3b21c4a..4a61c0c 100644
--- a/transport.h
+++ b/transport.h
@@ -175,4 +175,9 @@ void transport_print_push_status(const char *dest, struct ref *refs,
 typedef void alternate_ref_fn(const struct ref *, void *);
 extern void for_each_alternate_ref(alternate_ref_fn, void *);
 
+struct send_pack_args;
+extern int send_pack(struct send_pack_args *args,
+		     int fd[], struct child_process *conn,
+		     struct ref *remote_refs,
+		     struct extra_have_objects *extra_have);
 #endif
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH 6/7] fetch-pack: remove global (static) configuration variable "args"
From: Nguyễn Thái Ngọc Duy @ 2012-10-26 15:53 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1351266835-22610-1-git-send-email-pclouds@gmail.com>

This helps removes the hack in fetch_pack() that copies my_args to args.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/fetch-pack.c | 160 ++++++++++++++++++++++++++-------------------------
 1 file changed, 83 insertions(+), 77 deletions(-)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index e644398..d0fd707 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -20,9 +20,6 @@ static int no_done;
 static int fetch_fsck_objects = -1;
 static int transfer_fsck_objects = -1;
 static int agent_supported;
-static struct fetch_pack_args args = {
-	/* .uploadpack = */ "git-upload-pack",
-};
 
 static const char fetch_pack_usage[] =
 "git fetch-pack [--all] [--stdin] [--quiet|-q] [--keep|-k] [--thin] "
@@ -173,9 +170,9 @@ enum ack_type {
 	ACK_ready
 };
 
-static void consume_shallow_list(int fd)
+static void consume_shallow_list(struct fetch_pack_args *args, int fd)
 {
-	if (args.stateless_rpc && args.depth > 0) {
+	if (args->stateless_rpc && args->depth > 0) {
 		/* If we sent a depth we will get back "duplicate"
 		 * shallow and unshallow commands every time there
 		 * is a block of have lines exchanged.
@@ -246,9 +243,10 @@ static enum ack_type get_ack(int fd, unsigned char *result_sha1)
 	die("git fetch_pack: expected ACK/NAK, got '%s'", line);
 }
 
-static void send_request(int fd, struct strbuf *buf)
+static void send_request(struct fetch_pack_args *args,
+			 int fd, struct strbuf *buf)
 {
-	if (args.stateless_rpc) {
+	if (args->stateless_rpc) {
 		send_sideband(fd, -1, buf->buf, buf->len, LARGE_PACKET_MAX);
 		packet_flush(fd);
 	} else
@@ -264,9 +262,9 @@ static void insert_one_alternate_ref(const struct ref *ref, void *unused)
 #define PIPESAFE_FLUSH 32
 #define LARGE_FLUSH 1024
 
-static int next_flush(int count)
+static int next_flush(struct fetch_pack_args *args, int count)
 {
-	int flush_limit = args.stateless_rpc ? LARGE_FLUSH : PIPESAFE_FLUSH;
+	int flush_limit = args->stateless_rpc ? LARGE_FLUSH : PIPESAFE_FLUSH;
 
 	if (count < flush_limit)
 		count <<= 1;
@@ -275,7 +273,8 @@ static int next_flush(int count)
 	return count;
 }
 
-static int find_common(int fd[2], unsigned char *result_sha1,
+static int find_common(struct fetch_pack_args *args,
+		       int fd[2], unsigned char *result_sha1,
 		       struct ref *refs)
 {
 	int fetching;
@@ -287,7 +286,7 @@ static int find_common(int fd[2], unsigned char *result_sha1,
 	struct strbuf req_buf = STRBUF_INIT;
 	size_t state_len = 0;
 
-	if (args.stateless_rpc && multi_ack == 1)
+	if (args->stateless_rpc && multi_ack == 1)
 		die("--stateless-rpc requires multi_ack_detailed");
 	if (marked)
 		for_each_ref(clear_marks, NULL);
@@ -325,9 +324,9 @@ static int find_common(int fd[2], unsigned char *result_sha1,
 			if (no_done)            strbuf_addstr(&c, " no-done");
 			if (use_sideband == 2)  strbuf_addstr(&c, " side-band-64k");
 			if (use_sideband == 1)  strbuf_addstr(&c, " side-band");
-			if (args.use_thin_pack) strbuf_addstr(&c, " thin-pack");
-			if (args.no_progress)   strbuf_addstr(&c, " no-progress");
-			if (args.include_tag)   strbuf_addstr(&c, " include-tag");
+			if (args->use_thin_pack) strbuf_addstr(&c, " thin-pack");
+			if (args->no_progress)   strbuf_addstr(&c, " no-progress");
+			if (args->include_tag)   strbuf_addstr(&c, " include-tag");
 			if (prefer_ofs_delta)   strbuf_addstr(&c, " ofs-delta");
 			if (agent_supported)    strbuf_addf(&c, " agent=%s",
 							    git_user_agent_sanitized());
@@ -346,16 +345,16 @@ static int find_common(int fd[2], unsigned char *result_sha1,
 
 	if (is_repository_shallow())
 		write_shallow_commits(&req_buf, 1);
-	if (args.depth > 0)
-		packet_buf_write(&req_buf, "deepen %d", args.depth);
+	if (args->depth > 0)
+		packet_buf_write(&req_buf, "deepen %d", args->depth);
 	packet_buf_flush(&req_buf);
 	state_len = req_buf.len;
 
-	if (args.depth > 0) {
+	if (args->depth > 0) {
 		char line[1024];
 		unsigned char sha1[20];
 
-		send_request(fd[1], &req_buf);
+		send_request(args, fd[1], &req_buf);
 		while (packet_read_line(fd[0], line, sizeof(line))) {
 			if (!prefixcmp(line, "shallow ")) {
 				if (get_sha1_hex(line + 8, sha1))
@@ -377,10 +376,10 @@ static int find_common(int fd[2], unsigned char *result_sha1,
 			}
 			die("expected shallow/unshallow, got %s", line);
 		}
-	} else if (!args.stateless_rpc)
-		send_request(fd[1], &req_buf);
+	} else if (!args->stateless_rpc)
+		send_request(args, fd[1], &req_buf);
 
-	if (!args.stateless_rpc) {
+	if (!args->stateless_rpc) {
 		/* If we aren't using the stateless-rpc interface
 		 * we don't need to retain the headers.
 		 */
@@ -392,29 +391,29 @@ static int find_common(int fd[2], unsigned char *result_sha1,
 	retval = -1;
 	while ((sha1 = get_rev())) {
 		packet_buf_write(&req_buf, "have %s\n", sha1_to_hex(sha1));
-		if (args.verbose)
+		if (args->verbose)
 			fprintf(stderr, "have %s\n", sha1_to_hex(sha1));
 		in_vain++;
 		if (flush_at <= ++count) {
 			int ack;
 
 			packet_buf_flush(&req_buf);
-			send_request(fd[1], &req_buf);
+			send_request(args, fd[1], &req_buf);
 			strbuf_setlen(&req_buf, state_len);
 			flushes++;
-			flush_at = next_flush(count);
+			flush_at = next_flush(args, count);
 
 			/*
 			 * We keep one window "ahead" of the other side, and
 			 * will wait for an ACK only on the next one
 			 */
-			if (!args.stateless_rpc && count == INITIAL_FLUSH)
+			if (!args->stateless_rpc && count == INITIAL_FLUSH)
 				continue;
 
-			consume_shallow_list(fd[0]);
+			consume_shallow_list(args, fd[0]);
 			do {
 				ack = get_ack(fd[0], result_sha1);
-				if (args.verbose && ack)
+				if (args->verbose && ack)
 					fprintf(stderr, "got ack %d %s\n", ack,
 							sha1_to_hex(result_sha1));
 				switch (ack) {
@@ -430,7 +429,7 @@ static int find_common(int fd[2], unsigned char *result_sha1,
 						lookup_commit(result_sha1);
 					if (!commit)
 						die("invalid commit %s", sha1_to_hex(result_sha1));
-					if (args.stateless_rpc
+					if (args->stateless_rpc
 					 && ack == ACK_common
 					 && !(commit->object.flags & COMMON)) {
 						/* We need to replay the have for this object
@@ -455,7 +454,7 @@ static int find_common(int fd[2], unsigned char *result_sha1,
 			} while (ack);
 			flushes--;
 			if (got_continue && MAX_IN_VAIN < in_vain) {
-				if (args.verbose)
+				if (args->verbose)
 					fprintf(stderr, "giving up\n");
 				break; /* give up */
 			}
@@ -464,9 +463,9 @@ static int find_common(int fd[2], unsigned char *result_sha1,
 done:
 	if (!got_ready || !no_done) {
 		packet_buf_write(&req_buf, "done\n");
-		send_request(fd[1], &req_buf);
+		send_request(args, fd[1], &req_buf);
 	}
-	if (args.verbose)
+	if (args->verbose)
 		fprintf(stderr, "done\n");
 	if (retval != 0) {
 		multi_ack = 0;
@@ -474,11 +473,11 @@ done:
 	}
 	strbuf_release(&req_buf);
 
-	consume_shallow_list(fd[0]);
+	consume_shallow_list(args, fd[0]);
 	while (flushes || multi_ack) {
 		int ack = get_ack(fd[0], result_sha1);
 		if (ack) {
-			if (args.verbose)
+			if (args->verbose)
 				fprintf(stderr, "got ack (%d) %s\n", ack,
 					sha1_to_hex(result_sha1));
 			if (ack == ACK)
@@ -515,10 +514,11 @@ static int mark_complete(const char *refname, const unsigned char *sha1, int fla
 	return 0;
 }
 
-static void mark_recent_complete_commits(unsigned long cutoff)
+static void mark_recent_complete_commits(struct fetch_pack_args *args,
+					 unsigned long cutoff)
 {
 	while (complete && cutoff <= complete->item->date) {
-		if (args.verbose)
+		if (args->verbose)
 			fprintf(stderr, "Marking %s as complete\n",
 				sha1_to_hex(complete->item->object.sha1));
 		pop_most_recent_commit(&complete, COMPLETE);
@@ -535,7 +535,8 @@ static int non_matching_ref(struct string_list_item *item, void *unused)
 		return 1;
 }
 
-static void filter_refs(struct ref **refs, struct string_list *sought)
+static void filter_refs(struct fetch_pack_args *args,
+			struct ref **refs, struct string_list *sought)
 {
 	struct ref *newlist = NULL;
 	struct ref **newtail = &newlist;
@@ -564,8 +565,8 @@ static void filter_refs(struct ref **refs, struct string_list *sought)
 			}
 		}
 
-		if (! keep && args.fetch_all &&
-		    (!args.depth || prefixcmp(ref->name, "refs/tags/")))
+		if (! keep && args->fetch_all &&
+		    (!args->depth || prefixcmp(ref->name, "refs/tags/")))
 			keep = 1;
 
 		if (keep) {
@@ -586,7 +587,8 @@ static void mark_alternate_complete(const struct ref *ref, void *unused)
 	mark_complete(NULL, ref->old_sha1, 0, NULL);
 }
 
-static int everything_local(struct ref **refs, struct string_list *sought)
+static int everything_local(struct fetch_pack_args *args,
+			    struct ref **refs, struct string_list *sought)
 {
 	struct ref *ref;
 	int retval;
@@ -612,11 +614,11 @@ static int everything_local(struct ref **refs, struct string_list *sought)
 		}
 	}
 
-	if (!args.depth) {
+	if (!args->depth) {
 		for_each_ref(mark_complete, NULL);
 		for_each_alternate_ref(mark_alternate_complete, NULL);
 		if (cutoff)
-			mark_recent_complete_commits(cutoff);
+			mark_recent_complete_commits(args, cutoff);
 	}
 
 	/*
@@ -637,7 +639,7 @@ static int everything_local(struct ref **refs, struct string_list *sought)
 		}
 	}
 
-	filter_refs(refs, sought);
+	filter_refs(args, refs, sought);
 
 	for (retval = 1, ref = *refs; ref ; ref = ref->next) {
 		const unsigned char *remote = ref->old_sha1;
@@ -647,7 +649,7 @@ static int everything_local(struct ref **refs, struct string_list *sought)
 		o = lookup_object(remote);
 		if (!o || !(o->flags & COMPLETE)) {
 			retval = 0;
-			if (!args.verbose)
+			if (!args->verbose)
 				continue;
 			fprintf(stderr,
 				"want %s (%s)\n", sha1_to_hex(remote),
@@ -656,7 +658,7 @@ static int everything_local(struct ref **refs, struct string_list *sought)
 		}
 
 		hashcpy(ref->new_sha1, local);
-		if (!args.verbose)
+		if (!args->verbose)
 			continue;
 		fprintf(stderr,
 			"already have %s (%s)\n", sha1_to_hex(remote),
@@ -674,14 +676,15 @@ static int sideband_demux(int in, int out, void *data)
 	return ret;
 }
 
-static int get_pack(int xd[2], char **pack_lockfile)
+static int get_pack(struct fetch_pack_args *args,
+		    int xd[2], char **pack_lockfile)
 {
 	struct async demux;
 	const char *argv[20];
 	char keep_arg[256];
 	char hdr_arg[256];
 	const char **av;
-	int do_keep = args.keep_pack;
+	int do_keep = args->keep_pack;
 	struct child_process cmd;
 
 	memset(&demux, 0, sizeof(demux));
@@ -704,7 +707,7 @@ static int get_pack(int xd[2], char **pack_lockfile)
 	cmd.argv = argv;
 	av = argv;
 	*hdr_arg = 0;
-	if (!args.keep_pack && unpack_limit) {
+	if (!args->keep_pack && unpack_limit) {
 		struct pack_header header;
 
 		if (read_pack_header(demux.out, &header))
@@ -723,11 +726,11 @@ static int get_pack(int xd[2], char **pack_lockfile)
 			cmd.out = -1;
 		*av++ = "index-pack";
 		*av++ = "--stdin";
-		if (!args.quiet && !args.no_progress)
+		if (!args->quiet && !args->no_progress)
 			*av++ = "-v";
-		if (args.use_thin_pack)
+		if (args->use_thin_pack)
 			*av++ = "--fix-thin";
-		if (args.lock_pack || unpack_limit) {
+		if (args->lock_pack || unpack_limit) {
 			int s = sprintf(keep_arg,
 					"--keep=fetch-pack %"PRIuMAX " on ", (uintmax_t) getpid());
 			if (gethostname(keep_arg + s, sizeof(keep_arg) - s))
@@ -737,7 +740,7 @@ static int get_pack(int xd[2], char **pack_lockfile)
 	}
 	else {
 		*av++ = "unpack-objects";
-		if (args.quiet || args.no_progress)
+		if (args->quiet || args->no_progress)
 			*av++ = "-q";
 	}
 	if (*hdr_arg)
@@ -766,10 +769,11 @@ static int get_pack(int xd[2], char **pack_lockfile)
 	return 0;
 }
 
-static struct ref *do_fetch_pack(int fd[2],
-		const struct ref *orig_ref,
-		struct string_list *sought,
-		char **pack_lockfile)
+static struct ref *do_fetch_pack(struct fetch_pack_args *args,
+				 int fd[2],
+				 const struct ref *orig_ref,
+				 struct string_list *sought,
+				 char **pack_lockfile)
 {
 	struct ref *ref = copy_ref_list(orig_ref);
 	unsigned char sha1[20];
@@ -781,64 +785,64 @@ static struct ref *do_fetch_pack(int fd[2],
 	if (is_repository_shallow() && !server_supports("shallow"))
 		die("Server does not support shallow clients");
 	if (server_supports("multi_ack_detailed")) {
-		if (args.verbose)
+		if (args->verbose)
 			fprintf(stderr, "Server supports multi_ack_detailed\n");
 		multi_ack = 2;
 		if (server_supports("no-done")) {
-			if (args.verbose)
+			if (args->verbose)
 				fprintf(stderr, "Server supports no-done\n");
-			if (args.stateless_rpc)
+			if (args->stateless_rpc)
 				no_done = 1;
 		}
 	}
 	else if (server_supports("multi_ack")) {
-		if (args.verbose)
+		if (args->verbose)
 			fprintf(stderr, "Server supports multi_ack\n");
 		multi_ack = 1;
 	}
 	if (server_supports("side-band-64k")) {
-		if (args.verbose)
+		if (args->verbose)
 			fprintf(stderr, "Server supports side-band-64k\n");
 		use_sideband = 2;
 	}
 	else if (server_supports("side-band")) {
-		if (args.verbose)
+		if (args->verbose)
 			fprintf(stderr, "Server supports side-band\n");
 		use_sideband = 1;
 	}
 	if (!server_supports("thin-pack"))
-		args.use_thin_pack = 0;
+		args->use_thin_pack = 0;
 	if (!server_supports("no-progress"))
-		args.no_progress = 0;
+		args->no_progress = 0;
 	if (!server_supports("include-tag"))
-		args.include_tag = 0;
+		args->include_tag = 0;
 	if (server_supports("ofs-delta")) {
-		if (args.verbose)
+		if (args->verbose)
 			fprintf(stderr, "Server supports ofs-delta\n");
 	} else
 		prefer_ofs_delta = 0;
 
 	if ((agent_feature = server_feature_value("agent", &agent_len))) {
 		agent_supported = 1;
-		if (args.verbose && agent_len)
+		if (args->verbose && agent_len)
 			fprintf(stderr, "Server version is %.*s\n",
 				agent_len, agent_feature);
 	}
 
-	if (everything_local(&ref, sought)) {
+	if (everything_local(args, &ref, sought)) {
 		packet_flush(fd[1]);
 		goto all_done;
 	}
-	if (find_common(fd, sha1, ref) < 0)
-		if (!args.keep_pack)
+	if (find_common(args, fd, sha1, ref) < 0)
+		if (!args->keep_pack)
 			/* When cloning, it is not unusual to have
 			 * no common commit.
 			 */
 			warning("no common commits");
 
-	if (args.stateless_rpc)
+	if (args->stateless_rpc)
 		packet_flush(fd[1]);
-	if (get_pack(fd, pack_lockfile))
+	if (get_pack(args, fd, pack_lockfile))
 		die("git fetch-pack: fetch failed.");
 
  all_done:
@@ -900,9 +904,13 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
 	char *pack_lockfile = NULL;
 	char **pack_lockfile_ptr = NULL;
 	struct child_process *conn;
+	struct fetch_pack_args args;
 
 	packet_trace_identity("fetch-pack");
 
+	memset(&args, 0, sizeof(args));
+	args.uploadpack = "git-upload-pack";
+
 	for (i = 1; i < argc && *argv[i] == '-'; i++) {
 		const char *arg = argv[i];
 
@@ -1039,7 +1047,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
 	return ret;
 }
 
-struct ref *fetch_pack(struct fetch_pack_args *my_args,
+struct ref *fetch_pack(struct fetch_pack_args *args,
 		       int fd[], struct child_process *conn,
 		       const struct ref *ref,
 		       const char *dest,
@@ -1050,9 +1058,7 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
 	struct ref *ref_cpy;
 
 	fetch_pack_setup();
-	if (&args != my_args)
-		memcpy(&args, my_args, sizeof(args));
-	if (args.depth > 0) {
+	if (args->depth > 0) {
 		if (stat(git_path("shallow"), &st))
 			st.st_mtime = 0;
 	}
@@ -1066,9 +1072,9 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
 		packet_flush(fd[1]);
 		die("no matching remote head");
 	}
-	ref_cpy = do_fetch_pack(fd, ref, sought, pack_lockfile);
+	ref_cpy = do_fetch_pack(args, fd, ref, sought, pack_lockfile);
 
-	if (args.depth > 0) {
+	if (args->depth > 0) {
 		struct cache_time mtime;
 		struct strbuf sb = STRBUF_INIT;
 		char *shallow = git_path("shallow");
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH 7/7] fetch-pack: move core code to libgit.a
From: Nguyễn Thái Ngọc Duy @ 2012-10-26 15:53 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1351266835-22610-1-git-send-email-pclouds@gmail.com>

fetch_pack() is used by transport.c, part of libgit.a while it stays
in builtin/fetch-pack.c. Move it to fetch-pack.c so that we won't get
undefined reference if a program that uses libgit.a happens to pull it
in.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Makefile             |   1 +
 builtin/fetch-pack.c | 949 --------------------------------------------------
 fetch-pack.c         | 951 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 952 insertions(+), 949 deletions(-)
 create mode 100644 fetch-pack.c

diff --git a/Makefile b/Makefile
index 414f15f..2997908 100644
--- a/Makefile
+++ b/Makefile
@@ -745,6 +745,7 @@ LIB_OBJS += editor.o
 LIB_OBJS += entry.o
 LIB_OBJS += environment.o
 LIB_OBJS += exec_cmd.o
+LIB_OBJS += fetch-pack.o
 LIB_OBJS += fsck.o
 LIB_OBJS += gettext.o
 LIB_OBJS += gpg-interface.o
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index d0fd707..570d26f 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -1,899 +1,11 @@
 #include "builtin.h"
-#include "refs.h"
-#include "pkt-line.h"
-#include "commit.h"
-#include "tag.h"
-#include "exec_cmd.h"
-#include "pack.h"
-#include "sideband.h"
 #include "fetch-pack.h"
-#include "remote.h"
-#include "run-command.h"
-#include "transport.h"
-#include "version.h"
-
-static int transfer_unpack_limit = -1;
-static int fetch_unpack_limit = -1;
-static int unpack_limit = 100;
-static int prefer_ofs_delta = 1;
-static int no_done;
-static int fetch_fsck_objects = -1;
-static int transfer_fsck_objects = -1;
-static int agent_supported;
 
 static const char fetch_pack_usage[] =
 "git fetch-pack [--all] [--stdin] [--quiet|-q] [--keep|-k] [--thin] "
 "[--include-tag] [--upload-pack=<git-upload-pack>] [--depth=<n>] "
 "[--no-progress] [-v] [<host>:]<directory> [<refs>...]";
 
-#define COMPLETE	(1U << 0)
-#define COMMON		(1U << 1)
-#define COMMON_REF	(1U << 2)
-#define SEEN		(1U << 3)
-#define POPPED		(1U << 4)
-
-static int marked;
-
-/*
- * After sending this many "have"s if we do not get any new ACK , we
- * give up traversing our history.
- */
-#define MAX_IN_VAIN 256
-
-static struct commit_list *rev_list;
-static int non_common_revs, multi_ack, use_sideband;
-
-static void rev_list_push(struct commit *commit, int mark)
-{
-	if (!(commit->object.flags & mark)) {
-		commit->object.flags |= mark;
-
-		if (!(commit->object.parsed))
-			if (parse_commit(commit))
-				return;
-
-		commit_list_insert_by_date(commit, &rev_list);
-
-		if (!(commit->object.flags & COMMON))
-			non_common_revs++;
-	}
-}
-
-static int rev_list_insert_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
-{
-	struct object *o = deref_tag(parse_object(sha1), refname, 0);
-
-	if (o && o->type == OBJ_COMMIT)
-		rev_list_push((struct commit *)o, SEEN);
-
-	return 0;
-}
-
-static int clear_marks(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
-{
-	struct object *o = deref_tag(parse_object(sha1), refname, 0);
-
-	if (o && o->type == OBJ_COMMIT)
-		clear_commit_marks((struct commit *)o,
-				   COMMON | COMMON_REF | SEEN | POPPED);
-	return 0;
-}
-
-/*
-   This function marks a rev and its ancestors as common.
-   In some cases, it is desirable to mark only the ancestors (for example
-   when only the server does not yet know that they are common).
-*/
-
-static void mark_common(struct commit *commit,
-		int ancestors_only, int dont_parse)
-{
-	if (commit != NULL && !(commit->object.flags & COMMON)) {
-		struct object *o = (struct object *)commit;
-
-		if (!ancestors_only)
-			o->flags |= COMMON;
-
-		if (!(o->flags & SEEN))
-			rev_list_push(commit, SEEN);
-		else {
-			struct commit_list *parents;
-
-			if (!ancestors_only && !(o->flags & POPPED))
-				non_common_revs--;
-			if (!o->parsed && !dont_parse)
-				if (parse_commit(commit))
-					return;
-
-			for (parents = commit->parents;
-					parents;
-					parents = parents->next)
-				mark_common(parents->item, 0, dont_parse);
-		}
-	}
-}
-
-/*
-  Get the next rev to send, ignoring the common.
-*/
-
-static const unsigned char *get_rev(void)
-{
-	struct commit *commit = NULL;
-
-	while (commit == NULL) {
-		unsigned int mark;
-		struct commit_list *parents;
-
-		if (rev_list == NULL || non_common_revs == 0)
-			return NULL;
-
-		commit = rev_list->item;
-		if (!commit->object.parsed)
-			parse_commit(commit);
-		parents = commit->parents;
-
-		commit->object.flags |= POPPED;
-		if (!(commit->object.flags & COMMON))
-			non_common_revs--;
-
-		if (commit->object.flags & COMMON) {
-			/* do not send "have", and ignore ancestors */
-			commit = NULL;
-			mark = COMMON | SEEN;
-		} else if (commit->object.flags & COMMON_REF)
-			/* send "have", and ignore ancestors */
-			mark = COMMON | SEEN;
-		else
-			/* send "have", also for its ancestors */
-			mark = SEEN;
-
-		while (parents) {
-			if (!(parents->item->object.flags & SEEN))
-				rev_list_push(parents->item, mark);
-			if (mark & COMMON)
-				mark_common(parents->item, 1, 0);
-			parents = parents->next;
-		}
-
-		rev_list = rev_list->next;
-	}
-
-	return commit->object.sha1;
-}
-
-enum ack_type {
-	NAK = 0,
-	ACK,
-	ACK_continue,
-	ACK_common,
-	ACK_ready
-};
-
-static void consume_shallow_list(struct fetch_pack_args *args, int fd)
-{
-	if (args->stateless_rpc && args->depth > 0) {
-		/* If we sent a depth we will get back "duplicate"
-		 * shallow and unshallow commands every time there
-		 * is a block of have lines exchanged.
-		 */
-		char line[1000];
-		while (packet_read_line(fd, line, sizeof(line))) {
-			if (!prefixcmp(line, "shallow "))
-				continue;
-			if (!prefixcmp(line, "unshallow "))
-				continue;
-			die("git fetch-pack: expected shallow list");
-		}
-	}
-}
-
-struct write_shallow_data {
-	struct strbuf *out;
-	int use_pack_protocol;
-	int count;
-};
-
-static int write_one_shallow(const struct commit_graft *graft, void *cb_data)
-{
-	struct write_shallow_data *data = cb_data;
-	const char *hex = sha1_to_hex(graft->sha1);
-	data->count++;
-	if (data->use_pack_protocol)
-		packet_buf_write(data->out, "shallow %s", hex);
-	else {
-		strbuf_addstr(data->out, hex);
-		strbuf_addch(data->out, '\n');
-	}
-	return 0;
-}
-
-static int write_shallow_commits(struct strbuf *out, int use_pack_protocol)
-{
-	struct write_shallow_data data;
-	data.out = out;
-	data.use_pack_protocol = use_pack_protocol;
-	data.count = 0;
-	for_each_commit_graft(write_one_shallow, &data);
-	return data.count;
-}
-
-static enum ack_type get_ack(int fd, unsigned char *result_sha1)
-{
-	static char line[1000];
-	int len = packet_read_line(fd, line, sizeof(line));
-
-	if (!len)
-		die("git fetch-pack: expected ACK/NAK, got EOF");
-	if (line[len-1] == '\n')
-		line[--len] = 0;
-	if (!strcmp(line, "NAK"))
-		return NAK;
-	if (!prefixcmp(line, "ACK ")) {
-		if (!get_sha1_hex(line+4, result_sha1)) {
-			if (strstr(line+45, "continue"))
-				return ACK_continue;
-			if (strstr(line+45, "common"))
-				return ACK_common;
-			if (strstr(line+45, "ready"))
-				return ACK_ready;
-			return ACK;
-		}
-	}
-	die("git fetch_pack: expected ACK/NAK, got '%s'", line);
-}
-
-static void send_request(struct fetch_pack_args *args,
-			 int fd, struct strbuf *buf)
-{
-	if (args->stateless_rpc) {
-		send_sideband(fd, -1, buf->buf, buf->len, LARGE_PACKET_MAX);
-		packet_flush(fd);
-	} else
-		safe_write(fd, buf->buf, buf->len);
-}
-
-static void insert_one_alternate_ref(const struct ref *ref, void *unused)
-{
-	rev_list_insert_ref(NULL, ref->old_sha1, 0, NULL);
-}
-
-#define INITIAL_FLUSH 16
-#define PIPESAFE_FLUSH 32
-#define LARGE_FLUSH 1024
-
-static int next_flush(struct fetch_pack_args *args, int count)
-{
-	int flush_limit = args->stateless_rpc ? LARGE_FLUSH : PIPESAFE_FLUSH;
-
-	if (count < flush_limit)
-		count <<= 1;
-	else
-		count += flush_limit;
-	return count;
-}
-
-static int find_common(struct fetch_pack_args *args,
-		       int fd[2], unsigned char *result_sha1,
-		       struct ref *refs)
-{
-	int fetching;
-	int count = 0, flushes = 0, flush_at = INITIAL_FLUSH, retval;
-	const unsigned char *sha1;
-	unsigned in_vain = 0;
-	int got_continue = 0;
-	int got_ready = 0;
-	struct strbuf req_buf = STRBUF_INIT;
-	size_t state_len = 0;
-
-	if (args->stateless_rpc && multi_ack == 1)
-		die("--stateless-rpc requires multi_ack_detailed");
-	if (marked)
-		for_each_ref(clear_marks, NULL);
-	marked = 1;
-
-	for_each_ref(rev_list_insert_ref, NULL);
-	for_each_alternate_ref(insert_one_alternate_ref, NULL);
-
-	fetching = 0;
-	for ( ; refs ; refs = refs->next) {
-		unsigned char *remote = refs->old_sha1;
-		const char *remote_hex;
-		struct object *o;
-
-		/*
-		 * If that object is complete (i.e. it is an ancestor of a
-		 * local ref), we tell them we have it but do not have to
-		 * tell them about its ancestors, which they already know
-		 * about.
-		 *
-		 * We use lookup_object here because we are only
-		 * interested in the case we *know* the object is
-		 * reachable and we have already scanned it.
-		 */
-		if (((o = lookup_object(remote)) != NULL) &&
-				(o->flags & COMPLETE)) {
-			continue;
-		}
-
-		remote_hex = sha1_to_hex(remote);
-		if (!fetching) {
-			struct strbuf c = STRBUF_INIT;
-			if (multi_ack == 2)     strbuf_addstr(&c, " multi_ack_detailed");
-			if (multi_ack == 1)     strbuf_addstr(&c, " multi_ack");
-			if (no_done)            strbuf_addstr(&c, " no-done");
-			if (use_sideband == 2)  strbuf_addstr(&c, " side-band-64k");
-			if (use_sideband == 1)  strbuf_addstr(&c, " side-band");
-			if (args->use_thin_pack) strbuf_addstr(&c, " thin-pack");
-			if (args->no_progress)   strbuf_addstr(&c, " no-progress");
-			if (args->include_tag)   strbuf_addstr(&c, " include-tag");
-			if (prefer_ofs_delta)   strbuf_addstr(&c, " ofs-delta");
-			if (agent_supported)    strbuf_addf(&c, " agent=%s",
-							    git_user_agent_sanitized());
-			packet_buf_write(&req_buf, "want %s%s\n", remote_hex, c.buf);
-			strbuf_release(&c);
-		} else
-			packet_buf_write(&req_buf, "want %s\n", remote_hex);
-		fetching++;
-	}
-
-	if (!fetching) {
-		strbuf_release(&req_buf);
-		packet_flush(fd[1]);
-		return 1;
-	}
-
-	if (is_repository_shallow())
-		write_shallow_commits(&req_buf, 1);
-	if (args->depth > 0)
-		packet_buf_write(&req_buf, "deepen %d", args->depth);
-	packet_buf_flush(&req_buf);
-	state_len = req_buf.len;
-
-	if (args->depth > 0) {
-		char line[1024];
-		unsigned char sha1[20];
-
-		send_request(args, fd[1], &req_buf);
-		while (packet_read_line(fd[0], line, sizeof(line))) {
-			if (!prefixcmp(line, "shallow ")) {
-				if (get_sha1_hex(line + 8, sha1))
-					die("invalid shallow line: %s", line);
-				register_shallow(sha1);
-				continue;
-			}
-			if (!prefixcmp(line, "unshallow ")) {
-				if (get_sha1_hex(line + 10, sha1))
-					die("invalid unshallow line: %s", line);
-				if (!lookup_object(sha1))
-					die("object not found: %s", line);
-				/* make sure that it is parsed as shallow */
-				if (!parse_object(sha1))
-					die("error in object: %s", line);
-				if (unregister_shallow(sha1))
-					die("no shallow found: %s", line);
-				continue;
-			}
-			die("expected shallow/unshallow, got %s", line);
-		}
-	} else if (!args->stateless_rpc)
-		send_request(args, fd[1], &req_buf);
-
-	if (!args->stateless_rpc) {
-		/* If we aren't using the stateless-rpc interface
-		 * we don't need to retain the headers.
-		 */
-		strbuf_setlen(&req_buf, 0);
-		state_len = 0;
-	}
-
-	flushes = 0;
-	retval = -1;
-	while ((sha1 = get_rev())) {
-		packet_buf_write(&req_buf, "have %s\n", sha1_to_hex(sha1));
-		if (args->verbose)
-			fprintf(stderr, "have %s\n", sha1_to_hex(sha1));
-		in_vain++;
-		if (flush_at <= ++count) {
-			int ack;
-
-			packet_buf_flush(&req_buf);
-			send_request(args, fd[1], &req_buf);
-			strbuf_setlen(&req_buf, state_len);
-			flushes++;
-			flush_at = next_flush(args, count);
-
-			/*
-			 * We keep one window "ahead" of the other side, and
-			 * will wait for an ACK only on the next one
-			 */
-			if (!args->stateless_rpc && count == INITIAL_FLUSH)
-				continue;
-
-			consume_shallow_list(args, fd[0]);
-			do {
-				ack = get_ack(fd[0], result_sha1);
-				if (args->verbose && ack)
-					fprintf(stderr, "got ack %d %s\n", ack,
-							sha1_to_hex(result_sha1));
-				switch (ack) {
-				case ACK:
-					flushes = 0;
-					multi_ack = 0;
-					retval = 0;
-					goto done;
-				case ACK_common:
-				case ACK_ready:
-				case ACK_continue: {
-					struct commit *commit =
-						lookup_commit(result_sha1);
-					if (!commit)
-						die("invalid commit %s", sha1_to_hex(result_sha1));
-					if (args->stateless_rpc
-					 && ack == ACK_common
-					 && !(commit->object.flags & COMMON)) {
-						/* We need to replay the have for this object
-						 * on the next RPC request so the peer knows
-						 * it is in common with us.
-						 */
-						const char *hex = sha1_to_hex(result_sha1);
-						packet_buf_write(&req_buf, "have %s\n", hex);
-						state_len = req_buf.len;
-					}
-					mark_common(commit, 0, 1);
-					retval = 0;
-					in_vain = 0;
-					got_continue = 1;
-					if (ack == ACK_ready) {
-						rev_list = NULL;
-						got_ready = 1;
-					}
-					break;
-					}
-				}
-			} while (ack);
-			flushes--;
-			if (got_continue && MAX_IN_VAIN < in_vain) {
-				if (args->verbose)
-					fprintf(stderr, "giving up\n");
-				break; /* give up */
-			}
-		}
-	}
-done:
-	if (!got_ready || !no_done) {
-		packet_buf_write(&req_buf, "done\n");
-		send_request(args, fd[1], &req_buf);
-	}
-	if (args->verbose)
-		fprintf(stderr, "done\n");
-	if (retval != 0) {
-		multi_ack = 0;
-		flushes++;
-	}
-	strbuf_release(&req_buf);
-
-	consume_shallow_list(args, fd[0]);
-	while (flushes || multi_ack) {
-		int ack = get_ack(fd[0], result_sha1);
-		if (ack) {
-			if (args->verbose)
-				fprintf(stderr, "got ack (%d) %s\n", ack,
-					sha1_to_hex(result_sha1));
-			if (ack == ACK)
-				return 0;
-			multi_ack = 1;
-			continue;
-		}
-		flushes--;
-	}
-	/* it is no error to fetch into a completely empty repo */
-	return count ? retval : 0;
-}
-
-static struct commit_list *complete;
-
-static int mark_complete(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
-{
-	struct object *o = parse_object(sha1);
-
-	while (o && o->type == OBJ_TAG) {
-		struct tag *t = (struct tag *) o;
-		if (!t->tagged)
-			break; /* broken repository */
-		o->flags |= COMPLETE;
-		o = parse_object(t->tagged->sha1);
-	}
-	if (o && o->type == OBJ_COMMIT) {
-		struct commit *commit = (struct commit *)o;
-		if (!(commit->object.flags & COMPLETE)) {
-			commit->object.flags |= COMPLETE;
-			commit_list_insert_by_date(commit, &complete);
-		}
-	}
-	return 0;
-}
-
-static void mark_recent_complete_commits(struct fetch_pack_args *args,
-					 unsigned long cutoff)
-{
-	while (complete && cutoff <= complete->item->date) {
-		if (args->verbose)
-			fprintf(stderr, "Marking %s as complete\n",
-				sha1_to_hex(complete->item->object.sha1));
-		pop_most_recent_commit(&complete, COMPLETE);
-	}
-}
-
-static int non_matching_ref(struct string_list_item *item, void *unused)
-{
-	if (item->util) {
-		item->util = NULL;
-		return 0;
-	}
-	else
-		return 1;
-}
-
-static void filter_refs(struct fetch_pack_args *args,
-			struct ref **refs, struct string_list *sought)
-{
-	struct ref *newlist = NULL;
-	struct ref **newtail = &newlist;
-	struct ref *ref, *next;
-	int sought_pos;
-
-	sought_pos = 0;
-	for (ref = *refs; ref; ref = next) {
-		int keep = 0;
-		next = ref->next;
-		if (!memcmp(ref->name, "refs/", 5) &&
-		    check_refname_format(ref->name + 5, 0))
-			; /* trash */
-		else {
-			while (sought_pos < sought->nr) {
-				int cmp = strcmp(ref->name, sought->items[sought_pos].string);
-				if (cmp < 0)
-					break; /* definitely do not have it */
-				else if (cmp == 0) {
-					keep = 1; /* definitely have it */
-					sought->items[sought_pos++].util = "matched";
-					break;
-				}
-				else
-					sought_pos++; /* might have it; keep looking */
-			}
-		}
-
-		if (! keep && args->fetch_all &&
-		    (!args->depth || prefixcmp(ref->name, "refs/tags/")))
-			keep = 1;
-
-		if (keep) {
-			*newtail = ref;
-			ref->next = NULL;
-			newtail = &ref->next;
-		} else {
-			free(ref);
-		}
-	}
-
-	filter_string_list(sought, 0, non_matching_ref, NULL);
-	*refs = newlist;
-}
-
-static void mark_alternate_complete(const struct ref *ref, void *unused)
-{
-	mark_complete(NULL, ref->old_sha1, 0, NULL);
-}
-
-static int everything_local(struct fetch_pack_args *args,
-			    struct ref **refs, struct string_list *sought)
-{
-	struct ref *ref;
-	int retval;
-	unsigned long cutoff = 0;
-
-	save_commit_buffer = 0;
-
-	for (ref = *refs; ref; ref = ref->next) {
-		struct object *o;
-
-		o = parse_object(ref->old_sha1);
-		if (!o)
-			continue;
-
-		/* We already have it -- which may mean that we were
-		 * in sync with the other side at some time after
-		 * that (it is OK if we guess wrong here).
-		 */
-		if (o->type == OBJ_COMMIT) {
-			struct commit *commit = (struct commit *)o;
-			if (!cutoff || cutoff < commit->date)
-				cutoff = commit->date;
-		}
-	}
-
-	if (!args->depth) {
-		for_each_ref(mark_complete, NULL);
-		for_each_alternate_ref(mark_alternate_complete, NULL);
-		if (cutoff)
-			mark_recent_complete_commits(args, cutoff);
-	}
-
-	/*
-	 * Mark all complete remote refs as common refs.
-	 * Don't mark them common yet; the server has to be told so first.
-	 */
-	for (ref = *refs; ref; ref = ref->next) {
-		struct object *o = deref_tag(lookup_object(ref->old_sha1),
-					     NULL, 0);
-
-		if (!o || o->type != OBJ_COMMIT || !(o->flags & COMPLETE))
-			continue;
-
-		if (!(o->flags & SEEN)) {
-			rev_list_push((struct commit *)o, COMMON_REF | SEEN);
-
-			mark_common((struct commit *)o, 1, 1);
-		}
-	}
-
-	filter_refs(args, refs, sought);
-
-	for (retval = 1, ref = *refs; ref ; ref = ref->next) {
-		const unsigned char *remote = ref->old_sha1;
-		unsigned char local[20];
-		struct object *o;
-
-		o = lookup_object(remote);
-		if (!o || !(o->flags & COMPLETE)) {
-			retval = 0;
-			if (!args->verbose)
-				continue;
-			fprintf(stderr,
-				"want %s (%s)\n", sha1_to_hex(remote),
-				ref->name);
-			continue;
-		}
-
-		hashcpy(ref->new_sha1, local);
-		if (!args->verbose)
-			continue;
-		fprintf(stderr,
-			"already have %s (%s)\n", sha1_to_hex(remote),
-			ref->name);
-	}
-	return retval;
-}
-
-static int sideband_demux(int in, int out, void *data)
-{
-	int *xd = data;
-
-	int ret = recv_sideband("fetch-pack", xd[0], out);
-	close(out);
-	return ret;
-}
-
-static int get_pack(struct fetch_pack_args *args,
-		    int xd[2], char **pack_lockfile)
-{
-	struct async demux;
-	const char *argv[20];
-	char keep_arg[256];
-	char hdr_arg[256];
-	const char **av;
-	int do_keep = args->keep_pack;
-	struct child_process cmd;
-
-	memset(&demux, 0, sizeof(demux));
-	if (use_sideband) {
-		/* xd[] is talking with upload-pack; subprocess reads from
-		 * xd[0], spits out band#2 to stderr, and feeds us band#1
-		 * through demux->out.
-		 */
-		demux.proc = sideband_demux;
-		demux.data = xd;
-		demux.out = -1;
-		if (start_async(&demux))
-			die("fetch-pack: unable to fork off sideband"
-			    " demultiplexer");
-	}
-	else
-		demux.out = xd[0];
-
-	memset(&cmd, 0, sizeof(cmd));
-	cmd.argv = argv;
-	av = argv;
-	*hdr_arg = 0;
-	if (!args->keep_pack && unpack_limit) {
-		struct pack_header header;
-
-		if (read_pack_header(demux.out, &header))
-			die("protocol error: bad pack header");
-		snprintf(hdr_arg, sizeof(hdr_arg),
-			 "--pack_header=%"PRIu32",%"PRIu32,
-			 ntohl(header.hdr_version), ntohl(header.hdr_entries));
-		if (ntohl(header.hdr_entries) < unpack_limit)
-			do_keep = 0;
-		else
-			do_keep = 1;
-	}
-
-	if (do_keep) {
-		if (pack_lockfile)
-			cmd.out = -1;
-		*av++ = "index-pack";
-		*av++ = "--stdin";
-		if (!args->quiet && !args->no_progress)
-			*av++ = "-v";
-		if (args->use_thin_pack)
-			*av++ = "--fix-thin";
-		if (args->lock_pack || unpack_limit) {
-			int s = sprintf(keep_arg,
-					"--keep=fetch-pack %"PRIuMAX " on ", (uintmax_t) getpid());
-			if (gethostname(keep_arg + s, sizeof(keep_arg) - s))
-				strcpy(keep_arg + s, "localhost");
-			*av++ = keep_arg;
-		}
-	}
-	else {
-		*av++ = "unpack-objects";
-		if (args->quiet || args->no_progress)
-			*av++ = "-q";
-	}
-	if (*hdr_arg)
-		*av++ = hdr_arg;
-	if (fetch_fsck_objects >= 0
-	    ? fetch_fsck_objects
-	    : transfer_fsck_objects >= 0
-	    ? transfer_fsck_objects
-	    : 0)
-		*av++ = "--strict";
-	*av++ = NULL;
-
-	cmd.in = demux.out;
-	cmd.git_cmd = 1;
-	if (start_command(&cmd))
-		die("fetch-pack: unable to fork off %s", argv[0]);
-	if (do_keep && pack_lockfile) {
-		*pack_lockfile = index_pack_lockfile(cmd.out);
-		close(cmd.out);
-	}
-
-	if (finish_command(&cmd))
-		die("%s failed", argv[0]);
-	if (use_sideband && finish_async(&demux))
-		die("error in sideband demultiplexer");
-	return 0;
-}
-
-static struct ref *do_fetch_pack(struct fetch_pack_args *args,
-				 int fd[2],
-				 const struct ref *orig_ref,
-				 struct string_list *sought,
-				 char **pack_lockfile)
-{
-	struct ref *ref = copy_ref_list(orig_ref);
-	unsigned char sha1[20];
-	const char *agent_feature;
-	int agent_len;
-
-	sort_ref_list(&ref, ref_compare_name);
-
-	if (is_repository_shallow() && !server_supports("shallow"))
-		die("Server does not support shallow clients");
-	if (server_supports("multi_ack_detailed")) {
-		if (args->verbose)
-			fprintf(stderr, "Server supports multi_ack_detailed\n");
-		multi_ack = 2;
-		if (server_supports("no-done")) {
-			if (args->verbose)
-				fprintf(stderr, "Server supports no-done\n");
-			if (args->stateless_rpc)
-				no_done = 1;
-		}
-	}
-	else if (server_supports("multi_ack")) {
-		if (args->verbose)
-			fprintf(stderr, "Server supports multi_ack\n");
-		multi_ack = 1;
-	}
-	if (server_supports("side-band-64k")) {
-		if (args->verbose)
-			fprintf(stderr, "Server supports side-band-64k\n");
-		use_sideband = 2;
-	}
-	else if (server_supports("side-band")) {
-		if (args->verbose)
-			fprintf(stderr, "Server supports side-band\n");
-		use_sideband = 1;
-	}
-	if (!server_supports("thin-pack"))
-		args->use_thin_pack = 0;
-	if (!server_supports("no-progress"))
-		args->no_progress = 0;
-	if (!server_supports("include-tag"))
-		args->include_tag = 0;
-	if (server_supports("ofs-delta")) {
-		if (args->verbose)
-			fprintf(stderr, "Server supports ofs-delta\n");
-	} else
-		prefer_ofs_delta = 0;
-
-	if ((agent_feature = server_feature_value("agent", &agent_len))) {
-		agent_supported = 1;
-		if (args->verbose && agent_len)
-			fprintf(stderr, "Server version is %.*s\n",
-				agent_len, agent_feature);
-	}
-
-	if (everything_local(args, &ref, sought)) {
-		packet_flush(fd[1]);
-		goto all_done;
-	}
-	if (find_common(args, fd, sha1, ref) < 0)
-		if (!args->keep_pack)
-			/* When cloning, it is not unusual to have
-			 * no common commit.
-			 */
-			warning("no common commits");
-
-	if (args->stateless_rpc)
-		packet_flush(fd[1]);
-	if (get_pack(args, fd, pack_lockfile))
-		die("git fetch-pack: fetch failed.");
-
- all_done:
-	return ref;
-}
-
-static int fetch_pack_config(const char *var, const char *value, void *cb)
-{
-	if (strcmp(var, "fetch.unpacklimit") == 0) {
-		fetch_unpack_limit = git_config_int(var, value);
-		return 0;
-	}
-
-	if (strcmp(var, "transfer.unpacklimit") == 0) {
-		transfer_unpack_limit = git_config_int(var, value);
-		return 0;
-	}
-
-	if (strcmp(var, "repack.usedeltabaseoffset") == 0) {
-		prefer_ofs_delta = git_config_bool(var, value);
-		return 0;
-	}
-
-	if (!strcmp(var, "fetch.fsckobjects")) {
-		fetch_fsck_objects = git_config_bool(var, value);
-		return 0;
-	}
-
-	if (!strcmp(var, "transfer.fsckobjects")) {
-		transfer_fsck_objects = git_config_bool(var, value);
-		return 0;
-	}
-
-	return git_default_config(var, value, cb);
-}
-
-static struct lock_file lock;
-
-static void fetch_pack_setup(void)
-{
-	static int did_setup;
-	if (did_setup)
-		return;
-	git_config(fetch_pack_config, NULL);
-	if (0 <= transfer_unpack_limit)
-		unpack_limit = transfer_unpack_limit;
-	else if (0 <= fetch_unpack_limit)
-		unpack_limit = fetch_unpack_limit;
-	did_setup = 1;
-}
-
 int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
 {
 	int i, ret;
@@ -1046,64 +158,3 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
 
 	return ret;
 }
-
-struct ref *fetch_pack(struct fetch_pack_args *args,
-		       int fd[], struct child_process *conn,
-		       const struct ref *ref,
-		       const char *dest,
-		       struct string_list *sought,
-		       char **pack_lockfile)
-{
-	struct stat st;
-	struct ref *ref_cpy;
-
-	fetch_pack_setup();
-	if (args->depth > 0) {
-		if (stat(git_path("shallow"), &st))
-			st.st_mtime = 0;
-	}
-
-	if (sought->nr) {
-		sort_string_list(sought);
-		string_list_remove_duplicates(sought, 0);
-	}
-
-	if (!ref) {
-		packet_flush(fd[1]);
-		die("no matching remote head");
-	}
-	ref_cpy = do_fetch_pack(args, fd, ref, sought, pack_lockfile);
-
-	if (args->depth > 0) {
-		struct cache_time mtime;
-		struct strbuf sb = STRBUF_INIT;
-		char *shallow = git_path("shallow");
-		int fd;
-
-		mtime.sec = st.st_mtime;
-		mtime.nsec = ST_MTIME_NSEC(st);
-		if (stat(shallow, &st)) {
-			if (mtime.sec)
-				die("shallow file was removed during fetch");
-		} else if (st.st_mtime != mtime.sec
-#ifdef USE_NSEC
-				|| ST_MTIME_NSEC(st) != mtime.nsec
-#endif
-			  )
-			die("shallow file was changed during fetch");
-
-		fd = hold_lock_file_for_update(&lock, shallow,
-					       LOCK_DIE_ON_ERROR);
-		if (!write_shallow_commits(&sb, 0)
-		 || write_in_full(fd, sb.buf, sb.len) != sb.len) {
-			unlink_or_warn(shallow);
-			rollback_lock_file(&lock);
-		} else {
-			commit_lock_file(&lock);
-		}
-		strbuf_release(&sb);
-	}
-
-	reprepare_packed_git();
-	return ref_cpy;
-}
diff --git a/fetch-pack.c b/fetch-pack.c
new file mode 100644
index 0000000..099ff4d
--- /dev/null
+++ b/fetch-pack.c
@@ -0,0 +1,951 @@
+#include "cache.h"
+#include "refs.h"
+#include "pkt-line.h"
+#include "commit.h"
+#include "tag.h"
+#include "exec_cmd.h"
+#include "pack.h"
+#include "sideband.h"
+#include "fetch-pack.h"
+#include "remote.h"
+#include "run-command.h"
+#include "transport.h"
+#include "version.h"
+
+static int transfer_unpack_limit = -1;
+static int fetch_unpack_limit = -1;
+static int unpack_limit = 100;
+static int prefer_ofs_delta = 1;
+static int no_done;
+static int fetch_fsck_objects = -1;
+static int transfer_fsck_objects = -1;
+static int agent_supported;
+
+#define COMPLETE	(1U << 0)
+#define COMMON		(1U << 1)
+#define COMMON_REF	(1U << 2)
+#define SEEN		(1U << 3)
+#define POPPED		(1U << 4)
+
+static int marked;
+
+/*
+ * After sending this many "have"s if we do not get any new ACK , we
+ * give up traversing our history.
+ */
+#define MAX_IN_VAIN 256
+
+static struct commit_list *rev_list;
+static int non_common_revs, multi_ack, use_sideband;
+
+static void rev_list_push(struct commit *commit, int mark)
+{
+	if (!(commit->object.flags & mark)) {
+		commit->object.flags |= mark;
+
+		if (!(commit->object.parsed))
+			if (parse_commit(commit))
+				return;
+
+		commit_list_insert_by_date(commit, &rev_list);
+
+		if (!(commit->object.flags & COMMON))
+			non_common_revs++;
+	}
+}
+
+static int rev_list_insert_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
+{
+	struct object *o = deref_tag(parse_object(sha1), refname, 0);
+
+	if (o && o->type == OBJ_COMMIT)
+		rev_list_push((struct commit *)o, SEEN);
+
+	return 0;
+}
+
+static int clear_marks(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
+{
+	struct object *o = deref_tag(parse_object(sha1), refname, 0);
+
+	if (o && o->type == OBJ_COMMIT)
+		clear_commit_marks((struct commit *)o,
+				   COMMON | COMMON_REF | SEEN | POPPED);
+	return 0;
+}
+
+/*
+   This function marks a rev and its ancestors as common.
+   In some cases, it is desirable to mark only the ancestors (for example
+   when only the server does not yet know that they are common).
+*/
+
+static void mark_common(struct commit *commit,
+		int ancestors_only, int dont_parse)
+{
+	if (commit != NULL && !(commit->object.flags & COMMON)) {
+		struct object *o = (struct object *)commit;
+
+		if (!ancestors_only)
+			o->flags |= COMMON;
+
+		if (!(o->flags & SEEN))
+			rev_list_push(commit, SEEN);
+		else {
+			struct commit_list *parents;
+
+			if (!ancestors_only && !(o->flags & POPPED))
+				non_common_revs--;
+			if (!o->parsed && !dont_parse)
+				if (parse_commit(commit))
+					return;
+
+			for (parents = commit->parents;
+					parents;
+					parents = parents->next)
+				mark_common(parents->item, 0, dont_parse);
+		}
+	}
+}
+
+/*
+  Get the next rev to send, ignoring the common.
+*/
+
+static const unsigned char *get_rev(void)
+{
+	struct commit *commit = NULL;
+
+	while (commit == NULL) {
+		unsigned int mark;
+		struct commit_list *parents;
+
+		if (rev_list == NULL || non_common_revs == 0)
+			return NULL;
+
+		commit = rev_list->item;
+		if (!commit->object.parsed)
+			parse_commit(commit);
+		parents = commit->parents;
+
+		commit->object.flags |= POPPED;
+		if (!(commit->object.flags & COMMON))
+			non_common_revs--;
+
+		if (commit->object.flags & COMMON) {
+			/* do not send "have", and ignore ancestors */
+			commit = NULL;
+			mark = COMMON | SEEN;
+		} else if (commit->object.flags & COMMON_REF)
+			/* send "have", and ignore ancestors */
+			mark = COMMON | SEEN;
+		else
+			/* send "have", also for its ancestors */
+			mark = SEEN;
+
+		while (parents) {
+			if (!(parents->item->object.flags & SEEN))
+				rev_list_push(parents->item, mark);
+			if (mark & COMMON)
+				mark_common(parents->item, 1, 0);
+			parents = parents->next;
+		}
+
+		rev_list = rev_list->next;
+	}
+
+	return commit->object.sha1;
+}
+
+enum ack_type {
+	NAK = 0,
+	ACK,
+	ACK_continue,
+	ACK_common,
+	ACK_ready
+};
+
+static void consume_shallow_list(struct fetch_pack_args *args, int fd)
+{
+	if (args->stateless_rpc && args->depth > 0) {
+		/* If we sent a depth we will get back "duplicate"
+		 * shallow and unshallow commands every time there
+		 * is a block of have lines exchanged.
+		 */
+		char line[1000];
+		while (packet_read_line(fd, line, sizeof(line))) {
+			if (!prefixcmp(line, "shallow "))
+				continue;
+			if (!prefixcmp(line, "unshallow "))
+				continue;
+			die("git fetch-pack: expected shallow list");
+		}
+	}
+}
+
+struct write_shallow_data {
+	struct strbuf *out;
+	int use_pack_protocol;
+	int count;
+};
+
+static int write_one_shallow(const struct commit_graft *graft, void *cb_data)
+{
+	struct write_shallow_data *data = cb_data;
+	const char *hex = sha1_to_hex(graft->sha1);
+	data->count++;
+	if (data->use_pack_protocol)
+		packet_buf_write(data->out, "shallow %s", hex);
+	else {
+		strbuf_addstr(data->out, hex);
+		strbuf_addch(data->out, '\n');
+	}
+	return 0;
+}
+
+static int write_shallow_commits(struct strbuf *out, int use_pack_protocol)
+{
+	struct write_shallow_data data;
+	data.out = out;
+	data.use_pack_protocol = use_pack_protocol;
+	data.count = 0;
+	for_each_commit_graft(write_one_shallow, &data);
+	return data.count;
+}
+
+static enum ack_type get_ack(int fd, unsigned char *result_sha1)
+{
+	static char line[1000];
+	int len = packet_read_line(fd, line, sizeof(line));
+
+	if (!len)
+		die("git fetch-pack: expected ACK/NAK, got EOF");
+	if (line[len-1] == '\n')
+		line[--len] = 0;
+	if (!strcmp(line, "NAK"))
+		return NAK;
+	if (!prefixcmp(line, "ACK ")) {
+		if (!get_sha1_hex(line+4, result_sha1)) {
+			if (strstr(line+45, "continue"))
+				return ACK_continue;
+			if (strstr(line+45, "common"))
+				return ACK_common;
+			if (strstr(line+45, "ready"))
+				return ACK_ready;
+			return ACK;
+		}
+	}
+	die("git fetch_pack: expected ACK/NAK, got '%s'", line);
+}
+
+static void send_request(struct fetch_pack_args *args,
+			 int fd, struct strbuf *buf)
+{
+	if (args->stateless_rpc) {
+		send_sideband(fd, -1, buf->buf, buf->len, LARGE_PACKET_MAX);
+		packet_flush(fd);
+	} else
+		safe_write(fd, buf->buf, buf->len);
+}
+
+static void insert_one_alternate_ref(const struct ref *ref, void *unused)
+{
+	rev_list_insert_ref(NULL, ref->old_sha1, 0, NULL);
+}
+
+#define INITIAL_FLUSH 16
+#define PIPESAFE_FLUSH 32
+#define LARGE_FLUSH 1024
+
+static int next_flush(struct fetch_pack_args *args, int count)
+{
+	int flush_limit = args->stateless_rpc ? LARGE_FLUSH : PIPESAFE_FLUSH;
+
+	if (count < flush_limit)
+		count <<= 1;
+	else
+		count += flush_limit;
+	return count;
+}
+
+static int find_common(struct fetch_pack_args *args,
+		       int fd[2], unsigned char *result_sha1,
+		       struct ref *refs)
+{
+	int fetching;
+	int count = 0, flushes = 0, flush_at = INITIAL_FLUSH, retval;
+	const unsigned char *sha1;
+	unsigned in_vain = 0;
+	int got_continue = 0;
+	int got_ready = 0;
+	struct strbuf req_buf = STRBUF_INIT;
+	size_t state_len = 0;
+
+	if (args->stateless_rpc && multi_ack == 1)
+		die("--stateless-rpc requires multi_ack_detailed");
+	if (marked)
+		for_each_ref(clear_marks, NULL);
+	marked = 1;
+
+	for_each_ref(rev_list_insert_ref, NULL);
+	for_each_alternate_ref(insert_one_alternate_ref, NULL);
+
+	fetching = 0;
+	for ( ; refs ; refs = refs->next) {
+		unsigned char *remote = refs->old_sha1;
+		const char *remote_hex;
+		struct object *o;
+
+		/*
+		 * If that object is complete (i.e. it is an ancestor of a
+		 * local ref), we tell them we have it but do not have to
+		 * tell them about its ancestors, which they already know
+		 * about.
+		 *
+		 * We use lookup_object here because we are only
+		 * interested in the case we *know* the object is
+		 * reachable and we have already scanned it.
+		 */
+		if (((o = lookup_object(remote)) != NULL) &&
+				(o->flags & COMPLETE)) {
+			continue;
+		}
+
+		remote_hex = sha1_to_hex(remote);
+		if (!fetching) {
+			struct strbuf c = STRBUF_INIT;
+			if (multi_ack == 2)     strbuf_addstr(&c, " multi_ack_detailed");
+			if (multi_ack == 1)     strbuf_addstr(&c, " multi_ack");
+			if (no_done)            strbuf_addstr(&c, " no-done");
+			if (use_sideband == 2)  strbuf_addstr(&c, " side-band-64k");
+			if (use_sideband == 1)  strbuf_addstr(&c, " side-band");
+			if (args->use_thin_pack) strbuf_addstr(&c, " thin-pack");
+			if (args->no_progress)   strbuf_addstr(&c, " no-progress");
+			if (args->include_tag)   strbuf_addstr(&c, " include-tag");
+			if (prefer_ofs_delta)   strbuf_addstr(&c, " ofs-delta");
+			if (agent_supported)    strbuf_addf(&c, " agent=%s",
+							    git_user_agent_sanitized());
+			packet_buf_write(&req_buf, "want %s%s\n", remote_hex, c.buf);
+			strbuf_release(&c);
+		} else
+			packet_buf_write(&req_buf, "want %s\n", remote_hex);
+		fetching++;
+	}
+
+	if (!fetching) {
+		strbuf_release(&req_buf);
+		packet_flush(fd[1]);
+		return 1;
+	}
+
+	if (is_repository_shallow())
+		write_shallow_commits(&req_buf, 1);
+	if (args->depth > 0)
+		packet_buf_write(&req_buf, "deepen %d", args->depth);
+	packet_buf_flush(&req_buf);
+	state_len = req_buf.len;
+
+	if (args->depth > 0) {
+		char line[1024];
+		unsigned char sha1[20];
+
+		send_request(args, fd[1], &req_buf);
+		while (packet_read_line(fd[0], line, sizeof(line))) {
+			if (!prefixcmp(line, "shallow ")) {
+				if (get_sha1_hex(line + 8, sha1))
+					die("invalid shallow line: %s", line);
+				register_shallow(sha1);
+				continue;
+			}
+			if (!prefixcmp(line, "unshallow ")) {
+				if (get_sha1_hex(line + 10, sha1))
+					die("invalid unshallow line: %s", line);
+				if (!lookup_object(sha1))
+					die("object not found: %s", line);
+				/* make sure that it is parsed as shallow */
+				if (!parse_object(sha1))
+					die("error in object: %s", line);
+				if (unregister_shallow(sha1))
+					die("no shallow found: %s", line);
+				continue;
+			}
+			die("expected shallow/unshallow, got %s", line);
+		}
+	} else if (!args->stateless_rpc)
+		send_request(args, fd[1], &req_buf);
+
+	if (!args->stateless_rpc) {
+		/* If we aren't using the stateless-rpc interface
+		 * we don't need to retain the headers.
+		 */
+		strbuf_setlen(&req_buf, 0);
+		state_len = 0;
+	}
+
+	flushes = 0;
+	retval = -1;
+	while ((sha1 = get_rev())) {
+		packet_buf_write(&req_buf, "have %s\n", sha1_to_hex(sha1));
+		if (args->verbose)
+			fprintf(stderr, "have %s\n", sha1_to_hex(sha1));
+		in_vain++;
+		if (flush_at <= ++count) {
+			int ack;
+
+			packet_buf_flush(&req_buf);
+			send_request(args, fd[1], &req_buf);
+			strbuf_setlen(&req_buf, state_len);
+			flushes++;
+			flush_at = next_flush(args, count);
+
+			/*
+			 * We keep one window "ahead" of the other side, and
+			 * will wait for an ACK only on the next one
+			 */
+			if (!args->stateless_rpc && count == INITIAL_FLUSH)
+				continue;
+
+			consume_shallow_list(args, fd[0]);
+			do {
+				ack = get_ack(fd[0], result_sha1);
+				if (args->verbose && ack)
+					fprintf(stderr, "got ack %d %s\n", ack,
+							sha1_to_hex(result_sha1));
+				switch (ack) {
+				case ACK:
+					flushes = 0;
+					multi_ack = 0;
+					retval = 0;
+					goto done;
+				case ACK_common:
+				case ACK_ready:
+				case ACK_continue: {
+					struct commit *commit =
+						lookup_commit(result_sha1);
+					if (!commit)
+						die("invalid commit %s", sha1_to_hex(result_sha1));
+					if (args->stateless_rpc
+					 && ack == ACK_common
+					 && !(commit->object.flags & COMMON)) {
+						/* We need to replay the have for this object
+						 * on the next RPC request so the peer knows
+						 * it is in common with us.
+						 */
+						const char *hex = sha1_to_hex(result_sha1);
+						packet_buf_write(&req_buf, "have %s\n", hex);
+						state_len = req_buf.len;
+					}
+					mark_common(commit, 0, 1);
+					retval = 0;
+					in_vain = 0;
+					got_continue = 1;
+					if (ack == ACK_ready) {
+						rev_list = NULL;
+						got_ready = 1;
+					}
+					break;
+					}
+				}
+			} while (ack);
+			flushes--;
+			if (got_continue && MAX_IN_VAIN < in_vain) {
+				if (args->verbose)
+					fprintf(stderr, "giving up\n");
+				break; /* give up */
+			}
+		}
+	}
+done:
+	if (!got_ready || !no_done) {
+		packet_buf_write(&req_buf, "done\n");
+		send_request(args, fd[1], &req_buf);
+	}
+	if (args->verbose)
+		fprintf(stderr, "done\n");
+	if (retval != 0) {
+		multi_ack = 0;
+		flushes++;
+	}
+	strbuf_release(&req_buf);
+
+	consume_shallow_list(args, fd[0]);
+	while (flushes || multi_ack) {
+		int ack = get_ack(fd[0], result_sha1);
+		if (ack) {
+			if (args->verbose)
+				fprintf(stderr, "got ack (%d) %s\n", ack,
+					sha1_to_hex(result_sha1));
+			if (ack == ACK)
+				return 0;
+			multi_ack = 1;
+			continue;
+		}
+		flushes--;
+	}
+	/* it is no error to fetch into a completely empty repo */
+	return count ? retval : 0;
+}
+
+static struct commit_list *complete;
+
+static int mark_complete(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
+{
+	struct object *o = parse_object(sha1);
+
+	while (o && o->type == OBJ_TAG) {
+		struct tag *t = (struct tag *) o;
+		if (!t->tagged)
+			break; /* broken repository */
+		o->flags |= COMPLETE;
+		o = parse_object(t->tagged->sha1);
+	}
+	if (o && o->type == OBJ_COMMIT) {
+		struct commit *commit = (struct commit *)o;
+		if (!(commit->object.flags & COMPLETE)) {
+			commit->object.flags |= COMPLETE;
+			commit_list_insert_by_date(commit, &complete);
+		}
+	}
+	return 0;
+}
+
+static void mark_recent_complete_commits(struct fetch_pack_args *args,
+					 unsigned long cutoff)
+{
+	while (complete && cutoff <= complete->item->date) {
+		if (args->verbose)
+			fprintf(stderr, "Marking %s as complete\n",
+				sha1_to_hex(complete->item->object.sha1));
+		pop_most_recent_commit(&complete, COMPLETE);
+	}
+}
+
+static int non_matching_ref(struct string_list_item *item, void *unused)
+{
+	if (item->util) {
+		item->util = NULL;
+		return 0;
+	}
+	else
+		return 1;
+}
+
+static void filter_refs(struct fetch_pack_args *args,
+			struct ref **refs, struct string_list *sought)
+{
+	struct ref *newlist = NULL;
+	struct ref **newtail = &newlist;
+	struct ref *ref, *next;
+	int sought_pos;
+
+	sought_pos = 0;
+	for (ref = *refs; ref; ref = next) {
+		int keep = 0;
+		next = ref->next;
+		if (!memcmp(ref->name, "refs/", 5) &&
+		    check_refname_format(ref->name + 5, 0))
+			; /* trash */
+		else {
+			while (sought_pos < sought->nr) {
+				int cmp = strcmp(ref->name, sought->items[sought_pos].string);
+				if (cmp < 0)
+					break; /* definitely do not have it */
+				else if (cmp == 0) {
+					keep = 1; /* definitely have it */
+					sought->items[sought_pos++].util = "matched";
+					break;
+				}
+				else
+					sought_pos++; /* might have it; keep looking */
+			}
+		}
+
+		if (! keep && args->fetch_all &&
+		    (!args->depth || prefixcmp(ref->name, "refs/tags/")))
+			keep = 1;
+
+		if (keep) {
+			*newtail = ref;
+			ref->next = NULL;
+			newtail = &ref->next;
+		} else {
+			free(ref);
+		}
+	}
+
+	filter_string_list(sought, 0, non_matching_ref, NULL);
+	*refs = newlist;
+}
+
+static void mark_alternate_complete(const struct ref *ref, void *unused)
+{
+	mark_complete(NULL, ref->old_sha1, 0, NULL);
+}
+
+static int everything_local(struct fetch_pack_args *args,
+			    struct ref **refs, struct string_list *sought)
+{
+	struct ref *ref;
+	int retval;
+	unsigned long cutoff = 0;
+
+	save_commit_buffer = 0;
+
+	for (ref = *refs; ref; ref = ref->next) {
+		struct object *o;
+
+		o = parse_object(ref->old_sha1);
+		if (!o)
+			continue;
+
+		/* We already have it -- which may mean that we were
+		 * in sync with the other side at some time after
+		 * that (it is OK if we guess wrong here).
+		 */
+		if (o->type == OBJ_COMMIT) {
+			struct commit *commit = (struct commit *)o;
+			if (!cutoff || cutoff < commit->date)
+				cutoff = commit->date;
+		}
+	}
+
+	if (!args->depth) {
+		for_each_ref(mark_complete, NULL);
+		for_each_alternate_ref(mark_alternate_complete, NULL);
+		if (cutoff)
+			mark_recent_complete_commits(args, cutoff);
+	}
+
+	/*
+	 * Mark all complete remote refs as common refs.
+	 * Don't mark them common yet; the server has to be told so first.
+	 */
+	for (ref = *refs; ref; ref = ref->next) {
+		struct object *o = deref_tag(lookup_object(ref->old_sha1),
+					     NULL, 0);
+
+		if (!o || o->type != OBJ_COMMIT || !(o->flags & COMPLETE))
+			continue;
+
+		if (!(o->flags & SEEN)) {
+			rev_list_push((struct commit *)o, COMMON_REF | SEEN);
+
+			mark_common((struct commit *)o, 1, 1);
+		}
+	}
+
+	filter_refs(args, refs, sought);
+
+	for (retval = 1, ref = *refs; ref ; ref = ref->next) {
+		const unsigned char *remote = ref->old_sha1;
+		unsigned char local[20];
+		struct object *o;
+
+		o = lookup_object(remote);
+		if (!o || !(o->flags & COMPLETE)) {
+			retval = 0;
+			if (!args->verbose)
+				continue;
+			fprintf(stderr,
+				"want %s (%s)\n", sha1_to_hex(remote),
+				ref->name);
+			continue;
+		}
+
+		hashcpy(ref->new_sha1, local);
+		if (!args->verbose)
+			continue;
+		fprintf(stderr,
+			"already have %s (%s)\n", sha1_to_hex(remote),
+			ref->name);
+	}
+	return retval;
+}
+
+static int sideband_demux(int in, int out, void *data)
+{
+	int *xd = data;
+
+	int ret = recv_sideband("fetch-pack", xd[0], out);
+	close(out);
+	return ret;
+}
+
+static int get_pack(struct fetch_pack_args *args,
+		    int xd[2], char **pack_lockfile)
+{
+	struct async demux;
+	const char *argv[20];
+	char keep_arg[256];
+	char hdr_arg[256];
+	const char **av;
+	int do_keep = args->keep_pack;
+	struct child_process cmd;
+
+	memset(&demux, 0, sizeof(demux));
+	if (use_sideband) {
+		/* xd[] is talking with upload-pack; subprocess reads from
+		 * xd[0], spits out band#2 to stderr, and feeds us band#1
+		 * through demux->out.
+		 */
+		demux.proc = sideband_demux;
+		demux.data = xd;
+		demux.out = -1;
+		if (start_async(&demux))
+			die("fetch-pack: unable to fork off sideband"
+			    " demultiplexer");
+	}
+	else
+		demux.out = xd[0];
+
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.argv = argv;
+	av = argv;
+	*hdr_arg = 0;
+	if (!args->keep_pack && unpack_limit) {
+		struct pack_header header;
+
+		if (read_pack_header(demux.out, &header))
+			die("protocol error: bad pack header");
+		snprintf(hdr_arg, sizeof(hdr_arg),
+			 "--pack_header=%"PRIu32",%"PRIu32,
+			 ntohl(header.hdr_version), ntohl(header.hdr_entries));
+		if (ntohl(header.hdr_entries) < unpack_limit)
+			do_keep = 0;
+		else
+			do_keep = 1;
+	}
+
+	if (do_keep) {
+		if (pack_lockfile)
+			cmd.out = -1;
+		*av++ = "index-pack";
+		*av++ = "--stdin";
+		if (!args->quiet && !args->no_progress)
+			*av++ = "-v";
+		if (args->use_thin_pack)
+			*av++ = "--fix-thin";
+		if (args->lock_pack || unpack_limit) {
+			int s = sprintf(keep_arg,
+					"--keep=fetch-pack %"PRIuMAX " on ", (uintmax_t) getpid());
+			if (gethostname(keep_arg + s, sizeof(keep_arg) - s))
+				strcpy(keep_arg + s, "localhost");
+			*av++ = keep_arg;
+		}
+	}
+	else {
+		*av++ = "unpack-objects";
+		if (args->quiet || args->no_progress)
+			*av++ = "-q";
+	}
+	if (*hdr_arg)
+		*av++ = hdr_arg;
+	if (fetch_fsck_objects >= 0
+	    ? fetch_fsck_objects
+	    : transfer_fsck_objects >= 0
+	    ? transfer_fsck_objects
+	    : 0)
+		*av++ = "--strict";
+	*av++ = NULL;
+
+	cmd.in = demux.out;
+	cmd.git_cmd = 1;
+	if (start_command(&cmd))
+		die("fetch-pack: unable to fork off %s", argv[0]);
+	if (do_keep && pack_lockfile) {
+		*pack_lockfile = index_pack_lockfile(cmd.out);
+		close(cmd.out);
+	}
+
+	if (finish_command(&cmd))
+		die("%s failed", argv[0]);
+	if (use_sideband && finish_async(&demux))
+		die("error in sideband demultiplexer");
+	return 0;
+}
+
+static struct ref *do_fetch_pack(struct fetch_pack_args *args,
+				 int fd[2],
+				 const struct ref *orig_ref,
+				 struct string_list *sought,
+				 char **pack_lockfile)
+{
+	struct ref *ref = copy_ref_list(orig_ref);
+	unsigned char sha1[20];
+	const char *agent_feature;
+	int agent_len;
+
+	sort_ref_list(&ref, ref_compare_name);
+
+	if (is_repository_shallow() && !server_supports("shallow"))
+		die("Server does not support shallow clients");
+	if (server_supports("multi_ack_detailed")) {
+		if (args->verbose)
+			fprintf(stderr, "Server supports multi_ack_detailed\n");
+		multi_ack = 2;
+		if (server_supports("no-done")) {
+			if (args->verbose)
+				fprintf(stderr, "Server supports no-done\n");
+			if (args->stateless_rpc)
+				no_done = 1;
+		}
+	}
+	else if (server_supports("multi_ack")) {
+		if (args->verbose)
+			fprintf(stderr, "Server supports multi_ack\n");
+		multi_ack = 1;
+	}
+	if (server_supports("side-band-64k")) {
+		if (args->verbose)
+			fprintf(stderr, "Server supports side-band-64k\n");
+		use_sideband = 2;
+	}
+	else if (server_supports("side-band")) {
+		if (args->verbose)
+			fprintf(stderr, "Server supports side-band\n");
+		use_sideband = 1;
+	}
+	if (!server_supports("thin-pack"))
+		args->use_thin_pack = 0;
+	if (!server_supports("no-progress"))
+		args->no_progress = 0;
+	if (!server_supports("include-tag"))
+		args->include_tag = 0;
+	if (server_supports("ofs-delta")) {
+		if (args->verbose)
+			fprintf(stderr, "Server supports ofs-delta\n");
+	} else
+		prefer_ofs_delta = 0;
+
+	if ((agent_feature = server_feature_value("agent", &agent_len))) {
+		agent_supported = 1;
+		if (args->verbose && agent_len)
+			fprintf(stderr, "Server version is %.*s\n",
+				agent_len, agent_feature);
+	}
+
+	if (everything_local(args, &ref, sought)) {
+		packet_flush(fd[1]);
+		goto all_done;
+	}
+	if (find_common(args, fd, sha1, ref) < 0)
+		if (!args->keep_pack)
+			/* When cloning, it is not unusual to have
+			 * no common commit.
+			 */
+			warning("no common commits");
+
+	if (args->stateless_rpc)
+		packet_flush(fd[1]);
+	if (get_pack(args, fd, pack_lockfile))
+		die("git fetch-pack: fetch failed.");
+
+ all_done:
+	return ref;
+}
+
+static int fetch_pack_config(const char *var, const char *value, void *cb)
+{
+	if (strcmp(var, "fetch.unpacklimit") == 0) {
+		fetch_unpack_limit = git_config_int(var, value);
+		return 0;
+	}
+
+	if (strcmp(var, "transfer.unpacklimit") == 0) {
+		transfer_unpack_limit = git_config_int(var, value);
+		return 0;
+	}
+
+	if (strcmp(var, "repack.usedeltabaseoffset") == 0) {
+		prefer_ofs_delta = git_config_bool(var, value);
+		return 0;
+	}
+
+	if (!strcmp(var, "fetch.fsckobjects")) {
+		fetch_fsck_objects = git_config_bool(var, value);
+		return 0;
+	}
+
+	if (!strcmp(var, "transfer.fsckobjects")) {
+		transfer_fsck_objects = git_config_bool(var, value);
+		return 0;
+	}
+
+	return git_default_config(var, value, cb);
+}
+
+static struct lock_file lock;
+
+static void fetch_pack_setup(void)
+{
+	static int did_setup;
+	if (did_setup)
+		return;
+	git_config(fetch_pack_config, NULL);
+	if (0 <= transfer_unpack_limit)
+		unpack_limit = transfer_unpack_limit;
+	else if (0 <= fetch_unpack_limit)
+		unpack_limit = fetch_unpack_limit;
+	did_setup = 1;
+}
+
+struct ref *fetch_pack(struct fetch_pack_args *args,
+		       int fd[], struct child_process *conn,
+		       const struct ref *ref,
+		       const char *dest,
+		       struct string_list *sought,
+		       char **pack_lockfile)
+{
+	struct stat st;
+	struct ref *ref_cpy;
+
+	fetch_pack_setup();
+	if (args->depth > 0) {
+		if (stat(git_path("shallow"), &st))
+			st.st_mtime = 0;
+	}
+
+	if (sought->nr) {
+		sort_string_list(sought);
+		string_list_remove_duplicates(sought, 0);
+	}
+
+	if (!ref) {
+		packet_flush(fd[1]);
+		die("no matching remote head");
+	}
+	ref_cpy = do_fetch_pack(args, fd, ref, sought, pack_lockfile);
+
+	if (args->depth > 0) {
+		struct cache_time mtime;
+		struct strbuf sb = STRBUF_INIT;
+		char *shallow = git_path("shallow");
+		int fd;
+
+		mtime.sec = st.st_mtime;
+		mtime.nsec = ST_MTIME_NSEC(st);
+		if (stat(shallow, &st)) {
+			if (mtime.sec)
+				die("shallow file was removed during fetch");
+		} else if (st.st_mtime != mtime.sec
+#ifdef USE_NSEC
+				|| ST_MTIME_NSEC(st) != mtime.nsec
+#endif
+			  )
+			die("shallow file was changed during fetch");
+
+		fd = hold_lock_file_for_update(&lock, shallow,
+					       LOCK_DIE_ON_ERROR);
+		if (!write_shallow_commits(&sb, 0)
+		 || write_in_full(fd, sb.buf, sb.len) != sb.len) {
+			unlink_or_warn(shallow);
+			rollback_lock_file(&lock);
+		} else {
+			commit_lock_file(&lock);
+		}
+		strbuf_release(&sb);
+	}
+
+	reprepare_packed_git();
+	return ref_cpy;
+}
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH] Fix t9200 on case insensitive file systems
From: Torsten Bögershausen @ 2012-10-26 16:18 UTC (permalink / raw)
  To: git; +Cc: bdwalton, bosch, brian, robin.rosenberg, tboegi

t9200 defines $CVSROOT where cvs should init its repository
$CVSROOT is set to $PWD/cvsroot.
cvs init is supposed to create the repository inside $PWD/cvsroot/CVSROOT

"cvs init" (e.g. version  1.11.23) checks if the last element of the path is
"CVSROOT", and if a directory with e.g. $PWD/cvsroot/CVSROOT already exists.

For such a $CVSROOT cvs refuses to init a repository here:
"Cannot initialize repository under existing CVSROOT:

On a case insenstive file system cvsroot and CVSROOT are the same directories
and t9200 fails.

Solution: use $PWD/tmp/cvsroot instead of cvsroot $PWD/cvsroot

Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
 t/t9200-git-cvsexportcommit.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh
index b59be9a..69934b2 100755
--- a/t/t9200-git-cvsexportcommit.sh
+++ b/t/t9200-git-cvsexportcommit.sh
@@ -19,7 +19,7 @@ then
     test_done
 fi
 
-CVSROOT=$PWD/cvsroot
+CVSROOT=$PWD/tmpcvsroot
 CVSWORK=$PWD/cvswork
 GIT_DIR=$PWD/.git
 export CVSROOT CVSWORK GIT_DIR
-- 
1.7.12

^ permalink raw reply related

* Re: Subtree in Git
From: James Nylen @ 2012-10-26 16:54 UTC (permalink / raw)
  To: David Michael Barr
  Cc: Herman van Rink, dag, Junio C Hamano, greened, Hilco Wijbenga,
	Git Users
In-Reply-To: <2DDAA35052EA4F88A6EAC4FBDDF7FCCD@rr-dav.id.au>

On Fri, Oct 26, 2012 at 9:58 AM, David Michael Barr <b@rr-dav.id.au> wrote:
> From a quick survey, it appears there are no more than 55 patches
> squashed into the submitted patch.
> As I have an interest in git-subtree for maintaining the out-of-tree
> version of vcs-svn/ and a desire to improve my rebase-fu, I am tempted
> to make some sense of the organic growth that happened on GitHub.
> It doesn't appear that anyone else is willing to do this, so I doubt
> there will be any duplication of effort.

David, I think that would be great.  I wish I had time to work on
unwrapping the current patch but I don't.  I will definitely re-submit
my (simple) patch after this is done though.

^ permalink raw reply

* Re: merge --no-commit not able to report stats more verbosely?
From: Scott R. Godin @ 2012-10-26 17:18 UTC (permalink / raw)
  To: git
In-Reply-To: <k63iai$kbp$1@ger.gmane.org>

Looks like I got lost in the press of other issues. anyone?

On 10/22/2012 09:39 AM, Scott R. Godin wrote:
> As you can see from the below, I can't seem to get it to give me more
> verbose results of what's being merged (as in the actual merge below)
> with --stat or -v .. is it supposed to do that?
> 
> (develop)>$ git merge --no-commit --stat -v widget_twitter
> Automatic merge went well; stopped before committing as requested
> (develop|MERGING)>$ git merge --abort
> 
> (develop)>$ git merge widget_twitter
> Merge made by the 'recursive' strategy.
>  .../code/community/Dnd/Magentweet/Model/User.php   |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> 
> (develop)>$ git --version
> git version 1.7.7.6
> 
> 
> 


-- 
(please respond to the list as opposed to my email box directly,
unless you are supplying private information you don't want public
on the list)

^ permalink raw reply

* Re: [PATCH 1/2] Teach --recursive to submodule sync
From: Phil Hord @ 2012-10-26 17:19 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: git, phil.hord, Jeff King
In-Reply-To: <5089BBE1.3040107@web.de>


Jens Lehmann wrote:
> Am 24.10.2012 01:15, schrieb Phil Hord:
>> The submodule sync command was somehow left out when
>> --recursive was added to the other submodule commands.
>>
>> Teach sync to handle the --recursive switch by recursing
>> when we're in a submodule we are sync'ing.
>>
>> Change the report during sync to show submodule-path
>> instead of submodule-name to be consistent with the other
>> submodule commands and to help recursed paths make sense.
>>
>> Signed-off-by: Phil Hord <hordp@cisco.com>
> This makes perfect sense to me. Two things though:
>
> First it would be nice to initialize orig_flags like all the other
> call sites do:
>
> @@ -1003,6 +1003,7 @@ cmd_status()
>  #
>  cmd_sync()
>  {
> +	orig_flags=
>  	while test $# -ne 0
>  	do
>  		case "$1" in
>
>> ---
>>  git-submodule.sh | 18 +++++++++++++++---
>>  1 file changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/git-submodule.sh b/git-submodule.sh
>> index ab6b110..6dd2338 100755
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
>> @@ -11,7 +11,7 @@ USAGE="[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <r
>>     or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
>>     or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
>>     or: $dashless [--quiet] foreach [--recursive] <command>
>> -   or: $dashless [--quiet] sync [--] [<path>...]"
>> +   or: $dashless [--quiet] sync [--recursive] [--] [<path>...]"
>>  OPTIONS_SPEC=
>>  . git-sh-setup
>>  . git-sh-i18n
>> @@ -1008,7 +1008,9 @@ cmd_sync()
>>  		case "$1" in
>>  		-q|--quiet)
>>  			GIT_QUIET=1
>> -			shift
>> +			;;
>> +		--recursive)
>> +			recursive=1
>>  			;;
>>  		--)
>>  			shift
>> @@ -1021,6 +1023,8 @@ cmd_sync()
>>  			break
>>  			;;
>>  		esac
>> +		orig_flags="$orig_flags $(git rev-parse --sq-quote "$1")"
>> +		shift
>>  	done
>>  	cd_to_toplevel
>>  	module_list "$@" |
>> @@ -1051,7 +1055,7 @@ cmd_sync()
>>  
>>  		if git config "submodule.$name.url" >/dev/null 2>/dev/null
>>  		then
>> -			say "$(eval_gettext "Synchronizing submodule url for '\$name'")"
>> +			say "$(eval_gettext "Synchronizing submodule url for '\$prefix\$sm_path'")"
>>  			git config submodule."$name".url "$super_config_url"
>>  
>>  			if test -e "$sm_path"/.git
>> @@ -1061,6 +1065,14 @@ cmd_sync()
>>  				cd "$sm_path"
>>  				remote=$(get_default_remote)
>>  				git config remote."$remote".url "$sub_origin_url"
>> +
>> +				if test -n "$recursive"
>> +				then
>> +				(
>> +					prefix="$prefix$sm_path/"
>> +					eval cmd_sync "$orig_args"
> This should read 'eval cmd_sync "$orig_flags"'. I think you copied that
> from cmd_status(), where this is also incorrect, I just sent a patch to
> correct that one.

Yes, thanks for catching that.  I think I should add a test for that
except I notice that sync doesn't take any other flags useful for passing.

v2 is on the way.

>> +				)
>> +			fi
>>  			)
>>  			fi
>>  		fi
>>

^ permalink raw reply

* [PATCHv2 0/2] Reroll submodule sync --recursive
From: Phil Hord @ 2012-10-26 17:31 UTC (permalink / raw)
  To: git; +Cc: phil.hord, Jens Lehmann, Jeff King
In-Reply-To: <5089BBE1.3040107@web.de>


This one fixes "$orig_flags" problems noticed by Jens.
Copy-and-paste is gift from Satan.

Phil

^ permalink raw reply

* [PATCHv2 1/2] Teach --recursive to submodule sync
From: Phil Hord @ 2012-10-26 17:31 UTC (permalink / raw)
  To: git; +Cc: phil.hord, Jens Lehmann, Jeff King, Phil Hord
In-Reply-To: <1351272691-24718-1-git-send-email-hordp@cisco.com>

The submodule sync command was somehow left out when
--recursive was added to the other submodule commands.

Teach sync to handle the --recursive switch by recursing
when we're in a submodule we are sync'ing.

Change the report during sync to show submodule-path
instead of submodule-name to be consistent with the other
submodule commands and to help recursed paths make sense.

Signed-off-by: Phil Hord <hordp@cisco.com>
---
 git-submodule.sh | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index ab6b110..f40da72 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -11,7 +11,7 @@ USAGE="[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <r
    or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
    or: $dashless [--quiet] foreach [--recursive] <command>
-   or: $dashless [--quiet] sync [--] [<path>...]"
+   or: $dashless [--quiet] sync [--recursive] [--] [<path>...]"
 OPTIONS_SPEC=
 . git-sh-setup
 . git-sh-i18n
@@ -1003,12 +1003,15 @@ cmd_status()
 #
 cmd_sync()
 {
+	orig_flags=
 	while test $# -ne 0
 	do
 		case "$1" in
 		-q|--quiet)
 			GIT_QUIET=1
-			shift
+			;;
+		--recursive)
+			recursive=1
 			;;
 		--)
 			shift
@@ -1021,6 +1024,8 @@ cmd_sync()
 			break
 			;;
 		esac
+		orig_flags="$orig_flags $(git rev-parse --sq-quote "$1")"
+		shift
 	done
 	cd_to_toplevel
 	module_list "$@" |
@@ -1051,7 +1056,7 @@ cmd_sync()
 
 		if git config "submodule.$name.url" >/dev/null 2>/dev/null
 		then
-			say "$(eval_gettext "Synchronizing submodule url for '\$name'")"
+			say "$(eval_gettext "Synchronizing submodule url for '\$prefix\$sm_path'")"
 			git config submodule."$name".url "$super_config_url"
 
 			if test -e "$sm_path"/.git
@@ -1061,6 +1066,14 @@ cmd_sync()
 				cd "$sm_path"
 				remote=$(get_default_remote)
 				git config remote."$remote".url "$sub_origin_url"
+
+				if test -n "$recursive"
+				then
+				(
+					prefix="$prefix$sm_path/"
+					eval cmd_sync "$orig_flags"
+				)
+			fi
 			)
 			fi
 		fi
-- 
1.8.0.2.g35128e8

^ permalink raw reply related

* [PATCHv2 2/2] Add tests for submodule sync --recursive
From: Phil Hord @ 2012-10-26 17:31 UTC (permalink / raw)
  To: git; +Cc: phil.hord, Jens Lehmann, Jeff King, Phil Hord
In-Reply-To: <1351272691-24718-1-git-send-email-hordp@cisco.com>

Signed-off-by: Phil Hord <hordp@cisco.com>
---
 t/t7403-submodule-sync.sh | 55 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 53 insertions(+), 2 deletions(-)

diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh
index 524d5c1..94e26c4 100755
--- a/t/t7403-submodule-sync.sh
+++ b/t/t7403-submodule-sync.sh
@@ -17,18 +17,25 @@ test_expect_success setup '
 	git commit -m upstream &&
 	git clone . super &&
 	git clone super submodule &&
+	(cd submodule &&
+	 git submodule add ../submodule sub-submodule &&
+	 test_tick &&
+	 git commit -m "sub-submodule"
+	) &&
 	(cd super &&
 	 git submodule add ../submodule submodule &&
 	 test_tick &&
 	 git commit -m "submodule"
 	) &&
 	git clone super super-clone &&
-	(cd super-clone && git submodule update --init) &&
+	(cd super-clone && git submodule update --init --recursive) &&
 	git clone super empty-clone &&
 	(cd empty-clone && git submodule init) &&
 	git clone super top-only-clone &&
 	git clone super relative-clone &&
-	(cd relative-clone && git submodule update --init)
+	(cd relative-clone && git submodule update --init --recursive) &&
+	git clone super recursive-clone &&
+	(cd recursive-clone && git submodule update --init --recursive)
 '
 
 test_expect_success 'change submodule' '
@@ -46,6 +53,11 @@ test_expect_success 'change submodule url' '
 	 git pull
 	) &&
 	mv submodule moved-submodule &&
+	(cd moved-submodule &&
+	 git config -f .gitmodules submodule.sub-submodule.url ../moved-submodule &&
+	 test_tick &&
+	 git commit -a -m moved-sub-submodule
+	) &&
 	(cd super &&
 	 git config -f .gitmodules submodule.submodule.url ../moved-submodule &&
 	 test_tick &&
@@ -61,6 +73,9 @@ test_expect_success '"git submodule sync" should update submodule URLs' '
 	test -d "$(cd super-clone/submodule &&
 	 git config remote.origin.url
 	)" &&
+	test ! -d "$(cd super-clone/submodule/sub-submodule &&
+	 git config remote.origin.url
+	)" &&
 	(cd super-clone/submodule &&
 	 git checkout master &&
 	 git pull
@@ -70,6 +85,25 @@ test_expect_success '"git submodule sync" should update submodule URLs' '
 	)
 '
 
+test_expect_success '"git submodule sync --recursive" should update all submodule URLs' '
+	(cd super-clone &&
+	 (cd submodule &&
+	  git pull --no-recurse-submodules
+	 ) &&
+	 git submodule sync --recursive
+	) &&
+	test -d "$(cd super-clone/submodule &&
+	 git config remote.origin.url
+	)" &&
+	test -d "$(cd super-clone/submodule/sub-submodule &&
+	 git config remote.origin.url
+	)" &&
+	(cd super-clone/submodule/sub-submodule &&
+	 git checkout master &&
+	 git pull
+	)
+'
+
 test_expect_success '"git submodule sync" should update known submodule URLs' '
 	(cd empty-clone &&
 	 git pull &&
@@ -107,6 +141,23 @@ test_expect_success '"git submodule sync" handles origin URL of the form foo/bar
 	 #actual foo/submodule
 	 test "$(git config remote.origin.url)" = "../foo/submodule"
 	)
+	(cd submodule/sub-submodule &&
+	 test "$(git config remote.origin.url)" != "../../foo/submodule"
+	)
+	)
+'
+
+test_expect_success '"git submodule sync --recursive" propagates changes in origin' '
+	(cd recursive-clone &&
+	 git remote set-url origin foo/bar &&
+	 git submodule sync --recursive &&
+	(cd submodule &&
+	 #actual foo/submodule
+	 test "$(git config remote.origin.url)" = "../foo/submodule"
+	)
+	(cd submodule/sub-submodule &&
+	 test "$(git config remote.origin.url)" = "../../foo/submodule"
+	)
 	)
 '
 
-- 
1.8.0.2.g35128e8

^ permalink raw reply related

* Re: [PATCH 1/2] Teach --recursive to submodule sync
From: Phil Hord @ 2012-10-26 17:55 UTC (permalink / raw)
  To: Phil Hord; +Cc: Jens Lehmann, git, Jeff King
In-Reply-To: <508AC63E.6010502@cisco.com>

On Fri, Oct 26, 2012 at 1:19 PM, Phil Hord <hordp@cisco.com> wrote:
>
> Yes, thanks for catching that.  I think I should add a test for that
> except I notice that sync doesn't take any other flags useful for passing.

Which, of course, suggests that I should not add this
flag-propagating-machinery to submodule-sync at all. yes?

Phil

^ permalink raw reply

* Re: git push tags
From: Drew Northup @ 2012-10-26 18:07 UTC (permalink / raw)
  To: Kacper Kornet; +Cc: Angelo Borsotti, git
In-Reply-To: <20121026174255.GE10560@camk.edu.pl>

On Fri, Oct 26, 2012 at 1:42 PM, Kacper Kornet <draenog@pld-linux.org> wrote:
> On Thu, Oct 25, 2012 at 05:16:00PM -0400, Drew Northup wrote:
>> On Thu, Oct 25, 2012 at 3:05 PM, Angelo Borsotti
>> <angelo.borsotti@gmail.com> wrote:
>> > Are remote repositories less protected than the local ones? I
>> > think that to be consistent, the same strategy should be used on all
>> > repositories, i.e. rejecting changes on tags by default, unless they
>> > are forced.
>
>> So here we come to the core argument. Is sounds to me like you want
>> changes to remote tags to work differently from push updates to ALL
>> other references. The required change, if I'm not mistaken, would be
>> for tags to not permit fast-forward updates while all other references
>> would be pushed normally. From my brief and un-enlightened look at the
>> push code I can't see that being as easy as it sounds.
>
> I think the patch below obtains the requested behaviour:
>
> diff --git a/remote.c b/remote.c
> index 04fd9ea..7fcb51e 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -1320,7 +1320,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
>                         !ref->deletion &&
>                         !is_null_sha1(ref->old_sha1) &&
>                         (!has_sha1_file(ref->old_sha1)
> -                         || !ref_newer(ref->new_sha1, ref->old_sha1));
> +                         || !prefixcmp(ref->name, "refs/tags") || !ref_newer(ref->new_sha1, ref->old_sha1));
>
>                 if (ref->nonfastforward && !ref->force && !force_update) {
>                         ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
>
> --
>   Kacper Kornet

Kacper,
I obviously didn't dig deep enough. In any case, I presume that's what
he's asking for. I can't remember if git is still forcing tags to
always be located in "refs/tags" however. I didn't think it was.

-- 
-Drew Northup
--------------------------------------------------------------
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59

^ permalink raw reply

* Re: git push tags
From: Kacper Kornet @ 2012-10-26 17:42 UTC (permalink / raw)
  To: Drew Northup; +Cc: Angelo Borsotti, git
In-Reply-To: <CAM9Z-nkosbe1NXYnu7x6v4seLqCnMBWg-jrdH2eJ9RetaZBTyQ@mail.gmail.com>

On Thu, Oct 25, 2012 at 05:16:00PM -0400, Drew Northup wrote:
> On Thu, Oct 25, 2012 at 3:05 PM, Angelo Borsotti
> <angelo.borsotti@gmail.com> wrote:
> > Are remote repositories less protected than the local ones? I
> > think that to be consistent, the same strategy should be used on all
> > repositories, i.e. rejecting changes on tags by default, unless they
> > are forced.

> So here we come to the core argument. Is sounds to me like you want
> changes to remote tags to work differently from push updates to ALL
> other references. The required change, if I'm not mistaken, would be
> for tags to not permit fast-forward updates while all other references
> would be pushed normally. From my brief and un-enlightened look at the
> push code I can't see that being as easy as it sounds.

I think the patch below obtains the requested behaviour:

diff --git a/remote.c b/remote.c
index 04fd9ea..7fcb51e 100644
--- a/remote.c
+++ b/remote.c
@@ -1320,7 +1320,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 			!ref->deletion &&
 			!is_null_sha1(ref->old_sha1) &&
 			(!has_sha1_file(ref->old_sha1)
-			  || !ref_newer(ref->new_sha1, ref->old_sha1));
+			  || !prefixcmp(ref->name, "refs/tags") || !ref_newer(ref->new_sha1, ref->old_sha1));
 
 		if (ref->nonfastforward && !ref->force && !force_update) {
 			ref->status = REF_STATUS_REJECT_NONFASTFORWARD;

-- 
  Kacper Kornet

^ permalink raw reply related

* Re: git push tags
From: Kacper Kornet @ 2012-10-26 18:20 UTC (permalink / raw)
  To: Drew Northup; +Cc: Angelo Borsotti, git
In-Reply-To: <CAM9Z-nkBO1dbF-sBFLuxM_S_MT79Cx=gLEL+83XKB7ys8VTqNQ@mail.gmail.com>

On Fri, Oct 26, 2012 at 02:07:09PM -0400, Drew Northup wrote:
> On Fri, Oct 26, 2012 at 1:42 PM, Kacper Kornet <draenog@pld-linux.org> wrote:
> > On Thu, Oct 25, 2012 at 05:16:00PM -0400, Drew Northup wrote:
> >> On Thu, Oct 25, 2012 at 3:05 PM, Angelo Borsotti
> >> <angelo.borsotti@gmail.com> wrote:
> >> > Are remote repositories less protected than the local ones? I
> >> > think that to be consistent, the same strategy should be used on all
> >> > repositories, i.e. rejecting changes on tags by default, unless they
> >> > are forced.

> >> So here we come to the core argument. Is sounds to me like you want
> >> changes to remote tags to work differently from push updates to ALL
> >> other references. The required change, if I'm not mistaken, would be
> >> for tags to not permit fast-forward updates while all other references
> >> would be pushed normally. From my brief and un-enlightened look at the
> >> push code I can't see that being as easy as it sounds.

> > I think the patch below obtains the requested behaviour:

> > diff --git a/remote.c b/remote.c
> > index 04fd9ea..7fcb51e 100644
> > --- a/remote.c
> > +++ b/remote.c
> > @@ -1320,7 +1320,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
> >                         !ref->deletion &&
> >                         !is_null_sha1(ref->old_sha1) &&
> >                         (!has_sha1_file(ref->old_sha1)
> > -                         || !ref_newer(ref->new_sha1, ref->old_sha1));
> > +                         || !prefixcmp(ref->name, "refs/tags") || !ref_newer(ref->new_sha1, ref->old_sha1));

> >                 if (ref->nonfastforward && !ref->force && !force_update) {
> >                         ref->status = REF_STATUS_REJECT_NONFASTFORWARD;

> > --
> >   Kacper Kornet

> Kacper,
> I obviously didn't dig deep enough. In any case, I presume that's what
> he's asking for. I can't remember if git is still forcing tags to
> always be located in "refs/tags" however. I didn't think it was.

I have based my assumption about location of tags on gitglossary:

tag
 A ref under refs/tags/ namespace that points to an object of an
 arbitrary type (typically a tag points to either a tag or a commit
 object).

-- 
  Kacper

^ permalink raw reply

* Re: git push tags
From: Angelo Borsotti @ 2012-10-26 18:35 UTC (permalink / raw)
  To: Kacper Kornet; +Cc: Drew Northup, git
In-Reply-To: <20121026182020.GF10560@camk.edu.pl>

Hello

Drew,

I made some further tests on git-push to see if it handled branches
and tags in the same way, and have discovered the following
differences:

    - git push origin --delete master
      remote: error: By default, deleting the current branch is denied

    - git push origin --delete vx            (where vx is a tag)
      ... accepted

This is consistent with what is done on the local repo: deleting the
current branch is disallowed, but deleting a tag is allowed (even when
HEAD points to it).
That means that git-push does not handle branches and tags exactly the same.

Kacper

thank you for the patch. To keep downward compatibility, the denial to
update tags should perhaps be enabled with some option.

-Angelo

^ permalink raw reply

* Re: [PATCH 1/2] Teach --recursive to submodule sync
From: Jens Lehmann @ 2012-10-26 18:50 UTC (permalink / raw)
  To: Phil Hord; +Cc: Phil Hord, git, Jeff King
In-Reply-To: <CABURp0q0uELTS4DQ=Kqfr+8Pr-KgLcGJaBY_kJE_AiO=BZu+Bw@mail.gmail.com>

Am 26.10.2012 19:55, schrieb Phil Hord:
> On Fri, Oct 26, 2012 at 1:19 PM, Phil Hord <hordp@cisco.com> wrote:
>>
>> Yes, thanks for catching that.  I think I should add a test for that
>> except I notice that sync doesn't take any other flags useful for passing.
> 
> Which, of course, suggests that I should not add this
> flag-propagating-machinery to submodule-sync at all. yes?

Nope, the new --recursive option has to be passed on!

To catch that bug in your test you'd need another submodule inside your
sub-submodule. The first level submodule is initialized by sync anyways,
the sub-submodule is initialized by the --recursive logic you added but
the sub-sub-submodule would not have been synced because the option was
dropped. I really can't blame you for not adding that third level of
submodules ;-)

^ permalink raw reply

* Re: git push tags
From: Kacper Kornet @ 2012-10-26 19:00 UTC (permalink / raw)
  To: Angelo Borsotti; +Cc: Drew Northup, git
In-Reply-To: <CAB9Jk9AR7vFBH6E7-hFabyD9XgRrF5PVZU-HtABS85wkwVTt+Q@mail.gmail.com>

On Fri, Oct 26, 2012 at 08:35:50PM +0200, Angelo Borsotti wrote:
> Hello

> Drew,

> I made some further tests on git-push to see if it handled branches
> and tags in the same way, and have discovered the following
> differences:

>     - git push origin --delete master
>       remote: error: By default, deleting the current branch is denied

>     - git push origin --delete vx            (where vx is a tag)
>       ... accepted

> This is consistent with what is done on the local repo: deleting the
> current branch is disallowed, but deleting a tag is allowed (even when
> HEAD points to it).
> That means that git-push does not handle branches and tags exactly the same.

> Kacper

> thank you for the patch. To keep downward compatibility, the denial to
> update tags should perhaps be enabled with some option.

You are probably right. The proper submission should also contain a
test. I have sent a crude patch to show that the behaviour asked by you
is possible to obtain.

I will try to prepare a formal submission patch, but I can't say how
long it will take me. So if you want to do it by yourself feel free.

-- 
  Kacper Kornet

^ permalink raw reply

* Re: [PATCH] submodule status: properly pass options with --recursive
From: Phil Hord @ 2012-10-26 19:07 UTC (permalink / raw)
  To: Jeff King; +Cc: Jens Lehmann, Git Mailing List, Junio C Hamano
In-Reply-To: <20121026131507.GA2747@sigill.intra.peff.net>

On Fri, Oct 26, 2012 at 9:15 AM, Jeff King <peff@peff.net> wrote:
> On Fri, Oct 26, 2012 at 12:20:29AM +0200, Jens Lehmann wrote:
>
>> When renaming orig_args to orig_flags in 98dbe63d (submodule: only
>> preserve flags across recursive status/update invocations) the call site
>> of the recursive cmd_status was forgotten. At that place orig_args is
>> still passed into the recursion, which is always empty now. This clears
>> all options when recursing, as that variable is never set.
>>
>> Fix that by renaming orig_args to orig_flags there too and add a test to
>> catch that bug.
>
> Thanks. I back-ported your patch on top of 98dbe63d so it can go to
> 'maint'. I'm curious, though: why didn't the test right before (which
> checks recursion for --cached) catch this?

I was wondering the same thing about why 'git submodule sync
--recursive --quiet' succeeded, so I checked on it.  The answer is
that "--quiet" sets GIT_QUIET=1, which is then inherited by the
recursive call. Indeed, Jens' new test passes even without the
accompanying fix.  :-\

The 'status --recursive --cached' test passes for two reasons.  The
first is that the test is checking for a cached change in a level-1
submodule, but it is the level-2+ submodules which do not get the
flags passed down correctly in "$@".  The 2nd reason is that the
$cached variable is _also_ inherited by the recursive call to
cmd_status, specifically because it is a call to cmd_status() and not
to '$SHELL git submodule status', where the latter would have cleared
the flags at startup, but the former does not.

I'm a bit wary about "fixing" the flags for the recursion machinery.
I'm starting to think $orig_flags is moot in almost all cases.  It
looks like clearing all the flags on each iteration would break 'git
submodule foreach --recursive --quiet' because it does not use
$orig_flags at all.  Who knows what other surprises lurk there.

Here's a fix for the test, though, even though it still does not fail
in the absence of the $orig_flags patch.

-- >8 --

diff --git i/t/t7407-submodule-foreach.sh w/t/t7407-submodule-foreach.sh
index 9b69fe2..8442b32 100755
--- i/t/t7407-submodule-foreach.sh
+++ w/t/t7407-submodule-foreach.sh
@@ -226,14 +226,14 @@ test_expect_success 'test "status --recursive"' '
        test_cmp expect actual
 '

-sed -e "/nested1 /s/.*/+$nested1sha1 nested1 (file2~1)/;/sub[1-3]/d"
< expect > expect2
+sed -e "/nested2 /s/.*/+$nested2sha1 nested1\/nested2
(file2~1)/;/sub[1-3]/d" < expect > expect2
 mv -f expect2 expect

 test_expect_success 'ensure "status --cached --recursive" preserves
the --cached flag' '
        (
                cd clone3 &&
                (
-                       cd nested1 &&
+                       cd nested1/nested2 &&
                        test_commit file2
                ) &&
                git submodule status --cached --recursive -- nested1 > ../actual
@@ -245,6 +245,14 @@ test_expect_success 'ensure "status --cached
--recursive" preserves the --cached
        test_cmp expect actual
 '
 test_expect_success 'use "git clone --recursive" to checkout all submodules' '
        git clone --recursive super clone4 &&
        (

Phil

^ permalink raw reply related

* Re: Can't understand the behaviour of git-diff --submodule
From: Jens Lehmann @ 2012-10-26 19:08 UTC (permalink / raw)
  To: Francis Moreau; +Cc: git
In-Reply-To: <CAC9WiBgzbsury2f9FyAu=Pgn31f2uCtq7AvsVWGWEwoV6KbyjA@mail.gmail.com>

Am 26.10.2012 16:07, schrieb Francis Moreau:
> I'm trying to use the --submodule switch with git-diff but doesnt
> understand the following behaviour:
> 
> $ git diff 2c9a257718d1803de720f95766ff256d33accad5 HEAD
> diff --git a/configs b/configs
> index 16c6a89..ce12289 160000
> --- a/configs
> +++ b/configs
> @@ -1 +1 @@
> -Subproject commit 16c6a89f245f0eed7fb0bce8e027c59fcf1d543e
> +Subproject commit ce12289c5bfca7b2c423d9f1871c13ad1ba1dc32
> 
> but adding the --submodule option gives:
> 
> $ git diff --submodule=log 2c9a257718d1803de720f95766ff256d33accad5 HEAD
> Submodule configs 16c6a89...ce12289 (commits not present)
> 
> Could anybody enlight me ?

The output "Submodule configs 16c6a89...ce12289 (commits not present)"
contains same SHA-1s, only in their abbreviated form. That is the same
information you get without the --submodule option, but in shorter
format: it says the submodule moved from 16c6a89 to ce12289 in the
given commit range of the superproject (and the "..." part tells us it
wasn't a fast-forward). The "(commits not present)" part indicates that
even though git diff would have wanted to show you what happened in the
submodule between 16c6a89 and ce12289 by displaying the first line of
each commit message, it couldn't because these commit(s) are not present
in the submodule repo. If you do a "git log --oneline 16c6a89...ce12289"
inside the submodule you'll get an "unknown revision" error for the same
reason.

The "(commits not present)" part should go away and the output of the
short commit messages should appear when you do a "git fetch" inside the
submodule (at least unless someone forgot to push these submodule commits
upstream). As "git fetch" does attempt to fetch all referenced submodules
too, I suspect that you have unpushed commits (e.g. they might have been
dropped because of a rebase). In this case a "git submodule update" will
fail for the same reason.

Does that make it clearer?

^ permalink raw reply

* Re: git push tags
From: Drew Northup @ 2012-10-26 19:08 UTC (permalink / raw)
  To: Kacper Kornet; +Cc: Angelo Borsotti, git
In-Reply-To: <20121026190029.GB15328@camk.edu.pl>

On Fri, Oct 26, 2012 at 3:00 PM, Kacper Kornet <draenog@pld-linux.org> wrote:
> On Fri, Oct 26, 2012 at 08:35:50PM +0200, Angelo Borsotti wrote:
>> Hello
>
>> Drew,
....
>> Kacper
>
>> thank you for the patch. To keep downward compatibility, the denial to
>> update tags should perhaps be enabled with some option.
>
> You are probably right. The proper submission should also contain a
> test. I have sent a crude patch to show that the behaviour asked by you
> is possible to obtain.
>
> I will try to prepare a formal submission patch, but I can't say how
> long it will take me. So if you want to do it by yourself feel free.

Kacper,
I have been rebuilding my local so that generating a proper patch for
the manpage change part of the set won't make a horrible mess over
here.

It looks to me like the changeset requires the following:
Kacper's patch, some manpage changes, a test.

Angelo, may we use your original mail as the source for the cover page?

-- 
-Drew Northup
--------------------------------------------------------------
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59

^ permalink raw reply

* [PATCH] t7407: Fix recursive submodule test
From: Phil Hord @ 2012-10-26 19:13 UTC (permalink / raw)
  To: Git Mailing List; +Cc: phil.hord, Jens Lehmann, Jeff King, Phil Hord
In-Reply-To: <CABURp0op2+QUvusUmAFUxT8s8c02bB9V3=ag9gTTSiiN4t96OA@mail.gmail.com>

A test in t7404-submodule-foreach purports to test that
the --cached flag is properly noticed by --recursive calls
to the foreach command as it descends into nested
submodules.  However, the test really does not perform this
test since the change it looks for is in a top-level
submodule handled by the first invocation of the command.
To properly test for the flag being passed to recursive
invocations, the change must be buried deeper in the
hierarchy.

Move the change one level deeper so it properly verifies
the recursive machinery of the 'git submodule status'
command.

Signed-off-by: Phil Hord <hordp@cisco.com>
---
 t/t7407-submodule-foreach.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
index 9b69fe2..107b4b7 100755
--- a/t/t7407-submodule-foreach.sh
+++ b/t/t7407-submodule-foreach.sh
@@ -226,14 +226,14 @@ test_expect_success 'test "status --recursive"' '
 	test_cmp expect actual
 '
 
-sed -e "/nested1 /s/.*/+$nested1sha1 nested1 (file2~1)/;/sub[1-3]/d" < expect > expect2
+sed -e "/nested2 /s/.*/+$nested2sha1 nested1\/nested2 (file2~1)/;/sub[1-3]/d" < expect > expect2
 mv -f expect2 expect
 
 test_expect_success 'ensure "status --cached --recursive" preserves the --cached flag' '
 	(
 		cd clone3 &&
 		(
-			cd nested1 &&
+			cd nested1/nested2 &&
 			test_commit file2
 		) &&
 		git submodule status --cached --recursive -- nested1 > ../actual
-- 
1.8.0.2.gd008466.dirty

^ permalink raw reply related

* Re: [PATCH] submodule status: properly pass options with --recursive
From: Jens Lehmann @ 2012-10-26 19:26 UTC (permalink / raw)
  To: Phil Hord; +Cc: Jeff King, Git Mailing List, Junio C Hamano
In-Reply-To: <CABURp0op2+QUvusUmAFUxT8s8c02bB9V3=ag9gTTSiiN4t96OA@mail.gmail.com>

Am 26.10.2012 21:07, schrieb Phil Hord:
> On Fri, Oct 26, 2012 at 9:15 AM, Jeff King <peff@peff.net> wrote:
>> On Fri, Oct 26, 2012 at 12:20:29AM +0200, Jens Lehmann wrote:
>>
>>> When renaming orig_args to orig_flags in 98dbe63d (submodule: only
>>> preserve flags across recursive status/update invocations) the call site
>>> of the recursive cmd_status was forgotten. At that place orig_args is
>>> still passed into the recursion, which is always empty now. This clears
>>> all options when recursing, as that variable is never set.
>>>
>>> Fix that by renaming orig_args to orig_flags there too and add a test to
>>> catch that bug.
>>
>> Thanks. I back-ported your patch on top of 98dbe63d so it can go to
>> 'maint'. I'm curious, though: why didn't the test right before (which
>> checks recursion for --cached) catch this?
> 
> I was wondering the same thing about why 'git submodule sync
> --recursive --quiet' succeeded, so I checked on it.  The answer is
> that "--quiet" sets GIT_QUIET=1, which is then inherited by the
> recursive call. Indeed, Jens' new test passes even without the
> accompanying fix.  :-\

Dang, you're right! At least that explains why nobody noticed that
so far ... (and that's what you get for skipping the "does the test
fail without your fix?" part because the fix is so obvious :-( )

> The 'status --recursive --cached' test passes for two reasons.  The
> first is that the test is checking for a cached change in a level-1
> submodule, but it is the level-2+ submodules which do not get the
> flags passed down correctly in "$@".  The 2nd reason is that the
> $cached variable is _also_ inherited by the recursive call to
> cmd_status, specifically because it is a call to cmd_status() and not
> to '$SHELL git submodule status', where the latter would have cleared
> the flags at startup, but the former does not.
> 
> I'm a bit wary about "fixing" the flags for the recursion machinery.
> I'm starting to think $orig_flags is moot in almost all cases.  It
> looks like clearing all the flags on each iteration would break 'git
> submodule foreach --recursive --quiet' because it does not use
> $orig_flags at all.  Who knows what other surprises lurk there.

I agree, it looks like ripping out the orig_flags would be the way
to go.

^ permalink raw reply

* Re: [PATCH] t7407: Fix recursive submodule test
From: Jens Lehmann @ 2012-10-26 19:29 UTC (permalink / raw)
  To: Phil Hord; +Cc: Git Mailing List, phil.hord, Jeff King
In-Reply-To: <1351278834-28867-1-git-send-email-hordp@cisco.com>

Am 26.10.2012 21:13, schrieb Phil Hord:
> A test in t7404-submodule-foreach purports to test that
> the --cached flag is properly noticed by --recursive calls
> to the foreach command as it descends into nested
> submodules.  However, the test really does not perform this
> test since the change it looks for is in a top-level
> submodule handled by the first invocation of the command.
> To properly test for the flag being passed to recursive
> invocations, the change must be buried deeper in the
> hierarchy.
> 
> Move the change one level deeper so it properly verifies
> the recursive machinery of the 'git submodule status'
> command.

Me thinks we should definitely do this.

> Signed-off-by: Phil Hord <hordp@cisco.com>
> ---
>  t/t7407-submodule-foreach.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
> index 9b69fe2..107b4b7 100755
> --- a/t/t7407-submodule-foreach.sh
> +++ b/t/t7407-submodule-foreach.sh
> @@ -226,14 +226,14 @@ test_expect_success 'test "status --recursive"' '
>  	test_cmp expect actual
>  '
>  
> -sed -e "/nested1 /s/.*/+$nested1sha1 nested1 (file2~1)/;/sub[1-3]/d" < expect > expect2
> +sed -e "/nested2 /s/.*/+$nested2sha1 nested1\/nested2 (file2~1)/;/sub[1-3]/d" < expect > expect2
>  mv -f expect2 expect
>  
>  test_expect_success 'ensure "status --cached --recursive" preserves the --cached flag' '
>  	(
>  		cd clone3 &&
>  		(
> -			cd nested1 &&
> +			cd nested1/nested2 &&
>  			test_commit file2
>  		) &&
>  		git submodule status --cached --recursive -- nested1 > ../actual
> 

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox