Git development
 help / color / mirror / Atom feed
* Re: In favor of "git commit --no-parent"
From: Phil Hord @ 2011-09-29 22:07 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Michael Witten, Carlos Martín Nieto, vra5107,
	Michael J Gruber, Matthieu Moy, Eric Raible, Philip Oakley,
	Jeff King, Jay Soffian, git
In-Reply-To: <7v4nzvrp3k.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:
> Phil Hord <phil.hord@gmail.com> writes:
>
>> I think a user looking for this functionality -- either a new git user
>> or a user who seldom uses the "create secondary root commit" command
>> -- would first try 'git help init'.  It seems logical to me that I
>> should be able to do this:
>>
>>   cd my-git-repo
>>   git init --root=<newbranch> .
>>
>> This feels natural to me for this operation.
>
> Hmm, this does not feel very natural to me; unless "git init --root=work"
> uses 'work' branch instead of 'master' when creating a new repository,
> that is. But it is attractive that init is much less often used than
> checkout/commit and everybody knows it is somewhat a _special_ operation.

That's what I meant.  Sorry my example wasn't more clear about that.
I also considered "--new-branch" and "--new-root".  I like the latter
more, but I didn't like the extra hyphen.  Considering the rarity and
the prior art, I guess either one is more clear here.

>>> That leaves "Hidden History" the only useful use case. IOW, the answer to
>>> the first question above is not "Separate or Hidden History", but is
>>> "Hidden History and nothing else".
>>
>> I think you're saying that the "hidden history" scenario is more
>> special than the "separate history" one because of these reasons:
>
> Not at all.
>
> I am saying that "separate history" has no place in git workflow, if these
> multiple roots _originate_ in the same single repository with a working
> tree.

No place in *your* workflow.  Oh, wait.  Except it has, and you use it
in the git tree.  So, um...  I'm confused.

> And all of "git checkout --orphan", "git commit --root" and your
> "git init --root" are solutions to make multiple roots _originate_ in the
> same single repository with a working tree.
>
> I have no trouble in a single repository with multiple roots if that is
> done in a distribution point, which by definition does not need and
> typically does not have any working tree. Options to "checkout/commit"
> would not help as they need a working tree.

I'm not sure where you're going with the "working tree" objection.
Are you saying that it's ok to _create_ "separate histories" in a bare
repository but wrong to do so in a non-bare one?  But surely this
means it is ok to _have_ "separate histories" in a non-bare
repository, since that is what I will get when I do a simple git-clone
of the bare one.

If I understand right, the mechanics of the initial creation is what
bothers you.  Is that right?  If so, we're on the same page here,
because it bothers me too.  The commit or checkout alternatives seem
like two halves of a turd sandwich.  Both ends are wrong somehow, and
it's because of the state of the working directory in the interim.

If I don't understand you right, ignore the sandwich metaphor and
please explain.

> The way to do it is to work in multiple repositories, one for each of
> these roots, and push into a single repository from them.

That's one way to do it.

>>> And a half of the the answer to the second question is "checkout --orphan"
>>> (and the other half would be "filter-branch"). "checkout --orphan" does
>>> have major safety advantage than introducing "commit --no-parent", as Peff
>>> pointed out earlier (to which I agreed).
>>
>> The thing I don't understand about "checkout --orphan" is exactly what
>> you're getting when you do this.  I assume you get a populated index
>> and a non-existent HEAD. This seems a lot like "git init" to me,
>> especially in the non-existent HEAD area.
>
> It is "HEAD pointing at a branch that does not yet exist", but I find it
> strangely attractive ;-)
>
>> I didn't think git init would be much use for this scenario before,
>> but now I've changed my mind.
>>
>>   git init --root=<newbranch> --keep-index
>>
>> Again, this avoids complicating the common commands.  But maybe it
>> does overload init with extra baggage.
>
> I do not think you would even need --keep-index there (running "git init"
> in an existing repository to see what it does. It does not touch the index
> nor HEAD).

In my imagined extension, the "separate history" action needs a clean
index.  I guess 'git rm -fr *' will suffice, but I think an in-line
switch would be more useful... for this "non-git workflow". :-)

> I am not sure if "--root" is the right name but if "git init --root=work"
> that is run to create (not re-init) a new repository points HEAD at a yet
> to be created 'work' branch instead of 'master', I think it would be a
> reasonable alternative.

That's what I meant.  How about this?

   git init --new-root-branch=<new-branch-name>

> By the way, why did you drop the mailing list?

Clicked the wrong button.  Not used to gmail yet for this.  Sorry about that.

Phil

^ permalink raw reply

* [PATCH v2] refs: Use binary search to lookup refs faster
From: Julian Phillips @ 2011-09-29 22:04 UTC (permalink / raw)
  To: Julian Phillips
  Cc: Julian Phillips, Martin Fick, Christian Couder, git,
	Christian Couder, Thomas Rast
In-Reply-To: <20110929041811.5363.33396.julian@quantumfyre.co.uk>

Currently we linearly search through lists of refs when we need to
find a specific ref.  This can be very slow if we need to lookup a
large number of refs.  By changing to a binary search we can make this
faster.

In order to be able to use a binary search we need to change from
using linked lists to arrays, which we can manage using ALLOC_GROW.

We can now also use the standard library qsort function to sort the
refs arrays.

Signed-off-by: Julian Phillips <julian@quantumfyre.co.uk>
---

Previous version caused a regression in the test suite ... :$

 refs.c |  329 ++++++++++++++++++++++++++--------------------------------------
 1 files changed, 133 insertions(+), 196 deletions(-)

diff --git a/refs.c b/refs.c
index a49ff74..35bba97 100644
--- a/refs.c
+++ b/refs.c
@@ -8,14 +8,18 @@
 #define REF_KNOWS_PEELED 04
 #define REF_BROKEN 010
 
