Git development
 help / color / mirror / Atom feed
* [PATCH] pack-objects: Prefer shallower deltas if the size is equal
From: Brian Downing @ 2007-07-09  4:45 UTC (permalink / raw)
  To: git
In-Reply-To: <20070709044326.GH4087@lavos.net>

Change "try_delta" so that if it finds a delta that has the same size
but shallower depth than the existing delta, it will prefer the
shallower one.  This makes certain delta trees vastly less deep.

Signed-off-by: Brian Downing <bdowning@lavos.net>
---
 builtin-pack-objects.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 3d396ca..54b9d26 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1337,7 +1337,7 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,
 	if (max_size == 0)
 		return 0;
 	if (trg_entry->delta && trg_entry->delta_size <= max_size)
-		max_size = trg_entry->delta_size-1;
+		max_size = trg_entry->delta_size;
 	src_size = src_entry->size;
 	sizediff = src_size < trg_size ? trg_size - src_size : 0;
 	if (sizediff >= max_size)
@@ -1371,6 +1371,12 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,
 		return 0;
 
 	if (trg_entry->delta_data) {
+		/* Prefer only shallower same-sized deltas. */
+		if (delta_size == trg_entry->delta_size &&
+		    src_entry->depth + 1 >= trg_entry->depth) {
+			free(delta_buf);
+			return 0;
+		}
 		delta_cache_size -= trg_entry->delta_size;
 		free(trg_entry->delta_data);
 	}
-- 
1.5.2.GIT

^ permalink raw reply related

* Re: [PATCH] Per-path attribute based hunk header selection.
From: Junio C Hamano @ 2007-07-09  5:05 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Linus Torvalds, Johannes Schindelin, git
In-Reply-To: <alpine.LFD.0.999.0707082320480.26459@xanadu.home>

Nicolas Pitre <nico@cam.org> writes:

> On Fri, 6 Jul 2007, Junio C Hamano wrote:
>
>> About the comment from Johannes regarding hunk_header vs
>> funcname, I would actually prefer hunk_header, since that is
>> what this is about ("funcname" and "find_func" were misnomer
>> from the beginning), but I'd rename hunk_header to funcname for
>> the sake of consistency and minimizing the diff.
>
> I think "minimizing the diff" in this case is a bad reason.  Using 
> hunk_header is so much better than funcname IMHO.

Well, even then it turns out to be a good reason, as the patch
to rename function and field can be a separate patch.  After
adding that "latex pattern" stuff, I am even more inclined to
rename them.

^ permalink raw reply

* [PATCH v2] Make fetch-pack a builtin with an internal API
From: Daniel Barkalow @ 2007-07-09  5:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

In addition to making fetch-pack a builtin, this allows it to be called 
directly from other built-in code without generating and parsing argument 
lists, which will be useful for builtin-fetch.

Incidently, it makes git-fetch-pack not output lists of what it fetched 
when it fails.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
 Makefile                             |    1 +
 fetch-pack.c => builtin-fetch-pack.c |   92 ++++++++++++++++++++++++++--------
 builtin.h                            |    1 +
 fetch-pack.h                         |   16 ++++++
 git.c                                |    1 +
 5 files changed, 89 insertions(+), 22 deletions(-)
 rename fetch-pack.c => builtin-fetch-pack.c (92%)
 create mode 100644 fetch-pack.h

diff --git a/Makefile b/Makefile
index 4ea5e45..da750f8 100644
--- a/Makefile
+++ b/Makefile
@@ -342,6 +342,7 @@ BUILTIN_OBJS = \
 	builtin-diff-files.o \
 	builtin-diff-index.o \
 	builtin-diff-tree.o \
+	builtin-fetch-pack.o \
 	builtin-fetch--tool.o \
 	builtin-fmt-merge-msg.o \
 	builtin-for-each-ref.o \
diff --git a/fetch-pack.c b/builtin-fetch-pack.c
similarity index 92%
rename from fetch-pack.c
rename to builtin-fetch-pack.c
index 9c81305..27daa33 100644
--- a/fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -6,6 +6,7 @@
 #include "exec_cmd.h"
 #include "pack.h"
 #include "sideband.h"
+#include "fetch-pack.h"
 
 static int keep_pack;
 static int transfer_unpack_limit = -1;
@@ -573,7 +574,7 @@ static int get_pack(int xd[2])
 	die("%s died of unnatural causes %d", argv[0], status);
 }
 
-static int fetch_pack(int fd[2], int nr_match, char **match)
+static struct ref *do_fetch_pack(int fd[2], int nr_match, char **match)
 {
 	struct ref *ref;
 	unsigned char sha1[20];
@@ -615,12 +616,7 @@ static int fetch_pack(int fd[2], int nr_match, char **match)
 		die("git-fetch-pack: fetch failed.");
 
  all_done:
-	while (ref) {
-		printf("%s %s\n",
-		       sha1_to_hex(ref->old_sha1), ref->name);
-		ref = ref->next;
-	}
-	return 0;
+	return ref;
 }
 
 static int remove_duplicates(int nr_heads, char **heads)
@@ -663,15 +659,36 @@ static int fetch_pack_config(const char *var, const char *value)
 
 static struct lock_file lock;
 
