Re: auto-packing on kernel.org? please?

[Linus Torvalds <torvalds@osdl.org>]

On Mon, 21 Nov 2005, Junio C Hamano wrote:
> 
> One cop-out: do fsck-objects upfront before making a pack.  This
> would populate your buffer cache so it might not be a bad thing.

Well, it's extremely expensive most of the time. It's often as expensive 
as the packing itself. So I don't like that option very much.

> Alternatively:
> 
>         name=$( {
>                 git-rev-list --objects $rev_list $(git-rev-parse $rev_parse) ||
>                 echo Gaaahhh
>         } | git-pack-objects --non-empty $pack_objects .tmp-pack)

Actually, some dim memories prodded me to some man-page digging, and the 
"pipefail" option in particular.

It seems to be a common option to both ksh and bash, so

	set -o pipefail

seems like it should fix this. Sadly, I think it's pretty recent in bash 
(ksh apparently got it in -93, bash seems to have gotten it only as of 
version 3.0, which is definitely recent enough that we can't just assume 
it).

[ Also, bash seems to have a variable called $PIPESTATUS, but that's 
  bash-specific (I don't know when it was enabled). ]

Anyway, doing a

	set -o pipefail

should never be the wrong thing to do, but the problem is figuring out 
whether the option is available or not, since if it isn't available, it's 
considered an error ;/

So with all that, how about we take your "Gaah" idea, and simplify it: 
just pipe stderr too. That, together with making git-pack-objects tell 
what garbage it got, actually does the rigth thing:

	[torvalds@g5 git-clone]$ git repack -a -d
	fatal: expected sha1, got garbage:
	 error: Could not read 7f59dbbb8f8d479c1d31453eac06ec765436a780

with this pretty simple patch.

Whaddaya think?

			Linus

---
diff --git a/git-repack.sh b/git-repack.sh
index 4e16d34..c0f271d 100755
--- a/git-repack.sh
+++ b/git-repack.sh
@@ -41,7 +41,7 @@ esac
 if [ "$local" ]; then
 	pack_objects="$pack_objects --local"
 fi
-name=$(git-rev-list --objects $rev_list $(git-rev-parse $rev_parse) |
+name=$(git-rev-list --objects $rev_list $(git-rev-parse $rev_parse) 2>&1 |
 	git-pack-objects --non-empty $pack_objects .tmp-pack) ||
 	exit 1
 if [ -z "$name" ]; then
diff --git a/pack-objects.c b/pack-objects.c
index 4e941e7..8864a31 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -524,7 +524,7 @@ int main(int argc, char **argv)
 		unsigned char sha1[20];
 
 		if (get_sha1_hex(line, sha1))
-			die("expected sha1, got garbage");
+			die("expected sha1, got garbage:\n %s", line);
 		hash = 0;
 		p = line+40;
 		while (*p) {