-struct ref_list {
-	struct ref_list *next;
+struct ref_entry {
 	unsigned char flag; /* ISSYMREF? ISPACKED? */
 	unsigned char sha1[20];
 	unsigned char peeled[20];
 	char name[FLEX_ARRAY];
 };
 
+struct ref_array {
+	int nr, alloc;
+	struct ref_entry **refs;
+};
+
 static const char *parse_ref_line(char *line, unsigned char *sha1)
 {
 	/*
@@ -44,108 +48,58 @@ static const char *parse_ref_line(char *line, unsigned char *sha1)
 	return line;
 }
 
-static struct ref_list *add_ref(const char *name, const unsigned char *sha1,
-				int flag, struct ref_list *list,
-				struct ref_list **new_entry)
+static void add_ref(const char *name, const unsigned char *sha1,
+		    int flag, struct ref_array *refs,
+		    struct ref_entry **new_entry)
 {
 	int len;
-	struct ref_list *entry;
+	struct ref_entry *entry;
 
 	/* Allocate it and add it in.. */
 	len = strlen(name) + 1;
-	entry = xmalloc(sizeof(struct ref_list) + len);
+	entry = xmalloc(sizeof(struct ref) + len);
 	hashcpy(entry->sha1, sha1);
 	hashclr(entry->peeled);
 	memcpy(entry->name, name, len);
 	entry->flag = flag;
-	entry->next = list;
 	if (new_entry)
 		*new_entry = entry;
-	return entry;
+	ALLOC_GROW(refs->refs, refs->nr + 1, refs->alloc);
+	refs->refs[refs->nr++] = entry;
 }
 
-/* merge sort the ref list */
-static struct ref_list *sort_ref_list(struct ref_list *list)
+static int ref_entry_cmp(const void *a, const void *b)
 {
-	int psize, qsize, last_merge_count, cmp;
-	struct ref_list *p, *q, *l, *e;
-	struct ref_list *new_list = list;
-	int k = 1;
-	int merge_count = 0;
-
-	if (!list)
-		return list;
-
-	do {
-		last_merge_count = merge_count;
-		merge_count = 0;
-
-		psize = 0;
+	struct ref_entry *one = *(struct ref_entry **)a;
+	struct ref_entry *two = *(struct ref_entry **)b;
+	return strcmp(one->name, two->name);
+}
 
-		p = new_list;
-		q = new_list;
-		new_list = NULL;
-		l = NULL;
+static void sort_ref_array(struct ref_array *array)
+{
+	qsort(array->refs, array->nr, sizeof(*array->refs), ref_entry_cmp);
+}
 
-		while (p) {
-			merge_count++;
+static struct ref_entry *search_ref_array(struct ref_array *array, const char *name)
+{
+	struct ref_entry *e, **r;
+	int len;
 
-			while (psize < k && q->next) {
-				q = q->next;
-				psize++;
-			}
-			qsize = k;
-
-			while ((psize > 0) || (qsize > 0 && q)) {
-				if (qsize == 0 || !q) {
-					e = p;
-					p = p->next;
-					psize--;
-				} else if (psize == 0) {
-					e = q;
-					q = q->next;
-					qsize--;
-				} else {
-					cmp = strcmp(q->name, p->name);
-					if (cmp < 0) {
-						e = q;
-						q = q->next;
-						qsize--;
-					} else if (cmp > 0) {
-						e = p;
-						p = p->next;
-						psize--;
-					} else {
-						if (hashcmp(q->sha1, p->sha1))
-							die("Duplicated ref, and SHA1s don't match: %s",
-							    q->name);
-						warning("Duplicated ref: %s", q->name);
-						e = q;
-						q = q->next;
-						qsize--;
-						free(e);
-						e = p;
-						p = p->next;
-						psize--;
-					}
-				}
+	if (name == NULL)
+		return NULL;
 
-				e->next = NULL;
+	len = strlen(name) + 1;
+	e = xmalloc(sizeof(struct ref) + len);
+	memcpy(e->name, name, len);
 
-				if (l)
-					l->next = e;
-				if (!new_list)
-					new_list = e;
-				l = e;
-			}
+	r = bsearch(&e, array->refs, array->nr, sizeof(*array->refs), ref_entry_cmp);
 
-			p = q;
-		};
+	free(e);
 
-		k = k * 2;
-	} while ((last_merge_count != merge_count) || (last_merge_count != 1));
+	if (r == NULL)
+		return NULL;
 
-	return new_list;
+	return *r;
 }
 
 /*
@@ -155,38 +109,37 @@ static struct ref_list *sort_ref_list(struct ref_list *list)
 static struct cached_refs {
 	char did_loose;
 	char did_packed;
-	struct ref_list *loose;
-	struct ref_list *packed;
+	struct ref_array loose;
+	struct ref_array packed;
 } cached_refs, submodule_refs;
-static struct ref_list *current_ref;
+static struct ref_entry *current_ref;
 
-static struct ref_list *extra_refs;
+static struct ref_array extra_refs;
 
-static void free_ref_list(struct ref_list *list)
+static void free_ref_array(struct ref_array *array)
 {
-	struct ref_list *next;
-	for ( ; list; list = next) {
-		next = list->next;
-		free(list);
-	}
+	int i;
+	for (i = 0; i < array->nr; i++)
+		free(array->refs[i]);
+	free(array->refs);
+	array->nr = array->alloc = 0;
+	array->refs = NULL;
 }
 
 static void invalidate_cached_refs(void)
 {
 	struct cached_refs *ca = &cached_refs;
 
-	if (ca->did_loose && ca->loose)
-		free_ref_list(ca->loose);
-	if (ca->did_packed && ca->packed)
-		free_ref_list(ca->packed);
-	ca->loose = ca->packed = NULL;
+	if (ca->did_loose)
+		free_ref_array(&ca->loose);
+	if (ca->did_packed)
+		free_ref_array(&ca->packed);
 	ca->did_loose = ca->did_packed = 0;
 }
 
 static void read_packed_refs(FILE *f, struct cached_refs *cached_refs)
 {
-	struct ref_list *list = NULL;
-	struct ref_list *last = NULL;
+	struct ref_entry *last = NULL;
 	char refline[PATH_MAX];
 	int flag = REF_ISPACKED;
 
@@ -205,7 +158,7 @@ static void read_packed_refs(FILE *f, struct cached_refs *cached_refs)
 
 		name = parse_ref_line(refline, sha1);
 		if (name) {
-			list = add_ref(name, sha1, flag, list, &last);
+			add_ref(name, sha1, flag, &cached_refs->packed, &last);
 			continue;
 		}
 		if (last &&
@@ -215,21 +168,20 @@ static void read_packed_refs(FILE *f, struct cached_refs *cached_refs)
 		    !get_sha1_hex(refline + 1, sha1))
 			hashcpy(last->peeled, sha1);
 	}
-	cached_refs->packed = sort_ref_list(list);
+	sort_ref_array(&cached_refs->packed);
 }
 
 void add_extra_ref(const char *name, const unsigned char *sha1, int flag)
 {
-	extra_refs = add_ref(name, sha1, flag, extra_refs, NULL);
+	add_ref(name, sha1, flag, &extra_refs, NULL);
 }
 
 void clear_extra_refs(void)
 {
-	free_ref_list(extra_refs);
-	extra_refs = NULL;
+	free_ref_array(&extra_refs);
 }
 
-static struct ref_list *get_packed_refs(const char *submodule)
+static struct ref_array *get_packed_refs(const char *submodule)
 {
 	const char *packed_refs_file;
 	struct cached_refs *refs;
@@ -237,7 +189,7 @@ static struct ref_list *get_packed_refs(const char *submodule)
 	if (submodule) {
 		packed_refs_file = git_path_submodule(submodule, "packed-refs");
 		refs = &submodule_refs;
-		free_ref_list(refs->packed);
+		free_ref_array(&refs->packed);
 	} else {
 		packed_refs_file = git_path("packed-refs");
 		refs = &cached_refs;
@@ -245,18 +197,17 @@ static struct ref_list *get_packed_refs(const char *submodule)
 
 	if (!refs->did_packed || submodule) {
 		FILE *f = fopen(packed_refs_file, "r");
-		refs->packed = NULL;
 		if (f) {
 			read_packed_refs(f, refs);
 			fclose(f);
 		}
 		refs->did_packed = 1;
 	}
-	return refs->packed;
+	return &refs->packed;
 }
 
-static struct ref_list *get_ref_dir(const char *submodule, const char *base,
-				    struct ref_list *list)
+static void get_ref_dir(const char *submodule, const char *base,
+			struct ref_array *array)
 {
 	DIR *dir;
 	const char *path;
@@ -299,7 +250,7 @@ static struct ref_list *get_ref_dir(const char *submodule, const char *base,
 			if (stat(refdir, &st) < 0)
 				continue;
 			if (S_ISDIR(st.st_mode)) {
-				list = get_ref_dir(submodule, ref, list);
+				get_ref_dir(submodule, ref, array);
 				continue;
 			}
 			if (submodule) {
@@ -314,12 +265,11 @@ static struct ref_list *get_ref_dir(const char *submodule, const char *base,
 					hashclr(sha1);
 					flag |= REF_BROKEN;
 				}
-			list = add_ref(ref, sha1, flag, list, NULL);
+			add_ref(ref, sha1, flag, array, NULL);
 		}
 		free(ref);
 		closedir(dir);
 	}
-	return list;
 }
 
 struct warn_if_dangling_data {
@@ -356,21 +306,21 @@ void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname)
 	for_each_rawref(warn_if_dangling_symref, &data);
 }
 
-static struct ref_list *get_loose_refs(const char *submodule)
+static struct ref_array *get_loose_refs(const char *submodule)
 {
 	if (submodule) {
-		free_ref_list(submodule_refs.loose);
-		submodule_refs.loose = get_ref_dir(submodule, "refs", NULL);
-		submodule_refs.loose = sort_ref_list(submodule_refs.loose);
-		return submodule_refs.loose;
+		free_ref_array(&submodule_refs.loose);
+		get_ref_dir(submodule, "refs", &submodule_refs.loose);
+		sort_ref_array(&submodule_refs.loose);
+		return &submodule_refs.loose;
 	}
 
 	if (!cached_refs.did_loose) {
-		cached_refs.loose = get_ref_dir(NULL, "refs", NULL);
-		cached_refs.loose = sort_ref_list(cached_refs.loose);
+		get_ref_dir(NULL, "refs", &cached_refs.loose);
+		sort_ref_array(&cached_refs.loose);
 		cached_refs.did_loose = 1;
 	}
-	return cached_refs.loose;
+	return &cached_refs.loose;
 }
 
 /* We allow "recursive" symbolic refs. Only within reason, though */
@@ -381,8 +331,8 @@ static int resolve_gitlink_packed_ref(char *name, int pathlen, const char *refna
 {
 	FILE *f;
 	struct cached_refs refs;
-	struct ref_list *ref;
-	int retval;
+	struct ref_entry *ref;
+	int retval = -1;
 
 	strcpy(name + pathlen, "packed-refs");
 	f = fopen(name, "r");
@@ -390,17 +340,12 @@ static int resolve_gitlink_packed_ref(char *name, int pathlen, const char *refna
 		return -1;
 	read_packed_refs(f, &refs);
 	fclose(f);
-	ref = refs.packed;
-	retval = -1;
-	while (ref) {
-		if (!strcmp(ref->name, refname)) {
-			retval = 0;
-			memcpy(result, ref->sha1, 20);
-			break;
-		}
-		ref = ref->next;
+	ref = search_ref_array(&refs.packed, refname);
+	if (ref != NULL) {
+		memcpy(result, ref->sha1, 20);
+		retval = 0;
 	}
-	free_ref_list(refs.packed);
+	free_ref_array(&refs.packed);
 	return retval;
 }
 
@@ -501,15 +446,13 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
 		git_snpath(path, sizeof(path), "%s", ref);
 		/* Special case: non-existing file. */
 		if (lstat(path, &st) < 0) {
-			struct ref_list *list = get_packed_refs(NULL);
-			while (list) {
-				if (!strcmp(ref, list->name)) {
-					hashcpy(sha1, list->sha1);
-					if (flag)
-						*flag |= REF_ISPACKED;
-					return ref;
-				}
-				list = list->next;
+			struct ref_array *packed = get_packed_refs(NULL);
+			struct ref_entry *r = search_ref_array(packed, ref);
+			if (r != NULL) {
+				hashcpy(sha1, r->sha1);
+				if (flag)
+					*flag |= REF_ISPACKED;
+				return ref;
 			}
 			if (reading || errno != ENOENT)
 				return NULL;
@@ -584,7 +527,7 @@ int read_ref(const char *ref, unsigned char *sha1)
 
 #define DO_FOR_EACH_INCLUDE_BROKEN 01
 static int do_one_ref(const char *base, each_ref_fn fn, int trim,
-		      int flags, void *cb_data, struct ref_list *entry)
+		      int flags, void *cb_data, struct ref_entry *entry)
 {
 	if (prefixcmp(entry->name, base))
 		return 0;
@@ -630,18 +573,12 @@ int peel_ref(const char *ref, unsigned char *sha1)
 		return -1;
 
 	if ((flag & REF_ISPACKED)) {
-		struct ref_list *list = get_packed_refs(NULL);
+		struct ref_array *array = get_packed_refs(NULL);
+		struct ref_entry *r = search_ref_array(array, ref);
 
-		while (list) {
-			if (!strcmp(list->name, ref)) {
-				if (list->flag & REF_KNOWS_PEELED) {
-					hashcpy(sha1, list->peeled);
-					return 0;
-				}
-				/* older pack-refs did not leave peeled ones */
-				break;
-			}
-			list = list->next;
+		if (r != NULL && r->flag & REF_KNOWS_PEELED) {
+			hashcpy(sha1, r->peeled);
+			return 0;
 		}
 	}
 
@@ -660,36 +597,39 @@ fallback:
 static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn fn,
 			   int trim, int flags, void *cb_data)
 {
-	int retval = 0;
-	struct ref_list *packed = get_packed_refs(submodule);
-	struct ref_list *loose = get_loose_refs(submodule);
+	int retval = 0, i, p = 0, l = 0;
+	struct ref_array *packed = get_packed_refs(submodule);
+	struct ref_array *loose = get_loose_refs(submodule);
 
-	struct ref_list *extra;
+	struct ref_array *extra = &extra_refs;
 
-	for (extra = extra_refs; extra; extra = extra->next)
-		retval = do_one_ref(base, fn, trim, flags, cb_data, extra);
+	for (i = 0; i < extra->nr; i++)
+		retval = do_one_ref(base, fn, trim, flags, cb_data, extra->refs[i]);
 
-	while (packed && loose) {
-		struct ref_list *entry;
-		int cmp = strcmp(packed->name, loose->name);
+	while (p < packed->nr && l < loose->nr) {
+		struct ref_entry *entry;
+		int cmp = strcmp(packed->refs[p]->name, loose->refs[l]->name);
 		if (!cmp) {
-			packed = packed->next;
+			p++;
 			continue;
 		}
 		if (cmp > 0) {
-			entry = loose;
-			loose = loose->next;
+			entry = loose->refs[l++];
 		} else {
-			entry = packed;
-			packed = packed->next;
+			entry = packed->refs[p++];
 		}
 		retval = do_one_ref(base, fn, trim, flags, cb_data, entry);
 		if (retval)
 			goto end_each;
 	}
 
-	for (packed = packed ? packed : loose; packed; packed = packed->next) {
-		retval = do_one_ref(base, fn, trim, flags, cb_data, packed);
+	if (l < loose->nr) {
+		p = l;
+		packed = loose;
+	}
+
+	for (; p < packed->nr; p++) {
+		retval = do_one_ref(base, fn, trim, flags, cb_data, packed->refs[p]);
 		if (retval)
 			goto end_each;
 	}
@@ -1005,24 +945,24 @@ static int remove_empty_directories(const char *file)
 }
 
 static int is_refname_available(const char *ref, const char *oldref,
-				struct ref_list *list, int quiet)
-{
-	int namlen = strlen(ref); /* e.g. 'foo/bar' */
-	while (list) {
-		/* list->name could be 'foo' or 'foo/bar/baz' */
-		if (!oldref || strcmp(oldref, list->name)) {
-			int len = strlen(list->name);
+				struct ref_array *array, int quiet)
+{
+	int i, namlen = strlen(ref); /* e.g. 'foo/bar' */
+	for (i = 0; i < array->nr; i++ ) {
+		struct ref_entry *entry = array->refs[i];
+		/* entry->name could be 'foo' or 'foo/bar/baz' */
+		if (!oldref || strcmp(oldref, entry->name)) {
+			int len = strlen(entry->name);
 			int cmplen = (namlen < len) ? namlen : len;
-			const char *lead = (namlen < len) ? list->name : ref;
-			if (!strncmp(ref, list->name, cmplen) &&
+			const char *lead = (namlen < len) ? entry->name : ref;
+			if (!strncmp(ref, entry->name, cmplen) &&
 			    lead[cmplen] == '/') {
 				if (!quiet)
 					error("'%s' exists; cannot create '%s'",
-					      list->name, ref);
+					      entry->name, ref);
 				return 0;
 			}
 		}
-		list = list->next;
 	}
 	return 1;
 }
@@ -1129,18 +1069,13 @@ static struct lock_file packlock;
 
 static int repack_without_ref(const char *refname)
 {
-	struct ref_list *list, *packed_ref_list;
-	int fd;
-	int found = 0;
+	struct ref_array *packed;
+	struct ref_entry *ref;
+	int fd, i;
 
-	packed_ref_list = get_packed_refs(NULL);
-	for (list = packed_ref_list; list; list = list->next) {
-		if (!strcmp(refname, list->name)) {
-			found = 1;
-			break;
-		}
-	}
-	if (!found)
+	packed = get_packed_refs(NULL);
+	ref = search_ref_array(packed, refname);
+	if (ref == NULL)
 		return 0;
 	fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
 	if (fd < 0) {
@@ -1148,17 +1083,19 @@ static int repack_without_ref(const char *refname)
 		return error("cannot delete '%s' from packed refs", refname);
 	}
 
-	for (list = packed_ref_list; list; list = list->next) {
+	for (i = 0; i < packed->nr; i++) {
 		char line[PATH_MAX + 100];
 		int len;
 
-		if (!strcmp(refname, list->name))
+		ref = packed->refs[i];
+
+		if (!strcmp(refname, ref->name))
 			continue;
 		len = snprintf(line, sizeof(line), "%s %s\n",
-			       sha1_to_hex(list->sha1), list->name);
+			       sha1_to_hex(ref->sha1), ref->name);
 		/* this should not happen but just being defensive */
 		if (len > sizeof(line))
-			die("too long a refname '%s'", list->name);
+			die("too long a refname '%s'", ref->name);
 		write_or_die(fd, line, len);
 	}
 	return commit_lock_file(&packlock);
-- 
1.7.6.1

^ permalink raw reply related

* Re: [PATCH] refs: Use binary search to lookup refs faster
From: Junio C Hamano @ 2011-09-29 22:06 UTC (permalink / raw)
  To: Julian Phillips
  Cc: Martin Fick, Christian Couder, git, Christian Couder, Thomas Rast
In-Reply-To: <20110929041811.5363.33396.julian@quantumfyre.co.uk>

Julian Phillips <julian@quantumfyre.co.uk> writes:

> +static void add_ref(const char *name, const unsigned char *sha1,
> +		    int flag, struct ref_array *refs,
> +		    struct ref_entry **new_entry)
>  {
>  	int len;
> -	struct ref_list *entry;
> +	struct ref_entry *entry;
>  
>  	/* Allocate it and add it in.. */
>  	len = strlen(name) + 1;
> -	entry = xmalloc(sizeof(struct ref_list) + len);
> +	entry = xmalloc(sizeof(struct ref) + len);

This should be sizeof(struct ref_entry), no?  There is another such
misallocation in search_ref_array() where it prepares a temporary.

^ permalink raw reply

* Re: In favor of "git commit --no-parent"
From: Michael Witten @ 2011-09-29 22:01 UTC (permalink / raw)
  To: Phil Hord
  Cc: Junio C Hamano, Carlos Martín Nieto, vra5107,
	Michael J Gruber, Matthieu Moy, Eric Raible, Philip Oakley,
	Jeff King, Jay Soffian, git
In-Reply-To: <CABURp0qEyQB37Zx75Xa6EEocnJeWiAGdkFqO7iZw_B_hg69hRg@mail.gmail.com>

On Thu, Sep 29, 2011 at 21:50, Phil Hord <phil.hord@gmail.com> wrote:
> On Thu, Sep 29, 2011 at 5:28 PM, Michael Witten <mfwitten@gmail.com> wrote:
>> On Thu, Sep 29, 2011 at 21:02, Phil Hord <phil.hord@gmail.com> wrote:
>>> I think a user looking for this functionality -- either a new git user
>>> or a user who seldom uses the "create secondary root commit" command
>>> -- would first try 'git help init'.  It seems logical to me that I
>>> should be able to do this:
>>>
>>>  cd my-git-repo
>>>  git init --root=<newbranch> .
>>>
>>> This feels natural to me for this operation.
>>
>> That would be a good place for the "git checkout --no-parent" variant,
>> especially given that I think "git checkout --no-parent" should produce
>> an empty working tree and index, which we can all note is essentially
>> what "git init" gives us.
>>
>> Your suggestion seems like a corroboration of my stance.
>
> I'm not arguing the functionality; just the command spelling.
> Consider your stance corroborated.
>
> I don't like "git checkout" for this because
> 1. git-checkout is too popular already; oddball functions like this
> should live in the shadows.
> 2. git-checkout is conceptually wrong, imho.  git-checkout means
> "fetch me this commit" or "fetch me files from this commit".
> Technically it does the same thing that we're talking about here (it
> frobs the index, the workdir and HEAD), but conceptually it is very
> different.
>
> Conceptually, I think the functionality you're talking about is more
> akin to git-init.

Actually, I'd say that the purpose of "git checkout" is to set the
working tree and index; in that sense, "git init" could in fact be
implemented by using "git checkout".

The key to what I'm saying, though, is that 2 scenarios are involved:

  * Make a root starting with nothing.
  * Make a root based off something.

The user should be able to express that, so that commands like
"git status" make sense. Currently, the user is only able
to express the first scenario:

  * Make a root starting with nothing.

but existing stuff is automatically added in case the user
wants to do the second scenario, but that makes "git status"
essentially unusuable for the second scenario, anyway.

^ permalink raw reply

* Re: [PATCH] refs: Use binary search to lookup refs faster
From: Junio C Hamano @ 2011-09-29 21:57 UTC (permalink / raw)
  To: Julian Phillips
  Cc: Martin Fick, Christian Couder, git, Christian Couder, Thomas Rast
In-Reply-To: <20110929041811.5363.33396.julian@quantumfyre.co.uk>

Julian Phillips <julian@quantumfyre.co.uk> writes:

> Currently we linearly search through lists of refs when we need to
> find a specific ref.  This can be very slow if we need to lookup a
> large number of refs.  By changing to a binary search we can make this
> faster.
>
> In order to be able to use a binary search we need to change from
> using linked lists to arrays, which we can manage using ALLOC_GROW.
>
> We can now also use the standard library qsort function to sort the
> refs arrays.
>
> Signed-off-by: Julian Phillips <julian@quantumfyre.co.uk>
> ---
>
> Something like this?
>
>  refs.c |  328 ++++++++++++++++++++++++++--------------------------------------
>  1 files changed, 131 insertions(+), 197 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index a49ff74..e411bea 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -8,14 +8,18 @@
>  #define REF_KNOWS_PEELED 04
>  #define REF_BROKEN 010
>  
> -struct ref_list {
> -	struct ref_list *next;
> +struct ref_entry {
>  	unsigned char flag; /* ISSYMREF? ISPACKED? */
>  	unsigned char sha1[20];
>  	unsigned char peeled[20];
>  	char name[FLEX_ARRAY];
>  };
>  
> +struct ref_array {
> +	int nr, alloc;
> +	struct ref_entry **refs;
> +};
> +

Yeah, I can say "something like that" without looking at the rest of the
patch ;-) The rest should naturally follow from the above data structures.

^ permalink raw reply

* Re: In favor of "git commit --no-parent"
From: Junio C Hamano @ 2011-09-29 21:54 UTC (permalink / raw)
  To: git
In-Reply-To: <7vd3ejrqin.fsf@alter.siamese.dyndns.org>

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

> Phil Hord <phil.hord@gmail.com> writes:
>
>> I think a user looking for this functionality -- either a new git user
>> or a user who seldom uses the "create secondary root commit" command
>> -- would first try 'git help init'.  It seems logical to me that I
>> should be able to do this:
>>
>>   cd my-git-repo
>>   git init --root=<newbranch> .
>>
>> This feels natural to me for this operation.
>
> Hmm, this does not feel very natural to me; unless "git init --root=work"
> uses 'work' branch instead of 'master' when creating a new repository,
> that is. But it is attractive that init is much less often used than
> checkout/commit and everybody knows it is somewhat a _special_ operation.
>
>>> That leaves "Hidden History" the only useful use case. IOW, the answer to
>>> the first question above is not "Separate or Hidden History", but is
>>> "Hidden History and nothing else".
>>
>> I think you're saying that the "hidden history" scenario is more
>> special than the "separate history" one because of these reasons:
>
> Not at all.
>
> I am saying that "separate history" has no place in git workflow, if these
> multiple roots _originate_ in the same single repository with a working
> tree. And all of "git checkout --orphan", "git commit --root" and your
> "git init --root" are solutions to make multiple roots _originate_ in the
> same single repository with a working tree.
>
> I have no trouble in a single repository with multiple roots if that is
> done in a distribution point, which by definition does not need and
> typically does not have any working tree. Options to "checkout/commit"
> would not help as they need a working tree.
>
> The way to do it is to work in multiple repositories, one for each of
> these roots, and push into a single repository from them.
>
>>> And a half of the the answer to the second question is "checkout --orphan"
>>> (and the other half would be "filter-branch"). "checkout --orphan" does
>>> have major safety advantage than introducing "commit --no-parent", as Peff
>>> pointed out earlier (to which I agreed).
>>
>> The thing I don't understand about "checkout --orphan" is exactly what
>> you're getting when you do this.  I assume you get a populated index
>> and a non-existent HEAD. This seems a lot like "git init" to me,
>> especially in the non-existent HEAD area.
>
> It is "HEAD pointing at a branch that does not yet exist", but I find it
> strangely attractive ;-)
>
>> I didn't think git init would be much use for this scenario before,
>> but now I've changed my mind.
>>
>>   git init --root=<newbranch> --keep-index
>>
>> Again, this avoids complicating the common commands.  But maybe it
>> does overload init with extra baggage.
>
> I do not think you would even need --keep-index there (running "git init"
> in an existing repository to see what it does. It does not touch the index
> nor HEAD).
>
> I am not sure if "--root" is the right name but if "git init --root=work"
> that is run to create (not re-init) a new repository points HEAD at a yet
> to be created 'work' branch instead of 'master', I think it would be a
> reasonable alternative.
>
> By the way, why did you drop the mailing list?

^ permalink raw reply

* Re: In favor of "git commit --no-parent"
From: Phil Hord @ 2011-09-29 21:50 UTC (permalink / raw)
  To: Michael Witten
  Cc: Junio C Hamano, Carlos Martín Nieto, vra5107,
	Michael J Gruber, Matthieu Moy, Eric Raible, Philip Oakley,
	Jeff King, Jay Soffian, git
In-Reply-To: <CAMOZ1Bs9kei58AVJZRJM4g+Nh3QaY8dtUBctmLL8SVL3XW=aLw@mail.gmail.com>

On Thu, Sep 29, 2011 at 5:28 PM, Michael Witten <mfwitten@gmail.com> wrote:
> On Thu, Sep 29, 2011 at 21:02, Phil Hord <phil.hord@gmail.com> wrote:
>> I think a user looking for this functionality -- either a new git user
>> or a user who seldom uses the "create secondary root commit" command
>> -- would first try 'git help init'.  It seems logical to me that I
>> should be able to do this:
>>
>>  cd my-git-repo
>>  git init --root=<newbranch> .
>>
>> This feels natural to me for this operation.
>
> That would be a good place for the "git checkout --no-parent" variant,
> especially given that I think "git checkout --no-parent" should produce
> an empty working tree and index, which we can all note is essentially
> what "git init" gives us.
>
> Your suggestion seems like a corroboration of my stance.

I'm not arguing the functionality; just the command spelling.
Consider your stance corroborated.

I don't like "git checkout" for this because
1. git-checkout is too popular already; oddball functions like this
should live in the shadows.
2. git-checkout is conceptually wrong, imho.  git-checkout means
"fetch me this commit" or "fetch me files from this commit".
Technically it does the same thing that we're talking about here (it
frobs the index, the workdir and HEAD), but conceptually it is very
different.

Conceptually, I think the functionality you're talking about is more
akin to git-init.

Phil

^ permalink raw reply

* [PATCH] refs: Use binary search to lookup refs faster
From: Julian Phillips @ 2011-09-29  4:18 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Julian Phillips, Martin Fick, Christian Couder, git,
	Christian Couder, Thomas Rast
In-Reply-To: <7vy5x7rwq9.fsf@alter.siamese.dyndns.org>

Currently we linearly search through lists of refs when we need to
find a specific ref.  This can be very slow if we need to lookup a
large number of refs.  By changing to a binary search we can make this
faster.

In order to be able to use a binary search we need to change from
using linked lists to arrays, which we can manage using ALLOC_GROW.

We can now also use the standard library qsort function to sort the
refs arrays.

Signed-off-by: Julian Phillips <julian@quantumfyre.co.uk>
---

Something like this?

 refs.c |  328 ++++++++++++++++++++++++++--------------------------------------
 1 files changed, 131 insertions(+), 197 deletions(-)

diff --git a/refs.c b/refs.c
index a49ff74..e411bea 100644
--- a/refs.c
+++ b/refs.c
@@ -8,14 +8,18 @@
 #define REF_KNOWS_PEELED 04
 #define REF_BROKEN 010
 
-struct ref_list {
-	struct ref_list *next;
+struct ref_entry {
 	unsigned char flag; /* ISSYMREF? ISPACKED? */
 	unsigned char sha1[20];
 	unsigned char peeled[20];
 	char name[FLEX_ARRAY];
 };
 
+struct ref_array {
+	int nr, alloc;
+	struct ref_entry **refs;
+};
+
 static const char *parse_ref_line(char *line, unsigned char *sha1)
 {
 	/*
@@ -44,108 +48,55 @@ static const char *parse_ref_line(char *line, unsigned char *sha1)
 	return line;
 }
 
-static struct ref_list *add_ref(const char *name, const unsigned char *sha1,
-				int flag, struct ref_list *list,
-				struct ref_list **new_entry)
+static void add_ref(const char *name, const unsigned char *sha1,
+		    int flag, struct ref_array *refs,
+		    struct ref_entry **new_entry)
 {
 	int len;
-	struct ref_list *entry;
+	struct ref_entry *entry;
 
 	/* Allocate it and add it in.. */
 	len = strlen(name) + 1;
-	entry = xmalloc(sizeof(struct ref_list) + len);
+	entry = xmalloc(sizeof(struct ref) + len);
 	hashcpy(entry->sha1, sha1);
 	hashclr(entry->peeled);
 	memcpy(entry->name, name, len);
 	entry->flag = flag;
-	entry->next = list;
 	if (new_entry)
 		*new_entry = entry;
-	return entry;
+	ALLOC_GROW(refs->refs, refs->nr + 1, refs->alloc);
+	refs->refs[refs->nr++] = entry;
 }
 
-/* merge sort the ref list */
-static struct ref_list *sort_ref_list(struct ref_list *list)
+static int ref_entry_cmp(const void *a, const void *b)
 {
-	int psize, qsize, last_merge_count, cmp;
-	struct ref_list *p, *q, *l, *e;
-	struct ref_list *new_list = list;
-	int k = 1;
-	int merge_count = 0;
-
-	if (!list)
-		return list;
-
-	do {
-		last_merge_count = merge_count;
-		merge_count = 0;
-
-		psize = 0;
-
-		p = new_list;
-		q = new_list;
-		new_list = NULL;
-		l = NULL;
+	struct ref_entry *one = *(struct ref_entry **)a;
+	struct ref_entry *two = *(struct ref_entry **)b;
+	return strcmp(one->name, two->name);
+}
 
-		while (p) {
-			merge_count++;
+static void sort_ref_array(struct ref_array *array)
+{
+	qsort(array->refs, array->nr, sizeof(*array->refs), ref_entry_cmp);
+}
 
-			while (psize < k && q->next) {
-				q = q->next;
-				psize++;
-			}
-			qsize = k;
-
-			while ((psize > 0) || (qsize > 0 && q)) {
-				if (qsize == 0 || !q) {
-					e = p;
-					p = p->next;
-					psize--;
-				} else if (psize == 0) {
-					e = q;
-					q = q->next;
-					qsize--;
-				} else {
-					cmp = strcmp(q->name, p->name);
-					if (cmp < 0) {
-						e = q;
-						q = q->next;
-						qsize--;
-					} else if (cmp > 0) {
-						e = p;
-						p = p->next;
-						psize--;
-					} else {
-						if (hashcmp(q->sha1, p->sha1))
-							die("Duplicated ref, and SHA1s don't match: %s",
-							    q->name);
-						warning("Duplicated ref: %s", q->name);
-						e = q;
-						q = q->next;
-						qsize--;
-						free(e);
-						e = p;
-						p = p->next;
-						psize--;
-					}
-				}
+static struct ref_entry *search_ref_array(struct ref_array *array, const char *name)
+{
+	struct ref_entry *e, **r;
+	int len;
 
-				e->next = NULL;
+	len = strlen(name) + 1;
+	e = xmalloc(sizeof(struct ref) + len);
+	memcpy(e->name, name, len);
 
-				if (l)
-					l->next = e;
-				if (!new_list)
-					new_list = e;
-				l = e;
-			}
+	r = bsearch(&e, array->refs, array->nr, sizeof(*array->refs), ref_entry_cmp);
 
-			p = q;
-		};
+	free(e);
 
-		k = k * 2;
-	} while ((last_merge_count != merge_count) || (last_merge_count != 1));
+	if (r == NULL)
+		return NULL;
 
-	return new_list;
+	return *r;
 }
 
 /*
@@ -155,38 +106,37 @@ static struct ref_list *sort_ref_list(struct ref_list *list)
 static struct cached_refs {
 	char did_loose;
 	char did_packed;
-	struct ref_list *loose;
-	struct ref_list *packed;
+	struct ref_array loose;
+	struct ref_array packed;
 } cached_refs, submodule_refs;
-static struct ref_list *current_ref;
+static struct ref_entry *current_ref;
 
-static struct ref_list *extra_refs;
+static struct ref_array extra_refs;
 
-static void free_ref_list(struct ref_list *list)
+static void free_ref_array(struct ref_array *array)
 {
-	struct ref_list *next;
-	for ( ; list; list = next) {
-		next = list->next;
-		free(list);
-	}
+	int i;
+	for (i = 0; i < array->nr; i++)
+		free(array->refs[i]);
+	free(array->refs);
+	array->nr = array->alloc = 0;
+	array->refs = NULL;
 }
 
 static void invalidate_cached_refs(void)
 {
 	struct cached_refs *ca = &cached_refs;
 
-	if (ca->did_loose && ca->loose)
-		free_ref_list(ca->loose);
-	if (ca->did_packed && ca->packed)
-		free_ref_list(ca->packed);
-	ca->loose = ca->packed = NULL;
+	if (ca->did_loose)
+		free_ref_array(&ca->loose);
+	if (ca->did_packed)
+		free_ref_array(&ca->packed);
 	ca->did_loose = ca->did_packed = 0;
 }
 
 static void read_packed_refs(FILE *f, struct cached_refs *cached_refs)
 {
-	struct ref_list *list = NULL;
-	struct ref_list *last = NULL;
+	struct ref_entry *last = NULL;
 	char refline[PATH_MAX];
 	int flag = REF_ISPACKED;
 
@@ -205,7 +155,7 @@ static void read_packed_refs(FILE *f, struct cached_refs *cached_refs)
 
 		name = parse_ref_line(refline, sha1);
 		if (name) {
-			list = add_ref(name, sha1, flag, list, &last);
+			add_ref(name, sha1, flag, &cached_refs->packed, &last);
 			continue;
 		}
 		if (last &&
@@ -215,21 +165,20 @@ static void read_packed_refs(FILE *f, struct cached_refs *cached_refs)
 		    !get_sha1_hex(refline + 1, sha1))
 			hashcpy(last->peeled, sha1);
 	}
-	cached_refs->packed = sort_ref_list(list);
+	sort_ref_array(&cached_refs->packed);
 }
 
 void add_extra_ref(const char *name, const unsigned char *sha1, int flag)
 {
-	extra_refs = add_ref(name, sha1, flag, extra_refs, NULL);
+	add_ref(name, sha1, flag, &extra_refs, NULL);
 }
 
 void clear_extra_refs(void)
 {
-	free_ref_list(extra_refs);
-	extra_refs = NULL;
+	free_ref_array(&extra_refs);
 }
 
-static struct ref_list *get_packed_refs(const char *submodule)
+static struct ref_array *get_packed_refs(const char *submodule)
 {
 	const char *packed_refs_file;
 	struct cached_refs *refs;
@@ -237,7 +186,7 @@ static struct ref_list *get_packed_refs(const char *submodule)
 	if (submodule) {
 		packed_refs_file = git_path_submodule(submodule, "packed-refs");
 		refs = &submodule_refs;
-		free_ref_list(refs->packed);
+		free_ref_array(&refs->packed);
 	} else {
 		packed_refs_file = git_path("packed-refs");
 		refs = &cached_refs;
@@ -245,18 +194,17 @@ static struct ref_list *get_packed_refs(const char *submodule)
 
 	if (!refs->did_packed || submodule) {
 		FILE *f = fopen(packed_refs_file, "r");
-		refs->packed = NULL;
 		if (f) {
 			read_packed_refs(f, refs);
 			fclose(f);
 		}
 		refs->did_packed = 1;
 	}
-	return refs->packed;
+	return &refs->packed;
 }
 
-static struct ref_list *get_ref_dir(const char *submodule, const char *base,
-				    struct ref_list *list)
+static void get_ref_dir(const char *submodule, const char *base,
+			struct ref_array *array)
 {
 	DIR *dir;
 	const char *path;
@@ -299,7 +247,7 @@ static struct ref_list *get_ref_dir(const char *submodule, const char *base,
 			if (stat(refdir, &st) < 0)
 				continue;
 			if (S_ISDIR(st.st_mode)) {
-				list = get_ref_dir(submodule, ref, list);
+				get_ref_dir(submodule, ref, array);
 				continue;
 			}
 			if (submodule) {
@@ -314,12 +262,11 @@ static struct ref_list *get_ref_dir(const char *submodule, const char *base,
 					hashclr(sha1);
 					flag |= REF_BROKEN;
 				}
-			list = add_ref(ref, sha1, flag, list, NULL);
+			add_ref(ref, sha1, flag, array, NULL);
 		}
 		free(ref);
 		closedir(dir);
 	}
-	return list;
 }
 
 struct warn_if_dangling_data {
@@ -356,21 +303,21 @@ void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname)
 	for_each_rawref(warn_if_dangling_symref, &data);
 }
 
-static struct ref_list *get_loose_refs(const char *submodule)
+static struct ref_array *get_loose_refs(const char *submodule)
 {
 	if (submodule) {
-		free_ref_list(submodule_refs.loose);
-		submodule_refs.loose = get_ref_dir(submodule, "refs", NULL);
-		submodule_refs.loose = sort_ref_list(submodule_refs.loose);
-		return submodule_refs.loose;
+		free_ref_array(&submodule_refs.loose);
+		get_ref_dir(submodule, "refs", &submodule_refs.loose);
+		sort_ref_array(&submodule_refs.loose);
+		return &submodule_refs.loose;
 	}
 
 	if (!cached_refs.did_loose) {
-		cached_refs.loose = get_ref_dir(NULL, "refs", NULL);
-		cached_refs.loose = sort_ref_list(cached_refs.loose);
+		get_ref_dir(NULL, "refs", &cached_refs.loose);
+		sort_ref_array(&cached_refs.loose);
 		cached_refs.did_loose = 1;
 	}
-	return cached_refs.loose;
+	return &cached_refs.loose;
 }
 
 /* We allow "recursive" symbolic refs. Only within reason, though */
@@ -381,8 +328,8 @@ static int resolve_gitlink_packed_ref(char *name, int pathlen, const char *refna
 {
 	FILE *f;
 	struct cached_refs refs;
-	struct ref_list *ref;
-	int retval;
+	struct ref_entry *ref;
+	int retval = -1;
 
 	strcpy(name + pathlen, "packed-refs");
 	f = fopen(name, "r");
@@ -390,17 +337,12 @@ static int resolve_gitlink_packed_ref(char *name, int pathlen, const char *refna
 		return -1;
 	read_packed_refs(f, &refs);
 	fclose(f);
-	ref = refs.packed;
-	retval = -1;
-	while (ref) {
-		if (!strcmp(ref->name, refname)) {
-			retval = 0;
-			memcpy(result, ref->sha1, 20);
-			break;
-		}
-		ref = ref->next;
+	ref = search_ref_array(&refs.packed, refname);
+	if (ref != NULL) {
+		memcpy(result, ref->sha1, 20);
+		retval = 0;
 	}
-	free_ref_list(refs.packed);
+	free_ref_array(&refs.packed);
 	return retval;
 }
 
@@ -501,15 +443,13 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
 		git_snpath(path, sizeof(path), "%s", ref);
 		/* Special case: non-existing file. */
 		if (lstat(path, &st) < 0) {
-			struct ref_list *list = get_packed_refs(NULL);
-			while (list) {
-				if (!strcmp(ref, list->name)) {
-					hashcpy(sha1, list->sha1);
-					if (flag)
-						*flag |= REF_ISPACKED;
-					return ref;
-				}
-				list = list->next;
+			struct ref_array *packed = get_packed_refs(NULL);
+			struct ref_entry *r = search_ref_array(packed, ref);
+			if (r != NULL) {
+				hashcpy(sha1, r->sha1);
+				if (flag)
+					*flag |= REF_ISPACKED;
+				return ref;
 			}
 			if (reading || errno != ENOENT)
 				return NULL;
@@ -584,7 +524,7 @@ int read_ref(const char *ref, unsigned char *sha1)
 
 #define DO_FOR_EACH_INCLUDE_BROKEN 01
 static int do_one_ref(const char *base, each_ref_fn fn, int trim,
-		      int flags, void *cb_data, struct ref_list *entry)
+		      int flags, void *cb_data, struct ref_entry *entry)
 {
 	if (prefixcmp(entry->name, base))
 		return 0;
@@ -630,18 +570,12 @@ int peel_ref(const char *ref, unsigned char *sha1)
 		return -1;
 
 	if ((flag & REF_ISPACKED)) {
-		struct ref_list *list = get_packed_refs(NULL);
+		struct ref_array *array = get_packed_refs(NULL);
+		struct ref_entry *r = search_ref_array(array, ref);
 
-		while (list) {
-			if (!strcmp(list->name, ref)) {
-				if (list->flag & REF_KNOWS_PEELED) {
-					hashcpy(sha1, list->peeled);
-					return 0;
-				}
-				/* older pack-refs did not leave peeled ones */
-				break;
-			}
-			list = list->next;
+		if (r != NULL && r->flag & REF_KNOWS_PEELED) {
+			hashcpy(sha1, r->peeled);
+			return 0;
 		}
 	}
 
@@ -660,36 +594,39 @@ fallback:
 static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn fn,
 			   int trim, int flags, void *cb_data)
 {
-	int retval = 0;
-	struct ref_list *packed = get_packed_refs(submodule);
-	struct ref_list *loose = get_loose_refs(submodule);
+	int retval = 0, i, p = 0, l = 0;
+	struct ref_array *packed = get_packed_refs(submodule);
+	struct ref_array *loose = get_loose_refs(submodule);
 
-	struct ref_list *extra;
+	struct ref_array *extra = &extra_refs;
 
-	for (extra = extra_refs; extra; extra = extra->next)
-		retval = do_one_ref(base, fn, trim, flags, cb_data, extra);
+	for (i = 0; i < extra->nr; i++)
+		retval = do_one_ref(base, fn, trim, flags, cb_data, extra->refs[i]);
 
-	while (packed && loose) {
-		struct ref_list *entry;
-		int cmp = strcmp(packed->name, loose->name);
+	while (p < packed->nr && l < loose->nr) {
+		struct ref_entry *entry;
+		int cmp = strcmp(packed->refs[p]->name, loose->refs[l]->name);
 		if (!cmp) {
-			packed = packed->next;
+			p++;
 			continue;
 		}
 		if (cmp > 0) {
-			entry = loose;
-			loose = loose->next;
+			entry = loose->refs[l++];
 		} else {
-			entry = packed;
-			packed = packed->next;
+			entry = packed->refs[p++];
 		}
 		retval = do_one_ref(base, fn, trim, flags, cb_data, entry);
 		if (retval)
 			goto end_each;
 	}
 
-	for (packed = packed ? packed : loose; packed; packed = packed->next) {
-		retval = do_one_ref(base, fn, trim, flags, cb_data, packed);
+	if (l < loose->nr) {
+		p = l;
+		packed = loose;
+	}
+
+	for (; p < packed->nr; p++) {
+		retval = do_one_ref(base, fn, trim, flags, cb_data, packed->refs[p]);
 		if (retval)
 			goto end_each;
 	}
@@ -1005,24 +942,24 @@ static int remove_empty_directories(const char *file)
 }
 
 static int is_refname_available(const char *ref, const char *oldref,
-				struct ref_list *list, int quiet)
-{
-	int namlen = strlen(ref); /* e.g. 'foo/bar' */
-	while (list) {
-		/* list->name could be 'foo' or 'foo/bar/baz' */
-		if (!oldref || strcmp(oldref, list->name)) {
-			int len = strlen(list->name);
+				struct ref_array *array, int quiet)
+{
+	int i, namlen = strlen(ref); /* e.g. 'foo/bar' */
+	for (i = 0; i < array->nr; i++ ) {
+		struct ref_entry *entry = array->refs[i];
+		/* entry->name could be 'foo' or 'foo/bar/baz' */
+		if (!oldref || strcmp(oldref, entry->name)) {
+			int len = strlen(entry->name);
 			int cmplen = (namlen < len) ? namlen : len;
-			const char *lead = (namlen < len) ? list->name : ref;
-			if (!strncmp(ref, list->name, cmplen) &&
+			const char *lead = (namlen < len) ? entry->name : ref;
+			if (!strncmp(ref, entry->name, cmplen) &&
 			    lead[cmplen] == '/') {
 				if (!quiet)
 					error("'%s' exists; cannot create '%s'",
-					      list->name, ref);
+					      entry->name, ref);
 				return 0;
 			}
 		}
-		list = list->next;
 	}
 	return 1;
 }
@@ -1129,18 +1066,13 @@ static struct lock_file packlock;
 
 static int repack_without_ref(const char *refname)
 {
-	struct ref_list *list, *packed_ref_list;
-	int fd;
-	int found = 0;
+	struct ref_array *packed;
+	struct ref_entry *ref;
+	int fd, i;
 
-	packed_ref_list = get_packed_refs(NULL);
-	for (list = packed_ref_list; list; list = list->next) {
-		if (!strcmp(refname, list->name)) {
-			found = 1;
-			break;
-		}
-	}
-	if (!found)
+	packed = get_packed_refs(NULL);
+	ref = search_ref_array(packed, refname);
+	if (ref == NULL)
 		return 0;
 	fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
 	if (fd < 0) {
@@ -1148,17 +1080,19 @@ static int repack_without_ref(const char *refname)
 		return error("cannot delete '%s' from packed refs", refname);
 	}
 
-	for (list = packed_ref_list; list; list = list->next) {
+	for (i = 0; i < packed->nr; i++) {
 		char line[PATH_MAX + 100];
 		int len;
 
-		if (!strcmp(refname, list->name))
+		ref = packed->refs[i];
+
+		if (!strcmp(refname, ref->name))
 			continue;
 		len = snprintf(line, sizeof(line), "%s %s\n",
-			       sha1_to_hex(list->sha1), list->name);
+			       sha1_to_hex(ref->sha1), ref->name);
 		/* this should not happen but just being defensive */
 		if (len > sizeof(line))
-			die("too long a refname '%s'", list->name);
+			die("too long a refname '%s'", ref->name);
 		write_or_die(fd, line, len);
 	}
 	return commit_lock_file(&packlock);
-- 
1.7.6.1

^ permalink raw reply related

* Re: In favor of "git commit --no-parent"
From: Michael Witten @ 2011-09-29 21:28 UTC (permalink / raw)
  To: Phil Hord
  Cc: Junio C Hamano, Carlos Martín Nieto, vra5107,
	Michael J Gruber, Matthieu Moy, Eric Raible, Philip Oakley,
	Jeff King, Jay Soffian, git
In-Reply-To: <CABURp0q4wi5ruLmeaZtN=8QvuX2ftSFQo1uJL0_8-wtm1nYaGA@mail.gmail.com>

On Thu, Sep 29, 2011 at 21:02, Phil Hord <phil.hord@gmail.com> wrote:
> On Thu, Sep 29, 2011 at 2:11 PM, Junio C Hamano <gitster@pobox.com> wrote:
> [...]
>> Step back a bit. There are two independent issues:
>>
>>  - When does it make sense to originate two independent histories in a
>>   single repository that has a working tree?
>>
>>  - What is the best tool to originate a new independent history in a
>>   single repository that has a working tree?
>>
>> As I said number of times already, be it done with "checkout --orphan" or
>> "commit --no-parent", the "Separate History" use case is better done in a
>> separate repository. There is *no* advantage to originate the two separate
>> histories that do not share any resemblance of tree shape as branches in a
>> single repository with a working tree; "git checkout $branch" between the
>> two would actively work against you.
>
> I think a user looking for this functionality -- either a new git user
> or a user who seldom uses the "create secondary root commit" command
> -- would first try 'git help init'.  It seems logical to me that I
> should be able to do this:
>
>  cd my-git-repo
>  git init --root=<newbranch> .
>
> This feels natural to me for this operation.

That would be a good place for the "git checkout --no-parent" variant,
especially given that I think "git checkout --no-parent" should produce
an empty working tree and index, which we can all note is essentially
what "git init" gives us.

Your suggestion seems like a corroboration of my stance.

^ permalink raw reply

* Re: In favor of "git commit --no-parent"
From: Phil Hord @ 2011-09-29 21:02 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Michael Witten, Carlos Martín Nieto, vra5107,
	Michael J Gruber, Matthieu Moy, Eric Raible, Philip Oakley,
	Jeff King, Jay Soffian, git
In-Reply-To: <7vmxdnte0j.fsf@alter.siamese.dyndns.org>

On Thu, Sep 29, 2011 at 2:11 PM, Junio C Hamano <gitster@pobox.com> wrote:
[...]
> Step back a bit. There are two independent issues:
>
>  - When does it make sense to originate two independent histories in a
>   single repository that has a working tree?
>
>  - What is the best tool to originate a new independent history in a
>   single repository that has a working tree?
>
> As I said number of times already, be it done with "checkout --orphan" or
> "commit --no-parent", the "Separate History" use case is better done in a
> separate repository. There is *no* advantage to originate the two separate
> histories that do not share any resemblance of tree shape as branches in a
> single repository with a working tree; "git checkout $branch" between the
> two would actively work against you.

I think a user looking for this functionality -- either a new git user
or a user who seldom uses the "create secondary root commit" command
-- would first try 'git help init'.  It seems logical to me that I
should be able to do this:

 cd my-git-repo
 git init --root=<newbranch> .

This feels natural to me for this operation.

And it doesn't add "dangerous options" to the more commonly used
commands (checkout, commit)

[...]
> That leaves "Hidden History" the only useful use case. IOW, the answer to
> the first question above is not "Separate or Hidden History", but is
> "Hidden History and nothing else".

I think you're saying that the "hidden history" scenario is more
special than the "separate history" one because of these reasons:
1. It is already possible to create a "hidden history".

And that's it. No more reason than that.

But isn't it also possible already to create a "separate history" just
as easily?  I can think of at least three methods that do not involve
git-reset.  And I'm a relative newbie.

> And a half of the the answer to the second question is "checkout --orphan"
> (and the other half would be "filter-branch"). "checkout --orphan" does
> have major safety advantage than introducing "commit --no-parent", as Peff
> pointed out earlier (to which I agreed).

The thing I don't understand about "checkout --orphan" is exactly what
you're getting when you do this.  I assume you get a populated index
and a non-existent HEAD.  This seems a lot like "git init" to me,
especially in the non-existent HEAD area.

I didn't think git init would be much use for this scenario before,
but now I've changed my mind.

 git init --root=<newbranch> --keep-index

Again, this avoids complicating the common commands.  But maybe it
does overload init with extra baggage.

>From this user's perspective it still makes more sense.

Phil

p.s. Sorry for jumping in so late.

^ permalink raw reply

* Re: [PATCH v3 0/4] port upload-archive to Windows
From: Erik Faye-Lund @ 2011-09-29 21:01 UTC (permalink / raw)
  To: git; +Cc: peff, j6t, gitster, rene.scharfe
In-Reply-To: <1317329963-6656-1-git-send-email-kusmabite@gmail.com>

On Thu, Sep 29, 2011 at 10:59 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> It's been a while, but here's another updated version of this
> series.
>
> The only change since last time is that the series has been made
> compatible with Peff's "when remote, disable some features"
> changes.

Oh, and I forgot to mention here in the cover-letter: I dropped the
NO_MINGW-prereq from the archive --remote tests as well.

^ permalink raw reply

* [PATCH v3 4/4] upload-archive: use start_command instead of fork
From: Erik Faye-Lund @ 2011-09-29 20:59 UTC (permalink / raw)
  To: git; +Cc: peff, j6t, gitster, rene.scharfe
In-Reply-To: <1317329963-6656-1-git-send-email-kusmabite@gmail.com>

The POSIX-function fork is not supported on Windows. Use our
start_command API instead.

As this is the last call-site that depends on the fork-stub in
compat/mingw.h, remove that as well.

Add an undocumented flag to git-archive that tells it that the
action originated from a remote, so features can be disabled.
Thanks to Jeff King for work on this part.

Remove the NOT_MINGW-prereq for t5000, as git-archive --remote
now works.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
Helped-by: Jeff King <peff@peff.net>
---
 builtin/archive.c        |    6 +++-
 builtin/upload-archive.c |   68 ++++++++++++++-------------------------------
 compat/mingw.h           |    2 -
 t/t5000-tar-tree.sh      |   10 +++---
 4 files changed, 31 insertions(+), 55 deletions(-)

diff --git a/builtin/archive.c b/builtin/archive.c
index 883c009..6668340 100644
--- a/builtin/archive.c
+++ b/builtin/archive.c
@@ -85,6 +85,7 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
 	const char *exec = "git-upload-archive";
 	const char *output = NULL;
 	const char *remote = NULL;
+	int is_remote = 0;
 	struct option local_opts[] = {
 		OPT_STRING('o', "output", &output, "file",
 			"write the archive to this file"),
@@ -92,6 +93,9 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
 			"retrieve the archive from remote repository <repo>"),
 		OPT_STRING(0, "exec", &exec, "cmd",
 			"path to the remote git-upload-archive command"),
+		{ OPTION_BOOLEAN, 0, "remote-request", &is_remote, NULL,
+			"indicate we are serving a remote request",
+			PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },
 		OPT_END()
 	};
 
@@ -106,5 +110,5 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
 
 	setvbuf(stderr, NULL, _IOLBF, BUFSIZ);
 
-	return write_archive(argc, argv, prefix, 1, output, 0);
+	return write_archive(argc, argv, prefix, 1, output, is_remote);
 }
diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index 2d0b383..c57e8bd 100644
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -6,6 +6,7 @@
 #include "archive.h"
 #include "pkt-line.h"
 #include "sideband.h"
+#include "run-command.h"
 
 static const char upload_archive_usage[] =
 	"git upload-archive <repo>";
@@ -18,28 +19,17 @@ static const char lostchild[] =
 
 #define MAX_ARGS (64)
 
-static int run_upload_archive(int argc, const char **argv, const char *prefix)
+static void prepare_argv(const char **sent_argv, const char **argv)
 {
-	const char *sent_argv[MAX_ARGS];
 	const char *arg_cmd = "argument ";
 	char *p, buf[4096];
 	int sent_argc;
 	int len;
 
-	if (argc != 2)
-		usage(upload_archive_usage);
-
-	if (strlen(argv[1]) + 1 > sizeof(buf))
-		die("insanely long repository name");
-
-	strcpy(buf, argv[1]); /* enter-repo smudges its argument */
-
-	if (!enter_repo(buf, 0))
-		die("'%s' does not appear to be a git repository", buf);
-
 	/* put received options in sent_argv[] */
-	sent_argc = 1;
-	sent_argv[0] = "git-upload-archive";
+	sent_argc = 2;
+	sent_argv[0] = "archive";
+	sent_argv[1] = "--remote-request";
 	for (p = buf;;) {
 		/* This will die if not enough free space in buf */
 		len = packet_read_line(0, p, (buf + sizeof buf) - p);
@@ -62,9 +52,6 @@ static int run_upload_archive(int argc, const char **argv, const char *prefix)
 		*p++ = 0;
 	}
 	sent_argv[sent_argc] = NULL;
-
-	/* parse all options sent by the client */
-	return write_archive(sent_argc, sent_argv, prefix, 0, NULL, 1);
 }
 
 __attribute__((format (printf, 1, 2)))
@@ -96,38 +83,25 @@ static ssize_t process_input(int child_fd, int band)
 
 int cmd_upload_archive(int argc, const char **argv, const char *prefix)
 {
-	pid_t writer;
-	int fd1[2], fd2[2];
-	/*
-	 * Set up sideband subprocess.
-	 *
-	 * We (parent) monitor and read from child, sending its fd#1 and fd#2
-	 * multiplexed out to our fd#1.  If the child dies, we tell the other
-	 * end over channel #3.
-	 */
-	if (pipe(fd1) < 0 || pipe(fd2) < 0) {
-		int err = errno;
-		packet_write(1, "NACK pipe failed on the remote side\n");
-		die("upload-archive: %s", strerror(err));
-	}
-	writer = fork();
-	if (writer < 0) {
+	const char *sent_argv[MAX_ARGS];
+	struct child_process cld = { sent_argv };
+	cld.out = cld.err = -1;
+	cld.git_cmd = 1;
+
+	if (argc != 2)
+		usage(upload_archive_usage);
+
+	if (!enter_repo(argv[1], 0))
+		die("'%s' does not appear to be a git repository", argv[1]);
+
+	prepare_argv(sent_argv, argv);
+	if (start_command(&cld)) {
 		int err = errno;
 		packet_write(1, "NACK fork failed on the remote side\n");
 		die("upload-archive: %s", strerror(err));
 	}
-	if (!writer) {
-		/* child - connect fd#1 and fd#2 to the pipe */
-		dup2(fd1[1], 1);
-		dup2(fd2[1], 2);
-		close(fd1[1]); close(fd2[1]);
-		close(fd1[0]); close(fd2[0]); /* we do not read from pipe */
-
-		exit(run_upload_archive(argc, argv, prefix));
-	}
 
 	/* parent - read from child, multiplex and send out to fd#1 */
-	close(fd1[1]); close(fd2[1]); /* we do not write to pipe */
 	packet_write(1, "ACK\n");
 	packet_flush(1);
 
@@ -135,9 +109,9 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
 		struct pollfd pfd[2];
 		int status;
 
-		pfd[0].fd = fd1[0];
+		pfd[0].fd = cld.out;
 		pfd[0].events = POLLIN;
-		pfd[1].fd = fd2[0];
+		pfd[1].fd = cld.err;
 		pfd[1].events = POLLIN;
 		if (poll(pfd, 2, -1) < 0) {
 			if (errno != EINTR) {
@@ -156,7 +130,7 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
 			if (process_input(pfd[0].fd, 1))
 				continue;
 
-		if (waitpid(writer, &status, 0) < 0)
+		if (waitpid(cld.pid, &status, 0) < 0)
 			error_clnt("%s", lostchild);
 		else if (!WIFEXITED(status) || WEXITSTATUS(status) > 0)
 			error_clnt("%s", deadchild);
diff --git a/compat/mingw.h b/compat/mingw.h
index ce9dd98..9cb6eb9 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -85,8 +85,6 @@ static inline int symlink(const char *oldpath, const char *newpath)
 { errno = ENOSYS; return -1; }
 static inline int fchmod(int fildes, mode_t mode)
 { errno = ENOSYS; return -1; }
-static inline pid_t fork(void)
-{ errno = ENOSYS; return -1; }
 static inline unsigned int alarm(unsigned int seconds)
 { return 0; }
 static inline int fsync(int fd)
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index d906898..889842e 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -96,7 +96,7 @@ test_expect_success 'git archive with --output' \
     'git archive --output=b4.tar HEAD &&
     test_cmp b.tar b4.tar'
 
-test_expect_success NOT_MINGW 'git archive --remote' \
+test_expect_success 'git archive --remote' \
     'git archive --remote=. HEAD >b5.tar &&
     test_cmp b.tar b5.tar'
 
@@ -266,7 +266,7 @@ test_expect_success 'archive --list mentions user filter' '
 	grep "^bar\$" output
 '
 
-test_expect_success NOT_MINGW 'archive --list shows only enabled remote filters' '
+test_expect_success 'archive --list shows only enabled remote filters' '
 	git archive --list --remote=. >output &&
 	! grep "^tar\.foo\$" output &&
 	grep "^bar\$" output
@@ -298,7 +298,7 @@ test_expect_success 'extension matching requires dot' '
 	test_cmp b.tar config-implicittar.foo
 '
 
-test_expect_success NOT_MINGW 'only enabled filters are available remotely' '
+test_expect_success 'only enabled filters are available remotely' '
 	test_must_fail git archive --remote=. --format=tar.foo HEAD \
 		>remote.tar.foo &&
 	git archive --remote=. --format=bar >remote.bar HEAD &&
@@ -341,12 +341,12 @@ test_expect_success GZIP,GUNZIP 'extract tgz file' '
 	test_cmp b.tar j.tar
 '
 
-test_expect_success GZIP,NOT_MINGW 'remote tar.gz is allowed by default' '
+test_expect_success GZIP 'remote tar.gz is allowed by default' '
 	git archive --remote=. --format=tar.gz HEAD >remote.tar.gz &&
 	test_cmp j.tgz remote.tar.gz
 '
 
-test_expect_success GZIP,NOT_MINGW 'remote tar.gz can be disabled' '
+test_expect_success GZIP 'remote tar.gz can be disabled' '
 	git config tar.tar.gz.remote false &&
 	test_must_fail git archive --remote=. --format=tar.gz HEAD \
 		>remote.tar.gz
-- 
1.7.6.355.g842ba.dirty

^ permalink raw reply related

* [PATCH v3 3/4] enter_repo: do not modify input
From: Erik Faye-Lund @ 2011-09-29 20:59 UTC (permalink / raw)
  To: git; +Cc: peff, j6t, gitster, rene.scharfe
In-Reply-To: <1317329963-6656-1-git-send-email-kusmabite@gmail.com>

entr_repo(..., 0) currently modifies the input to strip away
trailing slashes. This means that we some times need to copy the
input to keep the original.

Change it to unconditionally copy it into the used_path buffer so
we can safely use the input without having to copy it.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
 cache.h  |    2 +-
 daemon.c |    4 ++--
 path.c   |   30 ++++++++++++++----------------
 3 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/cache.h b/cache.h
index 607c2ea..a7e4de1 100644
--- a/cache.h
+++ b/cache.h
@@ -734,7 +734,7 @@ int safe_create_leading_directories(char *path);
 int safe_create_leading_directories_const(const char *path);
 int mkdir_in_gitdir(const char *path);
 extern char *expand_user_path(const char *path);
-char *enter_repo(char *path, int strict);
+const char *enter_repo(const char *path, int strict);
 static inline int is_absolute_path(const char *path)
 {
 	return is_dir_sep(path[0]) || has_dos_drive_prefix(path);
diff --git a/daemon.c b/daemon.c
index 4c8346d..9253192 100644
--- a/daemon.c
+++ b/daemon.c
@@ -108,11 +108,11 @@ static void NORETURN daemon_die(const char *err, va_list params)
 	exit(1);
 }
 
-static char *path_ok(char *directory)
+static const char *path_ok(char *directory)
 {
 	static char rpath[PATH_MAX];
 	static char interp_path[PATH_MAX];
-	char *path;
+	const char *path;
 	char *dir;
 
 	dir = directory;
diff --git a/path.c b/path.c
index 6f3f5d5..f7dfd0b 100644
--- a/path.c
+++ b/path.c
@@ -283,7 +283,7 @@ return_null:
  * links.  User relative paths are also returned as they are given,
  * except DWIM suffixing.
  */
-char *enter_repo(char *path, int strict)
+const char *enter_repo(const char *path, int strict)
 {
 	static char used_path[PATH_MAX];
 	static char validated_path[PATH_MAX];
@@ -297,14 +297,15 @@ char *enter_repo(char *path, int strict)
 		};
 		int len = strlen(path);
 		int i;
-		while ((1 < len) && (path[len-1] == '/')) {
-			path[len-1] = 0;
+		while ((1 < len) && (path[len-1] == '/'))
 			len--;
-		}
+
 		if (PATH_MAX <= len)
 			return NULL;
-		if (path[0] == '~') {
-			char *newpath = expand_user_path(path);
+		strncpy(used_path, path, len);
+
+		if (used_path[0] == '~') {
+			char *newpath = expand_user_path(used_path);
 			if (!newpath || (PATH_MAX - 10 < strlen(newpath))) {
 				free(newpath);
 				return NULL;
@@ -316,24 +317,21 @@ char *enter_repo(char *path, int strict)
 			 * anyway.
 			 */
 			strcpy(used_path, newpath); free(newpath);
-			strcpy(validated_path, path);
-			path = used_path;
+			strcpy(validated_path, used_path);
 		}
 		else if (PATH_MAX - 10 < len)
 			return NULL;
-		else {
-			path = strcpy(used_path, path);
-			strcpy(validated_path, path);
-		}
-		len = strlen(path);
+		else
+			strcpy(validated_path, used_path);
+		len = strlen(used_path);
 		for (i = 0; suffix[i]; i++) {
-			strcpy(path + len, suffix[i]);
-			if (!access(path, F_OK)) {
+			strcpy(used_path + len, suffix[i]);
+			if (!access(used_path, F_OK)) {
 				strcat(validated_path, suffix[i]);
 				break;
 			}
 		}
-		if (!suffix[i] || chdir(path))
+		if (!suffix[i] || chdir(used_path))
 			return NULL;
 		path = validated_path;
 	}
-- 
1.7.6.355.g842ba.dirty

^ permalink raw reply related

* [PATCH v3 2/4] mingw: fix compilation of poll-emulation
From: Erik Faye-Lund @ 2011-09-29 20:59 UTC (permalink / raw)
  To: git; +Cc: peff, j6t, gitster, rene.scharfe
In-Reply-To: <1317329963-6656-1-git-send-email-kusmabite@gmail.com>

gnulib has changed the inclusion of poll.h from double quotes
to single-quotes. But because compat/win32/sys/ isn't in our
include-path, this breaks compilation. Change it back the way
it was.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
 compat/win32/sys/poll.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/compat/win32/sys/poll.c b/compat/win32/sys/poll.c
index 403eaa7..225ddce 100644
--- a/compat/win32/sys/poll.c
+++ b/compat/win32/sys/poll.c
@@ -29,7 +29,7 @@
 #include <sys/types.h>
 
 /* Specification.  */
-#include <poll.h>
+#include "poll.h"
 
 #include <errno.h>
 #include <limits.h>
-- 
1.7.6.355.g842ba.dirty

^ permalink raw reply related

* [PATCH v3 1/4] compat/win32/sys/poll.c: upgrade from upstream
From: Erik Faye-Lund @ 2011-09-29 20:59 UTC (permalink / raw)
  To: git; +Cc: peff, j6t, gitster, rene.scharfe
In-Reply-To: <1317329963-6656-1-git-send-email-kusmabite@gmail.com>

poll.c is updated from revision adc3a5b in
git://git.savannah.gnu.org/gnulib.git

The changes are applied with --whitespace=fix to reduce noise.

poll.h is not upgraded, because the most recent version now
contains template-stuff that breaks compilation for us.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
 compat/win32/sys/poll.c |   19 +++++++++++++------
 1 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/compat/win32/sys/poll.c b/compat/win32/sys/poll.c
index 708a6c9..403eaa7 100644
--- a/compat/win32/sys/poll.c
+++ b/compat/win32/sys/poll.c
@@ -1,7 +1,7 @@
 /* Emulation for poll(2)
    Contributed by Paolo Bonzini.
 
-   Copyright 2001-2003, 2006-2010 Free Software Foundation, Inc.
+   Copyright 2001-2003, 2006-2011 Free Software Foundation, Inc.
 
    This file is part of gnulib.
 
@@ -27,7 +27,10 @@
 #include <malloc.h>
 
 #include <sys/types.h>
-#include "poll.h"
+
+/* Specification.  */
+#include <poll.h>
+
 #include <errno.h>
 #include <limits.h>
 #include <assert.h>
@@ -314,10 +317,7 @@ compute_revents (int fd, int sought, fd_set *rfds, fd_set *wfds, fd_set *efds)
 #endif /* !MinGW */
 
 int
-poll (pfd, nfd, timeout)
-     struct pollfd *pfd;
-     nfds_t nfd;
-     int timeout;
+poll (struct pollfd *pfd, nfds_t nfd, int timeout)
 {
 #ifndef WIN32_NATIVE
   fd_set rfds, wfds, efds;
@@ -454,6 +454,7 @@ poll (pfd, nfd, timeout)
   if (!hEvent)
     hEvent = CreateEvent (NULL, FALSE, FALSE, NULL);
 
+restart:
   handle_array[0] = hEvent;
   nhandles = 1;
   FD_ZERO (&rfds);
@@ -594,6 +595,12 @@ poll (pfd, nfd, timeout)
 	rc++;
     }
 
+  if (!rc && timeout == INFTIM)
+    {
+      SwitchToThread();
+      goto restart;
+    }
+
   return rc;
 #endif
 }
-- 
1.7.6.355.g842ba.dirty

^ permalink raw reply related

* [PATCH v3 0/4] port upload-archive to Windows
From: Erik Faye-Lund @ 2011-09-29 20:59 UTC (permalink / raw)
  To: git; +Cc: peff, j6t, gitster, rene.scharfe

It's been a while, but here's another updated version of this
series.

The only change since last time is that the series has been made
compatible with Peff's "when remote, disable some features"
changes.

Erik Faye-Lund (4):
  compat/win32/sys/poll.c: upgrade from upstream
  mingw: fix compilation of poll-emulation
  enter_repo: do not modify input
  upload-archive: use start_command instead of fork

 builtin/archive.c        |    6 +++-
 builtin/upload-archive.c |   68 ++++++++++++++-------------------------------
 cache.h                  |    2 +-
 compat/mingw.h           |    2 -
 compat/win32/sys/poll.c  |   17 ++++++++---
 daemon.c                 |    4 +-
 path.c                   |   30 +++++++++-----------
 t/t5000-tar-tree.sh      |   10 +++---
 8 files changed, 60 insertions(+), 79 deletions(-)

-- 
1.7.6.355.g842ba.dirty

^ permalink raw reply

* Re: Git is not scalable with too many refs/*
From: Martin Fick @ 2011-09-29 20:44 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: René Scharfe, Julian Phillips, Christian Couder, git,
	Christian Couder, Thomas Rast
In-Reply-To: <7vy5x7rwq9.fsf@alter.siamese.dyndns.org>

On Thursday, September 29, 2011 01:10:06 pm Junio C Hamano 
wrote:
> Also we would probably want to drop "next" field from
> "struct ref_list" (i.e. making it not a linear list),
> introduce a new "struct ref_array" that is a
> ALLOC_GROW() managed array of pointers to "struct
> ref_list", make get_packed_refs() and get_loose_refs()
> return a pointer to "struct ref_array" after sorting the
> array contents by "name". Then resolve_ref() can do a
> bisection search in the packed refs array when it does
> not find a loose ref.

That would be nice, and I suspect it would shave a bit more 
of the orphan check and possibly even a fetch.  If I 
understood all that, I might try.  But I might need some 
hand holding, my C is pretty rusty... Is there a bisection 
search library in git already to use?  Is there a git 
sorting library for the array also?

-Martin


-- 
Employee of Qualcomm Innovation Center, Inc. which is a 
member of Code Aurora Forum

^ permalink raw reply

* RE: Lack of detached signatures
From: Olsen, Alan R @ 2011-09-29 20:31 UTC (permalink / raw)
  To: Ted Ts'o, Jeff King
  Cc: Junio C Hamano, Joseph Parmelee, Carlos Martín Nieto,
	Michael Witten, git@vger.kernel.org
In-Reply-To: <20110928230958.GJ19250@thunk.org>

[Sorry for the top posting. Fscking Outlook demands it.]

The value of detached signatures is not that the users catch a hacked version, it is that the automated build bots catch the hacked version.  There was a server compromised a number of years back that was not caught by users, but by the Gentoo build bot.

What I would eventually like to see is a similar way to check an existing git tree and make sure all the commits authenticate and have not been tampered with.  That is a harder problem since the actual patch can change during a merge. (Having cryptographic sign-offs would be helpful as well. I have seen a few cases internally where the signoffs were for bogus e-mail addresses that got generated by automated tools, as well as patches that got altered after the sign-off.)

-----Original Message-----
From: Ted Ts'o [mailto:tytso@mit.edu] 
Sent: Wednesday, September 28, 2011 4:10 PM
To: Jeff King
Cc: Junio C Hamano; Joseph Parmelee; Carlos Martín Nieto; Olsen, Alan R; Michael Witten; git@vger.kernel.org
Subject: Re: Lack of detached signatures

On Wed, Sep 28, 2011 at 06:25:43PM -0400, Jeff King wrote:
> [1] This is a minor nit, and probably not worth breaking away from the
> way the rest of the world does it, but it is somewhat silly to sign the
> compressed data. I couldn't care less about the exact bytes in the
> compressed version; what I care about is the actual tar file. The
> compression is just a transport.

The worry I have is that many users don't check the GPG checksum files
as it is.  If they have to decompress the file, and then run gpg to
check the checksum, they might never get around to doing it.

That being said, I'm not sure I have a good solution.  One is to ship
the file without using detached signatures, and ship a foo.tar.gz.gpg
file, and force them to use GPG to unwrap the file before it can be
unpacked.  But users would yell and scream if we did that...

	       	     	   	    	   - Ted

^ permalink raw reply

* Re: [PATCH v2 4/4] upload-archive: use start_command instead of fork
From: Erik Faye-Lund @ 2011-09-29 20:20 UTC (permalink / raw)
  To: René Scharfe; +Cc: Jeff King, msysGit, Johannes Sixt, Junio C Hamano, git
In-Reply-To: <4E84D29F.6020305@lsrfire.ath.cx>

On Thu, Sep 29, 2011 at 10:18 PM, René Scharfe
<rene.scharfe@lsrfire.ath.cx> wrote:
> Am 29.09.2011 21:54, schrieb Erik Faye-Lund:
>> So the way forward is probably to just change to a self-compiled gzip
>> in msysGit (and Git for Windows by proxy).
>>
>> I'll post the patch to build a native-gzip to the msysGit mailing list
>> (I only got 1.2.4 to compile on Windows, but it's the same version as
>> we have in msysGit so it's probably fine), and post the latest version
>> of this series soon...
>
> Good to hear that.
>
> By the way, are you aware of http://unxutils.sourceforge.net/ (includes
> a native gzip 1.2.4 binary)?

Yes, but we prefer to build our binaries ourselves in the msysGit project.

> Or http://zlib.net/pigz/ (parallel gzip, no binaries)?

I'm aware of pigz, but changing the gzip implementation isn't really
the scope of this change IMO.

^ permalink raw reply

* Re: [PATCH v2 4/4] upload-archive: use start_command instead of fork
From: René Scharfe @ 2011-09-29 20:18 UTC (permalink / raw)
  To: kusmabite; +Cc: Jeff King, msysGit, Johannes Sixt, Junio C Hamano, git
In-Reply-To: <CABPQNSba+nQ+AOmVEVNWhJi2e65EiuhRxfOWZsW03Pkc6zXdHg@mail.gmail.com>

Am 29.09.2011 21:54, schrieb Erik Faye-Lund:
> So the way forward is probably to just change to a self-compiled gzip
> in msysGit (and Git for Windows by proxy).
> 
> I'll post the patch to build a native-gzip to the msysGit mailing list
> (I only got 1.2.4 to compile on Windows, but it's the same version as
> we have in msysGit so it's probably fine), and post the latest version
> of this series soon...

Good to hear that.

By the way, are you aware of http://unxutils.sourceforge.net/ (includes
a native gzip 1.2.4 binary)?

Or http://zlib.net/pigz/ (parallel gzip, no binaries)?

René

^ permalink raw reply

* Re: Git is not scalable with too many refs/*
From: Martin Fick @ 2011-09-29 20:11 UTC (permalink / raw)
  To: René Scharfe
  Cc: Julian Phillips, Christian Couder, git, Christian Couder,
	Thomas Rast, Junio C Hamano
In-Reply-To: <4E84B89F.4060304@lsrfire.ath.cx>

On Thursday, September 29, 2011 12:27:43 pm René Scharfe 
wrote:
> Hmm.  Can we get rid of the multiple ref lookups fixed by
> the above *and* the overhead of dealing with a textual
> argument list at the same time by calling
> add_pending_object directly, like this?  (Factoring out
> add_pending_sha1 should be a separate patch..)
 
René,

Your patch works well for me.  It achieves about the same 
gains as Julian's patch. Thanks!

After all the performance fixes get merged for large ref 
counts, it sure should help the Gerrit community.  I wonder 
how it might impact Gerrit mirroring...

-Martin


Employee of Qualcomm Innovation Center, Inc. which is a 
member of Code Aurora Forum

^ permalink raw reply

* Re: [PATCH v4] Docs: git checkout --orphan: Copyedit,                          and s/root commit/orphan branch/
From: Philip Oakley @ 2011-09-29 20:05 UTC (permalink / raw)
  To: Michael Witten, Junio C Hamano
  Cc: Carlos Martín Nieto, vra5107, Michael J Gruber, Matthieu Moy,
	Eric Raible, Jeff King, Jay Soffian, git
In-Reply-To: <3cba6bb85bde4f96903b2b617190a2b8-mfwitten@gmail.com>

From: "Michael Witten" <mfwitten@gmail.com>
> It's copyediting; it's best just to read the damn patch, particularly when
> there are no subtle details to be considered beyond the exhausting game of
> making everybody involved in the email thread feel like he or she has 
> gotten
> a portion of the bikeshed painted a certain color.
>
> See:
>
>  Re: Can a git changeset be created with no parent
>  Carlos Martín Nieto <cmn@elego.de>
>  Message-ID: <1317073309.5579.9.camel@centaur.lab.cmartin.tk>
>
> Signed-off-by: Michael Witten <mfwitten@gmail.com>
> ---
> Documentation/git-checkout.txt |   69 
> ++++++++++++++++++++++++++++-----------
> 1 files changed, 49 insertions(+), 20 deletions(-)
>
> diff --git a/Documentation/git-checkout.txt 
> b/Documentation/git-checkout.txt
> index c0a96e6..bf042c2 100644
> --- a/Documentation/git-checkout.txt
> +++ b/Documentation/git-checkout.txt
> @@ -125,29 +125,58 @@ explicitly give a name with '-b' in such a case.
>  below for details.
>
> ---orphan::
> - Create a new 'orphan' branch, named <new_branch>, started from
> - <start_point> and switch to it.  The first commit made on this
> - new branch will have no parents and it will be the root of a new
> - history totally disconnected from all the other branches and
> - commits.
> -+
> -The index and the working tree are adjusted as if you had previously run
> -"git checkout <start_point>".  This allows you to start a new history
> -that records a set of paths similar to <start_point> by easily running
> -"git commit -a" to make the root commit.
> -+
> -This can be useful when you want to publish the tree from a commit
> -without exposing its full history. You might want to do this to publish
> -an open source branch of a project whose current tree is "clean", but
> -whose full history contains proprietary or otherwise encumbered bits of
> -code.
> -+
> -If you want to start a disconnected history that records a set of paths
> -that is totally different from the one of <start_point>, then you should
> -clear the index and the working tree right after creating the orphan
> -branch by running "git rm -rf ." from the top level of the working tree.
> -Afterwards you will be ready to prepare your new files, repopulating the
> -working tree, by copying them from elsewhere, extracting a tarball, etc.
> +--orphan::
> + Perform these two tasks:
> ++
> +--
> + * Adjust the working tree and index as if you ran
> +   "git checkout <start_point>".
> +
> + * Set up git to turn the next commit you create into a root commit
> +   (that is, a commit without any parent); creating the next commit
> +   is similar to creating the first commit after running "git init",
> +   except that the new commit will be referenced by <new_branch>
> +   rather than "master".
> +--
> ++
> +Then, by just running "git commit", you can create a root commit
> +with a tree that is exactly the same as the tree of <start_point>.
> +Alternatively, before creating the commit, you may manipulate the
> +index in any way you want; for example, to create a root commit with
> +a tree that is totally different from the tree of <start_point>,
> +just clear the working tree and index first: From the top level of
> +the working tree, run "git rm -rf .", and then prepare your new
> +working tree and index as desired.
> ++
> +There are two common uses for this option:
> ++
> +--
> + Separate history::

Can I suggest    "Distinct History::"    as this title.
'separate' is both a verb and a noun, and it is easy to misread the title as 
if it is the verb, which would then be describing the action of creating the 
hidden history (the second title), with resulting misunderstanding. 
"Distinct" avoids the problem. The use of 'seperate' at the end is in 
context,  so not an issue.

> + Suppose that for convenience, you want to maintain
> + in the same repository as your project some ancillary
> + material that is infrequently altered.  In such a case,
> + it may not make much sense to interleave the history of
> + that material with the history of your project; you can
> + use the "--orphan" option in order to create completely
> + separate histories.
> +
> + Hidden history::
> + Suppose you have a project that has proprietary
> + material that is never meant to be released to the
> + public, yet you now want to maintain an open source
> + history that may be published widely.
> ++
> +In this case, it would not be enough just to remove the proprietary
> +material from the working tree and then create a new commit, because
> +the proprietary material would still be accessible through the new
> +commit's ancestry; the proprietary history must be hidden from the new
> +commit, and the "--orphan" option allows you to do so by ensuring that
> +the new commit has no parent.
> ++
> +However, when there are multiple commits that are already suitable for
> +the open source history (or that you want to make suitable), you should
> +instead consider working with linkgit:git-filter-branch[1] and possibly
> +linkgit:git-rebase[1].
> +--
>
> -m::
> --merge::
> -- 
> 1.7.6.409.ge7a85
>

^ permalink raw reply

* Re: [PATCH v2 4/4] upload-archive: use start_command instead of fork
From: Erik Faye-Lund @ 2011-09-29 19:54 UTC (permalink / raw)
  To: Jeff King, msysGit; +Cc: René Scharfe, Johannes Sixt, Junio C Hamano, git
In-Reply-To: <20110809200504.GA31944@sigill.intra.peff.net>

OK, this is quite late, but I've got some news on the subject.

On Tue, Aug 9, 2011 at 10:05 PM, Jeff King <peff@peff.net> wrote:
> On Tue, Aug 09, 2011 at 12:25:42PM +0200, René Scharfe wrote:
>
>> > BTW, as nice as this "gzip -cn | cat" idea is, I think it needs to be
>> > wrapped in a shell script. With the code above, we will generate "gzip
>> > -cn | cat -9".
>>
>> Yes, the three added lines in the patch above would have to be moved
>> down two lines, after the compression level is added.  D'oh!
>
> Also, is adding "| cat" also sufficient for arbitrary shell code (i.e.,
> whatever the user hands us via the config)? I couldn't think of an
> example that wouldn't work.
>
>> OK, that's one way to do it; another would be let gzip (and bzip2 etc.)
>> do whatever cat does to avoid end of line conversions.  And yet another
>> is to take them from http://unxutils.sourceforge.net/.
>
> Yeah, I like all of those solutions better than hacking an extra pipe
> into git. I don't know enough to say how painful they are in practice,
> though.
>

It turns out, if I compile gzip myself from source the test pass just
fine. So the problem seems indeed to be MSYS vs non-MSYS applications.

So the way forward is probably to just change to a self-compiled gzip
in msysGit (and Git for Windows by proxy).

I'll post the patch to build a native-gzip to the msysGit mailing list
(I only got 1.2.4 to compile on Windows, but it's the same version as
we have in msysGit so it's probably fine), and post the latest version
of this series soon...

^ permalink raw reply

* Re: Git is not scalable with too many refs/*
From: Julian Phillips @ 2011-09-29 19:10 UTC (permalink / raw)
  To: René Scharfe
  Cc: Martin Fick, Christian Couder, git, Christian Couder, Thomas Rast,
	Junio C Hamano
In-Reply-To: <4E84B89F.4060304@lsrfire.ath.cx>

On Thu, 29 Sep 2011 20:27:43 +0200, René Scharfe wrote:
> Am 29.09.2011 04:19, schrieb Julian Phillips:
>> Does the following help?
>>
>> diff --git a/builtin/checkout.c b/builtin/checkout.c
>> index 5e356a6..f0f4ca1 100644
>> --- a/builtin/checkout.c
>> +++ b/builtin/checkout.c
>> @@ -605,7 +605,7 @@ static int add_one_ref_to_rev_list_arg(const 
>> char
>> *refname,
>>                                        int flags,
>>                                        void *cb_data)
>>  {
>> -       add_one_rev_list_arg(cb_data, refname);
>> +       add_one_rev_list_arg(cb_data, strdup(sha1_to_hex(sha1)));
>>         return 0;
>>  }
>
> Hmm.  Can we get rid of the multiple ref lookups fixed by the above
> *and* the overhead of dealing with a textual argument list at the 
> same
> time by calling add_pending_object directly, like this?  (Factoring
> out add_pending_sha1 should be a separate patch..)

Seems like a good idea.  I get the same sort of times as with my patch, 
but it makes the code _feel_ much nicer (and slightly smaller).  Mine 
was definitely more of a "it's 2am, but I think the problem is here" 
type of patch ;)

-- 
Julian

^ permalink raw reply

* Re: Git is not scalable with too many refs/*
From: Junio C Hamano @ 2011-09-29 19:10 UTC (permalink / raw)
  To: René Scharfe
  Cc: Julian Phillips, Martin Fick, Christian Couder, git,
	Christian Couder, Thomas Rast
In-Reply-To: <4E84B89F.4060304@lsrfire.ath.cx>

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> Hmm.  Can we get rid of the multiple ref lookups fixed by the above
> *and* the overhead of dealing with a textual argument list at the same
> time by calling add_pending_object directly, like this?  (Factoring
> out add_pending_sha1 should be a separate patch..)

I haven't tested it or thought about it through, but it smells right ;-)

Also we would probably want to drop "next" field from "struct ref_list"
(i.e. making it not a linear list), introduce a new "struct ref_array"
that is a ALLOC_GROW() managed array of pointers to "struct ref_list",
make get_packed_refs() and get_loose_refs() return a pointer to "struct
ref_array" after sorting the array contents by "name". Then resolve_ref()
can do a bisection search in the packed refs array when it does not find a
loose ref.

^ 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