-int main(int argc, char **argv)
+void setup_fetch_pack(const char *_uploadpack,
+		      int _quiet,
+		      int _keep_pack,
+		      int _unpacklimit,
+		      int _use_thin_pack,
+		      int _fetch_all,
+		      int _verbose,
+		      int _depth,
+		      int _no_progress)
+{
+	uploadpack = _uploadpack;
+	quiet = _quiet;
+	keep_pack = _keep_pack;
+	if (_unpacklimit >= 0)
+		unpack_limit = _unpacklimit;
+	if (keep_pack)
+		unpack_limit = 0;
+	use_thin_pack = _use_thin_pack;
+	fetch_all = _fetch_all;
+	verbose = _verbose;
+	depth = _depth;
+	no_progress = _no_progress;
+}
+
+int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
 {
 	int i, ret, nr_heads;
+	struct ref *ref;
 	char *dest = NULL, **heads;
-	int fd[2];
-	pid_t pid;
-	struct stat st;
 
-	setup_git_directory();
 	git_config(fetch_pack_config);
 
 	if (0 <= transfer_unpack_limit)
@@ -682,7 +699,7 @@ int main(int argc, char **argv)
 	nr_heads = 0;
 	heads = NULL;
 	for (i = 1; i < argc; i++) {
-		char *arg = argv[i];
+		const char *arg = argv[i];
 
 		if (*arg == '-') {
 			if (!prefixcmp(arg, "--upload-pack=")) {
@@ -716,8 +733,6 @@ int main(int argc, char **argv)
 			}
 			if (!prefixcmp(arg, "--depth=")) {
 				depth = strtol(arg + 8, NULL, 0);
-				if (stat(git_path("shallow"), &st))
-					st.st_mtime = 0;
 				continue;
 			}
 			if (!strcmp("--no-progress", arg)) {
@@ -726,22 +741,52 @@ int main(int argc, char **argv)
 			}
 			usage(fetch_pack_usage);
 		}
-		dest = arg;
-		heads = argv + i + 1;
+		dest = (char *)arg;
+		heads = (char **)(argv + i + 1);
 		nr_heads = argc - i - 1;
 		break;
 	}
 	if (!dest)
 		usage(fetch_pack_usage);
-	pid = git_connect(fd, dest, uploadpack, verbose ? CONNECT_VERBOSE : 0);
+
+	ref = fetch_pack(dest, nr_heads, heads);
+
+	ret = !ref;
+
+	while (ref) {
+		printf("%s %s\n",
+		       sha1_to_hex(ref->old_sha1), ref->name);
+		ref = ref->next;
+	}
+
+	return ret;
+}
+
+struct ref *fetch_pack(const char *dest, int nr_heads, char **heads)
+{
+	int i, ret;
+	int fd[2];
+	pid_t pid;
+	struct ref *ref;
+	struct stat st;
+
+	if (depth > 0) {
+		if (stat(git_path("shallow"), &st))
+			st.st_mtime = 0;
+	}
+
+	printf("connect to %s\n", dest);
+
+	pid = git_connect(fd, (char *)dest, uploadpack,
+                          verbose ? CONNECT_VERBOSE : 0);
 	if (pid < 0)
-		return 1;
+		return NULL;
 	if (heads && nr_heads)
 		nr_heads = remove_duplicates(nr_heads, heads);
-	ret = fetch_pack(fd, nr_heads, heads);
+	ref = do_fetch_pack(fd, nr_heads, heads);
 	close(fd[0]);
 	close(fd[1]);
-	ret |= finish_connect(pid);
+	ret = finish_connect(pid);
 
 	if (!ret && nr_heads) {
 		/* If the heads to pull were given, we should have
@@ -785,5 +830,8 @@ int main(int argc, char **argv)
 		}
 	}
 
-	return !!ret;
+	if (ret)
+		ref = NULL;
+
+	return ref;
 }
diff --git a/builtin.h b/builtin.h
index 661a92f..8fa38d4 100644
--- a/builtin.h
+++ b/builtin.h
@@ -31,6 +31,7 @@ extern int cmd_diff_files(int argc, const char **argv, const char *prefix);
 extern int cmd_diff_index(int argc, const char **argv, const char *prefix);
 extern int cmd_diff(int argc, const char **argv, const char *prefix);
 extern int cmd_diff_tree(int argc, const char **argv, const char *prefix);
+extern int cmd_fetch_pack(int argc, const char **argv, const char *prefix);
 extern int cmd_fetch__tool(int argc, const char **argv, const char *prefix);
 extern int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix);
 extern int cmd_for_each_ref(int argc, const char **argv, const char *prefix);
diff --git a/fetch-pack.h b/fetch-pack.h
new file mode 100644
index 0000000..2bd05a8
--- /dev/null
+++ b/fetch-pack.h
@@ -0,0 +1,16 @@
+#ifndef FETCH_PACK_API
+#define FETCH_PACK_API
+
+void setup_fetch_pack(const char *_uploadpack,
+		      int _quiet,
+		      int _keep_pack,
+		      int _unpacklimit,
+		      int _use_thin_pack,
+		      int _fetch_all,
+		      int _verbose,
+		      int _depth,
+		      int _no_progress);
+
+struct ref *fetch_pack(const char *dest, int nr_heads, char **heads);
+
+#endif
diff --git a/git.c b/git.c
index b949cbb..df45161 100644
--- a/git.c
+++ b/git.c
@@ -307,6 +307,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "diff-files", cmd_diff_files },
 		{ "diff-index", cmd_diff_index, RUN_SETUP },
 		{ "diff-tree", cmd_diff_tree, RUN_SETUP },
+		{ "fetch-pack", cmd_fetch_pack, RUN_SETUP },
 		{ "fetch--tool", cmd_fetch__tool, RUN_SETUP },
 		{ "fmt-merge-msg", cmd_fmt_merge_msg, RUN_SETUP },
 		{ "for-each-ref", cmd_for_each_ref, RUN_SETUP },
-- 
1.5.2.2.1399.g097d5-dirty

^ permalink raw reply related

* [PATCH] Report information on branches from remote.h
From: Daniel Barkalow @ 2007-07-09  5:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

This adds full parsing for branch.<name> sections and functions to
interpret the results usefully. It incidentally corrects the fetch
configuration information for legacy branches/* files with '#'
characters in the URLs.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
 remote.c |  157 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 remote.h |   18 +++++++
 2 files changed, 164 insertions(+), 11 deletions(-)

diff --git a/remote.c b/remote.c
index 500ca4d..4ea15b9 100644
--- a/remote.c
+++ b/remote.c
@@ -5,6 +5,12 @@
 static struct remote **remotes;
 static int allocated_remotes;
 
+static struct branch **branches;
+static int allocated_branches;
+
+static struct branch *current_branch;
+static const char *default_remote_name;
+
 #define BUF_SIZE (2048)
 static char buffer[BUF_SIZE];
 
@@ -67,6 +73,54 @@ static struct remote *make_remote(const char *name, int len)
 	return remotes[empty];
 }
 
+static void add_merge(struct branch *branch, const char *name)
+{
+	int nr = branch->merge_nr + 1;
+	branch->merge_name =
+		xrealloc(branch->merge_name, nr * sizeof(char *));
+	branch->merge_name[nr-1] = name;
+	branch->merge_nr = nr;
+}
+
+static struct branch *make_branch(const char *name, int len)
+{
+	int i, empty = -1;
+	char *refname;
+
+	for (i = 0; i < allocated_branches; i++) {
+		if (!branches[i]) {
+			if (empty < 0)
+				empty = i;
+		} else {
+			if (len ? (!strncmp(name, branches[i]->name, len) &&
+				   !branches[i]->name[len]) :
+			    !strcmp(name, branches[i]->name))
+				return branches[i];
+		}
+	}
+
+	if (empty < 0) {
+		empty = allocated_branches;
+		allocated_branches += allocated_branches ? allocated_branches : 1;
+		branches = xrealloc(branches,
+				   sizeof(*branches) * allocated_branches);
+		memset(branches + empty, 0,
+		       (allocated_branches - empty) * sizeof(*branches));
+	}
+	branches[empty] = xcalloc(1, sizeof(struct branch));
+	if (len)
+		branches[empty]->name = xstrndup(name, len);
+	else
+		branches[empty]->name = xstrdup(name);
+	refname = malloc(strlen(name) + strlen("refs/heads/") + 1);
+	strcpy(refname, "refs/heads/");
+	strcpy(refname + strlen("refs/heads/"),
+	       branches[empty]->name);
+	branches[empty]->refname = refname;
+
+	return branches[empty];
+}
+
 static void read_remotes_file(struct remote *remote)
 {
 	FILE *f = fopen(git_path("remotes/%s", remote->name), "r");
@@ -116,6 +170,8 @@ static void read_remotes_file(struct remote *remote)
 static void read_branches_file(struct remote *remote)
 {
 	const char *slash = strchr(remote->name, '/');
+	char *frag;
+	char *branch;
 	int n = slash ? slash - remote->name : 1000;
 	FILE *f = fopen(git_path("branches/%.*s", n, remote->name), "r");
 	char *s, *p;
@@ -141,23 +197,40 @@ static void read_branches_file(struct remote *remote)
 	strcpy(p, s);
 	if (slash)
 		strcat(p, slash);
+	frag = strchr(p, '#');
+	if (frag) {
+		*(frag++) = '\0';
+		branch = xmalloc(strlen(frag) + 12);
+		strcpy(branch, "refs/heads/");
+		strcat(branch, frag);
+	} else {
+		branch = "refs/heads/master";
+	}
 	add_uri(remote, p);
+	add_fetch_refspec(remote, branch);
 }
 
-static char *default_remote_name = NULL;
-static const char *current_branch = NULL;
-static int current_branch_len = 0;
-
 static int handle_config(const char *key, const char *value)
 {
 	const char *name;
 	const char *subkey;
 	struct remote *remote;
-	if (!prefixcmp(key, "branch.") && current_branch &&
-	    !strncmp(key + 7, current_branch, current_branch_len) &&
-	    !strcmp(key + 7 + current_branch_len, ".remote")) {
-		free(default_remote_name);
-		default_remote_name = xstrdup(value);
+	struct branch *branch;
+	if (!prefixcmp(key, "branch.")) {
+		name = key + 7;
+		subkey = strrchr(name, '.');
+		branch = make_branch(name, subkey - name);
+		if (!subkey)
+			return 0;
+		if (!value)
+			return 0;
+		if (!strcmp(subkey, ".remote")) {
+			branch->remote_name = xstrdup(value);
+			if (branch == current_branch)
+				default_remote_name = branch->remote_name;
+		} else if (!strcmp(subkey, ".merge"))
+			add_merge(branch, xstrdup(value));
+		return 0;
 	}
 	if (prefixcmp(key,  "remote."))
 		return 0;
@@ -212,8 +285,8 @@ static void read_config(void)
 	head_ref = resolve_ref("HEAD", sha1, 0, &flag);
 	if (head_ref && (flag & REF_ISSYMREF) &&
 	    !prefixcmp(head_ref, "refs/heads/")) {
-		current_branch = head_ref + strlen("refs/heads/");
-		current_branch_len = strlen(current_branch);
+		current_branch =
+			make_branch(head_ref + strlen("refs/heads/"), 0);
 	}
 	git_config(handle_config);
 }
@@ -289,6 +362,25 @@ int remote_has_uri(struct remote *remote, const char *uri)
 	return 0;
 }
 
+/*
+ * Returns true if, under the matching rules for fetching, name is the
+ * same as the given full name.
+ */
+static int ref_matches_abbrev(const char *name, const char *full)
+{
+	if (!prefixcmp(name, "refs/") || !strcmp(name, "HEAD"))
+		return !strcmp(name, full);
+	if (prefixcmp(full, "refs/"))
+		return 0;
+	if (!prefixcmp(name, "heads/") ||
+	    !prefixcmp(name, "tags/") ||
+	    !prefixcmp(name, "remotes/"))
+		return !strcmp(name, full + 5);
+	if (prefixcmp(full + 5, "heads/"))
+		return 0;
+	return !strcmp(full + 11, name);
+}
+
 int remote_find_tracking(struct remote *remote, struct refspec *refspec)
 {
 	int i;
@@ -574,3 +666,46 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
 	}
 	return 0;
 }
+
+struct branch *branch_get(const char *name)
+{
+	struct branch *ret;
+
+	read_config();
+	if (!name || !*name || !strcmp(name, "HEAD"))
+		ret = current_branch;
+	else
+		ret = make_branch(name, 0);
+	if (ret && ret->remote_name) {
+		ret->remote = remote_get(ret->remote_name);
+		if (ret->merge_nr) {
+			int i;
+			ret->merge = xcalloc(sizeof(*ret->merge),
+					     ret->merge_nr);
+			for (i = 0; i < ret->merge_nr; i++) {
+				ret->merge[i] = xcalloc(1, sizeof(**ret->merge));
+				ret->merge[i]->src = ret->merge_name[i];
+				remote_find_tracking(ret->remote,
+						     ret->merge[i]);
+			}
+		}
+	}
+	return ret;
+}
+
+int branch_has_merge_config(struct branch *branch)
+{
+	return branch && !!branch->merge;
+}
+
+int branch_merges(struct branch *branch, const char *refname)
+{
+	int i;
+	if (!branch)
+		return 0;
+	for (i = 0; i < branch->merge_nr; i++) {
+		if (ref_matches_abbrev(branch->merge[i]->src, refname))
+			return 1;
+	}
+	return 0;
+}
diff --git a/remote.h b/remote.h
index 01dbcef..9824a0d 100644
--- a/remote.h
+++ b/remote.h
@@ -38,4 +38,22 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
  */
 int remote_find_tracking(struct remote *remote, struct refspec *refspec);
 
+struct branch {
+	const char *name;
+	const char *refname;
+
+	const char *remote_name;
+	struct remote *remote;
+
+	const char **merge_name;
+	struct refspec **merge;
+	int merge_nr;
+};
+
+struct branch *branch_get(const char *name);
+
+int branch_has_merge_config(struct branch *branch);
+
+int branch_merges(struct branch *branch, const char *refname);
+
 #endif
-- 
1.5.2.2.1399.g097d5-dirty

^ permalink raw reply related

* [PATCH v2] Push code for transport library
From: Daniel Barkalow @ 2007-07-09  5:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Johannes Schindelin, Shawn O. Pearce

This moves the code to call push backends into a library that can be
extended to make matching fetch and push decisions based on the URL it
gets, and which could be changed to have built-in implementations
instead of calling external programs.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
Okay, no more C99 initializers.

 Makefile       |    3 +-
 builtin-push.c |   82 ++++++-----------------
 transport.c    |  197 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 transport.h    |   61 +++++++++++++++++
 4 files changed, 281 insertions(+), 62 deletions(-)
 create mode 100644 transport.c
 create mode 100644 transport.h

diff --git a/Makefile b/Makefile
index 4ea5e45..b8f9af1 100644
--- a/Makefile
+++ b/Makefile
@@ -321,7 +321,8 @@ LIB_OBJS = \
 	write_or_die.o trace.o list-objects.o grep.o match-trees.o \
 	alloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \
 	color.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \
-	convert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o
+	convert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o \
+	transport.o
 
 BUILTIN_OBJS = \
 	builtin-add.o \
diff --git a/builtin-push.c b/builtin-push.c
index 2612f07..845d6fc 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -6,10 +6,11 @@
 #include "run-command.h"
 #include "builtin.h"
 #include "remote.h"
+#include "transport.h"
 
 static const char push_usage[] = "git-push [--all] [--tags] [--receive-pack=<git-receive-pack>] [--repo=all] [-f | --force] [-v] [<repository> <refspec>...]";
 
-static int all, force, thin = 1, verbose;
+static int all, thin = 1, verbose;
 static const char *receivepack;
 
 static const char **refspec;
@@ -43,80 +44,38 @@ static void set_refspecs(const char **refs, int nr)
 	}
 }
 
