Skip to contentSkip to conversation

Add --strict switch to diff-cache to force SHA1 checking

PATCH In review Jonas Fonseca 3 participants 4 messages Jun 15, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Add --strict switch to diff-cache to force SHA1 checking

It seems by far the easiest to let diff-cache take care of skipping files which have not been modified. The alternative is to keep diff-cache's lazy checking and make cogito's diff jump through hoops. Note, although the new SHA1 signature is derived, diff-cache still prints the special no-SHA1 for the modified file.
-

Add --strict switch to diff-cache to force it to check the SHA1 signature of modified files so files are only listed if the mode or content changed.

diff-cache.c (mode:100644) renamed from diff-cache.c (mode:100644 sha1:2ec6c29ab6b79a10277a2ff9021a2032d656abf0) +32 -1
@@ -1,6 +1,7 @@
 #include "cache.h"
 
 static int cached_only = 0;
+static int strict_checking = 0;
 static int line_termination = '\n';
 
 /* A file entry went away or appeared */
@@ -10,6 +11,26 @@
 	       sha1_to_hex(ce->sha1), ce->name, line_termination);
 }
 
+static int check_modified_signature(struct cache_entry *old, struct stat *st)
+{
+	void *map;
+	int ret = -1;
+	int fd = open(old->name, O_RDONLY);
+
+	if (fd < 0)
+		return -1;
+
+	map = mmap(NULL, st->st_size, PROT_READ, MAP_PRIVATE, fd, 0);
+	if (map != MAP_FAILED) {
+		ret = check_sha1_signature(old->sha1, map, st->st_size, "blob");
+		munmap(map, st->st_size);
+	}
+
+	close(fd);
+
+	return ret;
+}
+
 static int show_modified(struct cache_entry *old, struct cache_entry *new)
 {
 	unsigned int mode = ntohl(new->ce_mode), oldmode;
@@ -27,6 +48,12 @@
 		changed = cache_match_stat(new, &st);
 		if (changed) {
 			mode = st.st_mode;
+
+			if (strict_checking
+			    && mode == ntohl(old->ce_mode)
+			    && check_modified_signature(old, &st) == 0)
+				return 0;
+
 			sha1 = no_sha1;
 		}
 	}
@@ -85,7 +112,7 @@
 	}
 }
 
-static char *diff_cache_usage = "diff-cache [-r] [-z] [--cached] <tree sha1>";
+static char *diff_cache_usage = "diff-cache [-r] [-z] [--cached] [--strict] <tree sha1>";
 
 int main(int argc, char **argv)
 {
@@ -110,6 +137,10 @@
 			cached_only = 1;
 			continue;
 		}
+		if (!strcmp(arg, "--strict")) {
+			strict_checking = 1;
+			continue;
+		}
 		usage(diff_cache_usage);
 	}
 
Signature · 1 line
Jonas Fonseca
#1 lore raw

PATCH 1/1 Use diff-cache --strict in gitdiff.sh

Use diff-cache --strict invokation to only show diff of changed files.

gitdiff.sh (mode:100755) renamed from gitdiff.sh (mode:100755 sha1:1478a0cfb23a85c259eae1a36cf2a48597fbe8a2) +1 -1
@@ -76,7 +76,7 @@
 	# FIXME: Update ret based on what did we match. And take "$@"
 	# to account after all.
 	ret=
-	diff-cache -r -z $tree | xargs -0 gitdiff-do "$tree" uncommitted "$filter"
+	diff-cache -r -z --strict $tree | xargs -0 gitdiff-do "$tree" uncommitted "$filter"
 
 	if [ "$id1" != " " ]; then
 		rm $GIT_INDEX_FILE
Signature · 1 line
Jonas Fonseca
#2 lore raw
In reply to Jonas Fonseca
Quoted 6 lines · Jonas Fonseca

It seems by far the easiest to let diff-cache take care of skipping files which have not been modified. The alternative is to keep diff-cache's lazy checking and make cogito's diff jump through hoops. Note, although the new SHA1 signature is derived, diff-cache still prints the special no-SHA1 for the modified file.

The reason I don't want this is that if the commands keep on silently fixing things like this up, then performance will go down the toilet.

What _should_ happen is that you do an "update-cache --refresh" before doing the diff-cache. That way you do _not_ end up having to check the sha1 match over and over again if you just did a "touch" on the file.

In other words, you're adding bandage for a problem that shouldn't exist, and you're doing it in a way which means that _if_ the problem exists, you'll never fix it, but you'll just rely on your bandage all the time.

So cogito definitely does not have to jump through any hoops at all. It should just make sure to keep the cache up-to-date if it ever cares (ie do "update-cache --refresh").

Some commands obviously won't care.

Linus

#3 lore raw

PATCH 1/1 fix segfault in fsck-cache (2nd attempt)

Here is how to trigger it:

echo blob 100 > .git/objects/00/ae4e8d3208e09f2cf7a38202a126f728cadb49

Then run fsck-cache. It will try to unpack after the header to calculate the hash, inflate returns total_out == 0 and memcpy() dies.

The patch below seems to work with ZLIB 1.1 and 1.2.

sha1_file.c +2 -0
@@ -155,6 +155,8 @@
 
        inflateInit(&stream);
        ret = inflate(&stream, 0);
+       if (ret < Z_OK)
+               return NULL;
        if (sscanf(buffer, "%10s %lu", type, size) != 2)
                return NULL;
 
Quoted 33 lines · Linus Torvalds
Quoted 6 lines · Jonas Fonseca

It seems by far the easiest to let diff-cache take care of skipping files which have not been modified. The alternative is to keep diff-cache's lazy checking and make cogito's diff jump through hoops. Note, although the new SHA1 signature is derived, diff-cache still prints the special no-SHA1 for the modified file.

The reason I don't want this is that if the commands keep on silently fixing things like this up, then performance will go down the toilet.

What _should_ happen is that you do an "update-cache --refresh" before doing the diff-cache. That way you do _not_ end up having to check the sha1 match over and over again if you just did a "touch" on the file.

In other words, you're adding bandage for a problem that shouldn't exist, and you're doing it in a way which means that _if_ the problem exists, you'll never fix it, but you'll just rely on your bandage all the time.

So cogito definitely does not have to jump through any hoops at all. It should just make sure to keep the cache up-to-date if it ever cares (ie do "update-cache --refresh").

Some commands obviously won't care.

Linus
-
To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html