Merge branch 'jc/index-pack'

* jc/index-pack:
  verify-pack: use index-pack --verify
  index-pack: show histogram when emulating "verify-pack -v"
  index-pack: start learning to emulate "verify-pack -v"
  index-pack: a miniscule refactor
  index-pack --verify: read anomalous offsets from v2 idx file
  write_idx_file: need_large_offset() helper function
  index-pack: --verify
  write_idx_file: introduce a struct to hold idx customization options
  index-pack: group the delta-base array entries also by type

Conflicts:
	builtin/verify-pack.c
	cache.h
	sha1_file.c

Author: gitster

builtin/index-pack.c

@@ -70,6 +70,7 @@ static int local;
 static int incremental;
 static int ignore_packed_keep;
 static int allow_ofs_delta;
+static struct pack_idx_option pack_idx_opts;
 static const char *base_name;
 static int progress = 1;
 static int window = 10;
@@ -493,8 +494,8 @@ static void write_pack_file(void)
 			const char *idx_tmp_name;
 			char tmpname[PATH_MAX];
 
-			idx_tmp_name = write_idx_file(NULL, written_list,
-						      nr_written, sha1);
+			idx_tmp_name = write_idx_file(NULL, written_list, nr_written,
+						      &pack_idx_opts, sha1);
 
 			snprintf(tmpname, sizeof(tmpname), "%s-%s.pack",
 				 base_name, sha1_to_hex(sha1));
@@ -1884,10 +1885,10 @@ static int git_pack_config(const char *k, const char *v, void *cb)
 		return 0;
 	}
 	if (!strcmp(k, "pack.indexversion")) {
-		pack_idx_default_version = git_config_int(k, v);
-		if (pack_idx_default_version > 2)
+		pack_idx_opts.version = git_config_int(k, v);
+		if (pack_idx_opts.version > 2)
 			die("bad pack.indexversion=%"PRIu32,
-				pack_idx_default_version);
+			    pack_idx_opts.version);
 		return 0;
 	}
 	if (!strcmp(k, "pack.packsizelimit")) {
@@ -2134,6 +2135,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 	rp_av[1] = "--objects"; /* --thin will make it --objects-edge */
 	rp_ac = 2;
 
+	reset_pack_idx_option(&pack_idx_opts);
 	git_config(git_pack_config, NULL);
 	if (!pack_compression_seen && core_compression_seen)
 		pack_compression_level = core_compression_level;
@@ -2278,12 +2280,12 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 		}
 		if (!prefixcmp(arg, "--index-version=")) {
 			char *c;
-			pack_idx_default_version = strtoul(arg + 16, &c, 10);
-			if (pack_idx_default_version > 2)
+			pack_idx_opts.version = strtoul(arg + 16, &c, 10);
+			if (pack_idx_opts.version > 2)
 				die("bad %s", arg);
 			if (*c == ',')
-				pack_idx_off32_limit = strtoul(c+1, &c, 0);
-			if (*c || pack_idx_off32_limit & 0x80000000)
+				pack_idx_opts.off32_limit = strtoul(c+1, &c, 0);
+			if (*c || pack_idx_opts.off32_limit & 0x80000000)
 				die("bad %s", arg);
 			continue;
 		}

builtin/pack-objects.c

@@ -1,134 +1,53 @@
 #include "builtin.h"
 #include "cache.h"
-#include "pack.h"
-#include "pack-revindex.h"
+#include "run-command.h"
 #include "parse-options.h"
 
-#define MAX_CHAIN 50
-
 #define VERIFY_PACK_VERBOSE 01
 #define VERIFY_PACK_STAT_ONLY 02
 