-static int do_push(const char *repo)
+static int do_push(const char *repo, int flags)
 {
 	int i, errs;
-	int common_argc;
-	const char **argv;
-	int argc;
 	struct remote *remote = remote_get(repo);
 
 	if (!remote)
 		die("bad repository '%s'", repo);
 
-	if (remote->receivepack) {
-		char *rp = xmalloc(strlen(remote->receivepack) + 16);
-		sprintf(rp, "--receive-pack=%s", remote->receivepack);
-		receivepack = rp;
-	}
 	if (!refspec && !all && remote->push_refspec_nr) {
 		refspec = remote->push_refspec;
 		refspec_nr = remote->push_refspec_nr;
 	}
-
-	argv = xmalloc((refspec_nr + 10) * sizeof(char *));
-	argv[0] = "dummy-send-pack";
-	argc = 1;
-	if (all)
-		argv[argc++] = "--all";
-	if (force)
-		argv[argc++] = "--force";
-	if (receivepack)
-		argv[argc++] = receivepack;
-	common_argc = argc;
-
 	errs = 0;
 	for (i = 0; i < remote->uri_nr; i++) {
+		struct transport *transport =
+			transport_get(remote, remote->uri[i], 0);
 		int err;
-		int dest_argc = common_argc;
-		int dest_refspec_nr = refspec_nr;
-		const char **dest_refspec = refspec;
-		const char *dest = remote->uri[i];
-		const char *sender = "send-pack";
-		if (!prefixcmp(dest, "http://") ||
-		    !prefixcmp(dest, "https://"))
-			sender = "http-push";
-		else {
-			char *rem = xmalloc(strlen(remote->name) + 10);
-			sprintf(rem, "--remote=%s", remote->name);
-			argv[dest_argc++] = rem;
-			if (thin)
-				argv[dest_argc++] = "--thin";
-		}
-		argv[0] = sender;
-		argv[dest_argc++] = dest;
-		while (dest_refspec_nr--)
-			argv[dest_argc++] = *dest_refspec++;
-		argv[dest_argc] = NULL;
+		if (receivepack)
+			transport_set_option(transport,
+					     TRANS_OPT_RECEIVEPACK, receivepack);
+		if (thin)
+			transport_set_option(transport, TRANS_OPT_THIN, "yes");
+
 		if (verbose)
-			fprintf(stderr, "Pushing to %s\n", dest);
-		err = run_command_v_opt(argv, RUN_GIT_CMD);
+			fprintf(stderr, "Pushing to %s\n", remote->uri[i]);
+		err = transport_push(transport, refspec_nr, refspec, flags);
+		err |= transport_disconnect(transport);
+
 		if (!err)
 			continue;
 
 		error("failed to push to '%s'", remote->uri[i]);
-		switch (err) {
-		case -ERR_RUN_COMMAND_FORK:
-			error("unable to fork for %s", sender);
-		case -ERR_RUN_COMMAND_EXEC:
-			error("unable to exec %s", sender);
-			break;
-		case -ERR_RUN_COMMAND_WAITPID:
-		case -ERR_RUN_COMMAND_WAITPID_WRONG_PID:
-		case -ERR_RUN_COMMAND_WAITPID_SIGNAL:
-		case -ERR_RUN_COMMAND_WAITPID_NOEXIT:
-			error("%s died with strange error", sender);
-		}
 		errs++;
 	}
 	return !!errs;
@@ -125,6 +84,7 @@ static int do_push(const char *repo)
 int cmd_push(int argc, const char **argv, const char *prefix)
 {
 	int i;
+	int flags = 0;
 	const char *repo = NULL;	/* default repository */
 
 	for (i = 1; i < argc; i++) {
@@ -144,7 +104,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 			continue;
 		}
 		if (!strcmp(arg, "--all")) {
-			all = 1;
+			flags |= TRANSPORT_PUSH_ALL;
 			continue;
 		}
 		if (!strcmp(arg, "--tags")) {
@@ -152,7 +112,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 			continue;
 		}
 		if (!strcmp(arg, "--force") || !strcmp(arg, "-f")) {
-			force = 1;
+			flags |= TRANSPORT_PUSH_FORCE;
 			continue;
 		}
 		if (!strcmp(arg, "--thin")) {
@@ -164,11 +124,11 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 			continue;
 		}
 		if (!prefixcmp(arg, "--receive-pack=")) {
-			receivepack = arg;
+			receivepack = arg + 15;
 			continue;
 		}
 		if (!prefixcmp(arg, "--exec=")) {
-			receivepack = arg;
+			receivepack = arg + 7;
 			continue;
 		}
 		usage(push_usage);
@@ -177,5 +137,5 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 	if (all && refspec)
 		usage(push_usage);
 
-	return do_push(repo);
+	return do_push(repo, flags);
 }
diff --git a/transport.c b/transport.c
new file mode 100644
index 0000000..ce03133
--- /dev/null
+++ b/transport.c
@@ -0,0 +1,197 @@
+#include "cache.h"
+#include "transport.h"
+#include "run-command.h"
+
+static const struct transport_ops rsync_transport = {
+};
+
+static int curl_transport_push(struct transport *transport, int refspec_nr, const char **refspec, int flags) {
+	const char **argv;
+	int argc;
+	int err;
+
+	argv = xmalloc((refspec_nr + 11) * sizeof(char *));
+	argv[0] = "http-push";
+	argc = 1;
+	if (flags & TRANSPORT_PUSH_ALL)
+		argv[argc++] = "--all";
+	if (flags & TRANSPORT_PUSH_FORCE)
+		argv[argc++] = "--force";
+	argv[argc++] = transport->url;
+	while (refspec_nr--)
+		argv[argc++] = *refspec++;
+	argv[argc] = NULL;
+	err = run_command_v_opt(argv, RUN_GIT_CMD);
+	switch (err) {
+	case -ERR_RUN_COMMAND_FORK:
+		error("unable to fork for %s", argv[0]);
+	case -ERR_RUN_COMMAND_EXEC:
+		error("unable to exec %s", argv[0]);
+		break;
+	case -ERR_RUN_COMMAND_WAITPID:
+	case -ERR_RUN_COMMAND_WAITPID_WRONG_PID:
+	case -ERR_RUN_COMMAND_WAITPID_SIGNAL:
+	case -ERR_RUN_COMMAND_WAITPID_NOEXIT:
+		error("%s died with strange error", argv[0]);
+	}
+	return !!err;
+}
+
+static const struct transport_ops curl_transport = {
+	/* set_option */	NULL,
+	/* push */		curl_transport_push
+};
+
+static const struct transport_ops bundle_transport = {
+};
+
+struct git_transport_data {
+	unsigned thin : 1;
+
+	const char *receivepack;
+};
+
+static int set_git_option(struct transport *connection,
+			  const char *name, const char *value)
+{
+	struct git_transport_data *data = connection->data;
+	if (!strcmp(name, TRANS_OPT_RECEIVEPACK)) {
+		data->receivepack = value;
+		return 0;
+	} else if (!strcmp(name, TRANS_OPT_THIN)) {
+		data->thin = !!value;
+		return 0;
+	}
+	return 1;
+}
+
+static int git_transport_push(struct transport *transport, int refspec_nr, const char **refspec, int flags) {
+	struct git_transport_data *data = transport->data;
+	const char **argv;
+	char *rem;
+	int argc;
+	int err;
+
+	argv = xmalloc((refspec_nr + 11) * sizeof(char *));
+	argv[0] = "send-pack";
+	argc = 1;
+	if (flags & TRANSPORT_PUSH_ALL)
+		argv[argc++] = "--all";
+	if (flags & TRANSPORT_PUSH_FORCE)
+		argv[argc++] = "--force";
+	if (data->receivepack) {
+		char *rp = xmalloc(strlen(data->receivepack) + 16);
+		sprintf(rp, "--receive-pack=%s", data->receivepack);
+		argv[argc++] = rp;
+	}
+	if (data->thin)
+		argv[argc++] = "--thin";
+	rem = xmalloc(strlen(transport->remote->name) + 10);
+	sprintf(rem, "--remote=%s", transport->remote->name);
+	argv[argc++] = rem;
+	argv[argc++] = transport->url;
+	while (refspec_nr--)
+		argv[argc++] = *refspec++;
+	argv[argc] = NULL;
+	err = run_command_v_opt(argv, RUN_GIT_CMD);
+	switch (err) {
+	case -ERR_RUN_COMMAND_FORK:
+		error("unable to fork for %s", argv[0]);
+	case -ERR_RUN_COMMAND_EXEC:
+		error("unable to exec %s", argv[0]);
+		break;
+	case -ERR_RUN_COMMAND_WAITPID:
+	case -ERR_RUN_COMMAND_WAITPID_WRONG_PID:
+	case -ERR_RUN_COMMAND_WAITPID_SIGNAL:
+	case -ERR_RUN_COMMAND_WAITPID_NOEXIT:
+		error("%s died with strange error", argv[0]);
+	}
+	return !!err;
+}
+
+static const struct transport_ops git_transport = {
+	/* set_option */	set_git_option,
+	/* push */		git_transport_push
+};
+
+static int is_local(const char *url)
+{
+	const char *colon = strchr(url, ':');
+	const char *slash = strchr(url, '/');
+	return !colon || (slash && slash < colon);
+}
+
+static int is_file(const char *url)
+{
+	struct stat buf;
+	if (stat(url, &buf))
+		return 0;
+	return S_ISREG(buf.st_mode);
+}
+
+struct transport *transport_get(struct remote *remote, const char *url,
+				int fetch)
+{
+	struct transport *ret = NULL;
+	if (!prefixcmp(url, "rsync://")) {
+		ret = xmalloc(sizeof(*ret));
+		ret->data = NULL;
+		ret->ops = &rsync_transport;
+	} else if (!prefixcmp(url, "http://") || !prefixcmp(url, "https://") ||
+		   !prefixcmp(url, "ftp://")) {
+		ret = xmalloc(sizeof(*ret));
+		ret->ops = &curl_transport;
+		ret->data = NULL;
+	} else if (is_local(url) && is_file(url)) {
+		ret = xmalloc(sizeof(*ret));
+		ret->data = NULL;
+		ret->ops = &bundle_transport;
+	} else {
+		struct git_transport_data *data = xcalloc(1, sizeof(*data));
+		ret = xcalloc(1, sizeof(*ret));
+		ret->data = data;
+		data->thin = 1;
+		data->receivepack = "git-receive-pack";
+		if (remote->receivepack)
+			data->receivepack = remote->receivepack;
+		ret->ops = &git_transport;
+	}
+	if (ret) {
+		ret->remote = remote;
+		ret->url = url;
+		ret->fetch = !!fetch;
+	}
+	return ret;
+}
+
+int transport_set_option(struct transport *transport,
+			 const char *name, const char *value)
+{
+	int ret = 1;
+	if (transport->ops->set_option)
+		ret = transport->ops->set_option(transport, name, value);
+	if (ret < 0)
+		fprintf(stderr, "For '%s' option %s cannot be set to '%s'\n",
+			transport->url, name, value);
+	if (ret > 0)
+		fprintf(stderr, "For '%s' option %s is ignored\n",
+			transport->url, name);
+	return ret;
+}
+
+int transport_push(struct transport *transport,
+		   int refspec_nr, const char **refspec, int flags)
+{
+	if (!transport->ops->push)
+		return 1;
+	return transport->ops->push(transport, refspec_nr, refspec, flags);
+}
+
+int transport_disconnect(struct transport *transport)
+{
+	int ret = 0;
+	if (transport->ops->disconnect)
+		ret = transport->ops->disconnect(transport);
+	free(transport);
+	return ret;
+}
diff --git a/transport.h b/transport.h
new file mode 100644
index 0000000..5c2eb95
--- /dev/null
+++ b/transport.h
@@ -0,0 +1,61 @@
+#ifndef TRANSPORT_H
+#define TRANSPORT_H
+
+#include "cache.h"
+#include "remote.h"
+
+struct transport {
+	unsigned verbose : 1;
+	unsigned fetch : 1;
+	struct remote *remote;
+	const char *url;
+
+	void *data;
+
+	struct ref *remote_refs;
+
+	const struct transport_ops *ops;
+};
+
+#define TRANSPORT_PUSH_ALL 1
+#define TRANSPORT_PUSH_FORCE 2
+
+struct transport_ops {
+	/**
+	 * Returns 0 if successful, positive if the option is not
+	 * recognized or is inapplicable, and negative if the option
+	 * is applicable but the value is invalid.
+	 **/
+	int (*set_option)(struct transport *connection, const char *name,
+			  const char *value);
+
+	int (*push)(struct transport *connection, int refspec_nr, const char **refspec, int flags);
+
+	int (*disconnect)(struct transport *connection);
+};
+
+/* Returns a transport suitable for the url */
+struct transport *transport_get(struct remote *remote, const char *url,
+				int fetch);
+
+/* Transport options which apply to git:// and scp-style URLs */
+
+/* The program to use on the remote side to receive a pack */
+#define TRANS_OPT_RECEIVEPACK "receivepack"
+
+/* Transfer the data as a thin pack if not null */
+#define TRANS_OPT_THIN "thin"
+
+/**
+ * Returns 0 if the option was used, non-zero otherwise. Prints a
+ * message to stderr if the option is not used.
+ **/
+int transport_set_option(struct transport *transport, const char *name,
+			 const char *value);
+
+int transport_push(struct transport *connection,
+		   int refspec_nr, const char **refspec, int flags);
+
+int transport_disconnect(struct transport *transport);
+
+#endif
-- 
1.5.2.2.1399.g097d5-dirty

^ permalink raw reply related

* Re: Preferring shallower deltas on repack
From: Junio C Hamano @ 2007-07-09  5:31 UTC (permalink / raw)
  To: Brian Downing; +Cc: git
In-Reply-To: <20070709044326.GH4087@lavos.net>

bdowning@lavos.net (Brian Downing) writes:

> I modified this to prefer shallower deltas of the same size.  This made
> the deltas for this file a very wide tree with a maximum depth of about
> 65.  Other (much smaller) improvements were seen elsewhere in the pack.
> Runtime does not seem to have been affected, as most of the work had
> already been done when it was tossing deltas before.
>
> Some simple statistics:
>
> SBCL, standard pack-objects, window 100, depth 1000:
>   Max depth: 980
>   Mean depth: 100.223622114502
>   Median depth: 12
>   Standard deviation: 188.214331919176
>
> SBCL, patched pack-objects, window 100, depth 1000:
>   Max depth: 787
>   Mean depth: 61.5669990817656
>   Median depth: 11
>   Standard deviation: 127.644652607399

Putting aside a potential argument that the way the file in
question, version.lisp-expr, is kept track of might be insane,
this is an interesting topic.

In addition to the above stats, it may be interesting to know:

 - pack generation time and memory footprint (/usr/bin/time);

   I suspect you would have to try_delta more candidates, so
   this may degrade a bit, but that is done for getting a better
   deltification, so we would need to see if the extra cost is
   within reason and worth spending.

 - resulting pack size (ls -l pack-*.pack)

   I do not expect your change would degrade in this area, as
   you are currently not trading size with shallower delta
   depth.

Regarding your patch, I think it does not look too bad, as you
never pick delta that is larger than the best-so-far in favor of
shallower depth.

It would become worrysome (*BUT* infinitely more interesting)
once you start talking about a tradeoff between slightly larger
delta and much shorter delta.  Such a tradeoff, if done right,
would make a lot of sense, but I do not offhand think of a way
to strike a proper balance between them efficiently.

^ permalink raw reply

* Re: [PATCH v2] Make fetch-pack a builtin with an internal API
From: Junio C Hamano @ 2007-07-09  5:39 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0707090104120.6977@iabervon.org>

Are _identifiers with leading underscore Kosher thing to do, I
wonder...  We do have ones with trailing ones (mostly qsort
functions) and I think they are done that way for the sake of
standards conformance.

Gone is a "#if 0/#endif" which is good.

diff --git a/fetch-pack.h b/fetch-pack.h
new file mode 100644
index 0000000..2bd05a8
--- /dev/null
+++ b/fetch-pack.h
@@ -0,0 +1,16 @@
+#ifndef FETCH_PACK_API

We seem to say "#ifndef FETCH_PACK_H" in such a case, though.
Nobody seems to call setup_fetch_pack() yet.  How complete is
this patch meant to be?

The program is somehow much more pleasant to follow, even though
there is no fundamental change anywhere.

^ permalink raw reply related

* Re: Preferring shallower deltas on repack
From: Linus Torvalds @ 2007-07-09  5:41 UTC (permalink / raw)
  To: Brian Downing; +Cc: git
In-Reply-To: <20070709044326.GH4087@lavos.net>



On Sun, 8 Jul 2007, Brian Downing wrote:
> 
> I modified this to prefer shallower deltas of the same size.  This made
> the deltas for this file a very wide tree with a maximum depth of about
> 65.  Other (much smaller) improvements were seen elsewhere in the pack.
> Runtime does not seem to have been affected, as most of the work had
> already been done when it was tossing deltas before.

This seems like a good thing to do, and on the face of it I think it's 
worth it. I can't see any real downsides, at least, and while the upside 
doesn't sound huge, it sounds real enough.

So here's at least an initial tentative "ack" from me.

		Linus

^ permalink raw reply

* [PATCH] cvsimport: added warning doc, cvsimport may fail to handle history
From: Steffen Prohaska @ 2007-07-09  5:42 UTC (permalink / raw)
  To: git; +Cc: Steffen Prohaska

There are discussions on the mailing list that git-cvsimport
fails to handle history. This should be noted in the documentation
of cvsimport.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 Documentation/git-cvsimport.txt |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-cvsimport.txt b/Documentation/git-cvsimport.txt
index fdd7ec7..b4a5ebd 100644
--- a/Documentation/git-cvsimport.txt
+++ b/Documentation/git-cvsimport.txt
@@ -31,6 +31,16 @@ to work with; after that, you need to 'git merge' incremental imports, or
 any CVS branches, yourself.  It is advisable to specify a named remote via
 -r to separate and protect the incoming branches.
 
+There were rumors on the mailing list that git-cvsimport fails to
+handle history correctly.  Especially cvs branches seem to cause 
+problems. See the following thread on the mailing list for more details
+link:http://marc.info/?t=118385570200002&r=1&w=2[http://marc.info/?t=118385570200002&r=1&w=2].
+Alternatives discussed on the mailing list are
+
+  * parsecvs from link:http://anongit.freedesktop.org/git/users/keithp/repos/parsecvs.git/[http://anongit.freedesktop.org/git/users/keithp/repos/parsecvs.git/]
+  * fromcvs from link:http://ww2.fs.ei.tum.de/%7Ecorecode/hg/fromcvs/[http://ww2.fs.ei.tum.de/%7Ecorecode/hg/fromcvs/]
+
+As of July 2007 there is no definite recommendation which tool to use.
 
 OPTIONS
 -------
-- 
1.5.3.rc0.22.gd22ed

^ permalink raw reply related

* Re: Preferring shallower deltas on repack
From: Junio C Hamano @ 2007-07-09  5:43 UTC (permalink / raw)
  To: Brian Downing; +Cc: git
In-Reply-To: <7v1wfixhvk.fsf@assigned-by-dhcp.cox.net>

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

> It would become worrysome (*BUT* infinitely more interesting)
> once you start talking about a tradeoff between slightly larger
> delta and much shorter delta.  Such a tradeoff, if done right,

s/and much shorter delta/and much shallower depth/.

Should be obvious from the context, but just in case -- without
the above correction what I said does not make any sense ;-).

