Git development
 help / color / mirror / Atom feed
* Re: [RFC/PATCH] git checkout $tree path
From: John Szakmeister @ 2011-09-30  1:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk48rq854.fsf@alter.siamese.dyndns.org>

On Thu, Sep 29, 2011 at 6:46 PM, Junio C Hamano <gitster@pobox.com> wrote:
[snip]
> According to that definition, because "master" has dir/file1, and the
> index is unchanged since "next", we would add dir/file1 to the index, and
> then check dir/file1 and dir/file3 out of the index. Hence, we end up
> resurrecting dir/file3 out of the index, even though "master" does not
> have that path.
>
> This is somewhat surprising.

That is surprising!  It explains something I saw just yesterday which
closely mirrors your recipe.

> It may make sense to tweak the semantics a little bit. We can grab the
> paths out of the named tree ("master" in this case), update the index, and
> update the working tree with only with these paths we grabbed from the
> named tree. By doing so, we will keep the local modification to dir/file3
> (in this case, the modification is to "delete", but the above observation
> hold equally true if dir/file3 were modified).

That seems sane.

> An alternative semantics could be to first remove paths that match the
> given pathspec from the index, then update the index with paths taken from
> the named tree, and update the working tree. "git checkout master dir"
> would then mean "replace anything currently in dir with whatever is in dir
> in master". It is more dangerous, and it can easily emulated by doing:

This is equally sane, but is probably closer to my expectation.

[snip]
>  * This is a behaviour change, but it may qualify as a bugfix. I dunno.

Personally, I lean towards it being a bugfix.

-John

^ permalink raw reply

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

On Thu, Sep 29, 2011 at 23:08, Junio C Hamano <gitster@pobox.com> wrote:

> The branch switching semantics of Git is designed to work well when all
> the branches you check out in the working tree are somewhat related
> content-wise. You create a new file, or make modifications to an existing
> file, realize that the change wants to go to a branch different from the
> current one. You _can_ switch to the branch the change should belong to,
> because the contents in the working tree is defined to be not tied to any
> branch, but is floating on top of the current branch.

That's exactly why "git commit --no-parent" is so useful.

Look at the difference:

  Creating a Hidden History (git commit --no-parent)

    $ cd repo
    $ git checkout -b hidden-history
    $ # Hack away as usual or not
    $ git status # As with any other commit.
    $ git commit --no-parent

  Creating a Hidden History (git checkout --orphan):

    $ cd repo
    $ git checkout --orphan hidden-history
    $ # Hack away as usual or not
    $ git status # lots of "new file" notifications obscuring my changes
    $ git commit

The main issue with "git commit --no-parent" is [supposedly] safety, but it
can be made pretty safe:

  $ cd repo
  $ # Hack away as usual or not
  $ git status # As with any other commit.
  $ git commit --no-parent
  Error! There must be another branch head directly referencing the
  same commit that is directly referenced by the current branch head!
  $ git checkout -b hidden-history
  $ git commit --no-parent

In the vast majority of cases, that rule will prevent people from
losing history inadvertantly, and no extra "--force" is required.

^ permalink raw reply

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

This version looks sane, although I have a suspicion that it may have
some interaction with what Michael may be working on.

Thanks.

^ permalink raw reply

* Re: In favor of "git commit --no-parent"
From: Junio C Hamano @ 2011-09-29 23:08 UTC (permalink / raw)
  To: Phil Hord
  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: <CABURp0rjBdx+=_8R5g16fNKWis3=GgJw9SQ9D53H6xu_-Tq3Uw@mail.gmail.com>

Phil Hord <phil.hord@gmail.com> writes:

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

No, no place in anybody's workflow.

I do carry non source html/man branches in the same distribution point
repository, but I did not create and I do not have these unrelated
branches in my development repository. Possibility to run "git checkout
html" in my development repository is just insane.

The branch switching semantics of Git is designed to work well when all
the branches you check out in the working tree are somewhat related
content-wise. You create a new file, or make modifications to an existing
file, realize that the change wants to go to a branch different from the
current one. You _can_ switch to the branch the change should belong to,
because the contents in the working tree is defined to be not tied to any
branch, but is floating on top of the current branch.

We often see new people who do not understand this (yet) wonder "I
modified a file, switched to another branch, but the modification is still
there. Why?" It is because the local changes in the working tree do not
belong to a specific branch, and local changes could be committed to any
branch.