-static void show_pack_info(struct packed_git *p, unsigned int flags)
-{
-	uint32_t nr_objects, i;
-	int cnt;
-	int stat_only = flags & VERIFY_PACK_STAT_ONLY;
-	unsigned long chain_histogram[MAX_CHAIN+1], baseobjects;
-
-	nr_objects = p->num_objects;
-	memset(chain_histogram, 0, sizeof(chain_histogram));
-	baseobjects = 0;
-
-	for (i = 0; i < nr_objects; i++) {
-		const unsigned char *sha1;
-		unsigned char base_sha1[20];
-		const char *type;
-		unsigned long size;
-		unsigned long store_size;
-		off_t offset;
-		unsigned int delta_chain_length;
-
-		sha1 = nth_packed_object_sha1(p, i);
-		if (!sha1)
-			die("internal error pack-check nth-packed-object");
-		offset = nth_packed_object_offset(p, i);
-		type = typename(packed_object_info_detail(p, offset, &size, &store_size,
-						 &delta_chain_length,
-						 base_sha1));
-		if (!stat_only)
-			printf("%s ", sha1_to_hex(sha1));
-		if (!delta_chain_length) {
-			if (!stat_only)
-				printf("%-6s %lu %lu %"PRIuMAX"\n",
-				       type, size, store_size, (uintmax_t)offset);
-			baseobjects++;
-		}
-		else {
-			if (!stat_only)
-				printf("%-6s %lu %lu %"PRIuMAX" %u %s\n",
-				       type, size, store_size, (uintmax_t)offset,
-				       delta_chain_length, sha1_to_hex(base_sha1));
-			if (delta_chain_length <= MAX_CHAIN)
-				chain_histogram[delta_chain_length]++;
-			else
-				chain_histogram[0]++;
-		}
-	}
-
-	if (baseobjects)
-		printf("non delta: %lu object%s\n",
-		       baseobjects, baseobjects > 1 ? "s" : "");
-
-	for (cnt = 1; cnt <= MAX_CHAIN; cnt++) {
-		if (!chain_histogram[cnt])
-			continue;
-		printf("chain length = %d: %lu object%s\n", cnt,
-		       chain_histogram[cnt],
-		       chain_histogram[cnt] > 1 ? "s" : "");
-	}
-	if (chain_histogram[0])
-		printf("chain length > %d: %lu object%s\n", MAX_CHAIN,
-		       chain_histogram[0],
-		       chain_histogram[0] > 1 ? "s" : "");
-}
-
 static int verify_one_pack(const char *path, unsigned int flags)
 {
-	char arg[PATH_MAX];
-	int len;
+	struct child_process index_pack;
+	const char *argv[] = {"index-pack", NULL, NULL, NULL };
+	struct strbuf arg = STRBUF_INIT;
 	int verbose = flags & VERIFY_PACK_VERBOSE;
 	int stat_only = flags & VERIFY_PACK_STAT_ONLY;
-	struct packed_git *pack;
 	int err;
 
-	len = strlcpy(arg, path, PATH_MAX);
-	if (len >= PATH_MAX)
-		return error("name too long: %s", path);
-
-	/*
-	 * In addition to "foo.idx" we accept "foo.pack" and "foo";
-	 * normalize these forms to "foo.idx" for add_packed_git().
-	 */
-	if (has_extension(arg, ".pack")) {
-		strcpy(arg + len - 5, ".idx");
-		len--;
-	} else if (!has_extension(arg, ".idx")) {
-		if (len + 4 >= PATH_MAX)
-			return error("name too long: %s.idx", arg);
-		strcpy(arg + len, ".idx");
-		len += 4;
-	}
+	if (stat_only)
+		argv[1] = "--verify-stat-only";
+	else if (verbose)
+		argv[1] = "--verify-stat";
+	else
+		argv[1] = "--verify";
 
 	/*
-	 * add_packed_git() uses our buffer (containing "foo.idx") to
-	 * build the pack filename ("foo.pack").  Make sure it fits.
+	 * In addition to "foo.pack" we accept "foo.idx" and "foo";
+	 * normalize these forms to "foo.pack" for "index-pack --verify".
 	 */
-	if (len + 1 >= PATH_MAX) {
-		arg[len - 4] = '\0';
-		return error("name too long: %s.pack", arg);
-	}
-
-	pack = add_packed_git(arg, len, 1);
-	if (!pack)
-		return error("packfile %s not found.", arg);
+	strbuf_addstr(&arg, path);
+	if (has_extension(arg.buf, ".idx"))
+		strbuf_splice(&arg, arg.len - 3, 3, "pack", 4);
+	else if (!has_extension(arg.buf, ".pack"))
+		strbuf_add(&arg, ".pack", 5);
+	argv[2] = arg.buf;
 
-	install_packed_git(pack);
+	memset(&index_pack, 0, sizeof(index_pack));
+	index_pack.argv = argv;
+	index_pack.git_cmd = 1;
 
-	if (!stat_only)
-		err = verify_pack(pack);
-	else
-		err = open_pack_index(pack);
+	err = run_command(&index_pack);
 
 	if (verbose || stat_only) {
 		if (err)
-			printf("%s: bad\n", pack->pack_name);
+			printf("%s: bad\n", arg.buf);
 		else {
-			show_pack_info(pack, flags);
 			if (!stat_only)
-				printf("%s: ok\n", pack->pack_name);
+				printf("%s: ok\n", arg.buf);
 		}
 	}
+	strbuf_release(&arg);
 
 	return err;
 }
@@ -159,7 +78,6 @@ int cmd_verify_pack(int argc, const char **argv, const char *prefix)
 	for (i = 0; i < argc; i++) {
 		if (verify_one_pack(argv[i], flags))
 			err = 1;
-		discard_revindex();
 	}
 
 	return err;

builtin/verify-pack.c

@@ -1010,7 +1010,6 @@ extern off_t find_pack_entry_one(const unsigned char *, struct packed_git *);
 extern void *unpack_entry(struct packed_git *, off_t, enum object_type *, unsigned long *);
 extern unsigned long unpack_object_header_buffer(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep);
 extern unsigned long get_size_from_delta(struct packed_git *, struct pack_window **, off_t);