> would make a lot of sense, but I do not offhand think of a way
> to strike a proper balance between them efficiently.

^ permalink raw reply

* Re: git-svn set-tree
From: Eric Wong @ 2007-07-09  5:45 UTC (permalink / raw)
  To: Tjernlund; +Cc: git
In-Reply-To: <001501c7c02a$4bc130a0$0e67a8c0@Jocke>

Tjernlund <tjernlund@tjernlund.se> wrote:
> I have noticed that if I do a git-svn set-tree, remotes/git-svn
> retains the parent from the branch where set-tree was performed.
> 
> If a coworker wants recreate my tree by using git-svn init && git-svn
> fetch he looses the parent I have in my tree.
> 
> I wonder if not git-svn set-tree can record the parent information in
> the svn repos log, so that git-svn init/fetch can recreate the parent
> relationship?

We could at yet another non-standardized property into SVN to handle
merges.  Currently there are at least two properties used in the SVN/SVK
world to represent merges (Sam Vilain can give you the fun details of
each one!).

I'm afraid adding a third incompatible yet similair property for git-svn
would just confuse people.

I've become very much against crazy stuff like set-tree which ends up
creating a M:N history mapping between git and svn.  1:1 is the simplest
and easiest.  I'm more than willing to sacrifice multi-parent histories
in git for easier compatibility with other systems.

