Git development
 help / color / mirror / Atom feed
* [RFC] t5500 considered dangerous
From: Johannes Schindelin @ 2005-12-26 23:35 UTC (permalink / raw)
  To: git


'corrupt a pack and see if verify catches' is bound to fail sometimes for 
non-obvious reasons:

The test case corrupts a pack, and then also the index of that pack, by 
writing "0" at certain offsets, and then tests if git-verify-pack fails.

However, said pack contains a commit, which is variable by virtue of 
storing two dates. Therefore, the deflated objects are no longer 
deterministic, and neither the pack. What happens is that in roughly every 
256th run, there *is* a 0 at one of these offsets, and the test fails.

This patch makes the test deterministic by skipping the commit.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

	I just hunted down a bug which showed up very rarely, and it did 
	not make the hunting easier.

 t/t5300-pack-object.sh |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index df6549c..d6b6697 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -20,12 +20,11 @@ test_expect_success \
      done &&
      cat c >d && echo foo >>d && git-update-index --add d &&
      tree=`git-write-tree` &&
-     commit=`git-commit-tree $tree </dev/null` && {
+     {
 	 echo $tree &&
-	 echo $commit &&
 	 git-ls-tree $tree | sed -e "s/.* \\([0-9a-f]*\\)	.*/\\1/"
      } >obj-list && {
-	 git-diff-tree --root -p $commit &&
+	 git-diff-tree --root -p $tree &&
 	 while read object
 	 do
 	    t=`git-cat-file -t $object` &&
@@ -99,7 +98,7 @@ test_expect_success \
      GIT_OBJECT_DIRECTORY=.git2/objects &&
      export GIT_OBJECT_DIRECTORY &&
      cp test-1-${packname_1}.pack test-1-${packname_1}.idx .git2/objects/pack && {
-	 git-diff-tree --root -p $commit &&
+	 git-diff-tree --root -p $tree &&
 	 while read object
 	 do
 	    t=`git-cat-file -t $object` &&
@@ -114,7 +113,7 @@ test_expect_success \
      export GIT_OBJECT_DIRECTORY &&
      rm -f .git2/objects/pack/test-?.idx &&
      cp test-2-${packname_2}.pack test-2-${packname_2}.idx .git2/objects/pack && {
-	 git-diff-tree --root -p $commit &&
+	 git-diff-tree --root -p $tree &&
 	 while read object
 	 do
 	    t=`git-cat-file -t $object` &&

^ permalink raw reply related

* Re: [PATCH] Handle symlinks graciously
From: Johannes Schindelin @ 2005-12-26 23:00 UTC (permalink / raw)
  To: git, junkio
In-Reply-To: <Pine.LNX.4.63.0512262231350.21076@wbgn013.biozentrum.uni-wuerzburg.de>

Hi,

to make the test t4011 pass all the time, this little patch is needed on 
top:

diff --git a/t/t4011-diff-symlink.sh b/t/t4011-diff-symlink.sh
index 9e70063..e3ebf38 100755
--- a/t/t4011-diff-symlink.sh
+++ b/t/t4011-diff-symlink.sh
@@ -32,6 +32,7 @@ test_expect_success \
 test_expect_success \
     'diff unchanged symlink' \
     'tree=$(git-write-tree) &&
+    git-update-index frotz &&
     test -z "$(git-diff-index --name-only $tree)"'
 
 cat > expected << EOF

Ciao,
Dscho

^ permalink raw reply related

* Re: [PATCH] avoid asking ?alloc() for zero bytes.
From: Johannes Schindelin @ 2005-12-26 22:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Eric Wong, git
In-Reply-To: <7vslsfikii.fsf_-_@assigned-by-dhcp.cox.net>

Hi,

On Mon, 26 Dec 2005, Junio C Hamano wrote:

> Avoid asking for zero bytes when that change simplifies overall
> logic.  Later we would change the wrapper to ask for 1 byte on
> platforms that return NULL for zero byte request.
> 
> Signed-off-by: Junio C Hamano <junkio@cox.net>
> 
> ---
> 
>  * Here is what I have now that roughly corresponds to your
>    patch, which has been somewhat tested.

Thanks.

>    The final phase of making sure we return something from
>    x*alloc() is not done yet.

Tomorrow I'll compare the two patches and find out what, if anything, is 
needed.

Ciao,
Dscho

^ permalink raw reply

* [PATCH] avoid asking ?alloc() for zero bytes.
From: Junio C Hamano @ 2005-12-26 22:03 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Eric Wong, git
In-Reply-To: <Pine.LNX.4.63.0512262134290.19331@wbgn013.biozentrum.uni-wuerzburg.de>

Avoid asking for zero bytes when that change simplifies overall
logic.  Later we would change the wrapper to ask for 1 byte on
platforms that return NULL for zero byte request.

Signed-off-by: Junio C Hamano <junkio@cox.net>

---

 * Here is what I have now that roughly corresponds to your
   patch, which has been somewhat tested.

   The final phase of making sure we return something from
   x*alloc() is not done yet.

 diff.c              |    6 +++---
 diffcore-order.c    |    6 +++++-
 diffcore-pathspec.c |    3 +++
 index-pack.c        |   22 ++++++++++++++--------
 read-tree.c         |   17 ++++++++++-------
 tree-diff.c         |    4 ++++
 6 files changed, 39 insertions(+), 19 deletions(-)

da95c79803d90ce8eaf82b0c04dbde11c096ad87
diff --git a/diff.c b/diff.c
index c815918..bfc864d 100644
--- a/diff.c
+++ b/diff.c
@@ -504,9 +504,9 @@ static void prepare_temp_file(const char
 		}
 		if (S_ISLNK(st.st_mode)) {
 			int ret;
-			char *buf, buf_[1024];
-			buf = ((sizeof(buf_) < st.st_size) ?
-			       xmalloc(st.st_size) : buf_);
+			char buf[PATH_MAX + 1]; /* ought to be SYMLINK_MAX */
+			if (sizeof(buf) <= st.st_size)
+				die("symlink too long: %s", name);
 			ret = readlink(name, buf, st.st_size);
 			if (ret < 0)
 				die("readlink(%s)", name);
diff --git a/diffcore-order.c b/diffcore-order.c
index b381223..0bc2b22 100644
--- a/diffcore-order.c
+++ b/diffcore-order.c
@@ -105,9 +105,13 @@ static int compare_pair_order(const void
 void diffcore_order(const char *orderfile)
 {
 	struct diff_queue_struct *q = &diff_queued_diff;
-	struct pair_order *o = xmalloc(sizeof(*o) * q->nr);
+	struct pair_order *o;
 	int i;
 
+	if (!q->nr)
+		return;
+
+	o = xmalloc(sizeof(*o) * q->nr);
 	prepare_order(orderfile);
 	for (i = 0; i < q->nr; i++) {
 		o[i].pair = q->queue[i];
diff --git a/diffcore-pathspec.c b/diffcore-pathspec.c
index 68fe009..139fe88 100644
--- a/diffcore-pathspec.c
+++ b/diffcore-pathspec.c
@@ -48,6 +48,9 @@ void diffcore_pathspec(const char **path
 	for (i = 0; pathspec[i]; i++)
 		;
 	speccnt = i;
+	if (!speccnt)
+		return;
+
 	spec = xmalloc(sizeof(*spec) * speccnt);
 	for (i = 0; pathspec[i]; i++) {
 		spec[i].spec = pathspec[i];
diff --git a/index-pack.c b/index-pack.c
index d4ce3af..541d7bc 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -352,18 +352,24 @@ static int sha1_compare(const void *_a, 
 static void write_index_file(const char *index_name, unsigned char *sha1)
 {
 	struct sha1file *f;
-	struct object_entry **sorted_by_sha =
-		xcalloc(nr_objects, sizeof(struct object_entry *));
-	struct object_entry **list = sorted_by_sha;
-	struct object_entry **last = sorted_by_sha + nr_objects;
+	struct object_entry **sorted_by_sha, **list, **last;
 	unsigned int array[256];
 	int i;
 	SHA_CTX ctx;
 
-	for (i = 0; i < nr_objects; ++i)
-		sorted_by_sha[i] = &objects[i];
-	qsort(sorted_by_sha, nr_objects, sizeof(sorted_by_sha[0]),
-	      sha1_compare);
+	if (nr_objects) {
+		sorted_by_sha =
+			xcalloc(nr_objects, sizeof(struct object_entry *));
+		list = sorted_by_sha;
+		last = sorted_by_sha + nr_objects;
+		for (i = 0; i < nr_objects; ++i)
+			sorted_by_sha[i] = &objects[i];
+		qsort(sorted_by_sha, nr_objects, sizeof(sorted_by_sha[0]),
+		      sha1_compare);
+
+	}
+	else
+		sorted_by_sha = list = last = NULL;
 
 	unlink(index_name);
 	f = sha1create("%s", index_name);
diff --git a/read-tree.c b/read-tree.c
index e3b9c0d..a46c6fe 100644
--- a/read-tree.c
+++ b/read-tree.c
@@ -294,17 +294,20 @@ static int unpack_trees(merge_fn_t fn)
 {
 	int indpos = 0;
 	unsigned len = object_list_length(trees);
-	struct tree_entry_list **posns = 
-		xmalloc(len * sizeof(struct tree_entry_list *));
+	struct tree_entry_list **posns;
 	int i;
 	struct object_list *posn = trees;
 	merge_size = len;
-	for (i = 0; i < len; i++) {
-		posns[i] = ((struct tree *) posn->item)->entries;
-		posn = posn->next;
+
+	if (len) {
+		posns = xmalloc(len * sizeof(struct tree_entry_list *));
+		for (i = 0; i < len; i++) {
+			posns[i] = ((struct tree *) posn->item)->entries;
+			posn = posn->next;
+		}
+		if (unpack_trees_rec(posns, len, "", fn, &indpos))
+			return -1;
 	}
-	if (unpack_trees_rec(posns, len, "", fn, &indpos))
-		return -1;
 
 	if (trivial_merges_only && nontrivial_merge)
 		die("Merge requires file-level merging");
diff --git a/tree-diff.c b/tree-diff.c
index 0ef06a9..382092b 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -263,6 +263,10 @@ void diff_tree_setup_paths(const char **
 
 		paths = p;
 		nr_paths = count_paths(paths);
+		if (nr_paths == 0) {
+			pathlens = NULL;
+			return;
+		}
 		pathlens = xmalloc(nr_paths * sizeof(int));
 		for (i=0; i<nr_paths; i++)
 			pathlens[i] = strlen(paths[i]);
-- 
1.0.GIT

^ permalink raw reply related

* [PATCH] Handle symlinks graciously
From: Johannes Schindelin @ 2005-12-26 21:31 UTC (permalink / raw)
  To: git, junkio


This patch converts a stat() to an lstat() call, thereby fixing the case
when the date of a symlink was not the same as the one recorded in the
index. The included test case demonstrates this.

This is for the case that the symlink points to a non-existing file. If
the file exists, worse things than just an error message happen.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

	A few weeks ago I hunted a bug in t4004 on OS X, just to discover
	that it only occurred rarely. The problem is when the atime or mtime
	differ: in this case, diff-index would try to stat() the symlink,
	not lstat() it.

 diff.c                  |    2 +
 t/t4011-diff-symlink.sh |   84 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+), 1 deletions(-)
 create mode 100755 t/t4011-diff-symlink.sh

bd43cfdf69d99c3552fab5a375d9a72657d3bcad
diff --git a/diff.c b/diff.c
index c815918..6f064df 100644
--- a/diff.c
+++ b/diff.c
@@ -650,7 +650,7 @@ static void diff_fill_sha1_info(struct d
 	if (DIFF_FILE_VALID(one)) {
 		if (!one->sha1_valid) {
 			struct stat st;
-			if (stat(one->path, &st) < 0)
+			if (lstat(one->path, &st) < 0)
 				die("stat %s", one->path);
 			if (index_path(one->sha1, one->path, &st, 0))
 				die("cannot hash %s\n", one->path);
diff --git a/t/t4011-diff-symlink.sh b/t/t4011-diff-symlink.sh
new file mode 100755
index 0000000..9e70063
--- /dev/null
+++ b/t/t4011-diff-symlink.sh
@@ -0,0 +1,84 @@
+#!/bin/sh
+#
+# Copyright (c) 2005 Johannes Schindelin
+#
+
+test_description='Test diff of symlinks.
+
+'
+. ./test-lib.sh
+. ../diff-lib.sh
+
+cat > expected << EOF
+diff --git a/frotz b/frotz
+new file mode 120000
+index 0000000..7c465af
+--- /dev/null
++++ b/frotz
+@@ -0,0 +1 @@
++xyzzy
+\ No newline at end of file
+EOF
+
+test_expect_success \
+    'diff new symlink' \
+    'ln -s xyzzy frotz &&
+    git-update-index &&
+    tree=$(git-write-tree) &&
+    git-update-index --add frotz &&
+    GIT_DIFF_OPTS=--unified=0 git-diff-index -M -p $tree > current &&
+    compare_diff_patch current expected'
+
+test_expect_success \
+    'diff unchanged symlink' \
+    'tree=$(git-write-tree) &&
+    test -z "$(git-diff-index --name-only $tree)"'
+
+cat > expected << EOF
+diff --git a/frotz b/frotz
+deleted file mode 120000
+index 7c465af..0000000
+--- a/frotz
++++ /dev/null
+@@ -1 +0,0 @@
+-xyzzy
+\ No newline at end of file
+EOF
+
+test_expect_success \
+    'diff removed symlink' \
+    'rm frotz &&
+    git-diff-index -M -p $tree > current &&
+    compare_diff_patch current expected'
+
+cat > expected << EOF
+diff --git a/frotz b/frotz
+EOF
+
+test_expect_success \
+    'diff identical, but newly created symlink' \
+    'sleep 1 &&
+    ln -s xyzzy frotz &&
+    git-diff-index -M -p $tree > current &&
+    compare_diff_patch current expected'
+
+cat > expected << EOF
+diff --git a/frotz b/frotz
+index 7c465af..df1db54 120000
+--- a/frotz
++++ b/frotz
+@@ -1 +1 @@
+-xyzzy
+\ No newline at end of file
++yxyyz
+\ No newline at end of file
+EOF
+
+test_expect_success \
+    'diff different symlink' \
+    'rm frotz &&
+    ln -s yxyyz frotz &&
+    git-diff-index -M -p $tree > current &&
+    compare_diff_patch current expected'
+
+test_done
-- 
1.0.GIT

^ permalink raw reply related

* Re: [PATCH] Avoid allocating 0 bytes, was Re: [PATCH 4/4] git-compat-util.h: dietlibc-friendly x{malloc,realloc,calloc}
From: Johannes Schindelin @ 2005-12-26 20:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Eric Wong, git
In-Reply-To: <7vzmmnisix.fsf@assigned-by-dhcp.cox.net>

Hi,

On Mon, 26 Dec 2005, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > diff --git a/csum-file.c b/csum-file.c
> > index 5f9249a..2c0f097 100644
> > --- a/csum-file.c
> > +++ b/csum-file.c
> > @@ -121,6 +121,9 @@ int sha1write_compressed(struct sha1file
> >  	unsigned long maxsize;
> >  	void *out;
> >  
> > +	if (size == 0)
> > +		return 0;
> > +
> >  	memset(&stream, 0, sizeof(stream));
> >  	deflateInit(&stream, Z_DEFAULT_COMPRESSION);
> >  	maxsize = deflateBound(&stream, size);
> 
> I think this and the one in sha1_file.c::write_sha1_file() are
> wrong; 0-size input would not result in 0-size output.  Have you
> tested them by actually exercising the codepaths you touched?

No, I did not test them. At least not consciously (I did the whole work on 
Dec 24, when I was lying in bed, ill, trying to distract myself from being 
miserable).

The reason I did it this way: If zlib inflates a buffer of 0 bytes, it 
makes no sense to expect anything than 0 bytes to come out of it, right? 
Therefore, if zlib encounters a deflated buffer of 0 bytes, it should 
inflate it to 0 bytes. So it is a good idea in any case to write out 0 
bytes.

However, thinking of it again, it might break backwards compatibility. But 
I don't think so.

> > diff --git a/diffcore-pathspec.c b/diffcore-pathspec.c
> > index 68fe009..a12337a 100644
> > --- a/diffcore-pathspec.c
> > +++ b/diffcore-pathspec.c
> > @@ -48,10 +48,14 @@ void diffcore_pathspec(const char **path
> 
> diffcore-pathspec and diffcore-order can probably return without
> touching diff_queued_diff if there is no work to be done.

To be honest: I did not completely understand what that code does. 
It seemed obvious, but I got a track record of breaking things. So I was 
cautious. I will not try to modify it myself without a good test case.

> > @@ -353,7 +353,8 @@ static void write_index_file(const char 
> >  {
> >  	struct sha1file *f;
> >  	struct object_entry **sorted_by_sha =
> > -		xcalloc(nr_objects, sizeof(struct object_entry *));
> > +		xcalloc(nr_objects ? nr_objects : 1,
> > +				sizeof(struct object_entry *));
> >  	struct object_entry **list = sorted_by_sha;
> >  	struct object_entry **last = sorted_by_sha + nr_objects;
> >  	unsigned int array[256];
> 
> This can be simplified by sorted_by_sha = list = last = NULL
> when nr_objects == 0 and avoiding qsort; that is what you did in
> pack-objects, I think.

If nr_objects == 0, we could write out a fixed index.

We could alternatively exit(1), because what sense does it make to have an 
index if the pack contains 0 objects? However, this has consequences: 
Every user of index-pack (does "fetch-pack --keep" call it?) has to be 
aware that empty packs are illegal.

Again, I did this so that the tool does not break. I'll try to come up 
with something better soon.

> > @@ -448,8 +449,10 @@ int main(int argc, char **argv)
> >  
> >  	open_pack_file();
> >  	parse_pack_header();
> > -	objects = xcalloc(nr_objects, sizeof(struct object_entry));
> > -	deltas = xcalloc(nr_objects, sizeof(struct delta_entry));
> > +	objects = xcalloc(nr_objects ? nr_objects : 1,
> > +			sizeof(struct object_entry));
> > +	deltas = xcalloc(nr_objects ? nr_objects : 1,
> > +			sizeof(struct delta_entry));
> >  	parse_pack_objects();
> >  	free(deltas);
> >  	write_index_file(index_name, sha1);
> 
> Likewise I suspect.  After all the special case is only when
> reindexing an empty pack ;-).

Right.

> My inclination is to do things in these steps:
> 
>  - apply cleanups that actually simplify the logic, while
>    leaving the ones that you needed to do (size ?  size : 1)
>    unmodified (BTW, must next_in/next_out point at non NULL when
>    avail_in/avail_out are zero?).

I was cautious. Maybe at some point in future, zlib relies on that. But it 
is probably better to try not calling zlib at all in these cases.

>  - change x*alloc like this, once the above is done:
> 
>         static inline void *xmalloc(size_t size)
>         {
>                 void *ret = malloc(size);
>         #ifdef MALLOC_CAN_RETURN_NULL_ON_0SIZE
>                 if (!ret && !size)
>                         ret = malloc(size+1);
>         #endif
>                 if (!ret)
>                         die("Out of memory, malloc failed");
>                 return ret;
>         }

Sounds easiest.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Avoid allocating 0 bytes, was Re: [PATCH 4/4] git-compat-util.h: dietlibc-friendly x{malloc,realloc,calloc}
From: Junio C Hamano @ 2005-12-26 19:10 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Eric Wong, git
In-Reply-To: <Pine.LNX.4.63.0512261916440.8435@wbgn013.biozentrum.uni-wuerzburg.de>

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

> This is the result of a relatively quick audit of the source code. There
> might still be a few odd places lurking out there, but I am quite certain
> I caught most if not all.

Thanks.

> diff --git a/csum-file.c b/csum-file.c
> index 5f9249a..2c0f097 100644
> --- a/csum-file.c
> +++ b/csum-file.c
> @@ -121,6 +121,9 @@ int sha1write_compressed(struct sha1file
>  	unsigned long maxsize;
>  	void *out;
>  
> +	if (size == 0)
> +		return 0;
> +
>  	memset(&stream, 0, sizeof(stream));
>  	deflateInit(&stream, Z_DEFAULT_COMPRESSION);
>  	maxsize = deflateBound(&stream, size);

I think this and the one in sha1_file.c::write_sha1_file() are
wrong; 0-size input would not result in 0-size output.  Have you
tested them by actually exercising the codepaths you touched?

> diff --git a/diffcore-pathspec.c b/diffcore-pathspec.c
> index 68fe009..a12337a 100644
> --- a/diffcore-pathspec.c
> +++ b/diffcore-pathspec.c
> @@ -48,10 +48,14 @@ void diffcore_pathspec(const char **path

diffcore-pathspec and diffcore-order can probably return without
touching diff_queued_diff if there is no work to be done.

> @@ -353,7 +353,8 @@ static void write_index_file(const char 
>  {
>  	struct sha1file *f;
>  	struct object_entry **sorted_by_sha =
> -		xcalloc(nr_objects, sizeof(struct object_entry *));
> +		xcalloc(nr_objects ? nr_objects : 1,
> +				sizeof(struct object_entry *));
>  	struct object_entry **list = sorted_by_sha;
>  	struct object_entry **last = sorted_by_sha + nr_objects;
>  	unsigned int array[256];

This can be simplified by sorted_by_sha = list = last = NULL
when nr_objects == 0 and avoiding qsort; that is what you did in
pack-objects, I think.

> @@ -448,8 +449,10 @@ int main(int argc, char **argv)
>  
>  	open_pack_file();
>  	parse_pack_header();
> -	objects = xcalloc(nr_objects, sizeof(struct object_entry));
> -	deltas = xcalloc(nr_objects, sizeof(struct delta_entry));
> +	objects = xcalloc(nr_objects ? nr_objects : 1,
> +			sizeof(struct object_entry));
> +	deltas = xcalloc(nr_objects ? nr_objects : 1,
> +			sizeof(struct delta_entry));
>  	parse_pack_objects();
>  	free(deltas);
>  	write_index_file(index_name, sha1);

Likewise I suspect.  After all the special case is only when
reindexing an empty pack ;-).

My inclination is to do things in these steps:

 - apply cleanups that actually simplify the logic, while
   leaving the ones that you needed to do (size ?  size : 1)
   unmodified (BTW, must next_in/next_out point at non NULL when
   avail_in/avail_out are zero?).

 - change x*alloc like this, once the above is done:

        static inline void *xmalloc(size_t size)
        {
                void *ret = malloc(size);
        #ifdef MALLOC_CAN_RETURN_NULL_ON_0SIZE
                if (!ret && !size)
                        ret = malloc(size+1);
        #endif
                if (!ret)
                        die("Out of memory, malloc failed");
                return ret;
        }

^ permalink raw reply

* [PATCH] Avoid allocating 0 bytes, was Re: [PATCH 4/4] git-compat-util.h: dietlibc-friendly x{malloc,realloc,calloc}
From: Johannes Schindelin @ 2005-12-26 18:16 UTC (permalink / raw)
  To: Eric Wong; +Cc: Junio C Hamano, git
In-Reply-To: <20051224211546.GG3963@mail.yhbt.net>


This is the result of a relatively quick audit of the source code. There
might still be a few odd places lurking out there, but I am quite certain
I caught most if not all.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

 csum-file.c         |    3 +++
 diff.c              |    6 +++--
 diffcore-order.c    |    8 ++++++-
 diffcore-pathspec.c |   12 +++++++----
 index-pack.c        |   11 ++++++----
 pack-objects.c      |    6 ++++-
 read-tree.c         |   17 +++++++++------
 sha1_file.c         |   56 ++++++++++++++++++++++++++-------------------------
 tree-diff.c         |    4 ++++
 unpack-objects.c    |    2 +-
 10 files changed, 76 insertions(+), 49 deletions(-)

751737aeec4dd3cc1afd1d1d3fa529cfc74535e0
diff --git a/csum-file.c b/csum-file.c
index 5f9249a..2c0f097 100644
--- a/csum-file.c
+++ b/csum-file.c
@@ -121,6 +121,9 @@ int sha1write_compressed(struct sha1file
 	unsigned long maxsize;
 	void *out;
 
+	if (size == 0)
+		return 0;
+
 	memset(&stream, 0, sizeof(stream));
 	deflateInit(&stream, Z_DEFAULT_COMPRESSION);
 	maxsize = deflateBound(&stream, size);
diff --git a/diff.c b/diff.c
index c815918..daee05b 100644
--- a/diff.c
+++ b/diff.c
@@ -504,9 +504,9 @@ static void prepare_temp_file(const char
 		}
 		if (S_ISLNK(st.st_mode)) {
 			int ret;
-			char *buf, buf_[1024];
-			buf = ((sizeof(buf_) < st.st_size) ?
-			       xmalloc(st.st_size) : buf_);
+			char buf[MAXPATHLEN + 1];
+			if (st.st_size > MAXPATHLEN)
+				die("Unbelievably long symlink: %", name);
 			ret = readlink(name, buf, st.st_size);
 			if (ret < 0)
 				die("readlink(%s)", name);
diff --git a/diffcore-order.c b/diffcore-order.c
index b381223..94a4996 100644
--- a/diffcore-order.c
+++ b/diffcore-order.c
@@ -61,6 +61,8 @@ static void prepare_order(const char *or
 		}
 		if (pass == 0) {
 			order_cnt = cnt;
+			if (cnt == 0)
+				return;
 			order = xmalloc(sizeof(*order) * cnt);
 		}
 	}
@@ -105,9 +107,13 @@ static int compare_pair_order(const void
 void diffcore_order(const char *orderfile)
 {
 	struct diff_queue_struct *q = &diff_queued_diff;
-	struct pair_order *o = xmalloc(sizeof(*o) * q->nr);
+	struct pair_order *o;
 	int i;
 
+	if (q->nr == 0)
+		return;
+
+	o = xmalloc(sizeof(*o) * q->nr);
 	prepare_order(orderfile);
 	for (i = 0; i < q->nr; i++) {
 		o[i].pair = q->queue[i];
diff --git a/diffcore-pathspec.c b/diffcore-pathspec.c
index 68fe009..a12337a 100644
--- a/diffcore-pathspec.c
+++ b/diffcore-pathspec.c
@@ -48,10 +48,14 @@ void diffcore_pathspec(const char **path
 	for (i = 0; pathspec[i]; i++)
 		;
 	speccnt = i;
-	spec = xmalloc(sizeof(*spec) * speccnt);
-	for (i = 0; pathspec[i]; i++) {
-		spec[i].spec = pathspec[i];
-		spec[i].len = strlen(pathspec[i]);
+	if (speccnt == 0)
+		spec = NULL;
+	else {
+		spec = xmalloc(sizeof(*spec) * speccnt);
+		for (i = 0; pathspec[i]; i++) {
+			spec[i].spec = pathspec[i];
+			spec[i].len = strlen(pathspec[i]);
+		}
 	}
 
 	for (i = 0; i < q->nr; i++) {
diff --git a/index-pack.c b/index-pack.c
index d4ce3af..2927632 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -103,7 +103,7 @@ static void *unpack_entry_data(unsigned 
 	unsigned long pack_limit = pack_size - 20;
 	unsigned long pos = *current_pos;
 	z_stream stream;
-	void *buf = xmalloc(size);
+	void *buf = xmalloc(size ? size : 1);
 
 	memset(&stream, 0, sizeof(stream));
 	stream.next_out = buf;
@@ -353,7 +353,8 @@ static void write_index_file(const char 
 {
 	struct sha1file *f;
 	struct object_entry **sorted_by_sha =
-		xcalloc(nr_objects, sizeof(struct object_entry *));
+		xcalloc(nr_objects ? nr_objects : 1,
+				sizeof(struct object_entry *));
 	struct object_entry **list = sorted_by_sha;
 	struct object_entry **last = sorted_by_sha + nr_objects;
 	unsigned int array[256];
@@ -448,8 +449,10 @@ int main(int argc, char **argv)
 
 	open_pack_file();
 	parse_pack_header();
-	objects = xcalloc(nr_objects, sizeof(struct object_entry));
-	deltas = xcalloc(nr_objects, sizeof(struct delta_entry));
+	objects = xcalloc(nr_objects ? nr_objects : 1,
+			sizeof(struct object_entry));
+	deltas = xcalloc(nr_objects ? nr_objects : 1,
+			sizeof(struct delta_entry));
 	parse_pack_objects();
 	free(deltas);
 	write_index_file(index_name, sha1);
diff --git a/pack-objects.c b/pack-objects.c
index caf3b6b..f5a147f 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -406,7 +406,8 @@ static void prepare_pack(int window, int
 
 	fprintf(stderr, "Packing %d objects\n", nr_objects);
 
-	sorted_by_type = create_sorted_list(type_size_sort);
+	if (nr_objects > 0)
+		sorted_by_type = create_sorted_list(type_size_sort);
 	if (window && depth)
 		find_deltas(sorted_by_type, window+1, depth);
 	write_pack_file();
@@ -540,7 +541,8 @@ int main(int argc, char **argv)
 	if (non_empty && !nr_objects)
 		return 0;
 
-	sorted_by_sha = create_sorted_list(sha1_sort);
+	if (nr_objects > 0)
+		sorted_by_sha = create_sorted_list(sha1_sort);
 	SHA1_Init(&ctx);
 	list = sorted_by_sha;
 	for (i = 0; i < nr_objects; i++) {
diff --git a/read-tree.c b/read-tree.c
index e3b9c0d..a46c6fe 100644
--- a/read-tree.c
+++ b/read-tree.c
@@ -294,17 +294,20 @@ static int unpack_trees(merge_fn_t fn)
 {
 	int indpos = 0;
 	unsigned len = object_list_length(trees);
-	struct tree_entry_list **posns = 
-		xmalloc(len * sizeof(struct tree_entry_list *));
+	struct tree_entry_list **posns;
 	int i;
 	struct object_list *posn = trees;
 	merge_size = len;
-	for (i = 0; i < len; i++) {
-		posns[i] = ((struct tree *) posn->item)->entries;
-		posn = posn->next;
+
+	if (len) {
+		posns = xmalloc(len * sizeof(struct tree_entry_list *));
+		for (i = 0; i < len; i++) {
+			posns[i] = ((struct tree *) posn->item)->entries;
+			posn = posn->next;
+		}
+		if (unpack_trees_rec(posns, len, "", fn, &indpos))
+			return -1;
 	}
-	if (unpack_trees_rec(posns, len, "", fn, &indpos))
-		return -1;
 
 	if (trivial_merges_only && nontrivial_merge)
 		die("Merge requires file-level merging");
diff --git a/sha1_file.c b/sha1_file.c
index 8bebbb2..0d60884 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1371,35 +1371,37 @@ int write_sha1_file(void *buf, unsigned 
 		return -1;
 	}
 
-	/* Set it up */
-	memset(&stream, 0, sizeof(stream));
-	deflateInit(&stream, Z_BEST_COMPRESSION);
-	size = deflateBound(&stream, len+hdrlen);
-	compressed = xmalloc(size);
-
-	/* Compress it */
-	stream.next_out = compressed;
-	stream.avail_out = size;
-
-	/* First header.. */
-	stream.next_in = hdr;
-	stream.avail_in = hdrlen;
-	while (deflate(&stream, 0) == Z_OK)
-		/* nothing */;
-
-	/* Then the data itself.. */
-	stream.next_in = buf;
-	stream.avail_in = len;
-	while (deflate(&stream, Z_FINISH) == Z_OK)
-		/* nothing */;
-	deflateEnd(&stream);
-	size = stream.total_out;
+	if (len + hdrlen > 0) {
+		/* Set it up */
+		memset(&stream, 0, sizeof(stream));
+		deflateInit(&stream, Z_BEST_COMPRESSION);
+		size = deflateBound(&stream, len+hdrlen);
+		compressed = xmalloc(size);
+
+		/* Compress it */
+		stream.next_out = compressed;
+		stream.avail_out = size;
 
-	if (write(fd, compressed, size) != size)
-		die("unable to write file");
+		/* First header.. */
+		stream.next_in = hdr;
+		stream.avail_in = hdrlen;
+		while (deflate(&stream, 0) == Z_OK)
+			/* nothing */;
+
+		/* Then the data itself.. */
+		stream.next_in = buf;
+		stream.avail_in = len;
+		while (deflate(&stream, Z_FINISH) == Z_OK)
+			/* nothing */;
+		deflateEnd(&stream);
+		size = stream.total_out;
+
+		if (write(fd, compressed, size) != size)
+			die("unable to write file");
+		free(compressed);
+	}
 	fchmod(fd, 0444);
 	close(fd);
-	free(compressed);
 
 	return move_temp_to_file(tmpfile, filename);
 }
@@ -1429,7 +1431,7 @@ int write_sha1_to_fd(int fd, const unsig
 		memset(&stream, 0, sizeof(stream));
 		deflateInit(&stream, Z_BEST_COMPRESSION);
 		size = deflateBound(&stream, len + hdrlen);
-		temp_obj = buf = xmalloc(size);
+		temp_obj = buf = xmalloc(size ? size : 1);
 
 		/* Compress it */
 		stream.next_out = buf;
diff --git a/tree-diff.c b/tree-diff.c
index 0ef06a9..382092b 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -263,6 +263,10 @@ void diff_tree_setup_paths(const char **
 
 		paths = p;
 		nr_paths = count_paths(paths);
+		if (nr_paths == 0) {
+			pathlens = NULL;
+			return;
+		}
 		pathlens = xmalloc(nr_paths * sizeof(int));
 		for (i=0; i<nr_paths; i++)
 			pathlens[i] = strlen(paths[i]);
diff --git a/unpack-objects.c b/unpack-objects.c
index 5c5cb12..572eeee 100644
--- a/unpack-objects.c
+++ b/unpack-objects.c
@@ -53,7 +53,7 @@ static void use(int bytes)
 static void *get_data(unsigned long size)
 {
 	z_stream stream;
-	void *buf = xmalloc(size);
+	void *buf = xmalloc(size ? size : 1);
 
 	memset(&stream, 0, sizeof(stream));
 
-- 
1.0.GIT

^ permalink raw reply related

* Re: [PATCH] add strcpy_user_path() and use it in init-db.c and git.c
From: Johannes Schindelin @ 2005-12-26 17:42 UTC (permalink / raw)
  To: Eric Wong; +Cc: Junio C Hamano, git list

[Sorry, this mail will probably be sent to you twice. The mailserver I 
use normally is kinda broken. It sends out the mails one or two days late, 
so paradoxically, the resent mail will arrive earlier than the original.]

Hi,

On Sat, 24 Dec 2005, Eric Wong wrote:

> Junio C Hamano <junkio@cox.net> wrote:
> > Eric Wong <normalperson@yhbt.net> writes:
> > 
> > > My home directories have different names on different machines I'm
> > > on, and I want to avoid having to recompile git for each one.
> > > I don't have root access to some of them, so installing globally in /usr
> > > or /usr/local isn't an option, either.
> > 
> > Then you probably need to use GIT_EXEC_PATH environment
> > variable.
> 
> That works with git.c but not init-db.  But then again I don't use
> git-init-db that often.  I'll just write a shell script wrapper for the
> latter if I do.

How about something like this?

---
[PATCH] Introduce environment variable for the path to the templates

The environment variable GIT_TEMPLATE_PATH can override the compiled-in
setting, and can be overridden with the '--template=' argument to init-db.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

	Eric said that GIT_EXEC_PATH is enough for most things, but not for
	init-db. I guess he wanted something like this.

 init-db.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

5d95ce750b09a14bcb86e07ba23077ab0825089c
diff --git a/init-db.c b/init-db.c
index 863ec1a..774a91f 100644
--- a/init-db.c
+++ b/init-db.c
@@ -238,7 +238,7 @@ int main(int argc, char **argv)
 {
 	const char *git_dir;
 	const char *sha1_dir;
-	char *path, *template_dir = NULL;
+	char *path, *template_dir = getenv("GIT_TEMPLATE_PATH");
 	int len, i;
 
 	for (i = 1; i < argc; i++, argv++) {
-- 
1.0.GIT

^ permalink raw reply related

* Re: [PATCH 1/4] git.c: extra #include for dietlibc (and possibly other C libraries)
From: Junio C Hamano @ 2005-12-26 17:01 UTC (permalink / raw)
  To: Eric Wong; +Cc: git
In-Reply-To: <20051224121117.GB19136@mail.yhbt.net>

Eric Wong <normalperson@yhbt.net> writes:

> struct winsize is defined in <termios.h>, and that's not pulled
> in by other #includes in that file

With glibc, sys/ioctl.h seems to pull it in.  Input from people
on other platforms is appreciated on this.

^ permalink raw reply

* Re: [PATCH] add strcpy_user_path() and use it in init-db.c and git.c
From: Johannes Schindelin @ 2005-12-26 16:40 UTC (permalink / raw)
  To: Eric Wong; +Cc: Junio C Hamano, git list
In-Reply-To: <20051224211925.GH3963@mail.yhbt.net>

Hi,

On Sat, 24 Dec 2005, Eric Wong wrote:

> Junio C Hamano <junkio@cox.net> wrote:
> > Eric Wong <normalperson@yhbt.net> writes:
> > 
> > > My home directories have different names on different machines I'm
> > > on, and I want to avoid having to recompile git for each one.
> > > I don't have root access to some of them, so installing globally in /usr
> > > or /usr/local isn't an option, either.
> > 
> > Then you probably need to use GIT_EXEC_PATH environment
> > variable.
> 
> That works with git.c but not init-db.  But then again I don't use
> git-init-db that often.  I'll just write a shell script wrapper for the
> latter if I do.

How about something like this?

---
[PATCH] Introduce environment variable for the path to the templates

The environment variable GIT_TEMPLATE_PATH can override the compiled-in
setting, and can be overridden with the '--template=' argument to init-db.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

	Eric said that GIT_EXEC_PATH is enough for most things, but not for
	init-db. I guess he wanted something like this.

 init-db.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

5d95ce750b09a14bcb86e07ba23077ab0825089c
diff --git a/init-db.c b/init-db.c
index 863ec1a..774a91f 100644
--- a/init-db.c
+++ b/init-db.c
@@ -238,7 +238,7 @@ int main(int argc, char **argv)
 {
 	const char *git_dir;
 	const char *sha1_dir;
-	char *path, *template_dir = NULL;
+	char *path, *template_dir = getenv("GIT_TEMPLATE_PATH");
 	int len, i;
 
 	for (i = 1; i < argc; i++, argv++) {
-- 
1.0.GIT

^ permalink raw reply related

* [PATCH]Fix cg-export to not specify excess argument to compressor
From: Alexey Nezhdanov @ 2005-12-26 14:20 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git

[-- Attachment #1: Type: text/plain, Size: 142 bytes --]

When gzip or bzip2 provided with empty string as argument
they failing to use stdin as the source of data.

-- 
Respectfully
Alexey Nezhdanov

[-- Attachment #2: cg-export.patch --]
[-- Type: text/x-diff, Size: 494 bytes --]

--- /home/snake/scm/cogito/cg-export	2005-12-26 17:14:49.000000000 +0300
+++ cg-export	2005-12-26 17:16:17.000000000 +0300
@@ -44,10 +44,10 @@
 		base="${base##*/}"
 		case "$ext" in
 		.tar.gz|.tgz)
-			git-tar-tree "$id" "$base" | gzip -c9 "$tar" >"$dest"
+			git-tar-tree "$id" "$base" | gzip -c9 >"$dest"
 			;;
 		.tar.bz2)
-			git-tar-tree "$id" "$base" | bzip2 -c "$tar" >"$dest"
+			git-tar-tree "$id" "$base" | bzip2 -c >"$dest"
 			;;
 		.tar)
 			git-tar-tree "$id" "$base" >"$dest"

^ permalink raw reply

* cg-update does not work with argument specified
From: Alexey Nezhdanov @ 2005-12-26 14:02 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git

Documentation/tutorial-script/script.sh line 117 fails when trying to execute
cg-update bobswork

The error message is:
cg-fetch: where to fetch from?

Quick investigation showed that cg-update runs cg-fetch as:
cg-fetch "$force" "$name"

When tried it manually with
cg-fetch "" bobswork got the same error.
Removing "" works fine.

Further investigation showed that cg-fetch uses 
name="${ARGS[0]}"
line to find out branch name but ARGS[0] contains empty string.

Probably something should be fixed about lines 391-404 in cg-Xlib where
ARGS is set but I do not know this bash feature so unfortunately can't write 
patch myself.

Hope this helps.

P.S. Please Cc: me I'm not following the list closely.

-- 
Respectfully
Alexey Nezhdanov

^ permalink raw reply

* [PATCH] send-pack/receive-pack: allow errors to be reported back to pusher.
From: Junio C Hamano @ 2005-12-26  7:25 UTC (permalink / raw)
  To: git; +Cc: pasky

This updates the protocol between git-send-pack/git-receive-pack
in a backward compatible way to allow failures at the receiving
end to be propagated back to the sender.  Most notably, versions
of git-push before this could not notice if the update hook on
the receiving end refused to update the ref for its own policy
reasons.

Signed-off-by: Junio C Hamano <junkio@cox.net>

---

 * This is something I've been meaning to do since I saw Pasky's
   patch to allow Cogito to notice the failure from
   git-send-pack.

   I am reasonably sure that the protocol is backward compatible
   and you should be able run older git-receive-pack with this
   git-send-pack (the older one does not identify itself of
   capable to report status, and the sender does not ask for
   it), and this git-receive-pack can be driven with older
   git-send-pack (the older one will ignore the funny ref that
   hides the capability string).  Tests and comments are very
   much appreciated.

 connect.c      |    8 ++---
 receive-pack.c |   98 ++++++++++++++++++++++++++++++++++++++++++--------------
 send-pack.c    |   59 ++++++++++++++++++++++++++++++++--
 3 files changed, 134 insertions(+), 31 deletions(-)

be754924ffb9824453e8c8c6567e80789c552999
diff --git a/connect.c b/connect.c
index 6b6d613..d6f4e4c 100644
--- a/connect.c
+++ b/connect.c
@@ -34,10 +34,6 @@ struct ref **get_remote_heads(int in, st
 			die("protocol error: expected sha/ref, got '%s'", buffer);
 		name = buffer + 41;
 
-		if (ignore_funny && 45 < len && !memcmp(name, "refs/", 5) &&
-		    check_ref_format(name + 5))
-			continue;
-
 		name_len = strlen(name);
 		if (len != name_len + 41) {
 			if (server_capabilities)
@@ -45,6 +41,10 @@ struct ref **get_remote_heads(int in, st
 			server_capabilities = strdup(name + name_len + 1);
 		}
 
+		if (ignore_funny && 45 < len && !memcmp(name, "refs/", 5) &&
+		    check_ref_format(name + 5))
+			continue;
+
 		if (nr_match && !path_match(name, nr_match, match))
 			continue;
 		ref = xcalloc(1, sizeof(*ref) + len - 40);
diff --git a/receive-pack.c b/receive-pack.c
index 92878ec..6963545 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -8,20 +8,26 @@ static const char receive_pack_usage[] =
 
 static const char unpacker[] = "git-unpack-objects";
 
+static int report_status = 0;
+
 static int show_ref(const char *path, const unsigned char *sha1)
 {
 	packet_write(1, "%s %s\n", sha1_to_hex(sha1), path);
 	return 0;
 }
 
+static char capabilities[] = "report-status";
+
 static void write_head_info(void)
 {
+	packet_write(1, "%s capabilities^{}%c%s\n",
+		     sha1_to_hex(null_sha1), 0, capabilities);
 	for_each_ref(show_ref);
 }
 
 struct command {
 	struct command *next;
-	unsigned char updated;
+	const char *error_string;
 	unsigned char old_sha1[20];
 	unsigned char new_sha1[20];
 	char ref_name[0];
@@ -89,15 +95,20 @@ static int run_update_hook(const char *r
 	}
 }
 
-static int update(const char *name,
-		  unsigned char *old_sha1, unsigned char *new_sha1)
+static int update(struct command *cmd)
 {
+	const char *name = cmd->ref_name;
+	unsigned char *old_sha1 = cmd->old_sha1;
+	unsigned char *new_sha1 = cmd->new_sha1;
 	char new_hex[60], *old_hex, *lock_name;
 	int newfd, namelen, written;
 
-	if (!strncmp(name, "refs/", 5) && check_ref_format(name + 5))
+	cmd->error_string = NULL;
+	if (!strncmp(name, "refs/", 5) && check_ref_format(name + 5)) {
+		cmd->error_string = "funny refname";
 		return error("refusing to create funny ref '%s' locally",
 			     name);
+	}
 
 	namelen = strlen(name);
 	lock_name = xmalloc(namelen + 10);
@@ -106,16 +117,19 @@ static int update(const char *name,
 
 	strcpy(new_hex, sha1_to_hex(new_sha1));
 	old_hex = sha1_to_hex(old_sha1);
-	if (!has_sha1_file(new_sha1))
+	if (!has_sha1_file(new_sha1)) {
+		cmd->error_string = "bad pack";
 		return error("unpack should have generated %s, "
 			     "but I can't find it!", new_hex);
-
+	}
 	safe_create_leading_directories(lock_name);
 
 	newfd = open(lock_name, O_CREAT | O_EXCL | O_WRONLY, 0666);
-	if (newfd < 0)
+	if (newfd < 0) {
+		cmd->error_string = "can't lock";
 		return error("unable to create %s (%s)",
 			     lock_name, strerror(errno));
+	}
 
 	/* Write the ref with an ending '\n' */
 	new_hex[40] = '\n';
@@ -127,18 +141,22 @@ static int update(const char *name,
 	close(newfd);
 	if (written != 41) {
 		unlink(lock_name);
+		cmd->error_string = "can't write";
 		return error("unable to write %s", lock_name);
 	}
 	if (verify_old_ref(name, old_hex) < 0) {
 		unlink(lock_name);
+		cmd->error_string = "raced";
 		return error("%s changed during push", name);
 	}
 	if (run_update_hook(name, old_hex, new_hex)) {
 		unlink(lock_name);
+		cmd->error_string = "hook declined";
 		return error("hook declined to update %s\n", name);
 	}
 	else if (rename(lock_name, name) < 0) {
 		unlink(lock_name);
+		cmd->error_string = "can't rename";
 		return error("unable to replace %s", name);
 	}
 	else {
@@ -158,7 +176,7 @@ static void run_update_post_hook(struct 
 	if (access(update_post_hook, X_OK) < 0)
 		return;
 	for (argc = 1, cmd_p = cmd; cmd_p; cmd_p = cmd_p->next) {
-		if (!cmd_p->updated)
+		if (cmd_p->error_string)
 			continue;
 		argc++;
 	}
@@ -166,7 +184,7 @@ static void run_update_post_hook(struct 
 	argv[0] = update_post_hook;
 
 	for (argc = 1, cmd_p = cmd; cmd_p; cmd_p = cmd_p->next) {
-		if (!cmd_p->updated)
+		if (cmd_p->error_string)
 			continue;
 		argv[argc] = xmalloc(strlen(cmd_p->ref_name) + 1);
 		strcpy(argv[argc], cmd_p->ref_name);
@@ -185,8 +203,7 @@ static void execute_commands(void)
 	struct command *cmd = commands;
 
 	while (cmd) {
-		cmd->updated = !update(cmd->ref_name,
-				       cmd->old_sha1, cmd->new_sha1);
+		update(cmd);
 		cmd = cmd->next;
 	}
 	run_update_post_hook(commands);
@@ -199,7 +216,8 @@ static void read_head_info(void)
 		static char line[1000];
 		unsigned char old_sha1[20], new_sha1[20];
 		struct command *cmd;
-		int len;
+		char *refname;
+		int len, reflen;
 
 		len = packet_read_line(0, line, sizeof(line));
 		if (!len)
@@ -211,38 +229,66 @@ static void read_head_info(void)
 		    line[81] != ' ' ||
 		    get_sha1_hex(line, old_sha1) ||
 		    get_sha1_hex(line + 41, new_sha1))
-			die("protocol error: expected old/new/ref, got '%s'", line);
+			die("protocol error: expected old/new/ref, got '%s'",
+			    line);
+
+		refname = line + 82;
+		reflen = strlen(refname);
+		if (reflen + 82 < len) {
+			if (strstr(refname + reflen + 1, "report-status"))
+				report_status = 1;
+		}
 		cmd = xmalloc(sizeof(struct command) + len - 80);
 		memcpy(cmd->old_sha1, old_sha1, 20);
 		memcpy(cmd->new_sha1, new_sha1, 20);
 		memcpy(cmd->ref_name, line + 82, len - 81);
+		cmd->error_string = "n/a (unpacker error)";
 		cmd->next = NULL;
 		*p = cmd;
 		p = &cmd->next;
 	}
 }
 
-static void unpack(void)
+static const char *unpack(int *error_code)
 {
 	int code = run_command(unpacker, NULL);
+
+	*error_code = 0;
 	switch (code) {
 	case 0:
-		return;
+		return NULL;
 	case -ERR_RUN_COMMAND_FORK:
-		die("unpack fork failed");
+		return "unpack fork failed";
 	case -ERR_RUN_COMMAND_EXEC:
-		die("unpack execute failed");
+		return "unpack execute failed";
 	case -ERR_RUN_COMMAND_WAITPID:
-		die("waitpid failed");
+		return "waitpid failed";
 	case -ERR_RUN_COMMAND_WAITPID_WRONG_PID:
-		die("waitpid is confused");
+		return "waitpid is confused";
 	case -ERR_RUN_COMMAND_WAITPID_SIGNAL:
-		die("%s died of signal", unpacker);
+		return "unpacker died of signal";
 	case -ERR_RUN_COMMAND_WAITPID_NOEXIT:
-		die("%s died strangely", unpacker);
+		return "unpacker died strangely";
 	default:
-		die("%s exited with error code %d", unpacker, -code);
+		*error_code = -code;
+		return "unpacker exited with error code";
+	}
+}
+
+static void report(const char *unpack_status)
+{
+	struct command *cmd;
+	packet_write(1, "unpack %s\n",
+		     unpack_status ? unpack_status : "ok");
+	for (cmd = commands; cmd; cmd = cmd->next) {
+		if (!cmd->error_string)
+			packet_write(1, "ok %s\n",
+				     cmd->ref_name);
+		else
+			packet_write(1, "ng %s %s\n",
+				     cmd->ref_name, cmd->error_string);
 	}
+	packet_flush(1);
 }
 
 int main(int argc, char **argv)
@@ -275,8 +321,12 @@ int main(int argc, char **argv)
 
 	read_head_info();
 	if (commands) {
-		unpack();
-		execute_commands();
+		int code;
+		const char *unpack_status = unpack(&code);
+		if (!unpack_status)
+			execute_commands();
+		if (report_status)
+			report(unpack_status);
 	}
 	return 0;
 }
diff --git a/send-pack.c b/send-pack.c
index 0d41f9a..cd36193 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -176,16 +176,53 @@ static void get_local_heads(void)
 	for_each_ref(one_local_ref);
 }
 
+static int receive_status(int in)
+{
+	char line[1000];
+	int ret = 0;
+	int len = packet_read_line(in, line, sizeof(line));
+	if (len < 10 || memcmp(line, "unpack ", 7)) {
+		fprintf(stderr, "did not receive status back\n");
+		return -1;
+	}
+	if (memcmp(line, "unpack ok\n", 10)) {
+		fputs(line, stderr);
+		ret = -1;
+	}
+	while (1) {
+		len = packet_read_line(in, line, sizeof(line));
+		if (!len)
+			break;
+		if (len < 3 ||
+		    (memcmp(line, "ok", 2) && memcmp(line, "ng", 2))) {
+			fprintf(stderr, "protocol error: %s\n", line);
+			ret = -1;
+			break;
+		}
+		if (!memcmp(line, "ok", 2))
+			continue;
+		fputs(line, stderr);
+		ret = -1;
+	}
+	return ret;
+}
+
 static int send_pack(int in, int out, int nr_refspec, char **refspec)
 {
 	struct ref *ref;
 	int new_refs;
 	int ret = 0;
+	int ask_for_status_report = 0;
+	int expect_status_report = 0;
 
 	/* No funny business with the matcher */
 	remote_tail = get_remote_heads(in, &remote_refs, 0, NULL, 1);
 	get_local_heads();
 
+	/* Does the other end support the reporting? */
+	if (server_supports("report-status"))
+		ask_for_status_report = 1;
+
 	/* match them up */
 	if (!remote_tail)
 		remote_tail = &remote_refs;
@@ -260,7 +297,17 @@ static int send_pack(int in, int out, in
 		new_refs++;
 		strcpy(old_hex, sha1_to_hex(ref->old_sha1));
 		new_hex = sha1_to_hex(ref->new_sha1);
-		packet_write(out, "%s %s %s", old_hex, new_hex, ref->name);
+
+		if (ask_for_status_report) {
+			packet_write(out, "%s %s %s%c%s",
+				     old_hex, new_hex, ref->name, 0,
+				     "report-status");
+			ask_for_status_report = 0;
+			expect_status_report = 1;
+		}
+		else
+			packet_write(out, "%s %s %s",
+				     old_hex, new_hex, ref->name);
 		fprintf(stderr, "updating '%s'", ref->name);
 		if (strcmp(ref->name, ref->peer_ref->name))
 			fprintf(stderr, " using '%s'", ref->peer_ref->name);
@@ -270,9 +317,15 @@ static int send_pack(int in, int out, in
 	packet_flush(out);
 	if (new_refs)
 		pack_objects(out, remote_refs);
-	else if (ret == 0)
-		fprintf(stderr, "Everything up-to-date\n");
 	close(out);
+
+	if (expect_status_report) {
+		if (receive_status(in))
+			ret = -4;
+	}
+
+	if (!new_refs && ret == 0)
+		fprintf(stderr, "Everything up-to-date\n");
 	return ret;
 }
 
-- 
1.0.GIT

^ permalink raw reply related

* Re: Add a "git-describe" command
From: M_ @ 2005-12-25 19:21 UTC (permalink / raw)
  To: git; +Cc: Linus Torvalds, Junio C Hamano
In-Reply-To: <Pine.LNX.4.64.0512241409300.14098@g5.osdl.org>

Alle 23:13, sabato 24 dicembre 2005, Linus Torvalds ha scritto:
>  - it should notice when "git-describe" doesn't exist, and fall back on
>    the old less-than-descriptive behaviour

And something like this?

GIT_VERSION = $(shell (git-describe HEAD || echo "1.0.GIT") | sed 's:refs/tags/::')

^ permalink raw reply

* Re: Possibility of a MinGW version?
From: Rob McDonald @ 2005-12-25 17:16 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Andreas Ericsson, git
In-Reply-To: <Pine.LNX.4.63.0512241509240.24308@wbgn013.biozentrum.uni-wuerzburg.de>

> Well, cygwin is everything but unmaintained. It's unfair to blaim the good
> work of the cygwin folks if you can't get it to work. I have *never* had a
> problem I could not solve with cygwin. In particular, *no* package got
> messed up when I installed/upgraded another package. Before you ask: I use
> cygwin extensively.

I'm sorry to have seemed harsh in my criticism.  I have not used Cygwin
seriously in a couple years.  At the time, as a user, I saw no noticable
progress.  The project seemed dead.  I always had trouble with their package
management program.  I'm glad you've never had similar problems.

> As I already stated, there are two *big* showstoppers when it comes to
> port git to MinGW.

Thanks very much for those comments, that is exactly the kind of information
I was hoping to get out of this thread.

> Okay, I'll bite. Could you please port python to MinGW?

As I said, my experience porting apps to MinGW has been very limited in
scope.  Essentially limited to programs that don't use any unixisms.  I see
no need to port Python, they already have a native version.  And, you could
always use Jython if they didn't.

> You're welcome. Just be sure to tackle the hard problems first, else you
> end up having wasted lots of valuable time for nothing.

Thanks for the insight.  I didn't think the shell and / vs \ were going to
be the real issues.

                 Rob

^ permalink raw reply

* Re: [PATCH 3/4] add xmktime() function that always accounts for the TZ env
From: Johannes Schindelin @ 2005-12-24 12:45 UTC (permalink / raw)
  To: Eric Wong; +Cc: git list
In-Reply-To: <20051224121339.GB3963@mail.yhbt.net>

Hi,

On Sat, 24 Dec 2005, Eric Wong wrote:

> This function was added because mktime in dietlibc doesn't seem to
> account for the TZ env.  Also, xmktime() now shares the same
> always-summer bug TZ parsing elsewhere, so at least we can
> be wrong about summer consistently.

How about making this a compat function, which is used when the Makefile 
variable MKTIME_IGNORES_TZ is set?

Ciao,
Dscho

^ permalink raw reply

* Re: Possibility of a MinGW version?
From: Johannes Schindelin @ 2005-12-24 12:07 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: skimo, Rob McDonald, git
In-Reply-To: <43AD21D7.5060306@op5.se>

Hi,

On Sat, 24 Dec 2005, Andreas Ericsson wrote:

> Sven Verdoolaege wrote:
> > On Sat, Dec 24, 2005 at 11:09:39AM +0100, Andreas Ericsson wrote:
> > 
> > > The worst trouble you're likely to run into is all the hardcoded paths.
> > > They are everywhere and ofcourse use the / for path entity separation.
> > 
> > 
> > AFAIR, '/' is a valid path separator on Windows.
> > It's just command.com (does that still exist?) that insisted on '\\'
> > separators.
>
> Are you sure? I've seen lots of porting patches that transliterate those to
> '\\'. Perhaps those who wrote those patches just took for granted that it was
> needed, the same as I did.

I tried it (IIRC even on Win98) and it worked with '/'. If not, it is 
relatively easy to introduce compat_mkdir and compat_open.

(We need compat_mkdir anyway, because the good MinGW people thought it a 
good idea to change the signature from standard mkdir(path, mode) to 
mkdir(path); a major annoyance when porting things to MinGW).

A bigger obstacle is the ubiquitous "c:/blabla". While you can make sure 
that git internally never uses such a rubbish (would be "/c/blabla" in 
MinGW), there are always the users.

> > > The fact that there are 39 bash'ish shell-scripts does little to help a
> > > native port, and although they can be fairly easily replaced by "real"
> > > programs it still means quite a bit of work with little real value for the
> > > unix-version, so I'm guessing you'll have to write those up for yourself.
> > 
> > 
> > Or just use MinGW's bash.

That is correct. MinGW comes with a current bash.

The biggest stoppers, however, are the absence of fork() and of ssh.

I don't know how many times fork() is used just to call another program, 
and then return the the original program. I believe there are some other 
cases, where there is bidirectional communication between the two. This is 
not so easy.

Without ssh, you have only http-push left to do the push, which in turn 
needs a WebServer set up with DAV. Furthermore, you can only fetch via 
HTTP or the git protocol, which both means setting up a server, i.e. you 
have no way to move your data directly between two private repositories.

Yes, there are other SSH's out there. But in order to communicate with the 
other side, MinGW would have to fork() with bidirectional communication. 
Bummer.

I can understand that people would prefer a MinGW version to a cygwin one, 
because of the performance hit, but unfortunately it is probably not worth 
the trouble. I, for one, would not do the work without getting paid very 
well for the job.

Ciao,
Dscho

^ permalink raw reply

* Re: Possibility of a MinGW version?
From: Johannes Schindelin @ 2005-12-24 14:16 UTC (permalink / raw)
  To: Rob McDonald; +Cc: Andreas Ericsson, git
In-Reply-To: <009701c60891$50893fd0$6900a8c0@sps>

Hi,

On Sat, 24 Dec 2005, Rob McDonald wrote:

> > Is there some reason you can't install Cygwin, which effectively
> > overcomes both those problems?
> 
> I've had consistently lousy luck with Cygwin which has left a bad taste in
> my mouth.  Cygwin is generally a lot slower than Mingw, although that is
> most noticeable when you're making extensive use of math.h.  Also, it seems
> that every time I install some package in Cygwin, something else I've
> installed gets messed up.  It just seems to me that there isn't any reason
> for an efficient command-line tool like git to depend on a large
> unmaintained project like Cygwin.

Well, cygwin is everything but unmaintained. It's unfair to blaim the good 
work of the cygwin folks if you can't get it to work. I have *never* had a 
problem I could not solve with cygwin. In particular, *no* package got 
messed up when I installed/upgraded another package. Before you ask: I use 
cygwin extensively.

> Of course, one could use -mno-cygwin (or whatever it is) to use the MinGW
> headers when compiling in Cygwin as an intermediate step.  That would give
> any speed advantages.

As I already stated, there are two *big* showstoppers when it comes to 
port git to MinGW.

> However, I've had great luck porting Linux apps using the gcc toolchain to
> Windows using MinGW.  All these programs 'just worked'.

Okay, I'll bite. Could you please port python to MinGW?

> Thanks for the comments.  The best idea may be to just try it....

You're welcome. Just be sure to tackle the hard problems first, else you 
end up having wasted lots of valuable time for nothing.

Hth,
Dscho

^ permalink raw reply

* Re: [PATCH 2/4] short circuit out of a few places where we would allocate zero bytes
From: Johannes Schindelin @ 2005-12-24 12:44 UTC (permalink / raw)
  To: Eric Wong; +Cc: git list
In-Reply-To: <20051224121243.GA3963@mail.yhbt.net>

Hi,

On Sat, 24 Dec 2005, Eric Wong wrote:

> dietlibc versions of malloc, calloc and realloc all return NULL if
> they're told to allocate 0 bytes, causes the x* wrappers to die().
> 
> There are several more places where these calls could end up asking
> for 0 bytes, too...
> 
> Maybe simply not die()-ing in the x* wrappers if 0/NULL is returned
> when the requested size is zero is a safer and easier way to go.

I would prefer that, too. But the cleanest way would be to prevent calls 
to *alloc if the size is 0...

Ciao,
Dscho

^ permalink raw reply

* Re: Add a "git-describe" command
From: Junio C Hamano @ 2005-12-25  9:44 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0512241939490.14098@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> My real goal was to see a real meaningful version when I do "git 
> --version". I just hate how it normally just says it's version "1.0.GIT", 
> and I have no idea how new/old it really is.

Hmph.  I am not so interested in "git --version", but I find
git-describe to be pretty attractive.

Last night I proposed to tighten pack naming which would promote
so-far just a convention to a rule, which would introduce an
backward incompatibility and wanted to know which released
versions are affected.  With git-whatchanged I can identify the
exact commit that changed the packname SHA1 computation with
ease:

        $ git whatchanged --abbrev --pretty=oneline \
          -S'	sorted_by_sha = create_sorted_list(sha1_sort);
                SHA1_Init(&ctx);
                list = sorted_by_sha;
                for (i = 0; i < nr_objects; i++) {
        ' HEAD -- pack-objects.c
        diff-tree 84c8d8a... (from 9cf6d33...)
        Fix packname hash generation.
        :100644 100644 3d62278... ef55cab... M	pack-objects.c

and from that I would have been able to find that the change was
between 0.99.8 and 0.99.9:

        $ git describe 84c8d8a
        refs/tags/v0.99.8-g84c8d8ae

I used "git-show-branch $that_commit tags/v0.99.?" to get that
information instead, but git describe would have been very
useful.

^ permalink raw reply

* Re: Add a "git-describe" command
From: Linus Torvalds @ 2005-12-25  3:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4q4yq7f2.fsf@assigned-by-dhcp.cox.net>



On Sat, 24 Dec 2005, Junio C Hamano wrote:
>
> I wonder if some tags are more important than others (we may
> want to pick a signed tag that is a bit further down than an
> autogenerated snapshot -git47 tag), but that depends on the
> usage.  For bug hunting -git47 is more useful than sparsely done
> signed tags e.g. -rc4.

I considered just printing every tag I could reach, and then you could do 

	git-describe xyz | head -1

to get the closest one.

But then I decided that almost always, you just want the most recent one. 
But we could add a "--history" cmd line option to show all the reachable 
ones (or limit it to <n> tags shown or something)..

But there's already some logic to cull out uninteresting references (ie 
the "--all" flag enables all kinds of references, normally we only look at 
tags), and yes, we could also just add some other culling logic. For 
example, maybe we only care about annotated tags (ie real tag objects with 
a message, rather than just direct references to commits).

My real goal was to see a real meaningful version when I do "git 
--version". I just hate how it normally just says it's version "1.0.GIT", 
and I have no idea how new/old it really is.

			Linus

^ permalink raw reply

* Re: Possibility of a MinGW version?
From: Junio C Hamano @ 2005-12-25  1:47 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: git
In-Reply-To: <43ADDD6F.7010502@op5.se>

Andreas Ericsson <ae@op5.se> writes:

> Junio C Hamano wrote:
>> Andreas Ericsson <ae@op5.se> writes:
>>
>>> The fact that there are 39 bash'ish shell-scripts does little to
>>> help a native port...
>> Can you defend that "bash'ism" comment for all 39?  The one I
>> know of and would want to get rid of its bashism by rewriting is
>> git-grep, but most of them I thought was plain POSIX.
>
> Not really, no. I meant "there are 39 shell-scripts known to work with 
> bash but not necessarily known to fail with any other shell",...

I am not a Windows person (once I thought heard one particular
version of NT was POSIX compliant but I did not believe it), but
I suspect bash or not they may have trouble with POSIXism (iow
UNIX heritage), and if the port is done the right way by adding
compatibility layer they would end up reinventing Cygwin or
something close to it...

^ permalink raw reply

* Re: Add a "git-describe" command
From: Junio C Hamano @ 2005-12-25  1:42 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0512241339120.14098@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> What it does is to take any random commit, and "name" it by looking up the 
> most recent commit that is tagged and reachable from that commit.

Sounds useful.

> Comments?

> +static struct commit_name {
> +	const struct commit *commit;
> +	char path[];
> +} **name_array = NULL;

I wonder if some tags are more important than others (we may
want to pick a signed tag that is a bit further down than an
autogenerated snapshot -git47 tag), but that depends on the
usage.  For bug hunting -git47 is more useful than sparsely done
signed tags e.g. -rc4.

> +static void describe(struct commit *cmit)
> +...
> +	list = NULL;
> +	commit_list_insert(cmit, &list);
> +	while (list) {
> +		struct commit *c = pop_most_recent_commit(&list, SEEN);
> +		n = match(c);
> +		if (n) {
> +			printf("%s-g%.8s\n", n->path, sha1_to_hex(cmit->object.sha1));
> +			return;
> +		}
> +	}
> +}

I think this leaks list elements but until/unless this is
libified we would not care too much about it.  We could use
find_unique_abbrev(cmit->object.sha1, 8) but probably it is an
overkill.

^ permalink raw reply

* Re: Possibility of a MinGW version?
From: Andreas Ericsson @ 2005-12-24 23:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwthus7gr.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> Andreas Ericsson <ae@op5.se> writes:
> 
> 
>>The fact that there are 39 bash'ish shell-scripts does little to help a 
>>native port...
> 
> 
> Can you defend that "bash'ism" comment for all 39?  The one I
> know of and would want to get rid of its bashism by rewriting is
> git-grep, but most of them I thought was plain POSIX.
> 

Not really, no. I meant "there are 39 shell-scripts known to work with 
bash but not necessarily known to fail with any other shell", which 
usually means "it will break with some shell or other, although the 
shell that breaks is unusual enough for everyone not to have tested it 
yet". I'm probably being a bit super-anal and anti-MS biased.

I wrote the first mail being terribly hung over and this one being 
slightly tipsy. I now get the feeling that everything will work out for 
the best. Happy christmas everyone.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ 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