-extern int packed_object_info_detail(struct packed_git *, off_t, unsigned long *, unsigned long *, unsigned int *, unsigned char *);
 extern int unpack_object_header(struct packed_git *, struct pack_window **, off_t *, unsigned long *);
 
 struct object_info {

cache.h

@@ -11,8 +11,20 @@
 #include "progress.h"
 #include "csum-file.h"
 
-static void flush(struct sha1file *f, void * buf, unsigned int count)
+static void flush(struct sha1file *f, void *buf, unsigned int count)
 {
+	if (0 <= f->check_fd && count)  {
+		unsigned char check_buffer[8192];
+		ssize_t ret = read_in_full(f->check_fd, check_buffer, count);
+
+		if (ret < 0)
+			die_errno("%s: sha1 file read error", f->name);
+		if (ret < count)
+			die("%s: sha1 file truncated", f->name);
+		if (memcmp(buf, check_buffer, count))
+			die("sha1 file '%s' validation error", f->name);
+	}
+
 	for (;;) {
 		int ret = xwrite(f->fd, buf, count);
 		if (ret > 0) {
@@ -59,6 +71,17 @@ int sha1close(struct sha1file *f, unsigned char *result, unsigned int flags)
 		fd = 0;
 	} else
 		fd = f->fd;
+	if (0 <= f->check_fd) {
+		char discard;
+		int cnt = read_in_full(f->check_fd, &discard, 1);
+		if (cnt < 0)
+			die_errno("%s: error when reading the tail of sha1 file",
+				  f->name);
+		if (cnt)
+			die("%s: sha1 file has trailing garbage", f->name);
+		if (close(f->check_fd))
+			die_errno("%s: sha1 file error on close", f->name);
+	}
 	free(f);
 	return fd;
 }
@@ -101,10 +124,31 @@ struct sha1file *sha1fd(int fd, const char *name)
 	return sha1fd_throughput(fd, name, NULL);
 }
 
+struct sha1file *sha1fd_check(const char *name)
+{
+	int sink, check;
+	struct sha1file *f;
+
+	sink = open("/dev/null", O_WRONLY);
+	if (sink < 0)
+		return NULL;
+	check = open(name, O_RDONLY);
+	if (check < 0) {
+		int saved_errno = errno;
+		close(sink);
+		errno = saved_errno;
+		return NULL;
+	}
+	f = sha1fd(sink, name);
+	f->check_fd = check;
+	return f;
+}
+
 struct sha1file *sha1fd_throughput(int fd, const char *name, struct progress *tp)
 {
 	struct sha1file *f = xmalloc(sizeof(*f));
 	f->fd = fd;
+	f->check_fd = -1;
 	f->offset = 0;
 	f->total = 0;
 	f->tp = tp;

csum-file.c

@@ -6,6 +6,7 @@ struct progress;
 /* A SHA1-protected file */
 struct sha1file {
 	int fd;
+	int check_fd;
 	unsigned int offset;
 	git_SHA_CTX ctx;
 	off_t total;
@@ -21,6 +22,7 @@ struct sha1file {
 #define CSUM_FSYNC	2
 
 extern struct sha1file *sha1fd(int fd, const char *name);
+extern struct sha1file *sha1fd_check(const char *name);
 extern struct sha1file *sha1fd_throughput(int fd, const char *name, struct progress *tp);
 extern int sha1close(struct sha1file *, unsigned char *, unsigned int);
 extern int sha1write(struct sha1file *, void *, unsigned int);

csum-file.h

@@ -304,6 +304,7 @@ static unsigned int atom_cnt;
 static struct atom_str **atom_table;
 
 /* The .pack file being generated */
+static struct pack_idx_option pack_idx_opts;
 static unsigned int pack_id;
 static struct sha1file *pack_file;
 static struct packed_git *pack_data;
@@ -896,7 +897,7 @@ static const char *create_index(void)
 	if (c != last)
 		die("internal consistency error creating the index");
 
-	tmpfile = write_idx_file(NULL, idx, object_count, pack_data->sha1);
+	tmpfile = write_idx_file(NULL, idx, object_count, &pack_idx_opts, pack_data->sha1);
 	free(idx);
 	return tmpfile;
 }
@@ -3195,10 +3196,10 @@ static int git_pack_config(const char *k, const char *v, void *cb)
 		return 0;
 	}
 	if (!strcmp(k, "pack.indexversion")) {
-		pack_idx_default_version = git_config_int(k, v);
-		if (pack_idx_default_version > 2)
+		pack_idx_opts.version = git_config_int(k, v);
+		if (pack_idx_opts.version > 2)
 			die("bad pack.indexversion=%"PRIu32,
-			    pack_idx_default_version);
+			    pack_idx_opts.version);
 		return 0;
 	}
 	if (!strcmp(k, "pack.packsizelimit")) {
@@ -3252,6 +3253,7 @@ int main(int argc, const char **argv)
 		usage(fast_import_usage);
 
 	setup_git_directory();
+	reset_pack_idx_option(&pack_idx_opts);
 	git_config(git_pack_config, NULL);
 	if (!pack_compression_seen && core_compression_seen)
 		pack_compression_level = core_compression_level;