Heck, linear history is just easier to deal with and probably preferable
in most/many cases.  I'm sure that the rising popularity of git-rebase,
quilt, stgit, guilt, mq and other like tools is a testament to that.

-- 
Eric Wong

^ permalink raw reply

* Re: [PATCH] Fix "git log --parent -m" from emitting duplicated parent info
From: Marco Costalba @ 2007-07-09  5:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, Git Mailing List
In-Reply-To: <7vps32xsof.fsf@assigned-by-dhcp.cox.net>

On 7/9/07, Junio C Hamano <gitster@pobox.com> wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> Second try, this time not doing the simplification when --full-history
> is given.
>

Probably I'm wrong but I would think the first original version with
post-filtering is more
correct.

Indeed we can use and _do_ use --full-history not only for printing
all revisions but to _walk_ the tree in a certain way, different from
default.

IMHO --parents + --full-history is absolutely legal because the first
flag indicate how to walk the tree and the second flag indicate what
to show to the user.

I agree the original patch is much uglier then your last version but I
would say is more correct.

Thanks
Marco

^ permalink raw reply

* Re: [PATCH] Fix "git log --parent -m" from emitting duplicated parent info
From: Junio C Hamano @ 2007-07-09  6:19 UTC (permalink / raw)
  To: Marco Costalba; +Cc: Junio C Hamano, Johannes Schindelin, Git Mailing List
In-Reply-To: <e5bfff550707082246k39286517o58fe89d80e05e08@mail.gmail.com>

"Marco Costalba" <mcostalba@gmail.com> writes:

> Indeed we can use and _do_ use --full-history not only for printing
> all revisions but to _walk_ the tree in a certain way, different from
> default.

Yes, --full-history essentially tells the machnery to walk all
side branches even when a merge takes all the specified paths
from one parent's version.  You will end up walking all five
branches from the first pentapus "addafaf" in git.git history,
even if history simplification with pathspec would have narrowed
it down to three paths if you did not give that option.

> IMHO --parents + --full-history is absolutely legal because the first
> flag indicate how to walk the tree and the second flag indicate what
> to show to the user.

I think we are in agreement.  You are telling the machinery to
walk all five branches with --full-history.  But you are not
telling the machinery to keep the intermediate commits (i.e. you
do not give --sparse), so these five branches can be simplified
to lead to the same parent.  I think I was confused by dense vs
simplify_history when I wrote that message in the thread
Johannes pointed out:

	http://thread.gmane.org/gmane.comp.version-control.git/29222

Making the duplicate parent removal depend on "dense" makes
sense, but the rewrite_parent() codepath is used only when dense
anyway.

Thanks for a timely objection and sanity checking.

^ permalink raw reply

* Re: [PATCH v2] Make fetch-pack a builtin with an internal API
From: Daniel Barkalow @ 2007-07-09  6:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwsxaw2xu.fsf@assigned-by-dhcp.cox.net>

On Sun, 8 Jul 2007, Junio C Hamano wrote:

> Are _identifiers with leading underscore Kosher thing to do, I
> wonder...  We do have ones with trailing ones (mostly qsort
> functions) and I think they are done that way for the sake of
> standards conformance.