What would happen if I were crazy enough to have the html branch in my
development repository and checked it out? In addition to the previous
build artifacts *.o files, new source files *.[ch] and documentation
sources Documentation/*.txt will stay and will be mixed in the checkout of
the html branch, where we all know there is no reason for them to be
committed on.

A sane way to use Git is to have a separate repository to keep track of
changes in the other unrelated material, and I have separate repositories
with checkouts for html and man. Of course I do not edit them manually;
these repositories are targets of "make install-doc" from the source
repository.

They happen to be pushed into the same distribution point repository, but
that is a mere historical artifact. It only started because at k.org I
only had a write access to /pub/scm/git/git.git but not to /pub/scm/git/
directory itself; I may have used /pub/scm/git/html and /pub/scm/git/man
repositories otherwise. 

And cloners are advised to "tar-tree" these out and extract them to
somewhere else, if they do not want to format the documentation themselves
but still want to look at them. Of course, they could "checkout", but you
would not edit these generated files in the working tree or commit to
these branches. So in that sense, yes they could "checkout", but that is
like saying they can run "rm -fr .git" too---they can do useful things and
not so useful things just alike.

I suspect that people often see those html/man branches in the
distribution point repository and get a wrong idea that having these
unrelated histories somehow add their coolness factor. It doesn't.

>> 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.
>
> That's one way to do it.

And I have been trying to teach why the other way is a wrong thing to do,
but there is no point in teaching a better practice if the listener is not
willing to learn. My time is better spent on other topics.

^ permalink raw reply

* [RFC/PATCH] git checkout $tree path
From: Junio C Hamano @ 2011-09-29 22:46 UTC (permalink / raw)
  To: git

Suppose you have two branches, "master" with dir/file1 and file2, and
"next" with dir/file3 and file4. You are currently on "next" branch.

    $ rm dir/file3
    $ git status -suno
     D dir/file3

Now, what should "git checkout master dir" do?

Checking paths out of a tree is (currently) defined to:

 - Grab the paths from the named tree that match the given pathspec,
   and add them to the index; and then

 - Check out the contents from the index for paths that match the
   pathspec to the working tree;

 - While at it, if the given pathspec did not match anything, suspect
   a typo from the command line and error out without updating the index
   nor the working tree.

According to that definition, because "master" has dir/file1, and the
index is unchanged since "next", we would add dir/file1 to the index, and
then check dir/file1 and dir/file3 out of the index. Hence, we end up
resurrecting dir/file3 out of the index, even though "master" does not
have that path.

This is somewhat surprising.

It may make sense to tweak the semantics a little bit. We can grab the
paths out of the named tree ("master" in this case), update the index, and
update the working tree with only with these paths we grabbed from the
named tree. By doing so, we will keep the local modification to dir/file3
(in this case, the modification is to "delete", but the above observation
hold equally true if dir/file3 were modified).

An alternative semantics could be to first remove paths that match the
given pathspec from the index, then update the index with paths taken from
the named tree, and update the working tree. "git checkout master dir"
would then mean "replace anything currently in dir with whatever is in dir
in master". It is more dangerous, and it can easily emulated by doing:

    $ git rm -rf dir
    $ git checkout master dir

so I did not go that far with this patch.

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

 * This is a behaviour change, but it may qualify as a bugfix. I dunno.

 builtin/checkout.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 5e356a6..75dbe76 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -71,7 +71,7 @@ static int update_some(const unsigned char *sha1, const char *base, int baselen,
 	hashcpy(ce->sha1, sha1);
 	memcpy(ce->name, base, baselen);
 	memcpy(ce->name + baselen, pathname, len - baselen);
-	ce->ce_flags = create_ce_flags(len, 0);
+	ce->ce_flags = create_ce_flags(len, 0) | CE_UPDATE;
 	ce->ce_mode = create_ce_mode(mode);
 	add_cache_entry(ce, ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE);
 	return 0;
@@ -228,6 +228,8 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
 
 	for (pos = 0; pos < active_nr; pos++) {
 		struct cache_entry *ce = active_cache[pos];
+		if (source_tree && !(ce->ce_flags & CE_UPDATE))
+			continue;
 		match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, ps_matched);
 	}
 
@@ -266,6 +268,8 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
 	state.refresh_cache = 1;
 	for (pos = 0; pos < active_nr; pos++) {
 		struct cache_entry *ce = active_cache[pos];
+		if (source_tree && !(ce->ce_flags & CE_UPDATE))
+			continue;
 		if (match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, NULL)) {
 			if (!ce_stage(ce)) {
 				errs |= checkout_entry(ce, &state, NULL);

^ permalink raw reply related

* Re: In favor of "git commit --no-parent"
From: Michael Witten @ 2011-09-29 22:32 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: <CAMOZ1Bspiz-J4Z7d8t7+jA_rKmzKcjhxuUpUSv7BrjJ=kCBfBQ@mail.gmail.com>

On Thu, Sep 29, 2011 at 22:29, Michael Witten <mfwitten@gmail.com> wrote:

> Consider trying the "Hidden History" scenario with the current
> approach, which only allows for "git checkout --orphan":
>
>  $ cd repo
>  $ git checkout --orphan separate-history
>  $ # Hack away as usual or not
>  $ git status # lots of "new file" notifications obscuring my changes
>  $ git commit

To avoid confusion, please note that the line:

  $ git checkout --orphan separate-history

was meant to read:

  $ git checkout --orphan hidden-history

^ permalink raw reply

* Re: In favor of "git commit --no-parent"
From: Michael Witten @ 2011-09-29 22:29 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: <CABURp0rjBdx+=_8R5g16fNKWis3=GgJw9SQ9D53H6xu_-Tq3Uw@mail.gmail.com>

On Thu, Sep 29, 2011 at 22:07, Phil Hord <phil.hord@gmail.com> wrote:
> 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.

Creating a Hidden History (Junio is OK with this usage):

  $ cd repo
  $ git checkout -b hidden-history
  $ # Hack away as usual or not
  $ git status # As with any other commit.
  $ git commit --no-parent

Creating a completely Separate History (Junio doesn't like this):

  $ cd repo
  $ git checkout --orphan separate-history
  $ git rm -rf . # This SHOULD be automatic.
  $ # add files as with an empty repo.
  $ git status # As with an empty new repo; lots of "new file"
  $ git commit

Those are the 2 halves, and they are correct for their purposes;
however, I would agree that "git init" is better instead of
"git checkout", especially when there should be an automatic
use of "git rm -rf .".

Consider trying the "Hidden History" scenario with the current
approach, which only allows for "git checkout --orphan":

  $ cd repo
  $ git checkout --orphan separate-history
  $ # Hack away as usual or not
  $ git status # lots of "new file" notifications obscuring my changes
  $ git commit

The main issue with "git commit --no-parent" is [supposedly] safety, but it
can be made pretty safe:

  $ cd repo
  $ # Hack away as usual or not
  $ git status # As with any other commit.
  $ git commit --no-parent
  Error! There must be another branch head directly referencing the
  same commit that is directly referenced by the current branch head!
  $ git checkout -b hidden-history
  $ git commit --no-parent

In the vast majority of cases, that rule will prevent people from
losing history inadvertantly, and no extra "--force" is required.

^ permalink raw reply

* [PATCH v3] refs: Use binary search to lookup refs faster
From: Julian Phillips @ 2011-09-29 22:11 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Martin Fick, Christian Couder, git, Christian Couder, Thomas Rast
In-Reply-To: <7vvcsbqa0k.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>
---

On Thu, 29 Sep 2011 15:06:03 -0700, Junio C Hamano wrote:
> 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.

Indeed, thanks.

Looks like two instances of not noticing that "struct ref" already existed
managed to survive.  Drat.  Of course since "struct ref" is bigger than "struct
ref_entry", everthing worked fine ... so no failed tests to tip me off.

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

diff --git a/refs.c b/refs.c
index a49ff74..4c01d79 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_entry) + 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_entry) + 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: In favor of "git commit --no-parent"
From: Michael Witten @ 2011-09-29 22:07 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4nzvrp3k.fsf@alter.siamese.dyndns.org>

On Thu, Sep 29, 2011 at 21:54, Junio C Hamano <gitster@pobox.com> wrote:

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

Why do you keep repeating this?

The "git commit --no-parent" is the CORRECT solution for
"HIDDEN history", NOT the "separate history" that you so loathe.

The existing "git checkout --orphan" is the CORRECT solution for
the "SEPARATE history" that you so loathe, NOT the "hidden history".

^ permalink raw reply

* 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


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