Re: [ANNOUNCE] GIT 1.4.3

From: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
Date: Oct 21, 2006, 14:29 UTC
Message-ID: <453A2ED2.1000709@lsrfire.ath.cx>
In-Reply-To: <20061021021235.GA29920@ftp.linux.org.uk>

Al Viro schrieb:

> Speaking of irritations...  There is a major (and AFAICS fixable)
> suckitude in git-cherry.

[...]

> For one thing, there are better ways to do set comparison than creating
> a file for each element in one set and going through another checking
> if corresponding files exist (join(1) and sort(1) or just use perl hashes).

[...]

> Note that we are calculating a function of commit; it _never_ changes.
> Even if we don't just calculate and memorize it at commit time, a cache
> somewhere under .git would speed the things up a lot...

How about this patch? It does away with using temporary files and instead creates persistent cache files under .git/patch-ids/. It is a very stupid cache layout: file name = commit SHA1, file contents = patch ID. Perhaps it needs fan-out directories like .git/objects/ has before it can be considered for merge.

The set compare is stupid, too, but at least it is in-shell now, using a space separated list and the is_in function.

And the cache file creation is not safe for multiple parallel git-cherry's.

It survives "make test" and is otherwise untested. Care to test drive this prototype? :-D

Thanks, René

diff --git a/git-cherry.sh b/git-cherry.sh
index 8832573..c88afc3 100755
--- a/git-cherry.sh
+++ b/git-cherry.sh
@@ -46,18 +46,29 @@ # not that the order in inup matters...
 inup=`git-rev-list ^$ours $upstream` &&
 ours=`git-rev-list $ours ^$limit` || exit
 
-tmp=.cherry-tmp$$
-patch=$tmp-patch
-mkdir $patch
-trap "rm -rf $tmp-*" 0 1 2 3 15
+is_in() {
+	what="$1"
+	while [ $# -gt 1 ]; do
+		shift
+		[ "$what" = "$1" ] && return 0
+	done
+	return 1
+}
 
+# prime patch-ID cache
+PATCH_ID_CACHE="$GIT_DIR/patch-ids"
+mkdir -p "$PATCH_ID_CACHE"
+for commit in $inup $ours; do
+	[ -f "$PATCH_ID_CACHE/$commit" ] && continue
+	set x `git-diff-tree -p $commit | git-patch-id`
+	echo "$2" >"$PATCH_ID_CACHE/$commit"
+done
+
+ids_inup=
 for c in $inup
 do
-	git-diff-tree -p $c
-done | git-patch-id |
-while read id name
-do
-	echo $name >>$patch/$id
+	read id <"$PATCH_ID_CACHE/$c"
+	ids_inup="$ids_inup $id"
 done
 
 LF='
@@ -66,10 +77,10 @@ LF='
 O=
 for c in $ours
 do
-	set x `git-diff-tree -p $c | git-patch-id`
-	if test "$2" != ""
+	read id <"$PATCH_ID_CACHE/$c"
+	if test "$id" != ""
 	then
-		if test -f "$patch/$2"
+		if is_in $id $ids_inup
 		then
 			sign=-
 		else