I'm not sure; I inherited that bit of code from Julian. Do we have a 
standard idiom for a function that sets a bunch of static variables?

> diff --git a/fetch-pack.h b/fetch-pack.h
> new file mode 100644
> index 0000000..2bd05a8
> --- /dev/null
> +++ b/fetch-pack.h
> @@ -0,0 +1,16 @@
> +#ifndef FETCH_PACK_API
> 
> We seem to say "#ifndef FETCH_PACK_H" in such a case, though.

I was trying to convey that this is the C API to call fetch-pack directly, 
rather than something used by the builtin, or by the wrapper for calling 
the builtin. But the inclusion guard is probably not going to be noticed 
anyway, and I don't think it's worth making the header filename verbose.

> Nobody seems to call setup_fetch_pack() yet.  How complete is
> this patch meant to be?

It's part of a series that leads up to making fetch a builtin. I'm trying 
to get in bits that are bounded by logical APIs. The roadmap here is that 
transport.{c,h} from one of my other patches will get a function to fetch 
a set of refs, and it will (for a suitable URL format) call 
setup_fetch_pack() with the appropriate options and then call 
fetch_pack(). builtin-fetch will use this function to actually get objects 
once it has determined which ones it should get.

I think I've now got the whole series to a point where everything's 
submittable, if you'd like to see the whole thing. It's actually composed 
of 6 initial independant sub-series (mostly single patches) of which I've 
submitted 4 (three today and the one that modularizes the commit-walker 
infrastructure and removes the obsolete ones), and a final series of 3 
that implements fetch on top of the rest. How should I number this?

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: Preferring shallower deltas on repack
From: Brian Downing @ 2007-07-09  6:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v1wfixhvk.fsf@assigned-by-dhcp.cox.net>

On Sun, Jul 08, 2007 at 10:31:43PM -0700, Junio C Hamano wrote:
> Putting aside a potential argument that the way the file in
> question, version.lisp-expr, is kept track of might be insane,
> this is an interesting topic.

Yeah, that version numbering system worked quite well for CVS, given its
lack of any other kind of useful whole-tree versioning, and the fact
that there wasn't much branching and merging, due to it being a pain in
the ass.  If an when we move to something like Git, something else will
have to be done, as that file will /always/ be in conflict.

> In addition to the above stats, it may be interesting to know:
> 
>  - pack generation time and memory footprint (/usr/bin/time);
> 
>    I suspect you would have to try_delta more candidates, so
>    this may degrade a bit, but that is done for getting a better
>    deltification, so we would need to see if the extra cost is
>    within reason and worth spending.

It was already try_delta'ing everything in the window.  The only
difference now is that create_delta may generate one more byte of delta
before giving up.  That doesn't seem to have affected things at all
outside of sampling noise:

(These timings are for the Git pack on Linux/amd64, --window and --depth
both 100.  Since /usr/bin/time doesn't seem to report any useful memory
statistics on Linux, I also have a "ps aux" line from when the memory
size looked stable.  This was different from run to run but it shows the
two are in the same order of magnitude.)

Unpatched:
54.99user 0.18system 0:56.80elapsed 97%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (14major+32417minor)pagefaults 0swaps
bdowning  5290 98.7  4.5 106788 92900 pts/1    R+   01:26   0:49 git pack-obj

Patched:
55.37user 0.19system 0:56.35elapsed 98%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+32249minor)pagefaults 0swaps
bdowning  6086  100  4.5 106880 92996 pts/1    R+   01:29   0:49 git pack-obj

>  - resulting pack size (ls -l pack-*.pack)
> 
>    I do not expect your change would degrade in this area, as
>    you are currently not trading size with shallower delta
>    depth.

The patched version is actually smaller in both SBCL's and Git's case
(again, --window 100 and --depth 100):

SBCL: 61696 bytes smaller (13294225-13232529)
Git:  16010 bytes smaller (12690424-12674414)

I believe the reason for this is that more deltas can get in under the
depth limit.  If I repack the Git pack with --depth=999999999, the patched
version generates a pack that is 1793 bytes smaller.  (12334183-12332390)
(Hmm, I was expecting that to be the same, I'm not sure why it's not.
Padding?)

> Regarding your patch, I think it does not look too bad, as you
> never pick delta that is larger than the best-so-far in favor of
> shallower depth.
> 
> It would become worrysome (*BUT* infinitely more interesting)
> once you start talking about a tradeoff between slightly larger
> delta and much shorter delta.  Such a tradeoff, if done right,
> would make a lot of sense, but I do not offhand think of a way
> to strike a proper balance between them efficiently.

Yeah, I was thinking about that too, and came to the same conclusion.
I suspect you'd have to save a /lot/ of delta depth to want to pay any
more I/O, though.

Another thing that might be iffy (and complicated) is that if you keep
making a good low-depth delta off of a particular object, it might be
good to promote it so it stays in the window for longer.

-bcd

^ permalink raw reply

* rerere fails to record resolution if file doesn't exist in merge base
From: Uwe Kleine-König @ 2007-07-09  7:07 UTC (permalink / raw)
  To: Git Mailing List

Hello,

Some time ago, I sent a test for that[1], but the patch doesn't apply
anymore.

Anyhow, the failure still exists, even though the original report was
sent when rerere was still a perl script ...

	zeisberg@cassiopeia:/tmp/rerere$ git init
	Initialized empty Git repository in .git/
	zeisberg@cassiopeia:/tmp/rerere$ mkdir .git/rerere
	zeisberg@cassiopeia:/tmp/rerere$ echo just something to commit > file
	zeisberg@cassiopeia:/tmp/rerere$ git add file
	zeisberg@cassiopeia:/tmp/rerere$ git commit -m 1
	Created initial commit 51384cb: 1
	 1 files changed, 1 insertions(+), 0 deletions(-)
	 create mode 100644 file
	zeisberg@cassiopeia:/tmp/rerere$ git branch b
	zeisberg@cassiopeia:/tmp/rerere$ echo lala > lolo
	zeisberg@cassiopeia:/tmp/rerere$ git add lolo
	zeisberg@cassiopeia:/tmp/rerere$ git commit -m 2
	Created commit 98b91cc: 2
	 1 files changed, 1 insertions(+), 0 deletions(-)
	 create mode 100644 lolo
	zeisberg@cassiopeia:/tmp/rerere$ git checkout b
	Switched to branch "b"
	zeisberg@cassiopeia:/tmp/rerere$ echo lali > lolo
	zeisberg@cassiopeia:/tmp/rerere$ git add lolo
	zeisberg@cassiopeia:/tmp/rerere$ git commit -m 2a
	Created commit bb900f3: 2a
	 1 files changed, 1 insertions(+), 0 deletions(-)
	 create mode 100644 lolo
	zeisberg@cassiopeia:/tmp/rerere$ git pull . master
	Auto-merged lolo
	CONFLICT (add/add): Merge conflict in lolo
	Automatic merge failed; fix conflicts and then commit the result.
	zeisberg@cassiopeia:/tmp/rerere$ perl -n -i -e "print if /^l/" lolo
	zeisberg@cassiopeia:/tmp/rerere$ cat lolo
	lali
	lala
	zeisberg@cassiopeia:/tmp/rerere$ git rerere
	zeisberg@cassiopeia:/tmp/rerere$ 

In my eyes the last command should have recorded the resolution for
lolo, shouldn't it?

Best regards
Uwe

[1] http://article.gmane.org/gmane.comp.version-control.git/19267

-- 
Uwe Kleine-König

http://www.google.com/search?q=1+stone%3D

^ permalink raw reply

* Re: Preferring shallower deltas on repack
From: Junio C Hamano @ 2007-07-09  7:27 UTC (permalink / raw)
  To: Brian Downing; +Cc: git
In-Reply-To: <20070709065235.GJ4087@lavos.net>

bdowning@lavos.net (Brian Downing) writes:

> (These timings are for the Git pack on Linux/amd64, --window and --depth
> both 100.  Since /usr/bin/time doesn't seem to report any useful memory
> statistics on Linux, I also have a "ps aux" line from when the memory
> size looked stable.  This was different from run to run but it shows the
> two are in the same order of magnitude.)
>
> Unpatched:
> 54.99user 0.18system 0:56.80elapsed 97%CPU (0avgtext+0avgdata 0maxresident)k
> 0inputs+0outputs (14major+32417minor)pagefaults 0swaps
> bdowning  5290 98.7  4.5 106788 92900 pts/1    R+   01:26   0:49 git pack-obj
>
> Patched:
> 55.37user 0.19system 0:56.35elapsed 98%CPU (0avgtext+0avgdata 0maxresident)k
> 0inputs+0outputs (0major+32249minor)pagefaults 0swaps
> bdowning  6086  100  4.5 106880 92996 pts/1    R+   01:29   0:49 git pack-obj

The number of minor faults are comparable (slightly favorable),
which is a good sign.

> The patched version is actually smaller in both SBCL's and Git's case
> (again, --window 100 and --depth 100):
>
> SBCL: 61696 bytes smaller (13294225-13232529)
> Git:  16010 bytes smaller (12690424-12674414)
>
> I believe the reason for this is that more deltas can get in under the
> depth limit.

Very sensible indeed.

>> It would become worrysome (*BUT* infinitely more interesting)
>> once you start talking about a tradeoff between slightly larger
>> delta and much shorter delta.  Such a tradeoff, if done right,
>> would make a lot of sense, but I do not offhand think of a way
>> to strike a proper balance between them efficiently.
>
> Yeah, I was thinking about that too, and came to the same conclusion.
> I suspect you'd have to save a /lot/ of delta depth to want to pay any
> more I/O, though.

That may not be so.  Deeper delta also means more I/O (and
worse, because they can be from discontiguous areas) plus delta
application.

^ permalink raw reply

* Re: Preferring shallower deltas on repack
From: Brian Downing @ 2007-07-09  7:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vbqemvxyb.fsf@assigned-by-dhcp.cox.net>

On Mon, Jul 09, 2007 at 12:27:24AM -0700, Junio C Hamano wrote:
> >> It would become worrysome (*BUT* infinitely more interesting)
> >> once you start talking about a tradeoff between slightly larger
> >> delta and much shorter delta.  Such a tradeoff, if done right,
> >> would make a lot of sense, but I do not offhand think of a way
> >> to strike a proper balance between them efficiently.
> >
> > Yeah, I was thinking about that too, and came to the same conclusion.
> > I suspect you'd have to save a /lot/ of delta depth to want to pay any
> > more I/O, though.
> 
> That may not be so.  Deeper delta also means more I/O (and
> worse, because they can be from discontiguous areas) plus delta
> application.

Good point.

I guess if I were to think about a metric for deciding whether to go for
a small deep delta or a larger shallow one, I would probably keep track
of the total path size (the sum of the base size and all the delta sizes)
for each entry, and play with a weighting formula of single delta size
verses total path size.

-bcd

^ permalink raw reply

* Re: [PATCH] git-gui: Allow users to set commit.signoff from options.
From: Gerrit Pape @ 2007-07-09  7:43 UTC (permalink / raw)
  To: Shawn O. Pearce, git
In-Reply-To: <20070708214832.GC4436@spearce.org>

On Sun, Jul 08, 2007 at 05:48:32PM -0400, Shawn O. Pearce wrote:
> Gerrit Pape <pape@smarden.org> wrote:
> > Users may want to automatically sign-off any commit for a specific
> > repository.  If they are mostly a git-gui user they should be able to
> > view/set this option from within the git-gui environment, rather than
> > needing to edit a raw text file on their local filesystem.
> 
> Sure.  But your patch to git-gui actually just lets the user set
> the flag, but doesn't make git-gui honor it.  So the user can set
> "Automatically Sign-Off" through git-gui but it will have no effect
> within git-gui (git-gui doesn't use git-commit.sh, it has its own
> pure-Tcl implementation).

Ups, sorry.

> I'm not applying this to git-gui, for the very same reason that
> Junio already gave as to why he won't apply the git-commit.sh patch.
> 
> Within git-gui adding a signoff is either one mouse click (the
> button on the toolbar), a single keystroke (Ctrl-S) or a menu action
> (Commit->Signoff).  Three easy ways to insert the signoff line.
> But it still needs to be a choice from the user, every time they
> make a commit.

Yes, thanks, Gerrit.

^ permalink raw reply

* [PATCH 1/2] stash: implement "stash create"
From: Junio C Hamano @ 2007-07-09  7:57 UTC (permalink / raw)
  To: git

This subcommand creates a stash from the current state and writes out the
resulting commit object ID to the standard output, without updating the
stash ref nor resetting the tree.  It is intended to be used by scripts
to temporarily rewind the working tree to a clean state.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * This is currently only needed for the next one, which I do
   not think is ready for 1.5.3, so both will stay in either
   'pu' or perhaps in 'next'.

 git-stash.sh |   25 +++++++++++++++++++++----
 1 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index de13dd1..8c22cd4 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -23,7 +23,7 @@ clear_stash () {
 	: >"$logfile"
 }
 
-save_stash () {
+create_stash () {
 	stash_msg="$1"
 
 	if no_changes
@@ -31,9 +31,6 @@ save_stash () {
 		echo >&2 'No local changes to save'
 		exit 0
 	fi
-	test -f "$GIT_DIR/logs/$ref_stash" ||
-		clear_stash || die "Cannot initialize stash"
-
 	# state of the base commit
 	if b_commit=$(git rev-parse --verify HEAD)
 	then
@@ -79,7 +76,20 @@ save_stash () {
 	w_commit=$(printf '%s\n' "$stash_msg" |
 		git commit-tree $w_tree -p $b_commit -p $i_commit) ||
 		die "Cannot record working tree state"
+}
+
+save_stash () {
+	stash_msg="$1"
+
+	if no_changes
+	then
+		echo >&2 'No local changes to save'
+		exit 0
+	fi
+	test -f "$GIT_DIR/logs/$ref_stash" ||
+		clear_stash || die "Cannot initialize stash"
 
+	create_stash "$stash_msg"
 	git update-ref -m "$stash_msg" $ref_stash $w_commit ||
 		die "Cannot save the current status"
 	printf >&2 'Saved "%s"\n' "$stash_msg"
@@ -185,6 +195,13 @@ apply)
 clear)
 	clear_stash
 	;;
+create)
+	if test $# -gt 0 && test "$1" = create
+	then
+		shift
+	fi
+	create_stash "$*" && echo "$w_commit"
+	;;
 help | usage)
 	usage
 	;;
-- 
1.5.3.rc0.81.g1ed84

^ permalink raw reply related

* [PATCH 2/2] rebase: allow starting from a dirty tree.
From: Junio C Hamano @ 2007-07-09  7:59 UTC (permalink / raw)
  To: git

This uses the new "git stash create" interface to stash away the dirty state
you have in your working tree before starting a rebase, and then replaying
it when you are done with stashing.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * Not 'master' material yet without heavy field testing, and it is
   not a good time to do so before 1.5.3-rc1, so this will stay
   in 'pu' and perhaps in 'next' for now.

 git-rebase.sh |   66 ++++++++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 52 insertions(+), 14 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index cbafa14..602723a 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -116,10 +116,25 @@ call_merge () {
 }
 
 finish_rb_merge () {
+	if test -f "$dotest/stash"
+	then
+		stash=$(cat "$dotest/stash")
+		git stash apply --index "$stash"
+	fi
 	rm -r "$dotest"
 	echo "All done."
 }
 
+unstash_and_exit () {
+	err=$?
+	if test -f "$1" && test $err = 0
+	then
+		stash=$(cat "$1")
+		git stash apply --index "$stash"
+	fi
+	exit $err
+}
+
 is_interactive () {
 	test -f "$dotest"/interactive ||
 	while case $#,"$1" in 0,|*,-i|*,--interactive) break ;; esac
@@ -155,7 +170,7 @@ do
 			exit
 		fi
 		git am --resolved --3way --resolvemsg="$RESOLVEMSG"
-		exit
+		unstash_and_exit .dotest/stash
 		;;
 	--skip)
 		if test -d "$dotest"
@@ -175,20 +190,29 @@ do
 			exit
 		fi
 		git am -3 --skip --resolvemsg="$RESOLVEMSG"
-		exit
+		unstash_and_exit .dotest/stash
 		;;
 	--abort)
 		git rerere clear
 		if test -d "$dotest"
 		then
+			if test -f "$dotest/stash"
+			then
+				stash=$(cat "$dotest/stash")
+			fi
 			rm -r "$dotest"
 		elif test -d .dotest
 		then
+			if test -f ".dotest/stash"
+			then
+				stash=$(cat ".dotest/stash")
+			fi
 			rm -r .dotest
 		else
 			die "No rebase in progress?"
 		fi
 		git reset --hard ORIG_HEAD
+		test -z "$stash" || git stash apply --index "$stash"
 		exit
 		;;
 	--onto)
@@ -252,16 +276,6 @@ else
 	fi
 fi
 
-# The tree must be really really clean.
-git update-index --refresh || exit
-diff=$(git diff-index --cached --name-status -r HEAD)
-case "$diff" in
-?*)	echo "cannot rebase: your index is not up-to-date"
-	echo "$diff"
-	exit 1
-	;;
-esac
-
 # The upstream head must be given.  Make sure it is valid.
 upstream_name="$1"
 upstream=`git rev-parse --verify "${upstream_name}^0"` ||
@@ -271,11 +285,20 @@ upstream=`git rev-parse --verify "${upstream_name}^0"` ||
 onto_name=${newbase-"$upstream_name"}
 onto=$(git rev-parse --verify "${onto_name}^0") || exit
 
+# The tree must be clean enough for us to create a stash
+stash=$(git stash create) || exit
+if test -n "$stash"
+then
+	echo >&2 "Stashed away your working tree changes"
+	git reset --hard
+fi
+
 # If a hook exists, give it a chance to interrupt
 if test -x "$GIT_DIR/hooks/pre-rebase"
 then
 	"$GIT_DIR/hooks/pre-rebase" ${1+"$@"} || {
 		echo >&2 "The pre-rebase hook refused to rebase."
+		test -z "$stash" || git stash apply --index "$stash"
 		exit 1
 	}
 fi
@@ -284,7 +307,10 @@ fi
 case "$#" in
 2)
 	branch_name="$2"
-	git-checkout "$2" || usage
+	git-checkout "$2" || {
+		test -z "$stash" || git stash apply --index "$stash"
+		usage
+	}
 	;;
 *)
 	if branch_name=`git symbolic-ref -q HEAD`
@@ -307,6 +333,7 @@ if test "$upstream" = "$onto" && test "$mb" = "$onto" &&
 	! git rev-list --parents "$onto".."$branch" | grep " .* " > /dev/null
 then
 	echo >&2 "Current branch $branch_name is up to date."
+	test -z "$stash" || git stash apply --index "$stash"
 	exit 0
 fi
 
@@ -326,6 +353,7 @@ git-reset --hard "$onto"
 if test "$mb" = "$branch"
 then
 	echo >&2 "Fast-forwarded $branch_name to $onto_name."
+	test -z "$stash" || git stash apply --index "$stash"
 	exit 0
 fi
 
@@ -333,7 +361,16 @@ if test -z "$do_merge"
 then
 	git format-patch -k --stdout --full-index --ignore-if-in-upstream "$upstream"..ORIG_HEAD |
 	git am $git_am_opt --binary -3 -k --resolvemsg="$RESOLVEMSG"
-	exit $?
+	err=$?
+
+	if test $err = 0
+	then
+		test -z "$stash" || git stash apply --index "$stash"
+		exit
+	else
+		test -z "$stash" || echo "$stash" >.dotest/stash
+		exit $err
+	fi
 fi
 
 # start doing a rebase with git-merge
@@ -344,6 +381,7 @@ echo "$onto" > "$dotest/onto"
 echo "$onto_name" > "$dotest/onto_name"
 prev_head=`git rev-parse HEAD^0`
 echo "$prev_head" > "$dotest/prev_head"
+test -z "$stash" || echo "$stash" >"$dotest/stash"
 
 msgnum=0
 for cmt in `git rev-list --reverse --no-merges "$upstream"..ORIG_HEAD`
-- 
1.5.3.rc0.81.g1ed84

^ permalink raw reply related

* Re: rerere fails to record resolution if file doesn't exist in merge base
From: Junio C Hamano @ 2007-07-09  8:22 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: Git Mailing List
In-Reply-To: <20070709070725.GA4445@lala>

Uwe Kleine-König  <ukleinek@informatik.uni-freiburg.de> writes:

> Anyhow, the failure still exists, even though the original report was
> sent when rerere was still a perl script ...
>
> 	zeisberg@cassiopeia:/tmp/rerere$ git init
> 	Initialized empty Git repository in .git/
> 	zeisberg@cassiopeia:/tmp/rerere$ mkdir .git/rerere

This should be creating .git/rr-cache, but what you said in your
message is correct regardless.

> In my eyes the last command should have recorded the resolution for
> lolo, shouldn't it?

I think two-file merge (ancestor did not have it, and you and he
added the path differently) is rare enough that it was dropped
outside of the radar.  A fix would probably be a trivial change
to builtin-rerere.c::find_conflict(), I think.  While it would
still be sane to insist that we do not do rerere for symlinks,
and require to have stages #2 and #3, we should be able to drop
the requirement to have stage #1.  rerere does not use
information from there anyway.

Not even compile tested, but something like this should do.

 builtin-rerere.c |   17 +++++++----------
 1 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/builtin-rerere.c b/builtin-rerere.c
index c25b3d5..6ffc43d 100644
--- a/builtin-rerere.c
+++ b/builtin-rerere.c
@@ -168,19 +168,16 @@ static int find_conflict(struct path_list *conflict)
 	int i;
 	if (read_cache() < 0)
 		return error("Could not read index");
-	for (i = 0; i + 2 < active_nr; i++) {
-		struct cache_entry *e1 = active_cache[i];
-		struct cache_entry *e2 = active_cache[i+1];
-		struct cache_entry *e3 = active_cache[i+2];
-		if (ce_stage(e1) == 1 &&
-		    ce_stage(e2) == 2 &&
+	for (i = 0; i+1 < active_nr; i++) {
+		struct cache_entry *e2 = active_cache[i];
+		struct cache_entry *e3 = active_cache[i+1];
+		if (ce_stage(e2) == 2 &&
 		    ce_stage(e3) == 3 &&
-		    ce_same_name(e1, e2) && ce_same_name(e1, e3) &&
-		    S_ISREG(ntohl(e1->ce_mode)) &&
+		    ce_same_name(e2, e3) &&
 		    S_ISREG(ntohl(e2->ce_mode)) &&
 		    S_ISREG(ntohl(e3->ce_mode))) {
-			path_list_insert((const char *)e1->name, conflict);
-			i += 2;
+			path_list_insert((const char *)e2->name, conflict);
+			i++; /* skip over both #2 and #3 */
 		}
 	}
 	return 0;

^ permalink raw reply related

* Re: [mingw port] Problem starting git subcommands
From: Johannes Sixt @ 2007-07-09  8:49 UTC (permalink / raw)
  To: git
In-Reply-To: <87eacd830707081126x69b2edb3o9fd89733ff2258f4@mail.gmail.com>

Henning Rogge wrote:
> 
> I have experimented with GIT on my linux system successfully but one
> of my projects has to be done on Windows (neither VMware Workstation
> nor Wine can help at the moment) so I got a copy of the mingw port of
> GIT (http://lilypond.org/git/binaries/mingw/git-1.5.2.1-1.mingw.exe).
> 
> Installation succeeded without a single error but when I tried to run
> a few GIT commands I noticed a "bug":
> 
> the git.exe command does not recognize any scripted subcommand
> (git-clone in my case) !
> > C:\Programme\Git>git clone
> > git: 'clone' is not a git-command
> 
> Do I need another software package ? I tried to install the Mingw
> package itself, but it did not help at all.

Yes, you need a shell and some Posix tools. Install MSYS, e.g.

   MSYS-1.0.11-2004.04.30-1.exe

You will also want a ssh and perl, e.g. from

   msysDTK-1.0.1.exe

and MinGW's tcl/tk from

   tcltk-8.4.1-1.exe

Also, git-am seems to require bash 3.1, e.g. from

   bash-3.1-MSYS-1.0.11-snapshot.tar.bz2

-- Hannes

^ permalink raw reply

* Re: [PATCH 4/4] Add git-rewrite-commits
From: Johannes Sixt @ 2007-07-09  9:01 UTC (permalink / raw)
  To: git
In-Reply-To: <20070708211034.GO1528MdfPADPa@greensroom.kotnet.org>

Sven Verdoolaege wrote:
> I guess the major thing that is missing is --subdirectory-filter.
> Anything else?

Yes, how about this:

  $ git rewrite-commits --index-map '
         testresult=$($HOME/bin/expensive-test);
         [ $testresult = t ] && $HOME/bin/tweak-index ' \
     --commit-map '
         [ $testresult = t ] && $HOME/bin/tweak-commit '

:-P

-- Hannes

^ permalink raw reply

* Re: [RFC][PATCH] Re: git-rm isn't the inverse action of git-add
From: Matthieu Moy @ 2007-07-09  9:45 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Jan Hudec, Yann Dirson, Christian Jaeger
In-Reply-To: <Pine.LNX.4.64.0707082240510.4248@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi,
>
> On Sun, 8 Jul 2007, Matthieu Moy wrote:
>
>> I'm not sure whether this is really wrong. The things git should
>> really care about are the index and the repository itself, and the
>> proposed behavior is consistant regarding that (either remove all
>> files from the index, or remove none).
>
> Well, I think it is wrong for the same reason as it is wrong to apply the 
> changes to _any_ file when one would fail.  And since "git apply" shares 
> my understanding, I think "git rm" should, too.

OK, let's say I'm convinced ;-).

>> > I suspect that this case does never fail. 0 means success for 
>> > remove_file().  Not good.  You should at least have a way to ensure that 
>> > it removed the files from the working tree from a script.  Otherwise there 
>> > is not much point in returning a value to begin with.
>> 
>> I've changed it to have exit_status = 1 if git-rm aborted before
>> starting, and 2 if git-rm skiped some file removals (and of course, 0
>> if everything is done as expected).
>
> Oh, so you do not take the return value of this function to determine if 
> it has or has not done something with the files?  That's a bit confusing.

I did, but previously, I kept the code that "die()"s if the first call
to remove_file() "fails". In remove_file_maybe(), not removing a file
because it's not sure it's safe to delete it is not a failure, so I
had to put a "return 0;" here to avoid the fatal error. My first patch
had return status !=0 if we tried to remove the file, and it failed. I
changed that.

> I meant to complain about your OP, but this time it is even worse.  The 
> best way to guarantee that a patch gets lost in a thread is to move it _at 
> the end_ of a reply.

I had posted the patch for info, but I did expect this one to get
lost, since it's definitely not complete.

I'll post an updated patch with a testcase and an appropriate subject
line within a few days (I don't have time right now).

Thanks,

-- 
Matthieu

^ 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