Git development
 help / color / mirror / Atom feed
* [PATCH 28/32] builtin/index-pack:  Add a simple oid index
From: Eric W. Biederman @ 2023-09-08 23:10 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, brian m. carlson, Eric W. Biederman
In-Reply-To: <87sf7ol0z3.fsf@email.froward.int.ebiederm.org>

To support computing the compatibility hash a way to lookup objects by
their oid is needed.  This adds a simple hash table to enable looking
up objects by their oid.  The implementation is inspired by the hash
table for looking up object_entries by their oid in struct packing_data,
and implemented in pack-objects.c

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 builtin/index-pack.c | 68 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 67 insertions(+), 1 deletion(-)

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 006ffdc9c550..75c2113e455c 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -126,6 +126,9 @@ static int ref_deltas_alloc;
 static int nr_resolved_deltas;
 static int nr_threads;
 
+static int32_t *oid_index;
+static uint32_t oid_index_size;
+
 static int from_stdin;
 static int strict;
 static int do_fsck_object;
@@ -183,6 +186,62 @@ static inline void unlock_mutex(pthread_mutex_t *mutex)
 		pthread_mutex_unlock(mutex);
 }
 
+static uint32_t locate_oid_index(const struct object_id *oid, int *found)
+{
+	uint32_t i, mask = (oid_index_size - 1);
+
+	i = oidhash(oid) & mask;
+
+	while (oid_index[i] > 0) {
+		uint32_t pos = oid_index[i] - 1;
+
+		if (oideq(oid, &objects[pos].idx.oid)) {
+			*found = 1;
+			return i;
+		}
+
+		i = (i + 1) & mask;
+	}
+
+	*found = 0;
+	return i;
+}
+
+static void place_in_oid_index(struct object_entry *obj)
+{
+	int found;
+	uint32_t pos = locate_oid_index(&obj->idx.oid, &found);
+
+	/* Ignore duplicates */
+	if (found)
+		return;
+
+	oid_index[pos] = (obj - objects) + 1;
+}
+
+static struct object_entry *find_in_oid_index(struct object_id *oid)
+{
+	uint32_t i;
+	int found;
+
+	i = locate_oid_index(oid, &found);
+	if (!found)
+		return NULL;
+
+	return &objects[oid_index[i] - 1];
+}
+
+static inline uint32_t closest_pow2(uint32_t v)
+{
+	v = v - 1;
+	v |= v >> 1;
+	v |= v >> 2;
+	v |= v >> 4;
+	v |= v >> 8;
+	v |= v >> 16;
+	return v + 1;
+}
+
 /*
  * Mutex and conditional variable can't be statically-initialized on Windows.
  */
@@ -987,6 +1046,7 @@ static struct base_data *resolve_delta(struct object_entry *delta_obj,
 		bad_object(delta_obj->idx.offset, _("failed to apply delta"));
 	hash_object_file(the_hash_algo, result_data, result_size,
 			 delta_obj->real_type, &delta_obj->idx.oid);
+	place_in_oid_index(delta_obj);
 	sha1_object(result_data, NULL, result_size, delta_obj->real_type,
 		    &delta_obj->idx.oid);
 
@@ -1188,12 +1248,16 @@ static void parse_pack_objects(unsigned char *hash)
 			ref_deltas[nr_ref_deltas].obj_no = i;
 			nr_ref_deltas++;
 		} else if (!data) {
+			place_in_oid_index(obj);
+
 			/* large blobs, check later */
 			obj->real_type = OBJ_BAD;
 			nr_delays++;
-		} else
+		} else {
+			place_in_oid_index(obj);
 			sha1_object(data, NULL, obj->size, obj->type,
 				    &obj->idx.oid);
+		}
 		free(data);
 		display_progress(progress, i+1);
 	}
@@ -1918,6 +1982,8 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
 	if (show_stat)
 		CALLOC_ARRAY(obj_stat, st_add(nr_objects, 1));
 	CALLOC_ARRAY(ofs_deltas, nr_objects);
+	oid_index_size = closest_pow2(nr_objects * 3);
+	CALLOC_ARRAY(oid_index, oid_index_size);
 	parse_pack_objects(pack_hash);
 	if (report_end_of_input)
 		write_in_full(2, "\0", 1);
-- 
2.41.0


^ permalink raw reply related

* [PATCH 25/32] pack-compat-map:  Add support for .compat files of a packfile
From: Eric W. Biederman @ 2023-09-08 23:10 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, brian m. carlson, Eric W. Biederman
In-Reply-To: <87sf7ol0z3.fsf@email.froward.int.ebiederm.org>

These .compat files hold a bidirectional mapping between the names of
stored objects between sha1 and sha256.

Care has been taken so that index-pack --verify can be supported to
validate an existing compat map file is not currupted.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 Makefile                  |   2 +
 builtin.h                 |   1 +
 builtin/show-compat-map.c | 139 ++++++++++++++++
 git.c                     |   1 +
 object-file-convert.c     |   7 +
 object-name.c             |  18 ++
 object-store-ll.h         |   6 +
 pack-compat-map.c         | 334 ++++++++++++++++++++++++++++++++++++++
 pack-compat-map.h         |  27 +++
 pack-write.c              | 158 ++++++++++++++++++
 packfile.c                |  12 ++
 11 files changed, 705 insertions(+)
 create mode 100644 builtin/show-compat-map.c
 create mode 100644 pack-compat-map.c
 create mode 100644 pack-compat-map.h

diff --git a/Makefile b/Makefile
index 3c18664def9a..b3f3dbe7bfeb 100644
--- a/Makefile
+++ b/Makefile
@@ -1088,6 +1088,7 @@ LIB_OBJS += pack-check.o
 LIB_OBJS += pack-mtimes.o
 LIB_OBJS += pack-objects.o
 LIB_OBJS += pack-revindex.o
+LIB_OBJS += pack-compat-map.o
 LIB_OBJS += pack-write.o
 LIB_OBJS += packfile.o
 LIB_OBJS += pager.o
@@ -1299,6 +1300,7 @@ BUILTIN_OBJS += builtin/send-pack.o
 BUILTIN_OBJS += builtin/shortlog.o
 BUILTIN_OBJS += builtin/show-branch.o
 BUILTIN_OBJS += builtin/show-index.o
+BUILTIN_OBJS += builtin/show-compat-map.o
 BUILTIN_OBJS += builtin/show-ref.o
 BUILTIN_OBJS += builtin/sparse-checkout.o
 BUILTIN_OBJS += builtin/stash.o
diff --git a/builtin.h b/builtin.h
index d560baa6618a..25882d281dd2 100644
--- a/builtin.h
+++ b/builtin.h
@@ -223,6 +223,7 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix);
 int cmd_show(int argc, const char **argv, const char *prefix);
 int cmd_show_branch(int argc, const char **argv, const char *prefix);
 int cmd_show_index(int argc, const char **argv, const char *prefix);
+int cmd_show_compat_map(int argc, const char **argv, const char *prefix);
 int cmd_sparse_checkout(int argc, const char **argv, const char *prefix);
 int cmd_status(int argc, const char **argv, const char *prefix);
 int cmd_stash(int argc, const char **argv, const char *prefix);
diff --git a/builtin/show-compat-map.c b/builtin/show-compat-map.c
new file mode 100644
index 000000000000..8cc10bdaab61
--- /dev/null
+++ b/builtin/show-compat-map.c
@@ -0,0 +1,139 @@
+#include "builtin.h"
+#include "gettext.h"
+#include "hash.h"
+#include "hex.h"
+#include "pack.h"
+#include "parse-options.h"
+#include "repository.h"
+
+static const char *const show_compat_map_usage[] = {
+	"git show-compat-map [--verbose] ",
+	NULL
+};
+
+struct pack_compat_map_header {
+	uint8_t sig[4];
+	uint8_t version;
+	uint8_t first_oid_version;
+	uint8_t second_oid_version;
+	uint8_t mbz1;
+	uint32_t nr_objects;
+	uint8_t first_abbrev_len;
+	uint8_t mbz2;
+	uint8_t second_abbrev_len;
+	uint8_t mbz3;
+};
+
+struct map_entry {
+	struct object_id oid;
+	uint32_t index;
+};
+
+static const struct git_hash_algo *from_oid_version(unsigned oid_version)
+{
+	if (oid_version == 1) {
+		return &hash_algos[GIT_HASH_SHA1];
+	} else if (oid_version == 2) {
+		return &hash_algos[GIT_HASH_SHA256];
+	}
+	die("unknown oid version %u\n", oid_version);
+}
+
+static void read_half_map(struct map_entry *map, unsigned nr,
+		     const struct git_hash_algo *algo)
+{
+	unsigned i;
+	for (i = 0; i < nr; i++) {
+		uint32_t index;
+		if (fread(map[i].oid.hash, algo->rawsz, 1, stdin) != 1)
+			die("unable to read hash of %s entry %u/%u",
+			    algo->name, i, nr);
+		if (fread(&index, 4, 1, stdin) != 1)
+			die("unable to read index of %s entry %u/%u",
+			    algo->name, i, nr);
+		map[i].oid.algo = hash_algo_by_ptr(algo);
+		map[i].index = ntohl(index);
+	}
+}
+
+static void print_half_map(const struct map_entry *map,
+			   unsigned nr)
+{
+	unsigned i;
+	for (i = 0; i < nr; i++) {
+		printf("%s %"PRIu32"\n",
+		       oid_to_hex(&map[i].oid),
+		       map[i].index);
+	}
+}
+
+static void print_map(const struct map_entry *map,
+		      const struct map_entry *compat_map,
+		      unsigned nr)
+{
+	unsigned i;
+	for (i = 0; i < nr; i++) {
+		printf("%s ",
+		       oid_to_hex(&map[i].oid));
+		printf("%s\n",
+		       oid_to_hex(&compat_map[map[i].index].oid));
+	}
+}
+
+int cmd_show_compat_map(int argc, const char **argv, const char *prefix)
+{
+	const struct git_hash_algo *algo = NULL, *compat = NULL;
+	unsigned nr;
+	struct pack_compat_map_header hdr;
+	struct map_entry *map, *compat_map;
+	int verbose = 0;
+	const struct option show_comapt_map_options[] = {
+		OPT_BOOL(0, "verbose", &verbose,
+			 N_("print implementation details of the map file")),
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, prefix, show_comapt_map_options,
+			     show_compat_map_usage, 0);
+
+	if (fread(&hdr, sizeof(hdr), 1, stdin) != 1)
+		die("unable to read header");
+	if ((hdr.sig[0] != 'C') ||
+	    (hdr.sig[1] != 'M') ||
+	    (hdr.sig[2] != 'A') ||
+	    (hdr.sig[3] != 'P'))
+		die("Missing map signature");
+	if (hdr.version != 1)
+		die("Unknown map version");
+	if ((hdr.mbz1 != 0) ||
+	    (hdr.mbz2 != 0) ||
+	    (hdr.mbz3 != 0))
+		die("Must be zero fields non-zero");
+
+	nr = ntohl(hdr.nr_objects);
+
+	algo = from_oid_version(hdr.first_oid_version);
+	compat = from_oid_version(hdr.second_oid_version);
+
+
+	if (verbose) {
+		printf("Map v%u for %u objects from %s to %s abbrevs (%u:%u)\n",
+		       hdr.version,
+		       nr,
+		       algo->name, compat->name,
+		       hdr.first_abbrev_len,
+		       hdr.second_abbrev_len);
+	}
+	ALLOC_ARRAY(map, nr);
+	ALLOC_ARRAY(compat_map, nr);
+	read_half_map(map, nr, algo);
+	read_half_map(compat_map, nr, compat);
+	if (verbose) {
+		print_half_map(map, nr);
+		print_half_map(compat_map, nr);
+	}
+	print_map(map, compat_map, nr);
+	free(compat_map);
+	free(map);
+	return 0;
+}
diff --git a/git.c b/git.c
index c67e44dd82d2..bfaeece5ae0e 100644
--- a/git.c
+++ b/git.c
@@ -606,6 +606,7 @@ static struct cmd_struct commands[] = {
 	{ "show", cmd_show, RUN_SETUP },
 	{ "show-branch", cmd_show_branch, RUN_SETUP },
 	{ "show-index", cmd_show_index, RUN_SETUP_GENTLY },
+	{ "show-compat-map", cmd_show_compat_map, RUN_SETUP_GENTLY },
 	{ "show-ref", cmd_show_ref, RUN_SETUP },
 	{ "sparse-checkout", cmd_sparse_checkout, RUN_SETUP },
 	{ "stage", cmd_add, RUN_SETUP | NEED_WORK_TREE },
diff --git a/object-file-convert.c b/object-file-convert.c
index d381d3d2ea65..7978aa63dfa9 100644
--- a/object-file-convert.c
+++ b/object-file-convert.c
@@ -9,6 +9,7 @@
 #include "loose.h"
 #include "commit.h"
 #include "gpg-interface.h"
+#include "pack-compat-map.h"
 #include "object-file-convert.h"
 
 int repo_oid_to_algop(struct repository *repo, const struct object_id *src,
@@ -27,6 +28,12 @@ int repo_oid_to_algop(struct repository *repo, const struct object_id *src,
 		return 0;
 	}
 	if (repo_loose_object_map_oid(repo, dest, to, src)) {
+		/*
+		 * It's not in the loose object map, so let's see if it's in a
+		 * pack.
+		 */
+		if (!repo_packed_oid_to_algop(repo, src, to, dest))
+			return 0;
 		/*
 		 * We may have loaded the object map at repo initialization but
 		 * another process (perhaps upstream of a pipe from us) may have
diff --git a/object-name.c b/object-name.c
index ebe87f5c4fdd..d33c82bc96ba 100644
--- a/object-name.c
+++ b/object-name.c
@@ -26,6 +26,7 @@
 #include "commit-reach.h"
 #include "date.h"
 #include "object-file-convert.h"
+#include "pack-compat-map.h"
 
 static int get_oid_oneline(struct repository *r, const char *, struct object_id *, struct commit_list *);
 
@@ -210,6 +211,19 @@ static void find_short_packed_object(struct disambiguate_state *ds)
 		unique_in_pack(p, ds);
 }
 
+static void find_short_packed_compat_object(struct disambiguate_state *ds)
+{
+	struct packed_git *p;
+
+	/* Skip, unless compatibility oids are wanted */
+	if (!ds->algo && (&hash_algos[ds->algo] != ds->repo->compat_hash_algo))
+		return;
+
+	for (p = get_packed_git(ds->repo); p && !ds->ambiguous; p = p->next)
+		pack_compat_map_each(ds->repo, p, ds->bin_pfx.hash, ds->len,
+				     match_prefix, ds);
+}
+
 static int finish_object_disambiguation(struct disambiguate_state *ds,
 					struct object_id *oid)
 {
@@ -581,6 +595,7 @@ static enum get_oid_result get_short_oid(struct repository *r,
 
 	find_short_object_filename(&ds);
 	find_short_packed_object(&ds);
+	find_short_packed_compat_object(&ds);
 	status = finish_object_disambiguation(&ds, oid);
 
 	/*
@@ -592,6 +607,7 @@ static enum get_oid_result get_short_oid(struct repository *r,
 		reprepare_packed_git(r);
 		find_short_object_filename(&ds);
 		find_short_packed_object(&ds);
+		find_short_packed_compat_object(&ds);
 		status = finish_object_disambiguation(&ds, oid);
 	}
 
@@ -659,6 +675,7 @@ int repo_for_each_abbrev(struct repository *r, const char *prefix,
 	ds.cb_data = &collect;
 	find_short_object_filename(&ds);
 	find_short_packed_object(&ds);
+	find_short_packed_compat_object(&ds);
 
 	ret = oid_array_for_each_unique(&collect, fn, cb_data);
 	oid_array_clear(&collect);
@@ -871,6 +888,7 @@ int repo_find_unique_abbrev_r(struct repository *r, char *hex,
 	ds.cb_data = (void *)&mad;
 
 	find_short_object_filename(&ds);
+	find_short_packed_compat_object(&ds);
 	(void)finish_object_disambiguation(&ds, &oid_ret);
 
 	hex[mad.cur_len] = 0;
diff --git a/object-store-ll.h b/object-store-ll.h
index c5f2bb2fc2fe..c37c19ada0c3 100644
--- a/object-store-ll.h
+++ b/object-store-ll.h
@@ -135,6 +135,12 @@ struct packed_git {
 	 */
 	const uint32_t *mtimes_map;
 	size_t mtimes_size;
+
+	const void *compat_mapping;
+	size_t compat_mapping_size;
+	const uint8_t *hash_map;
+	const uint8_t *compat_hash_map;
+
 	/* something like ".git/objects/pack/xxxxx.pack" */
 	char pack_name[FLEX_ARRAY]; /* more */
 };
diff --git a/pack-compat-map.c b/pack-compat-map.c
new file mode 100644
index 000000000000..3a992095ebe3
--- /dev/null
+++ b/pack-compat-map.c
@@ -0,0 +1,334 @@
+#include "git-compat-util.h"
+#include "gettext.h"
+#include "hex.h"
+#include "hash-ll.h"
+#include "hash.h"
+#include "object-store.h"
+#include "object-file.h"
+#include "packfile.h"
+#include "pack-compat-map.h"
+#include "packfile.h"
+
+struct pack_compat_map_header {
+	uint8_t sig[4];
+	uint8_t version;
+	uint8_t first_oid_version;
+	uint8_t second_oid_version;
+	uint8_t mbz1;
+	uint32_t nr_objects;
+	uint8_t first_abbrev_len;
+	uint8_t mbz2;
+	uint8_t second_abbrev_len;
+	uint8_t mbz3;
+};
+
+static char *pack_compat_map_filename(struct packed_git *p)
+{
+	size_t len;
+	if (!strip_suffix(p->pack_name, ".pack", &len))
+		BUG("pack_name does not end in .pack");
+	return xstrfmt("%.*s.compat", (int)len, p->pack_name);
+}
+
+static int oid_version_match(const char *filename,
+			     unsigned oid_version,
+			     const struct git_hash_algo *algo)
+{
+	const struct git_hash_algo *found = NULL;
+	int ret = 0;
+
+	if (oid_version == 1) {
+		found = &hash_algos[GIT_HASH_SHA1];
+	} else if (oid_version == 2) {
+		found = &hash_algos[GIT_HASH_SHA256];
+	}
+	if (found == NULL) {
+		ret = error(_("compat map file %s hash version %u unknown"),
+			    filename, oid_version);
+	}
+	else if (found != algo) {
+		ret = error(_("compat map file %s found hash %s expected hash %s"),
+			    filename, found->name, algo->name);
+	}
+	return ret;
+}
+
+
+static int load_pack_compat_map_file(char *compat_map_file,
+				     struct repository *repo,
+				     struct packed_git *p)
+{
+	const struct pack_compat_map_header *hdr;
+	unsigned compat_map_objects = 0;
+	const uint8_t *data = NULL;
+	const uint8_t *packs_hash = NULL;
+	int fd, ret = 0;
+	struct stat st;
+	size_t size, map1sz, map2sz, expected_size;
+
+	fd = git_open(compat_map_file);
+
+	if (fd < 0) {
+		ret = -1;
+		goto cleanup;
+	}
+	if (fstat(fd, &st)) {
+		ret = error_errno(_("failed to read %s"), compat_map_file);
+		goto cleanup;
+	}
+
+	size = xsize_t(st.st_size);
+
+	if (size < sizeof(struct pack_compat_map_header)) {
+		ret = error(_("compat map file %s is too small"), compat_map_file);
+		goto cleanup;
+	}
+
+	data = xmmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0);
+
+	hdr = (const struct pack_compat_map_header *)data;
+	if ((hdr->sig[0] != 'C') ||
+	    (hdr->sig[1] != 'M') ||
+	    (hdr->sig[2] != 'A') ||
+	    (hdr->sig[3] != 'P')) {
+		ret = error(_("compat map file %s has unknown signature"),
+			    compat_map_file);
+		goto cleanup;
+	}
+
+	if (hdr->version != 1) {
+		ret = error(_("compat map file %s has unsupported version %"PRIu8),
+			    compat_map_file, hdr->version);
+		goto cleanup;
+	}
+
+	ret = oid_version_match(compat_map_file, hdr->first_oid_version, repo->hash_algo);
+	if (ret)
+		goto cleanup;
+	ret = oid_version_match(compat_map_file, hdr->second_oid_version, repo->compat_hash_algo);
+	if (ret)
+		goto cleanup;
+	compat_map_objects = ntohl(hdr->nr_objects);
+	if (compat_map_objects != p->num_objects) {
+		ret = error(_("compat map file %s number of objects found %u wanted %u"),
+			    compat_map_file, compat_map_objects, p->num_objects);
+		goto cleanup;
+	}
+
+	map1sz = st_mult(repo->hash_algo->rawsz + 4, compat_map_objects);
+	map2sz = st_mult(repo->compat_hash_algo->rawsz + 4, compat_map_objects);
+
+	expected_size = sizeof(struct pack_compat_map_header);
+	expected_size = st_add(expected_size, map1sz);
+	expected_size = st_add(expected_size, map2sz);
+	expected_size = st_add(expected_size, 2 * repo->hash_algo->rawsz);
+
+	if (size != expected_size) {
+		ret = error(_("compat map file %s is corrupt size %zu expected %zu objects %u sz1 %zu sz2 %zu"),
+			    compat_map_file, size, expected_size, compat_map_objects,
+			    map1sz, map2sz
+			);
+		goto cleanup;
+	}
+
+	packs_hash = data + sizeof(struct pack_compat_map_header) + map1sz + map2sz;
+	if (hashcmp(packs_hash, p->hash)) {
+		ret = error(_("compat map file %s does not match pack %s\n"),
+			      compat_map_file, hash_to_hex(p->hash));
+	}
+
+
+	p->compat_mapping = data;
+	p->compat_mapping_size = size;
+
+	p->hash_map = data + sizeof(struct pack_compat_map_header);
+	p->compat_hash_map = p->hash_map + map1sz;
+
+cleanup:
+	if (ret) {
+		if (data) {
+			munmap((void *)data, size);
+		}
+	}
+	if (fd >= 0)
+		close(fd);
+	return ret;
+}
+
+int load_pack_compat_map(struct repository *repo, struct packed_git *p)
+{
+	char *compat_map_name = NULL;
+	int ret = 0;
+
+	if (p->compat_mapping)
+		return ret;	/* already loaded */
+
+	if (!repo->compat_hash_algo)
+		return 1;		/* Nothing to do */
+
+	ret = open_pack_index(p);
+	if (ret < 0)
+		goto cleanup;
+
+	compat_map_name = pack_compat_map_filename(p);
+	ret = load_pack_compat_map_file(compat_map_name, repo, p);
+cleanup:
+	free(compat_map_name);
+	return ret;
+}
+
+static int keycmp(const unsigned char *a, const unsigned char *b,
+		  size_t key_hex_size)
+{
+	size_t key_byte_size = key_hex_size / 2;
+	unsigned a_last, b_last, mask = (key_hex_size & 1) ? 0xf0 : 0;
+	int cmp = memcmp(a, b, key_byte_size);
+	if (cmp)
+		return cmp;
+
+	a_last = a[key_byte_size] & mask;
+	b_last = b[key_byte_size] & mask;
+
+	if (a_last == b_last)
+		cmp = 0;
+	else if (a_last < b_last)
+		cmp = -1;
+	else
+		cmp = 1;
+
+	return cmp;
+}
+
+static const uint8_t *bsearch_map(const unsigned char *hash,
+				  const uint8_t *table, unsigned nr,
+				  size_t entry_size, size_t key_hex_size)
+{
+	uint32_t hi, lo;
+
+	hi = nr - 1;
+	lo = 0;
+	while (lo < hi) {
+		unsigned mi = lo + ((hi - lo) / 2);
+		const unsigned char *entry = table + (mi * entry_size);
+		int cmp = keycmp(entry, hash, key_hex_size);
+		if (!cmp)
+			return entry;
+		if (cmp > 0)
+			hi = mi;
+		else
+			lo = mi + 1;
+	}
+	if (lo == hi) {
+		const unsigned char *entry = table + (lo * entry_size);
+		int cmp = keycmp(entry, hash, key_hex_size);
+		if (!cmp)
+			return entry;
+	}
+	return NULL;
+}
+
+static void map_each(const struct git_hash_algo *compat,
+		     const unsigned char *prefix, size_t prefix_hexsz,
+		     const uint8_t *table, unsigned nr, size_t entry_bytes,
+		     compat_map_iter_t iter, void *data)
+{
+	const uint8_t *found, *last = table + (entry_bytes * nr);
+
+	found = bsearch_map(prefix, table, nr, entry_bytes, prefix_hexsz);
+	if (!found)
+		return;
+
+	/* Visit each matching key */
+	do {
+		struct object_id oid;
+
+		if (keycmp(found, prefix, prefix_hexsz) != 0)
+			break;
+
+		oidread_algop(&oid, found, compat);
+		if (iter(&oid, data) == CB_BREAK)
+			break;
+
+		found = found + entry_bytes;
+	} while (found < last);
+}
+
+void pack_compat_map_each(struct repository *repo, struct packed_git *p,
+			 const unsigned char *prefix, size_t prefix_hexsz,
+			 compat_map_iter_t iter, void *data)
+{
+	const struct git_hash_algo *compat = repo->compat_hash_algo;
+
+	if (!p->num_objects ||
+	    (!p->compat_mapping && load_pack_compat_map(repo, p)))
+		return;
+
+	if (prefix_hexsz > compat->hexsz)
+		prefix_hexsz = compat->hexsz;
+
+	map_each(compat, prefix, prefix_hexsz,
+		 p->compat_hash_map, p->num_objects, compat->rawsz + 4,
+		 iter, data);
+}
+
+static int compat_map_to_algop(const struct object_id *src,
+			       const struct git_hash_algo *to,
+			       const struct git_hash_algo *from,
+			       const uint8_t *to_table,
+			       const uint8_t *from_table,
+			       unsigned nr,
+			       struct object_id *dest)
+{
+	const uint8_t *found;
+	uint32_t index;
+
+	if (src->algo != hash_algo_by_ptr(from))
+		return -1;
+
+	found = bsearch_map(src->hash,
+			    from_table, nr,
+			    from->rawsz + 4,
+			    from->hexsz);
+	if (!found)
+		return -1;
+
+	index = ntohl(*(uint32_t *)(found + from->rawsz));
+	oidread_algop(dest, to_table + index * (to->rawsz + 4), to);
+	return 0;
+}
+
+static int pack_to_algop(struct repository *repo, struct packed_git *p,
+			 const struct object_id *src,
+			 const struct git_hash_algo *to, struct object_id *dest)
+{
+	if (!p->compat_mapping && load_pack_compat_map(repo, p))
+		return -1;
+
+	if (to == repo->hash_algo) {
+		return compat_map_to_algop(src, to, repo->compat_hash_algo,
+					   p->hash_map,
+					   p->compat_hash_map,
+					   p->num_objects, dest);
+	}
+	else if (to == repo->compat_hash_algo) {
+		return compat_map_to_algop(src, to, repo->hash_algo,
+					   p->compat_hash_map,
+					   p->hash_map,
+					   p->num_objects, dest);
+	}
+	else
+		return -1;
+}
+
+int repo_packed_oid_to_algop(struct repository *repo,
+			     const struct object_id *src,
+			     const struct git_hash_algo *to,
+			     struct object_id *dest)
+{
+	struct packed_git *p;
+	for (p = get_packed_git(repo); p; p = p->next) {
+		if (!pack_to_algop(repo, p, src, to, dest))
+			return 0;
+	}
+	return -1;
+}
diff --git a/pack-compat-map.h b/pack-compat-map.h
new file mode 100644
index 000000000000..2a4561ffdff6
--- /dev/null
+++ b/pack-compat-map.h
@@ -0,0 +1,27 @@
+#ifndef PACK_COMPAT_MAP_H
+#define PACK_COMPAT_MAP_H
+
+#include "cbtree.h"
+struct repository;
+struct packed_git;
+struct object_id;
+struct git_hash_algo;
+struct pack_idx_entry;
+
+int load_pack_compat_map(struct repository *repo, struct packed_git *p);
+
+typedef enum cb_next (*compat_map_iter_t)(const struct object_id *, void *data);
+void pack_compat_map_each(struct repository *repo, struct packed_git *p,
+			 const unsigned char *prefix, size_t prefix_hexsz,
+			 compat_map_iter_t, void *data);
+
+int repo_packed_oid_to_algop(struct repository *repo,
+			     const struct object_id *src,
+			     const struct git_hash_algo *to,
+			     struct object_id *dest);
+
+const char *write_compat_map_file(const char *compat_map_name,
+				  struct pack_idx_entry **objects,
+				  int nr_objects, const unsigned char *hash);
+
+#endif /* PACK_COMPAT_MAP_H */
diff --git a/pack-write.c b/pack-write.c
index b19ddf15b284..f22eea964f77 100644
--- a/pack-write.c
+++ b/pack-write.c
@@ -12,6 +12,7 @@
 #include "pack-revindex.h"
 #include "path.h"
 #include "strbuf.h"
+#include "object-file-convert.h"
 
 void reset_pack_idx_option(struct pack_idx_option *opts)
 {
@@ -345,6 +346,157 @@ static char *write_mtimes_file(struct packing_data *to_pack,
 	return mtimes_name;
 }
 
+struct map_entry {
+	const struct pack_idx_entry *idx;
+	uint32_t oid_index;
+	uint32_t compat_oid_index;
+};
+
+static int map_oid_cmp(const void *_a, const void *_b)
+{
+	struct map_entry *a = *(struct map_entry **)_a;
+	struct map_entry *b = *(struct map_entry **)_b;
+	return oidcmp(&a->idx->oid, &b->idx->oid);
+}
+
+static int map_compat_oid_cmp(const void *_a, const void *_b)
+{
+	struct map_entry *a = *(struct map_entry **)_a;
+	struct map_entry *b = *(struct map_entry **)_b;
+	return oidcmp(&a->idx->compat_oid, &b->idx->compat_oid);
+}
+
+struct pack_compat_map_header {
+	uint8_t sig[4];
+	uint8_t version;
+	uint8_t first_oid_version;
+	uint8_t second_oid_version;
+	uint8_t mbz1;
+	uint32_t nr_objects;
+	uint8_t first_abbrev_len;
+	uint8_t mbz2;
+	uint8_t second_abbrev_len;
+	uint8_t mbz3;
+};
+
+static inline unsigned last_matching_offset(const struct object_id *a,
+					    const struct object_id *b,
+					    const struct git_hash_algo *algop)
+{
+	unsigned i;
+	for (i = 0; i < algop->rawsz; i++)
+		if (a->hash[i] != b->hash[i])
+			return i;
+	/* We should never hit this case. */
+	return i;
+}
+
+/*
+ * The *hash contains the pack content hash.
+ * The objects array is passed in sorted.
+ */
+const char *write_compat_map_file(const char *compat_map_name,
+				  struct pack_idx_entry **objects,
+				  int nr_objects, const unsigned char *hash)
+{
+	struct repository *repo = the_repository;
+	const struct git_hash_algo *algo = repo->hash_algo;
+	const struct git_hash_algo *compat = repo->compat_hash_algo;
+	unsigned short_name_len, compat_short_name_len;
+	struct hashfile *f;
+	struct map_entry *map_entries, **map;
+	struct pack_compat_map_header hdr;
+	unsigned i;
+	int fd;
+
+	if (!compat || !nr_objects)
+		return NULL;
+
+	ALLOC_ARRAY(map_entries, nr_objects);
+	ALLOC_ARRAY(map, nr_objects);
+	short_name_len = 1;
+	for (i = 0; i < nr_objects; ++i) {
+		unsigned offset;
+
+		map[i] = &map_entries[i];
+		map_entries[i].idx = objects[i];
+		if (!objects[i]->compat_oid.algo)
+			BUG("No mapping from %s to %s\n",
+			    oid_to_hex(&objects[i]->oid),
+			    compat->name);
+
+		map_entries[i].oid_index = i;
+		map_entries[i].compat_oid_index = 0;
+		if (i == 0)
+			continue;
+
+		offset = last_matching_offset(&map_entries[i].idx->oid,
+					      &map_entries[i - 1].idx->oid,
+					      algo);
+		if (offset > short_name_len)
+			short_name_len = offset;
+	}
+	QSORT(map, nr_objects, map_compat_oid_cmp);
+	compat_short_name_len = 1;
+	for (i = 0; i < nr_objects; ++i) {
+		unsigned offset;
+
+		map[i]->compat_oid_index = i;
+
+		if (i == 0)
+			continue;
+
+		offset = last_matching_offset(&map[i]->idx->compat_oid,
+					      &map[i - 1]->idx->compat_oid,
+					      compat);
+		if (offset > compat_short_name_len)
+			compat_short_name_len = offset;
+	}
+
+	if (compat_map_name) {
+		/* Verify an existing compat map file */
+		f = hashfd_check(compat_map_name);
+	} else {
+		struct strbuf tmp_file = STRBUF_INIT;
+		fd = odb_mkstemp(&tmp_file, "pack/tmp_compat_map_XXXXXX");
+		compat_map_name = strbuf_detach(&tmp_file, NULL);
+		f = hashfd(fd, compat_map_name);
+	}
+
+	hdr.sig[0] = 'C';
+	hdr.sig[1] = 'M';
+	hdr.sig[2] = 'A';
+	hdr.sig[3] = 'P';
+	hdr.version = 1;
+	hdr.first_oid_version = oid_version(algo);
+	hdr.second_oid_version = oid_version(compat);
+	hdr.mbz1 = 0;
+	hdr.nr_objects = htonl(nr_objects);
+	hdr.first_abbrev_len = short_name_len;
+	hdr.mbz2 = 0;
+	hdr.second_abbrev_len = compat_short_name_len;
+	hdr.mbz3 = 0;
+	hashwrite(f, &hdr, sizeof(hdr));
+
+	QSORT(map, nr_objects, map_oid_cmp);
+	for (i = 0; i < nr_objects; i++) {
+		hashwrite(f, map[i]->idx->oid.hash, algo->rawsz);
+		hashwrite_be32(f, map[i]->compat_oid_index);
+	}
+	QSORT(map, nr_objects, map_compat_oid_cmp);
+	for (i = 0; i < nr_objects; i++) {
+		hashwrite(f, map[i]->idx->compat_oid.hash, compat->rawsz);
+		hashwrite_be32(f, map[i]->oid_index);
+	}
+
+	hashwrite(f, hash, algo->rawsz);
+	finalize_hashfile(f, NULL, FSYNC_COMPONENT_PACK_METADATA,
+			  CSUM_HASH_IN_STREAM | CSUM_CLOSE | CSUM_FSYNC);
+	free(map);
+	free(map_entries);
+	return compat_map_name;
+}
+
 off_t write_pack_header(struct hashfile *f, uint32_t nr_entries)
 {
 	struct pack_header hdr;
@@ -548,6 +700,7 @@ void stage_tmp_packfiles(struct strbuf *name_buffer,
 {
 	const char *rev_tmp_name = NULL;
 	char *mtimes_tmp_name = NULL;
+	const char *compat_map_tmp_name = NULL;
 
 	if (adjust_shared_perm(pack_tmp_name))
 		die_errno("unable to make temporary pack file readable");
@@ -566,11 +719,16 @@ void stage_tmp_packfiles(struct strbuf *name_buffer,
 						    hash);
 	}
 
+	compat_map_tmp_name = write_compat_map_file(NULL, written_list,
+						    nr_written, hash);
+
 	rename_tmp_packfile(name_buffer, pack_tmp_name, "pack");
 	if (rev_tmp_name)
 		rename_tmp_packfile(name_buffer, rev_tmp_name, "rev");
 	if (mtimes_tmp_name)
 		rename_tmp_packfile(name_buffer, mtimes_tmp_name, "mtimes");
+	if (compat_map_tmp_name)
+		rename_tmp_packfile(name_buffer, compat_map_tmp_name, "compat");
 
 	free((char *)rev_tmp_name);
 	free(mtimes_tmp_name);
diff --git a/packfile.c b/packfile.c
index 1fae0fcdd9e7..c1a6bd9bc6b3 100644
--- a/packfile.c
+++ b/packfile.c
@@ -349,6 +349,17 @@ static void close_pack_mtimes(struct packed_git *p)
 	p->mtimes_map = NULL;
 }
 
+static void close_pack_compat_map(struct packed_git *p)
+{
+	if (!p->compat_mapping)
+		return;
+
+	munmap((void *)p->compat_mapping, p->compat_mapping_size);
+	p->compat_mapping = NULL;
+	p->hash_map = NULL;
+	p->compat_hash_map = NULL;
+}
+
 void close_pack(struct packed_git *p)
 {
 	close_pack_windows(p);
@@ -356,6 +367,7 @@ void close_pack(struct packed_git *p)
 	close_pack_index(p);
 	close_pack_revindex(p);
 	close_pack_mtimes(p);
+	close_pack_compat_map(p);
 	oidset_clear(&p->bad_objects);
 }
 
-- 
2.41.0


^ permalink raw reply related

* [PATCH 21/32] tree-walk: init_tree_desc take an oid to get the hash algorithm
From: Eric W. Biederman @ 2023-09-08 23:10 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, brian m. carlson, Eric W. Biederman
In-Reply-To: <87sf7ol0z3.fsf@email.froward.int.ebiederm.org>

To make it possible for git ls-tree to display the tree encoded
in the hash algorithm of the oid specified to git ls-tree, update
init_tree_desc to take as a parameter the oid of the tree object.

Update all callers of init_tree_desc and init_tree_desc_gently
to pass the oid of the tree object.

Use the oid of the tree object to discover the hash algorithm
of the oid and store that hash algorithm in struct tree_desc.

Use the hash algorithm in decode_tree_entry and
update_tree_entry_internal to handle reading a tree object encoded in
a hash algorithm that differs from the repositories hash algorithm.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 archive.c              |  3 ++-
 builtin/am.c           |  6 +++---
 builtin/checkout.c     |  8 +++++---
 builtin/clone.c        |  2 +-
 builtin/commit.c       |  2 +-
 builtin/grep.c         |  8 ++++----
 builtin/merge.c        |  3 ++-
 builtin/pack-objects.c |  6 ++++--
 builtin/read-tree.c    |  2 +-
 builtin/stash.c        |  5 +++--
 cache-tree.c           |  2 +-
 delta-islands.c        |  2 +-
 diff-lib.c             |  2 +-
 fsck.c                 |  6 ++++--
 http-push.c            |  2 +-
 list-objects.c         |  2 +-
 match-trees.c          |  4 ++--
 merge-ort.c            | 11 ++++++-----
 merge-recursive.c      |  2 +-
 merge.c                |  3 ++-
 pack-bitmap-write.c    |  2 +-
 packfile.c             |  3 ++-
 reflog.c               |  2 +-
 revision.c             |  4 ++--
 tree-walk.c            | 36 +++++++++++++++++++++---------------
 tree-walk.h            |  7 +++++--
 tree.c                 |  2 +-
 walker.c               |  2 +-
 28 files changed, 80 insertions(+), 59 deletions(-)

diff --git a/archive.c b/archive.c
index ca11db185b15..b10269aee7be 100644
--- a/archive.c
+++ b/archive.c
@@ -339,7 +339,8 @@ int write_archive_entries(struct archiver_args *args,
 		opts.src_index = args->repo->index;
 		opts.dst_index = args->repo->index;
 		opts.fn = oneway_merge;
-		init_tree_desc(&t, args->tree->buffer, args->tree->size);
+		init_tree_desc(&t, &args->tree->object.oid,
+			       args->tree->buffer, args->tree->size);
 		if (unpack_trees(1, &t, &opts))
 			return -1;
 		git_attr_set_direction(GIT_ATTR_INDEX);
diff --git a/builtin/am.c b/builtin/am.c
index 8bde034fae68..4dfd714b910e 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1991,8 +1991,8 @@ static int fast_forward_to(struct tree *head, struct tree *remote, int reset)
 	opts.reset = reset ? UNPACK_RESET_PROTECT_UNTRACKED : 0;
 	opts.preserve_ignored = 0; /* FIXME: !overwrite_ignore */
 	opts.fn = twoway_merge;
-	init_tree_desc(&t[0], head->buffer, head->size);
-	init_tree_desc(&t[1], remote->buffer, remote->size);
+	init_tree_desc(&t[0], &head->object.oid, head->buffer, head->size);
+	init_tree_desc(&t[1], &remote->object.oid, remote->buffer, remote->size);
 
 	if (unpack_trees(2, t, &opts)) {
 		rollback_lock_file(&lock_file);
@@ -2026,7 +2026,7 @@ static int merge_tree(struct tree *tree)
 	opts.dst_index = &the_index;
 	opts.merge = 1;
 	opts.fn = oneway_merge;
-	init_tree_desc(&t[0], tree->buffer, tree->size);
+	init_tree_desc(&t[0], &tree->object.oid, tree->buffer, tree->size);
 
 	if (unpack_trees(1, t, &opts)) {
 		rollback_lock_file(&lock_file);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index f53612f46870..03eff73fd031 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -701,7 +701,7 @@ static int reset_tree(struct tree *tree, const struct checkout_opts *o,
 			       info->commit ? &info->commit->object.oid : null_oid(),
 			       NULL);
 	parse_tree(tree);
-	init_tree_desc(&tree_desc, tree->buffer, tree->size);
+	init_tree_desc(&tree_desc, &tree->object.oid, tree->buffer, tree->size);
 	switch (unpack_trees(1, &tree_desc, &opts)) {
 	case -2:
 		*writeout_error = 1;
@@ -815,10 +815,12 @@ static int merge_working_tree(const struct checkout_opts *opts,
 			die(_("unable to parse commit %s"),
 				oid_to_hex(old_commit_oid));
 
-		init_tree_desc(&trees[0], tree->buffer, tree->size);
+		init_tree_desc(&trees[0], &tree->object.oid,
+			       tree->buffer, tree->size);
 		parse_tree(new_tree);
 		tree = new_tree;
-		init_tree_desc(&trees[1], tree->buffer, tree->size);
+		init_tree_desc(&trees[1], &tree->object.oid,
+			       tree->buffer, tree->size);
 
 		ret = unpack_trees(2, trees, &topts);
 		clear_unpack_trees_porcelain(&topts);
diff --git a/builtin/clone.c b/builtin/clone.c
index c6357af94989..79ceefb93995 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -737,7 +737,7 @@ static int checkout(int submodule_progress, int filter_submodules)
 	if (!tree)
 		die(_("unable to parse commit %s"), oid_to_hex(&oid));
 	parse_tree(tree);
-	init_tree_desc(&t, tree->buffer, tree->size);
+	init_tree_desc(&t, &tree->object.oid, tree->buffer, tree->size);
 	if (unpack_trees(1, &t, &opts) < 0)
 		die(_("unable to checkout working tree"));
 
diff --git a/builtin/commit.c b/builtin/commit.c
index 7da5f924484d..537319932b65 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -340,7 +340,7 @@ static void create_base_index(const struct commit *current_head)
 	if (!tree)
 		die(_("failed to unpack HEAD tree object"));
 	parse_tree(tree);
-	init_tree_desc(&t, tree->buffer, tree->size);
+	init_tree_desc(&t, &tree->object.oid, tree->buffer, tree->size);
 	if (unpack_trees(1, &t, &opts))
 		exit(128); /* We've already reported the error, finish dying */
 }
diff --git a/builtin/grep.c b/builtin/grep.c
index 50e712a18479..0c2b8a376f8e 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -530,7 +530,7 @@ static int grep_submodule(struct grep_opt *opt,
 		strbuf_addstr(&base, filename);
 		strbuf_addch(&base, '/');
 
-		init_tree_desc(&tree, data, size);
+		init_tree_desc(&tree, oid, data, size);
 		hit = grep_tree(&subopt, pathspec, &tree, &base, base.len,
 				object_type == OBJ_COMMIT);
 		strbuf_release(&base);
@@ -574,7 +574,7 @@ static int grep_cache(struct grep_opt *opt,
 
 			data = repo_read_object_file(the_repository, &ce->oid,
 						     &type, &size);
-			init_tree_desc(&tree, data, size);
+			init_tree_desc(&tree, &ce->oid, data, size);
 
 			hit |= grep_tree(opt, pathspec, &tree, &name, 0, 0);
 			strbuf_setlen(&name, name_base_len);
@@ -670,7 +670,7 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
 				    oid_to_hex(&entry.oid));
 
 			strbuf_addch(base, '/');
-			init_tree_desc(&sub, data, size);
+			init_tree_desc(&sub, &entry.oid, data, size);
 			hit |= grep_tree(opt, pathspec, &sub, base, tn_len,
 					 check_attr);
 			free(data);
@@ -714,7 +714,7 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,
 			strbuf_add(&base, name, len);
 			strbuf_addch(&base, ':');
 		}
-		init_tree_desc(&tree, data, size);
+		init_tree_desc(&tree, &obj->oid, data, size);
 		hit = grep_tree(opt, pathspec, &tree, &base, base.len,
 				obj->type == OBJ_COMMIT);
 		strbuf_release(&base);
diff --git a/builtin/merge.c b/builtin/merge.c
index de68910177fb..718165d45917 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -704,7 +704,8 @@ static int read_tree_trivial(struct object_id *common, struct object_id *head,
 	cache_tree_free(&the_index.cache_tree);
 	for (i = 0; i < nr_trees; i++) {
 		parse_tree(trees[i]);
-		init_tree_desc(t+i, trees[i]->buffer, trees[i]->size);
+		init_tree_desc(t+i, &trees[i]->object.oid,
+			       trees[i]->buffer, trees[i]->size);
 	}
 	if (unpack_trees(nr_trees, t, &opts))
 		return -1;
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index d2a162d52804..d34902002656 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1756,7 +1756,8 @@ static void add_pbase_object(struct tree_desc *tree,
 			tree = pbase_tree_get(&entry.oid);
 			if (!tree)
 				return;
-			init_tree_desc(&sub, tree->tree_data, tree->tree_size);
+			init_tree_desc(&sub, &tree->oid,
+				       tree->tree_data, tree->tree_size);
 
 			add_pbase_object(&sub, down, downlen, fullname);
 			pbase_tree_put(tree);
@@ -1816,7 +1817,8 @@ static void add_preferred_base_object(const char *name)
 		}
 		else {
 			struct tree_desc tree;
-			init_tree_desc(&tree, it->pcache.tree_data, it->pcache.tree_size);
+			init_tree_desc(&tree, &it->pcache.oid,
+				       it->pcache.tree_data, it->pcache.tree_size);
 			add_pbase_object(&tree, name, cmplen, name);
 		}
 	}
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 1fec702a04fa..24d6d156d3a2 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -264,7 +264,7 @@ int cmd_read_tree(int argc, const char **argv, const char *cmd_prefix)
 	for (i = 0; i < nr_trees; i++) {
 		struct tree *tree = trees[i];
 		parse_tree(tree);
-		init_tree_desc(t+i, tree->buffer, tree->size);
+		init_tree_desc(t+i, &tree->object.oid, tree->buffer, tree->size);
 	}
 	if (unpack_trees(nr_trees, t, &opts))
 		return 128;
diff --git a/builtin/stash.c b/builtin/stash.c
index fe64cde9ce30..9ee52af4d28e 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -285,7 +285,7 @@ static int reset_tree(struct object_id *i_tree, int update, int reset)
 	if (parse_tree(tree))
 		return -1;
 
-	init_tree_desc(t, tree->buffer, tree->size);
+	init_tree_desc(t, &tree->object.oid, tree->buffer, tree->size);
 
 	opts.head_idx = 1;
 	opts.src_index = &the_index;
@@ -871,7 +871,8 @@ static void diff_include_untracked(const struct stash_info *info, struct diff_op
 		tree[i] = parse_tree_indirect(oid[i]);
 		if (parse_tree(tree[i]) < 0)
 			die(_("failed to parse tree"));
-		init_tree_desc(&tree_desc[i], tree[i]->buffer, tree[i]->size);
+		init_tree_desc(&tree_desc[i], &tree[i]->object.oid,
+			       tree[i]->buffer, tree[i]->size);
 	}
 
 	unpack_tree_opt.head_idx = -1;
diff --git a/cache-tree.c b/cache-tree.c
index ddc7d3d86959..334973a01cee 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -770,7 +770,7 @@ static void prime_cache_tree_rec(struct repository *r,
 
 	oidcpy(&it->oid, &tree->object.oid);
 
-	init_tree_desc(&desc, tree->buffer, tree->size);
+	init_tree_desc(&desc, &tree->object.oid, tree->buffer, tree->size);
 	cnt = 0;
 	while (tree_entry(&desc, &entry)) {
 		if (!S_ISDIR(entry.mode))
diff --git a/delta-islands.c b/delta-islands.c
index 5de5759f3f13..1ff3506b10f2 100644
--- a/delta-islands.c
+++ b/delta-islands.c
@@ -289,7 +289,7 @@ void resolve_tree_islands(struct repository *r,
 		if (!tree || parse_tree(tree) < 0)
 			die(_("bad tree object %s"), oid_to_hex(&ent->idx.oid));
 
-		init_tree_desc(&desc, tree->buffer, tree->size);
+		init_tree_desc(&desc, &tree->object.oid, tree->buffer, tree->size);
 		while (tree_entry(&desc, &entry)) {
 			struct object *obj;
 
diff --git a/diff-lib.c b/diff-lib.c
index 6b0c6a7180cc..add323f5628d 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -558,7 +558,7 @@ static int diff_cache(struct rev_info *revs,
 	opts.pathspec = &revs->diffopt.pathspec;
 	opts.pathspec->recursive = 1;
 
-	init_tree_desc(&t, tree->buffer, tree->size);
+	init_tree_desc(&t, &tree->object.oid, tree->buffer, tree->size);
 	return unpack_trees(1, &t, &opts);
 }
 
diff --git a/fsck.c b/fsck.c
index 2b1e348005b7..6b492a48da82 100644
--- a/fsck.c
+++ b/fsck.c
@@ -313,7 +313,8 @@ static int fsck_walk_tree(struct tree *tree, void *data, struct fsck_options *op
 		return -1;
 
 	name = fsck_get_object_name(options, &tree->object.oid);
-	if (init_tree_desc_gently(&desc, tree->buffer, tree->size, 0))
+	if (init_tree_desc_gently(&desc, &tree->object.oid,
+				  tree->buffer, tree->size, 0))
 		return -1;
 	while (tree_entry_gently(&desc, &entry)) {
 		struct object *obj;
@@ -583,7 +584,8 @@ static int fsck_tree(const struct object_id *tree_oid,
 	const char *o_name;
 	struct name_stack df_dup_candidates = { NULL };
 
-	if (init_tree_desc_gently(&desc, buffer, size, TREE_DESC_RAW_MODES)) {
+	if (init_tree_desc_gently(&desc, tree_oid, buffer, size,
+				  TREE_DESC_RAW_MODES)) {
 		retval += report(options, tree_oid, OBJ_TREE,
 				 FSCK_MSG_BAD_TREE,
 				 "cannot be parsed as a tree");
diff --git a/http-push.c b/http-push.c
index a704f490fdb2..81c35b5e96f7 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1308,7 +1308,7 @@ static struct object_list **process_tree(struct tree *tree,
 	obj->flags |= SEEN;
 	p = add_one_object(obj, p);
 
-	init_tree_desc(&desc, tree->buffer, tree->size);
+	init_tree_desc(&desc, &tree->object.oid, tree->buffer, tree->size);
 
 	while (tree_entry(&desc, &entry))
 		switch (object_type(entry.mode)) {
diff --git a/list-objects.c b/list-objects.c
index e60a6cd5b46e..312335c8a7f2 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -97,7 +97,7 @@ static void process_tree_contents(struct traversal_context *ctx,
 	enum interesting match = ctx->revs->diffopt.pathspec.nr == 0 ?
 		all_entries_interesting : entry_not_interesting;
 
-	init_tree_desc(&desc, tree->buffer, tree->size);
+	init_tree_desc(&desc, &tree->object.oid, tree->buffer, tree->size);
 
 	while (tree_entry(&desc, &entry)) {
 		if (match != all_entries_interesting) {
diff --git a/match-trees.c b/match-trees.c
index 0885ac681cd5..3412b6a1401d 100644
--- a/match-trees.c
+++ b/match-trees.c
@@ -63,7 +63,7 @@ static void *fill_tree_desc_strict(struct tree_desc *desc,
 		die("unable to read tree (%s)", oid_to_hex(hash));
 	if (type != OBJ_TREE)
 		die("%s is not a tree", oid_to_hex(hash));
-	init_tree_desc(desc, buffer, size);
+	init_tree_desc(desc, hash, buffer, size);
 	return buffer;
 }
 
@@ -194,7 +194,7 @@ static int splice_tree(const struct object_id *oid1, const char *prefix,
 	buf = repo_read_object_file(the_repository, oid1, &type, &sz);
 	if (!buf)
 		die("cannot read tree %s", oid_to_hex(oid1));
-	init_tree_desc(&desc, buf, sz);
+	init_tree_desc(&desc, oid1, buf, sz);
 
 	rewrite_here = NULL;
 	while (desc.size) {
diff --git a/merge-ort.c b/merge-ort.c
index 8631c997002d..3a5729c91e48 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -1679,9 +1679,10 @@ static int collect_merge_info(struct merge_options *opt,
 	parse_tree(merge_base);
 	parse_tree(side1);
 	parse_tree(side2);
-	init_tree_desc(t + 0, merge_base->buffer, merge_base->size);
-	init_tree_desc(t + 1, side1->buffer, side1->size);
-	init_tree_desc(t + 2, side2->buffer, side2->size);
+	init_tree_desc(t + 0, &merge_base->object.oid,
+		       merge_base->buffer, merge_base->size);
+	init_tree_desc(t + 1, &side1->object.oid, side1->buffer, side1->size);
+	init_tree_desc(t + 2, &side2->object.oid, side2->buffer, side2->size);
 
 	trace2_region_enter("merge", "traverse_trees", opt->repo);
 	ret = traverse_trees(NULL, 3, t, &info);
@@ -4400,9 +4401,9 @@ static int checkout(struct merge_options *opt,
 	unpack_opts.fn = twoway_merge;
 	unpack_opts.preserve_ignored = 0; /* FIXME: !opts->overwrite_ignore */
 	parse_tree(prev);
-	init_tree_desc(&trees[0], prev->buffer, prev->size);
+	init_tree_desc(&trees[0], &prev->object.oid, prev->buffer, prev->size);
 	parse_tree(next);
-	init_tree_desc(&trees[1], next->buffer, next->size);
+	init_tree_desc(&trees[1], &next->object.oid, next->buffer, next->size);
 
 	ret = unpack_trees(2, trees, &unpack_opts);
 	clear_unpack_trees_porcelain(&unpack_opts);
diff --git a/merge-recursive.c b/merge-recursive.c
index 6a4081bb0f52..93df9eecdd95 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -411,7 +411,7 @@ static inline int merge_detect_rename(struct merge_options *opt)
 static void init_tree_desc_from_tree(struct tree_desc *desc, struct tree *tree)
 {
 	parse_tree(tree);
-	init_tree_desc(desc, tree->buffer, tree->size);
+	init_tree_desc(desc, &tree->object.oid, tree->buffer, tree->size);
 }
 
 static int unpack_trees_start(struct merge_options *opt,
diff --git a/merge.c b/merge.c
index b60925459c29..86179c34102d 100644
--- a/merge.c
+++ b/merge.c
@@ -81,7 +81,8 @@ int checkout_fast_forward(struct repository *r,
 	}
 	for (i = 0; i < nr_trees; i++) {
 		parse_tree(trees[i]);
-		init_tree_desc(t+i, trees[i]->buffer, trees[i]->size);
+		init_tree_desc(t+i, &trees[i]->object.oid,
+			       trees[i]->buffer, trees[i]->size);
 	}
 
 	memset(&opts, 0, sizeof(opts));
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index f6757c3cbf20..9211e08f0127 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -366,7 +366,7 @@ static int fill_bitmap_tree(struct bitmap *bitmap,
 	if (parse_tree(tree) < 0)
 		die("unable to load tree object %s",
 		    oid_to_hex(&tree->object.oid));
-	init_tree_desc(&desc, tree->buffer, tree->size);
+	init_tree_desc(&desc, &tree->object.oid, tree->buffer, tree->size);
 
 	while (tree_entry(&desc, &entry)) {
 		switch (object_type(entry.mode)) {
diff --git a/packfile.c b/packfile.c
index 9cc0a2e37a83..1fae0fcdd9e7 100644
--- a/packfile.c
+++ b/packfile.c
@@ -2250,7 +2250,8 @@ static int add_promisor_object(const struct object_id *oid,
 		struct tree *tree = (struct tree *)obj;
 		struct tree_desc desc;
 		struct name_entry entry;
-		if (init_tree_desc_gently(&desc, tree->buffer, tree->size, 0))
+		if (init_tree_desc_gently(&desc, &tree->object.oid,
+					  tree->buffer, tree->size, 0))
 			/*
 			 * Error messages are given when packs are
 			 * verified, so do not print any here.
diff --git a/reflog.c b/reflog.c
index 9ad50e7d93e4..c6992a19268f 100644
--- a/reflog.c
+++ b/reflog.c
@@ -40,7 +40,7 @@ static int tree_is_complete(const struct object_id *oid)
 		tree->buffer = data;
 		tree->size = size;
 	}
-	init_tree_desc(&desc, tree->buffer, tree->size);
+	init_tree_desc(&desc, &tree->object.oid, tree->buffer, tree->size);
 	complete = 1;
 	while (tree_entry(&desc, &entry)) {
 		if (!repo_has_object_file(the_repository, &entry.oid) ||
diff --git a/revision.c b/revision.c
index 2f4c53ea207b..a60dfc23a2a5 100644
--- a/revision.c
+++ b/revision.c
@@ -82,7 +82,7 @@ static void mark_tree_contents_uninteresting(struct repository *r,
 	if (parse_tree_gently(tree, 1) < 0)
 		return;
 
-	init_tree_desc(&desc, tree->buffer, tree->size);
+	init_tree_desc(&desc, &tree->object.oid, tree->buffer, tree->size);
 	while (tree_entry(&desc, &entry)) {
 		switch (object_type(entry.mode)) {
 		case OBJ_TREE:
@@ -189,7 +189,7 @@ static void add_children_by_path(struct repository *r,
 	if (parse_tree_gently(tree, 1) < 0)
 		return;
 
-	init_tree_desc(&desc, tree->buffer, tree->size);
+	init_tree_desc(&desc, &tree->object.oid, tree->buffer, tree->size);
 	while (tree_entry(&desc, &entry)) {
 		switch (object_type(entry.mode)) {
 		case OBJ_TREE:
diff --git a/tree-walk.c b/tree-walk.c
index 3af50a01c2c7..0b44ec7c75ff 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -15,7 +15,7 @@ static int decode_tree_entry(struct tree_desc *desc, const char *buf, unsigned l
 	const char *path;
 	unsigned int len;
 	uint16_t mode;
-	const unsigned hashsz = the_hash_algo->rawsz;
+	const unsigned hashsz = desc->algo->rawsz;
 
 	if (size < hashsz + 3 || buf[size - (hashsz + 1)]) {
 		strbuf_addstr(err, _("too-short tree object"));
@@ -37,15 +37,19 @@ static int decode_tree_entry(struct tree_desc *desc, const char *buf, unsigned l
 	desc->entry.path = path;
 	desc->entry.mode = (desc->flags & TREE_DESC_RAW_MODES) ? mode : canon_mode(mode);
 	desc->entry.pathlen = len - 1;
-	oidread(&desc->entry.oid, (const unsigned char *)path + len);
+	oidread_algop(&desc->entry.oid, (const unsigned char *)path + len,
+		      desc->algo);
 
 	return 0;
 }
 
-static int init_tree_desc_internal(struct tree_desc *desc, const void *buffer,
-				   unsigned long size, struct strbuf *err,
+static int init_tree_desc_internal(struct tree_desc *desc,
+				   const struct object_id *oid,
+				   const void *buffer, unsigned long size,
+				   struct strbuf *err,
 				   enum tree_desc_flags flags)
 {
+	desc->algo = (oid && oid->algo) ? &hash_algos[oid->algo] : the_hash_algo;
 	desc->buffer = buffer;
 	desc->size = size;
 	desc->flags = flags;
@@ -54,19 +58,21 @@ static int init_tree_desc_internal(struct tree_desc *desc, const void *buffer,
 	return 0;
 }
 
-void init_tree_desc(struct tree_desc *desc, const void *buffer, unsigned long size)
+void init_tree_desc(struct tree_desc *desc, const struct object_id *tree_oid,
+		    const void *buffer, unsigned long size)
 {
 	struct strbuf err = STRBUF_INIT;
-	if (init_tree_desc_internal(desc, buffer, size, &err, 0))
+	if (init_tree_desc_internal(desc, tree_oid, buffer, size, &err, 0))
 		die("%s", err.buf);
 	strbuf_release(&err);
 }
 
-int init_tree_desc_gently(struct tree_desc *desc, const void *buffer, unsigned long size,
+int init_tree_desc_gently(struct tree_desc *desc, const struct object_id *oid,
+			  const void *buffer, unsigned long size,
 			  enum tree_desc_flags flags)
 {
 	struct strbuf err = STRBUF_INIT;
-	int result = init_tree_desc_internal(desc, buffer, size, &err, flags);
+	int result = init_tree_desc_internal(desc, oid, buffer, size, &err, flags);
 	if (result)
 		error("%s", err.buf);
 	strbuf_release(&err);
@@ -85,7 +91,7 @@ void *fill_tree_descriptor(struct repository *r,
 		if (!buf)
 			die("unable to read tree %s", oid_to_hex(oid));
 	}
-	init_tree_desc(desc, buf, size);
+	init_tree_desc(desc, oid, buf, size);
 	return buf;
 }
 
@@ -102,7 +108,7 @@ static void entry_extract(struct tree_desc *t, struct name_entry *a)
 static int update_tree_entry_internal(struct tree_desc *desc, struct strbuf *err)
 {
 	const void *buf = desc->buffer;
-	const unsigned char *end = (const unsigned char *)desc->entry.path + desc->entry.pathlen + 1 + the_hash_algo->rawsz;
+	const unsigned char *end = (const unsigned char *)desc->entry.path + desc->entry.pathlen + 1 + desc->algo->rawsz;
 	unsigned long size = desc->size;
 	unsigned long len = end - (const unsigned char *)buf;
 
@@ -611,7 +617,7 @@ int get_tree_entry(struct repository *r,
 		retval = -1;
 	} else {
 		struct tree_desc t;
-		init_tree_desc(&t, tree, size);
+		init_tree_desc(&t, tree_oid, tree, size);
 		retval = find_tree_entry(r, &t, name, oid, mode);
 	}
 	free(tree);
@@ -654,7 +660,7 @@ enum get_oid_result get_tree_entry_follow_symlinks(struct repository *r,
 	struct tree_desc t;
 	int follows_remaining = GET_TREE_ENTRY_FOLLOW_SYMLINKS_MAX_LINKS;
 
-	init_tree_desc(&t, NULL, 0UL);
+	init_tree_desc(&t, NULL, NULL, 0UL);
 	strbuf_addstr(&namebuf, name);
 	oidcpy(&current_tree_oid, tree_oid);
 
@@ -690,7 +696,7 @@ enum get_oid_result get_tree_entry_follow_symlinks(struct repository *r,
 				goto done;
 
 			/* descend */
-			init_tree_desc(&t, tree, size);
+			init_tree_desc(&t, &current_tree_oid, tree, size);
 		}
 
 		/* Handle symlinks to e.g. a//b by removing leading slashes */
@@ -724,7 +730,7 @@ enum get_oid_result get_tree_entry_follow_symlinks(struct repository *r,
 			free(parent->tree);
 			parents_nr--;
 			parent = &parents[parents_nr - 1];
-			init_tree_desc(&t, parent->tree, parent->size);
+			init_tree_desc(&t, &parent->oid, parent->tree, parent->size);
 			strbuf_remove(&namebuf, 0, remainder ? 3 : 2);
 			continue;
 		}
@@ -804,7 +810,7 @@ enum get_oid_result get_tree_entry_follow_symlinks(struct repository *r,
 			contents_start = contents;
 
 			parent = &parents[parents_nr - 1];
-			init_tree_desc(&t, parent->tree, parent->size);
+			init_tree_desc(&t, &parent->oid, parent->tree, parent->size);
 			strbuf_splice(&namebuf, 0, len,
 				      contents_start, link_len);
 			if (remainder)
diff --git a/tree-walk.h b/tree-walk.h
index 74cdceb3fed2..cf54d01019e9 100644
--- a/tree-walk.h
+++ b/tree-walk.h
@@ -26,6 +26,7 @@ struct name_entry {
  * A semi-opaque data structure used to maintain the current state of the walk.
  */
 struct tree_desc {
+	const struct git_hash_algo *algo;
 	/*
 	 * pointer into the memory representation of the tree. It always
 	 * points at the current entry being visited.
@@ -85,9 +86,11 @@ int update_tree_entry_gently(struct tree_desc *);
  * size parameters are assumed to be the same as the buffer and size
  * members of `struct tree`.
  */
-void init_tree_desc(struct tree_desc *desc, const void *buf, unsigned long size);
+void init_tree_desc(struct tree_desc *desc, const struct object_id *tree_oid,
+		    const void *buf, unsigned long size);
 
-int init_tree_desc_gently(struct tree_desc *desc, const void *buf, unsigned long size,
+int init_tree_desc_gently(struct tree_desc *desc, const struct object_id *oid,
+			  const void *buf, unsigned long size,
 			  enum tree_desc_flags flags);
 
 /*
diff --git a/tree.c b/tree.c
index c745462f968e..44bcf728f10a 100644
--- a/tree.c
+++ b/tree.c
@@ -27,7 +27,7 @@ int read_tree_at(struct repository *r,
 	if (parse_tree(tree))
 		return -1;
 
-	init_tree_desc(&desc, tree->buffer, tree->size);
+	init_tree_desc(&desc, &tree->object.oid, tree->buffer, tree->size);
 
 	while (tree_entry(&desc, &entry)) {
 		if (retval != all_entries_interesting) {
diff --git a/walker.c b/walker.c
index 65002a7220ad..c0fd632d921c 100644
--- a/walker.c
+++ b/walker.c
@@ -45,7 +45,7 @@ static int process_tree(struct walker *walker, struct tree *tree)
 	if (parse_tree(tree))
 		return -1;
 
-	init_tree_desc(&desc, tree->buffer, tree->size);
+	init_tree_desc(&desc, &tree->object.oid, tree->buffer, tree->size);
 	while (tree_entry(&desc, &entry)) {
 		struct object *obj = NULL;
 
-- 
2.41.0


^ permalink raw reply related

* [PATCH 24/32] builtin/pack-objects:  Communicate the compatibility hash through struct pack_idx_entry
From: Eric W. Biederman @ 2023-09-08 23:10 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, brian m. carlson, Eric W. Biederman
In-Reply-To: <87sf7ol0z3.fsf@email.froward.int.ebiederm.org>

When pack-objects is run all objects in the repository should already
have a compatibilty hash computed so it is just necessary to read
the existing mappings and store the value in struct pack_idx_entry.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 builtin/pack-objects.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index d34902002656..ff04660a18fd 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -42,6 +42,7 @@
 #include "promisor-remote.h"
 #include "pack-mtimes.h"
 #include "parse-options.h"
+#include "object-file-convert.h"
 
 /*
  * Objects we are going to pack are collected in the `to_pack` structure.
@@ -1547,10 +1548,16 @@ static struct object_entry *create_object_entry(const struct object_id *oid,
 						struct packed_git *found_pack,
 						off_t found_offset)
 {
+	struct repository *repo = the_repository;
+	const struct git_hash_algo *compat = repo->compat_hash_algo;
 	struct object_entry *entry;
 
 	entry = packlist_alloc(&to_pack, oid);
 	entry->hash = hash;
+	if (compat &&
+	    repo_oid_to_algop(repo, &entry->idx.oid, compat,
+			      &entry->idx.compat_oid))
+		die(_("can't map object %s while writing pack"), oid_to_hex(oid));
 	oe_set_type(entry, type);
 	if (exclude)
 		entry->preferred_base = 1;
-- 
2.41.0


^ permalink raw reply related

* [PATCH 18/32] object-file-convert: convert commit objects when writing
From: Eric W. Biederman @ 2023-09-08 23:10 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, brian m. carlson, Eric W . Biederman
In-Reply-To: <87sf7ol0z3.fsf@email.froward.int.ebiederm.org>

From: "brian m. carlson" <sandals@crustytoothpaste.net>

When writing a commit object in a repository with both SHA-1 and
SHA-256, we'll need to convert our commit objects so that we can write
the hash values for both into the repository.  To do so, let's add a
function to convert commit objects.

Read the commit object and map the tree value and any of the parent
values, and copy the rest of the commit through unmodified.  Note that
we don't need to modify the signature headers, because they are the same
under both algorithms.

****
- made static and moved to object-file-convert.c
- Renamed the variable compat_oid to mapped_oid for clarity
- Replaced repo_map_object with oid_to_algop
-- EWB

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 object-file-convert.c | 44 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/object-file-convert.c b/object-file-convert.c
index f266c8c6cc95..9c715a9864d5 100644
--- a/object-file-convert.c
+++ b/object-file-convert.c
@@ -83,6 +83,48 @@ static int convert_tree_object(struct strbuf *out,
 	return 0;
 }
 
+static int convert_commit_object(struct strbuf *out,
+				 const struct git_hash_algo *from,
+				 const struct git_hash_algo *to,
+				 const char *buffer, size_t size)
+{
+	const char *tail = buffer;
+	const char *bufptr = buffer;
+	const int tree_entry_len = from->hexsz + 5;
+	const int parent_entry_len = from->hexsz + 7;
+	struct object_id oid, mapped_oid;
+	const char *p;
+
+	tail += size;
+	if (tail <= bufptr + tree_entry_len + 1 || memcmp(bufptr, "tree ", 5) ||
+			bufptr[tree_entry_len] != '\n')
+		return error("bogus commit object");
+	if (parse_oid_hex_algop(bufptr + 5, &oid, &p, from) < 0)
+		return error("bad tree pointer");
+
+	if (repo_oid_to_algop(the_repository, &oid, to, &mapped_oid))
+		return error("unable to map tree %s in commit object",
+			     oid_to_hex(&oid));
+	strbuf_addf(out, "tree %s\n", oid_to_hex(&mapped_oid));
+	bufptr = p + 1;
+
+	while (bufptr + parent_entry_len < tail && !memcmp(bufptr, "parent ", 7)) {
+		if (tail <= bufptr + parent_entry_len + 1 ||
+		    parse_oid_hex_algop(bufptr + 7, &oid, &p, from) ||
+		    *p != '\n')
+			return error("bad parents in commit");
+
+		if (repo_oid_to_algop(the_repository, &oid, to, &mapped_oid))
+			return error("unable to map parent %s in commit object",
+				     oid_to_hex(&oid));
+
+		strbuf_addf(out, "parent %s\n", oid_to_hex(&mapped_oid));
+		bufptr = p + 1;
+	}
+	strbuf_add(out, bufptr, tail - bufptr);
+	return 0;
+}
+
 int convert_object_file(struct strbuf *outbuf,
 			const struct git_hash_algo *from,
 			const struct git_hash_algo *to,
@@ -101,6 +143,8 @@ int convert_object_file(struct strbuf *outbuf,
 		ret = convert_tree_object(outbuf, from, to, buf, len);
 		break;
 	case OBJ_COMMIT:
+		ret = convert_commit_object(outbuf, from, to, buf, len);
+		break;
 	case OBJ_TAG:
 	default:
 		/* Not implemented yet, so fail. */
-- 
2.41.0


^ permalink raw reply related

* [PATCH 17/32] object-file-convert: add a function to convert trees between algorithms
From: Eric W. Biederman @ 2023-09-08 23:10 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, brian m. carlson, Eric W . Biederman
In-Reply-To: <87sf7ol0z3.fsf@email.froward.int.ebiederm.org>

From: "brian m. carlson" <sandals@crustytoothpaste.net>

In the future, we're going to want to provide SHA-256 repositories that
have compatibility support for SHA-1 as well.  In order to do so, we'll
need to be able to convert tree objects from SHA-256 to SHA-1 by writing
a tree with each SHA-256 object ID mapped to a SHA-1 object ID.

We implement a function, convert_tree_object, that takes an existing
tree buffer and writes it to a new strbuf, converting between
algorithms.  Let's make this function generic, because while we only
need it to convert from the main algorithm to the compatibility
algorithm now, we may need to do the other way around in the future,
such as for transport.

We avoid reusing the code in decode_tree_entry because that code
normalizes data, and we don't want that here.  We want to produce a
complete round trip of data, so if, for example, the old entry had a
wrongly zero-padded mode, we'd want to preserve that when converting to
ensure a stable hash value.

****
- Removed the repository parameter to convert_tree_object
- Removed setting from and to defaults in convert_tree_object
- Replaced repo_map_object with oid_to_algop
- Replaced get_mode with parse_mode
- Made convert_tree_object static.
- Called convert_tree_object from convert_object_file.

-- EWB

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 object-file-convert.c | 51 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/object-file-convert.c b/object-file-convert.c
index e7c62434016d..f266c8c6cc95 100644
--- a/object-file-convert.c
+++ b/object-file-convert.c
@@ -1,8 +1,10 @@
 #include "git-compat-util.h"
 #include "gettext.h"
 #include "strbuf.h"
+#include "hex.h"
 #include "repository.h"
 #include "hash-ll.h"
+#include "hash.h"
 #include "object.h"
 #include "loose.h"
 #include "object-file-convert.h"
@@ -36,6 +38,51 @@ int repo_oid_to_algop(struct repository *repo, const struct object_id *src,
 	return 0;
 }
 
+static int decode_tree_entry_raw(struct object_id *oid, const char **path,
+				 size_t *len, const struct git_hash_algo *algo,
+				 const char *buf, unsigned long size)
+{
+	uint16_t mode;
+	const unsigned hashsz = algo->rawsz;
+
+	if (size < hashsz + 3 || buf[size - (hashsz + 1)]) {
+		return -1;
+	}
+
+	*path = parse_mode(buf, &mode);
+	if (!*path || !**path)
+		return -1;
+	*len = strlen(*path) + 1;
+
+	oidread_algop(oid, (const unsigned char *)*path + *len, algo);
+	return 0;
+}
+
+static int convert_tree_object(struct strbuf *out,
+			       const struct git_hash_algo *from,
+			       const struct git_hash_algo *to,
+			       const char *buffer, size_t size)
+{
+	const char *p = buffer, *end = buffer + size;
+
+	while (p < end) {
+		struct object_id entry_oid, mapped_oid;
+		const char *path = NULL;
+		size_t pathlen;
+
+		if (decode_tree_entry_raw(&entry_oid, &path, &pathlen, from, p,
+					  end - p))
+			return error(_("failed to decode tree entry"));
+		if (repo_oid_to_algop(the_repository, &entry_oid, to, &mapped_oid))
+			return error(_("failed to map tree entry for %s"), oid_to_hex(&entry_oid));
+		strbuf_add(out, p, path - p);
+		strbuf_add(out, path, pathlen);
+		strbuf_add(out, mapped_oid.hash, to->rawsz);
+		p = path + pathlen + from->rawsz;
+	}
+	return 0;
+}
+
 int convert_object_file(struct strbuf *outbuf,
 			const struct git_hash_algo *from,
 			const struct git_hash_algo *to,
@@ -50,8 +97,10 @@ int convert_object_file(struct strbuf *outbuf,
 		die("Refusing noop object file conversion");
 
 	switch (type) {
-	case OBJ_COMMIT:
 	case OBJ_TREE:
+		ret = convert_tree_object(outbuf, from, to, buf, len);
+		break;
+	case OBJ_COMMIT:
 	case OBJ_TAG:
 	default:
 		/* Not implemented yet, so fail. */
-- 
2.41.0


^ permalink raw reply related

* [PATCH 09/32] object-file: Update the loose object map when writing loose objects
From: Eric W. Biederman @ 2023-09-08 23:10 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, brian m. carlson, Eric W. Biederman
In-Reply-To: <87sf7ol0z3.fsf@email.froward.int.ebiederm.org>

To implement SHA1 compatibility on SHA256 repositories the loose
object map needs to be updated whenver a loose object is written.
Updating the loose object map this way allows git to support
the old hash algorithm in constant time.

The functions write_loose_object, and stream_loose_object are
the only two functions that write to the loose object store.

Update stream_loose_object to compute the compatibiilty hash, update
the loose object, and then call repo_add_loose_object_map to update
the loose object map.

Update write_object_file_flags to convert the object into
it's compatibility encoding, hash the compatibility encoding,
write the object, and then update the loose object map.

Update force_object_loose to lookup the hash of the compatibility
encoding, write the loose object, and then update the loose object
map.

Update write_object_file_litterally to refuse to write any objects
when a compatibility encoding is enabled.  The problem is that
write_object_file_literally is frequently used to write ill-formed
objects.  Especially when the type of those objects is changed there
is by definition no possibile way to convert them, as no converstion
has been defined.

Since a compatibilty encoding can not be found and a compatibility
mapping can not be written the cleanest behavior is to simply
disallow write_object_file_literraly from writing files.

Except that the loose objects are updated before the loose object map
I have not done any analysis to see how robust this scheme is in the
event of failure.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 object-file.c | 94 +++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 76 insertions(+), 18 deletions(-)

diff --git a/object-file.c b/object-file.c
index 7f24f19b8a68..6a14b8875343 100644
--- a/object-file.c
+++ b/object-file.c
@@ -44,6 +44,7 @@
 #include "setup.h"
 #include "submodule.h"
 #include "fsck.h"
+#include "loose.h"
 
 /* The maximum size for an object header. */
 #define MAX_HEADER_LEN 32
@@ -2035,9 +2036,12 @@ static int start_loose_object_common(struct strbuf *tmp_file,
 				     const char *filename, unsigned flags,
 				     git_zstream *stream,
 				     unsigned char *buf, size_t buflen,
-				     git_hash_ctx *c,
+				     git_hash_ctx *c, git_hash_ctx *compat_c,
 				     char *hdr, int hdrlen)
 {
+	struct repository *repo = the_repository;
+	const struct git_hash_algo *algo = repo->hash_algo;
+	const struct git_hash_algo *compat = repo->compat_hash_algo;
 	int fd;
 
 	fd = create_tmpfile(tmp_file, filename);
@@ -2057,14 +2061,18 @@ static int start_loose_object_common(struct strbuf *tmp_file,
 	git_deflate_init(stream, zlib_compression_level);
 	stream->next_out = buf;
 	stream->avail_out = buflen;
-	the_hash_algo->init_fn(c);
+	algo->init_fn(c);
+	if (compat && compat_c)
+		compat->init_fn(compat_c);
 
 	/*  Start to feed header to zlib stream */
 	stream->next_in = (unsigned char *)hdr;
 	stream->avail_in = hdrlen;
 	while (git_deflate(stream, 0) == Z_OK)
 		; /* nothing */
-	the_hash_algo->update_fn(c, hdr, hdrlen);
+	algo->update_fn(c, hdr, hdrlen);
+	if (compat && compat_c)
+		compat->update_fn(compat_c, hdr, hdrlen);
 
 	return fd;
 }
@@ -2073,16 +2081,21 @@ static int start_loose_object_common(struct strbuf *tmp_file,
  * Common steps for the inner git_deflate() loop for writing loose
  * objects. Returns what git_deflate() returns.
  */
-static int write_loose_object_common(git_hash_ctx *c,
+static int write_loose_object_common(git_hash_ctx *c, git_hash_ctx *compat_c,
 				     git_zstream *stream, const int flush,
 				     unsigned char *in0, const int fd,
 				     unsigned char *compressed,
 				     const size_t compressed_len)
 {
+	struct repository *repo = the_repository;
+	const struct git_hash_algo *algo = repo->hash_algo;
+	const struct git_hash_algo *compat = repo->compat_hash_algo;
 	int ret;
 
 	ret = git_deflate(stream, flush ? Z_FINISH : 0);
-	the_hash_algo->update_fn(c, in0, stream->next_in - in0);
+	algo->update_fn(c, in0, stream->next_in - in0);
+	if (compat && compat_c)
+		compat->update_fn(compat_c, in0, stream->next_in - in0);
 	if (write_in_full(fd, compressed, stream->next_out - compressed) < 0)
 		die_errno(_("unable to write loose object file"));
 	stream->next_out = compressed;
@@ -2097,15 +2110,21 @@ static int write_loose_object_common(git_hash_ctx *c,
  * - End the compression of zlib stream.
  * - Get the calculated oid to "oid".
  */
-static int end_loose_object_common(git_hash_ctx *c, git_zstream *stream,
-				   struct object_id *oid)
+static int end_loose_object_common(git_hash_ctx *c, git_hash_ctx *compat_c,
+				   git_zstream *stream, struct object_id *oid,
+				   struct object_id *compat_oid)
 {
+	struct repository *repo = the_repository;
+	const struct git_hash_algo *algo = repo->hash_algo;
+	const struct git_hash_algo *compat = repo->compat_hash_algo;
 	int ret;
 
 	ret = git_deflate_end_gently(stream);
 	if (ret != Z_OK)
 		return ret;
-	the_hash_algo->final_oid_fn(oid, c);
+	algo->final_oid_fn(oid, c);
+	if (compat && compat_c)
+		compat->final_oid_fn(compat_oid, compat_c);
 
 	return Z_OK;
 }
@@ -2129,7 +2148,7 @@ static int write_loose_object(const struct object_id *oid, char *hdr,
 
 	fd = start_loose_object_common(&tmp_file, filename.buf, flags,
 				       &stream, compressed, sizeof(compressed),
-				       &c, hdr, hdrlen);
+				       &c, NULL, hdr, hdrlen);
 	if (fd < 0)
 		return -1;
 
@@ -2139,14 +2158,14 @@ static int write_loose_object(const struct object_id *oid, char *hdr,
 	do {
 		unsigned char *in0 = stream.next_in;
 
-		ret = write_loose_object_common(&c, &stream, 1, in0, fd,
+		ret = write_loose_object_common(&c, NULL, &stream, 1, in0, fd,
 						compressed, sizeof(compressed));
 	} while (ret == Z_OK);
 
 	if (ret != Z_STREAM_END)
 		die(_("unable to deflate new object %s (%d)"), oid_to_hex(oid),
 		    ret);
-	ret = end_loose_object_common(&c, &stream, &parano_oid);
+	ret = end_loose_object_common(&c, NULL, &stream, &parano_oid, NULL);
 	if (ret != Z_OK)
 		die(_("deflateEnd on object %s failed (%d)"), oid_to_hex(oid),
 		    ret);
@@ -2191,10 +2210,12 @@ static int freshen_packed_object(const struct object_id *oid)
 int stream_loose_object(struct input_stream *in_stream, size_t len,
 			struct object_id *oid)
 {
+	const struct git_hash_algo *compat = the_repository->compat_hash_algo;
+	struct object_id compat_oid;
 	int fd, ret, err = 0, flush = 0;
 	unsigned char compressed[4096];
 	git_zstream stream;
-	git_hash_ctx c;
+	git_hash_ctx c, compat_c;
 	struct strbuf tmp_file = STRBUF_INIT;
 	struct strbuf filename = STRBUF_INIT;
 	int dirlen;
@@ -2218,7 +2239,7 @@ int stream_loose_object(struct input_stream *in_stream, size_t len,
 	 */
 	fd = start_loose_object_common(&tmp_file, filename.buf, 0,
 				       &stream, compressed, sizeof(compressed),
-				       &c, hdr, hdrlen);
+				       &c, &compat_c, hdr, hdrlen);
 	if (fd < 0) {
 		err = -1;
 		goto cleanup;
@@ -2236,7 +2257,7 @@ int stream_loose_object(struct input_stream *in_stream, size_t len,
 			if (in_stream->is_finished)
 				flush = 1;
 		}
-		ret = write_loose_object_common(&c, &stream, flush, in0, fd,
+		ret = write_loose_object_common(&c, &compat_c, &stream, flush, in0, fd,
 						compressed, sizeof(compressed));
 		/*
 		 * Unlike write_loose_object(), we do not have the entire
@@ -2259,7 +2280,7 @@ int stream_loose_object(struct input_stream *in_stream, size_t len,
 	 */
 	if (ret != Z_STREAM_END)
 		die(_("unable to stream deflate new object (%d)"), ret);
-	ret = end_loose_object_common(&c, &stream, oid);
+	ret = end_loose_object_common(&c, &compat_c, &stream, oid, &compat_oid);
 	if (ret != Z_OK)
 		die(_("deflateEnd on stream object failed (%d)"), ret);
 	close_loose_object(fd, tmp_file.buf);
@@ -2286,6 +2307,8 @@ int stream_loose_object(struct input_stream *in_stream, size_t len,
 	}
 
 	err = finalize_object_file(tmp_file.buf, filename.buf);
+	if (!err && compat)
+		err = repo_add_loose_object_map(the_repository, oid, &compat_oid);
 cleanup:
 	strbuf_release(&tmp_file);
 	strbuf_release(&filename);
@@ -2296,17 +2319,38 @@ int write_object_file_flags(const void *buf, unsigned long len,
 			    enum object_type type, struct object_id *oid,
 			    unsigned flags)
 {
+	struct repository *repo = the_repository;
+	const struct git_hash_algo *algo = repo->hash_algo;
+	const struct git_hash_algo *compat = repo->compat_hash_algo;
+	struct object_id compat_oid;
 	char hdr[MAX_HEADER_LEN];
 	int hdrlen = sizeof(hdr);
 
+	/* Generate compat_oid */
+	if (compat) {
+		if (type == OBJ_BLOB)
+			hash_object_file(compat, buf, len, type, &compat_oid);
+		else {
+			struct strbuf converted = STRBUF_INIT;
+			convert_object_file(&converted, algo, compat,
+					    buf, len, type, 0);
+			hash_object_file(compat, converted.buf, converted.len,
+					 type, &compat_oid);
+			strbuf_release(&converted);
+		}
+	}
+
 	/* Normally if we have it in the pack then we do not bother writing
 	 * it out into .git/objects/??/?{38} file.
 	 */
-	write_object_file_prepare(the_hash_algo, buf, len, type, oid, hdr,
-				  &hdrlen);
+	write_object_file_prepare(algo, buf, len, type, oid, hdr, &hdrlen);
 	if (freshen_packed_object(oid) || freshen_loose_object(oid))
 		return 0;
-	return write_loose_object(oid, hdr, hdrlen, buf, len, 0, flags);
+	if (write_loose_object(oid, hdr, hdrlen, buf, len, 0, flags))
+		return -1;
+	if (compat)
+		return repo_add_loose_object_map(repo, oid, &compat_oid);
+	return 0;
 }
 
 int write_object_file_literally(const void *buf, unsigned long len,
@@ -2324,6 +2368,10 @@ int write_object_file_literally(const void *buf, unsigned long len,
 
 	if (!(flags & HASH_WRITE_OBJECT))
 		goto cleanup;
+	else if (the_repository->compat_hash_algo) {
+		status = -1;
+		goto cleanup;
+	}
 	if (freshen_packed_object(oid) || freshen_loose_object(oid))
 		goto cleanup;
 	status = write_loose_object(oid, header, hdrlen, buf, len, 0, 0);
@@ -2335,9 +2383,12 @@ int write_object_file_literally(const void *buf, unsigned long len,
 
 int force_object_loose(const struct object_id *oid, time_t mtime)
 {
+	struct repository *repo = the_repository;
+	const struct git_hash_algo *compat = repo->compat_hash_algo;
 	void *buf;
 	unsigned long len;
 	struct object_info oi = OBJECT_INFO_INIT;
+	struct object_id compat_oid;
 	enum object_type type;
 	char hdr[MAX_HEADER_LEN];
 	int hdrlen;
@@ -2350,8 +2401,15 @@ int force_object_loose(const struct object_id *oid, time_t mtime)
 	oi.contentp = &buf;
 	if (oid_object_info_extended(the_repository, oid, &oi, 0))
 		return error(_("cannot read object for %s"), oid_to_hex(oid));
+	if (compat) {
+		if (repo_oid_to_algop(repo, oid, compat, &compat_oid))
+			return error(_("cannot map object %s to %s"),
+				     oid_to_hex(oid), compat->name);
+	}
 	hdrlen = format_object_header(hdr, sizeof(hdr), type, len);
 	ret = write_loose_object(oid, hdr, hdrlen, buf, len, mtime, 0);
+	if (!ret && compat)
+		ret = repo_add_loose_object_map(the_repository, oid, &compat_oid);
 	free(buf);
 
 	return ret;
-- 
2.41.0


^ permalink raw reply related

* [PATCH 11/32] pack: Communicate the compat_oid through struct pack_idx_entry
From: Eric W. Biederman @ 2023-09-08 23:10 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, brian m. carlson, Eric W. Biederman
In-Reply-To: <87sf7ol0z3.fsf@email.froward.int.ebiederm.org>

Add compat_oid into struct pack_idx_entry to allow communicating the
the compat hash value of the objects being indexed to the code that
builds the indexes for a pack.

Having a mechanism that communicates the compat_oid from the code
building the pack is necessary for bulk-checkin, fast-import, and
index-pack.  Only pack-objects could rely on the existing
comaptibility mappings, but there is not point since the
other creators of indexes can't.

Unfortunately this adds a 4 byte hole into struct pack_idx_entry.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 pack.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/pack.h b/pack.h
index 3ab9e3f60c0b..321d38374f70 100644
--- a/pack.h
+++ b/pack.h
@@ -75,6 +75,7 @@ struct pack_idx_header {
  */
 struct pack_idx_entry {
 	struct object_id oid;
+	struct object_id compat_oid;
 	uint32_t crc32;
 	off_t offset;
 };
-- 
2.41.0


^ permalink raw reply related

* [PATCH 05/32] repository: add a compatibility hash algorithm
From: Eric W. Biederman @ 2023-09-08 23:10 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, brian m. carlson, Eric W. Biederman
In-Reply-To: <87sf7ol0z3.fsf@email.froward.int.ebiederm.org>

We currently have support for using a full stage 4 SHA-256
implementation.  However, we'd like to support interoperability with
SHA-1 repositories as well.  The transition plan anticipates a
compatibility hash algorithm configuration option that we can use to
implement support for this.  Let's add an element to the repository
structure that indicates the compatibility hash algorithm so we can use
it when we need to consider interoperability between algorithms.

For now, we always set it to NULL, but we'll initialize it differently
in the future.

Inspired-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 repository.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/repository.h b/repository.h
index 5f18486f6465..6c4130f0c36e 100644
--- a/repository.h
+++ b/repository.h
@@ -160,6 +160,9 @@ struct repository {
 	/* Repository's current hash algorithm, as serialized on disk. */
 	const struct git_hash_algo *hash_algo;
 
+	/* Repository's compatibility hash algorithm. */
+	const struct git_hash_algo *compat_hash_algo;
+
 	/* A unique-id for tracing purposes. */
 	int trace2_repo_id;
 
-- 
2.41.0


^ permalink raw reply related

* [PATCH 13/32] object-file: Add a compat_oid_in parameter to write_object_file_flags
From: Eric W. Biederman @ 2023-09-08 23:10 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, brian m. carlson, Eric W. Biederman
In-Reply-To: <87sf7ol0z3.fsf@email.froward.int.ebiederm.org>

To create the proper signatures for commit objects both versions of
the commit object need to be generated and signed.  After that it is
a waste to throw away the work of generating the compatibility hash
so update write_object_file_flags to take a compatibility hash input
parameter that it can use to skip the work of generating the
compatability hash.

Update the places that don't generate the compatability hash to
pass NULL so it is easy to tell write_object_file_flags should
not attempt to use their compatability hash.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 cache-tree.c      | 2 +-
 object-file.c     | 6 ++++--
 object-store-ll.h | 4 ++--
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/cache-tree.c b/cache-tree.c
index 641427ed410a..ddc7d3d86959 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -448,7 +448,7 @@ static int update_one(struct cache_tree *it,
 		hash_object_file(the_hash_algo, buffer.buf, buffer.len,
 				 OBJ_TREE, &it->oid);
 	} else if (write_object_file_flags(buffer.buf, buffer.len, OBJ_TREE,
-					   &it->oid, flags & WRITE_TREE_SILENT
+					   &it->oid, NULL, flags & WRITE_TREE_SILENT
 					   ? HASH_SILENT : 0)) {
 		strbuf_release(&buffer);
 		return -1;
diff --git a/object-file.c b/object-file.c
index 6cc4ae1fd957..fd420dd303df 100644
--- a/object-file.c
+++ b/object-file.c
@@ -2317,7 +2317,7 @@ int stream_loose_object(struct input_stream *in_stream, size_t len,
 
 int write_object_file_flags(const void *buf, unsigned long len,
 			    enum object_type type, struct object_id *oid,
-			    unsigned flags)
+			    struct object_id *compat_oid_in, unsigned flags)
 {
 	struct repository *repo = the_repository;
 	const struct git_hash_algo *algo = repo->hash_algo;
@@ -2328,7 +2328,9 @@ int write_object_file_flags(const void *buf, unsigned long len,
 
 	/* Generate compat_oid */
 	if (compat) {
-		if (type == OBJ_BLOB)
+		if (compat_oid_in)
+			oidcpy(&compat_oid, compat_oid_in);
+		else if (type == OBJ_BLOB)
 			hash_object_file(compat, buf, len, type, &compat_oid);
 		else {
 			struct strbuf converted = STRBUF_INIT;
diff --git a/object-store-ll.h b/object-store-ll.h
index bc76d6bec80d..c5f2bb2fc2fe 100644
--- a/object-store-ll.h
+++ b/object-store-ll.h
@@ -255,11 +255,11 @@ void hash_object_file(const struct git_hash_algo *algo, const void *buf,
 
 int write_object_file_flags(const void *buf, unsigned long len,
 			    enum object_type type, struct object_id *oid,
-			    unsigned flags);
+			    struct object_id *comapt_oid_in, unsigned flags);
 static inline int write_object_file(const void *buf, unsigned long len,
 				    enum object_type type, struct object_id *oid)
 {
-	return write_object_file_flags(buf, len, type, oid, 0);
+	return write_object_file_flags(buf, len, type, oid, NULL, 0);
 }
 
 int write_object_file_literally(const void *buf, unsigned long len,
-- 
2.41.0


^ permalink raw reply related

* Re: [PATCH v3 6/6] git-std-lib: add test file to call git-std-lib.a functions
From: Junio C Hamano @ 2023-09-09  5:26 UTC (permalink / raw)
  To: Calvin Wan; +Cc: git, nasamuffin, jonathantanmy, linusa, phillip.wood123, vdye
In-Reply-To: <20230908174443.1027716-6-calvinwan@google.com>

Calvin Wan <calvinwan@google.com> writes:

> +
> +test-git-std-lib:
> +	cc -It -o stdlib-test stdlib-test.c -L. -l:../git-std-lib.a

Yuck, no.  Try to share as much with the main Makefile one level up.

> +	./stdlib-test
> diff --git a/t/stdlib-test.c b/t/stdlib-test.c
> new file mode 100644
> index 0000000000..76fed9ecbf
> --- /dev/null
> +++ b/t/stdlib-test.c
> @@ -0,0 +1,231 @@
> +#include "../git-compat-util.h"
> +#include "../abspath.h"
> +#include "../hex-ll.h"
> +#include "../parse.h"
> +#include "../strbuf.h"
> +#include "../string-list.h"

Use -I.. or something, to match what the main Makefile does, so that
you do not have to have these "../".  With -I.., you could even say

    #include <hex-ll.h>
    #include <parse.h>

etc.


> +	// skip_to_optional_arg_default(const char *str, const char *prefix,
> +	// 			 const char **arg, const char *def)

No // comments in this codebase, please.

> +	strbuf_addchars(sb, 1, 1);
> +	strbuf_addf(sb, "%s", "foo");

https://github.com/git/git/actions/runs/6126669144/job/16631124765#step:4:657


^ permalink raw reply

* [PATCH v2 1/6] trailer: separate public from internal portion of trailer_iterator
From: Linus Arver via GitGitGadget @ 2023-09-09  6:16 UTC (permalink / raw)
  To: git; +Cc: Glen Choo, Christian Couder, Phillip Wood, Linus Arver,
	Linus Arver
In-Reply-To: <pull.1563.v2.git.1694240177.gitgitgadget@gmail.com>

From: Linus Arver <linusa@google.com>

The fields here are not meant to be used by downstream callers, so put
them behind an anonymous struct named as "internal" to warn against
their use. This follows the pattern in 576de3d956 (unpack_trees: start
splitting internal fields from public API, 2023-02-27).

Signed-off-by: Linus Arver <linusa@google.com>
---
 trailer.c | 10 +++++-----
 trailer.h |  6 ++++--
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/trailer.c b/trailer.c
index f408f9b058d..de4bdece847 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1220,14 +1220,14 @@ void trailer_iterator_init(struct trailer_iterator *iter, const char *msg)
 	strbuf_init(&iter->key, 0);
 	strbuf_init(&iter->val, 0);
 	opts.no_divider = 1;
-	trailer_info_get(&iter->info, msg, &opts);
-	iter->cur = 0;
+	trailer_info_get(&iter->internal.info, msg, &opts);
+	iter->internal.cur = 0;
 }
 
 int trailer_iterator_advance(struct trailer_iterator *iter)
 {
-	while (iter->cur < iter->info.trailer_nr) {
-		char *trailer = iter->info.trailers[iter->cur++];
+	while (iter->internal.cur < iter->internal.info.trailer_nr) {
+		char *trailer = iter->internal.info.trailers[iter->internal.cur++];
 		int separator_pos = find_separator(trailer, separators);
 
 		if (separator_pos < 1)
@@ -1245,7 +1245,7 @@ int trailer_iterator_advance(struct trailer_iterator *iter)
 
 void trailer_iterator_release(struct trailer_iterator *iter)
 {
-	trailer_info_release(&iter->info);
+	trailer_info_release(&iter->internal.info);
 	strbuf_release(&iter->val);
 	strbuf_release(&iter->key);
 }
diff --git a/trailer.h b/trailer.h
index 795d2fccfd9..ab2cd017567 100644
--- a/trailer.h
+++ b/trailer.h
@@ -119,8 +119,10 @@ struct trailer_iterator {
 	struct strbuf val;
 
 	/* private */
-	struct trailer_info info;
-	size_t cur;
+	struct {
+		struct trailer_info info;
+		size_t cur;
+	} internal;
 };
 
 /*
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v2 0/6] Trailer readability cleanups
From: Linus Arver via GitGitGadget @ 2023-09-09  6:16 UTC (permalink / raw)
  To: git; +Cc: Glen Choo, Christian Couder, Phillip Wood, Linus Arver
In-Reply-To: <pull.1563.git.1691211879.gitgitgadget@gmail.com>

These patches were created while digging into the trailer code to better
understand how it works, in preparation for making the trailer.{c,h} files
as small as possible to make them available as a library for external users.
This series was originally created as part of [1], but are sent here
separately because the changes here are arguably more subjective in nature.
I think Patch 1 is the most important in this series. The others can wait,
if folks are opposed to adding them on their own merits at this point in
time.

These patches do not add or change any features. Instead, their goal is to
make the code easier to understand for new contributors (like myself), by
making various cleanups and improvements. Ultimately, my hope is that with
such cleanups, we are better positioned to make larger changes (especially
the broader libification effort, as in "Introduce Git Standard Library"
[2]).

Patch 1 was inspired by 576de3d956 (unpack_trees: start splitting internal
fields from public API, 2023-02-27) [3], and is in preparation for a
libification effort in the future around the trailer code. Independent of
libification, it still makes sense to discourage callers from peeking into
these trailer-internal fields.

Patches 2-3 aim to make some functions do a little less multitasking.

Patch 4 makes the find_patch_start function care about the "--no-divider"
option, because it that option matters for determining the start of the
"patch part" of the input.

Patch 5 is a renaming change to reduce overloaded language in the codebase.
It is inspired by 229d6ab6bf (doc: trailer: examples: avoid the word
"message" by itself, 2023-06-15) [4], which did a similar thing for the
interpret-trailers documentation.

Patch 6 makes trailer_info use offsets for trailer_start and trailer_end.


Updates in v2
=============

 * Patch 1: Drop the use of a #define. Instead just use an anonymous struct
   named internal.
 * Patch 2: Don't free info out parameter inside parse_trailers(). Instead
   free it from the caller, process_trailers(). Update comment in
   parse_trailers().
 * Patch 3: Reword commit message.
 * Patch 4: Mention be3d654343 (commit: pass --no-divider to
   interpret-trailers, 2023-06-17) in commit message.
 * Added Patch 6 to make trailer_info use offsets for trailer_start and
   trailer_end (thanks to Glen Choo for the suggestion).

[1]
https://lore.kernel.org/git/pull.1564.git.1691210737.gitgitgadget@gmail.com/T/#mb044012670663d8eb7a548924bbcc933bef116de
[2]
https://lore.kernel.org/git/20230627195251.1973421-1-calvinwan@google.com/
[3]
https://lore.kernel.org/git/pull.1149.git.1677143700.gitgitgadget@gmail.com/
[4]
https://lore.kernel.org/git/6b4cb31b17077181a311ca87e82464a1e2ad67dd.1686797630.git.gitgitgadget@gmail.com/

Linus Arver (6):
  trailer: separate public from internal portion of trailer_iterator
  trailer: split process_input_file into separate pieces
  trailer: split process_command_line_args into separate functions
  trailer: teach find_patch_start about --no-divider
  trailer: rename *_DEFAULT enums to *_UNSPECIFIED
  trailer: use offsets for trailer_start/trailer_end

 trailer.c | 126 +++++++++++++++++++++++++++++-------------------------
 trailer.h |  19 ++++----
 2 files changed, 77 insertions(+), 68 deletions(-)


base-commit: 1b0a5129563ebe720330fdc8f5c6843d27641137
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1563%2Flistx%2Ftrailer-libification-prep-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1563/listx/trailer-libification-prep-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1563

Range-diff vs v1:

 1:  0bce4d4b0d5 ! 1:  4f116d2550f trailer: separate public from internal portion of trailer_iterator
     @@ Commit message
          trailer: separate public from internal portion of trailer_iterator
      
          The fields here are not meant to be used by downstream callers, so put
     -    them behind an anonymous struct named as
     -    "__private_to_trailer_c__do_not_use" to warn against their use.
     +    them behind an anonymous struct named as "internal" to warn against
     +    their use. This follows the pattern in 576de3d956 (unpack_trees: start
     +    splitting internal fields from public API, 2023-02-27).
      
     -    Internally, use a "#define" to keep the code tidy.
     -
     -    Helped-by: Junio C Hamano <gitster@pobox.com>
          Signed-off-by: Linus Arver <linusa@google.com>
      
       ## trailer.c ##
     -@@ trailer.c: void format_trailers_from_commit(struct strbuf *out, const char *msg,
     - 	trailer_info_release(&info);
     - }
     - 
     -+#define private __private_to_trailer_c__do_not_use
     -+
     - void trailer_iterator_init(struct trailer_iterator *iter, const char *msg)
     - {
     - 	struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
     +@@ trailer.c: void trailer_iterator_init(struct trailer_iterator *iter, const char *msg)
       	strbuf_init(&iter->key, 0);
       	strbuf_init(&iter->val, 0);
       	opts.no_divider = 1;
      -	trailer_info_get(&iter->info, msg, &opts);
      -	iter->cur = 0;
     -+	trailer_info_get(&iter->private.info, msg, &opts);
     -+	iter->private.cur = 0;
     ++	trailer_info_get(&iter->internal.info, msg, &opts);
     ++	iter->internal.cur = 0;
       }
       
       int trailer_iterator_advance(struct trailer_iterator *iter)
       {
      -	while (iter->cur < iter->info.trailer_nr) {
      -		char *trailer = iter->info.trailers[iter->cur++];
     -+	while (iter->private.cur < iter->private.info.trailer_nr) {
     -+		char *trailer = iter->private.info.trailers[iter->private.cur++];
     ++	while (iter->internal.cur < iter->internal.info.trailer_nr) {
     ++		char *trailer = iter->internal.info.trailers[iter->internal.cur++];
       		int separator_pos = find_separator(trailer, separators);
       
       		if (separator_pos < 1)
     @@ trailer.c: int trailer_iterator_advance(struct trailer_iterator *iter)
       void trailer_iterator_release(struct trailer_iterator *iter)
       {
      -	trailer_info_release(&iter->info);
     -+	trailer_info_release(&iter->private.info);
     ++	trailer_info_release(&iter->internal.info);
       	strbuf_release(&iter->val);
       	strbuf_release(&iter->key);
       }
     @@ trailer.h: struct trailer_iterator {
      +	struct {
      +		struct trailer_info info;
      +		size_t cur;
     -+	} __private_to_trailer_c__do_not_use;
     ++	} internal;
       };
       
       /*
 2:  d023c297dca ! 2:  c00f4623d0b trailer: split process_input_file into separate pieces
     @@ trailer.c: static void unfold_value(struct strbuf *val)
      -				 struct list_head *head,
      -				 const struct process_trailer_options *opts)
      +/*
     -+ * Parse trailers in "str" and populate the "head" linked list structure.
     ++ * Parse trailers in "str", populating the trailer info and "head"
     ++ * linked list structure.
      + */
      +static void parse_trailers(struct trailer_info *info,
      +			     const char *str,
     @@ trailer.c: static void unfold_value(struct strbuf *val)
       			continue;
       		separator_pos = find_separator(trailer, separators);
      @@ trailer.c: static size_t process_input_file(FILE *outfile,
     + 					 strbuf_detach(&val, NULL));
       		}
       	}
     - 
     +-
      -	trailer_info_release(&info);
      -
      -	return info.trailer_end - str;
     -+	trailer_info_release(info);
       }
       
       static void free_all(struct list_head *head)
     @@ trailer.c: void process_trailers(const char *file,
       
       	if (!opts->only_input) {
       		LIST_HEAD(arg_head);
     +@@ trailer.c: void process_trailers(const char *file,
     + 	print_all(outfile, &head, opts);
     + 
     + 	free_all(&head);
     ++	trailer_info_release(&info);
     + 
     + 	/* Print the lines after the trailers as is */
     + 	if (!opts->only_trailers)
 3:  c8bb0136621 ! 3:  f78c2345fad trailer: split process_command_line_args into separate functions
     @@ Commit message
              (1) parse trailers from the configuration, and
              (2) parse trailers defined on the command line.
      
     -    Separate these concerns into parse_trailers_from_config and
     -    parse_trailers_from_command_line_args, respectively. Remove (now
     -    redundant) process_command_line_args.
     +    Separate (1) outside to a new function, parse_trailers_from_config.
     +    Rename the remaining logic to parse_trailers_from_command_line_args.
      
          Signed-off-by: Linus Arver <linusa@google.com>
      
 4:  1fc060041db ! 4:  f5f507c4c6c trailer: teach find_patch_start about --no-divider
     @@ Commit message
      
          Instead, make find_patch_start aware of "--no-divider" and make it
          handle that case as well. This means we no longer need to call strlen at
     -    all and can just rely on the existing code in find_patch_start.
     +    all and can just rely on the existing code in find_patch_start. By
     +    forcing callers to consider this important option, we avoid the kind of
     +    mistake described in be3d654343 (commit: pass --no-divider to
     +    interpret-trailers, 2023-06-17).
      
          This patch will make unit testing a bit more pleasant in this area in
          the future when we adopt a unit testing framework, because we would not
 5:  7c9b63c2616 ! 5:  52958c3557c trailer: rename *_DEFAULT enums to *_UNSPECIFIED
     @@ Commit message
          (2) "Default" can also mean the "trailer.*" configurations themselves,
              because these configurations are used by "default" (ahead of the
              hardcoded defaults in (1)) if no command line arguments are
     -        provided.
     +        provided. This concept of defaulting back to the configurations was
     +        introduced in 0ea5292e6b (interpret-trailers: add options for
     +        actions, 2017-08-01).
      
          In addition, the corresponding *_DEFAULT values are chosen when the user
          provides the "--no-where", "--no-if-exists", or "--no-if-missing" flags
 -:  ----------- > 6:  0463066ebe0 trailer: use offsets for trailer_start/trailer_end

-- 
gitgitgadget

^ permalink raw reply

* [PATCH v2 2/6] trailer: split process_input_file into separate pieces
From: Linus Arver via GitGitGadget @ 2023-09-09  6:16 UTC (permalink / raw)
  To: git; +Cc: Glen Choo, Christian Couder, Phillip Wood, Linus Arver,
	Linus Arver
In-Reply-To: <pull.1563.v2.git.1694240177.gitgitgadget@gmail.com>

From: Linus Arver <linusa@google.com>

Currently, process_input_file does three things:

    (1) parse the input string for trailers,
    (2) print text before the trailers, and
    (3) calculate the position of the input where the trailers end.

Rename this function to parse_trailers(), and make it only do
(1). The caller of this function, process_trailers, becomes responsible
for (2) and (3). These items belong inside process_trailers because they
are both concerned with printing the surrounding text around
trailers (which is already one of the immediate concerns of
process_trailers).

Signed-off-by: Linus Arver <linusa@google.com>
---
 trailer.c | 42 ++++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/trailer.c b/trailer.c
index de4bdece847..2c56cbc4a2e 100644
--- a/trailer.c
+++ b/trailer.c
@@ -961,28 +961,24 @@ static void unfold_value(struct strbuf *val)
 	strbuf_release(&out);
 }
 
-static size_t process_input_file(FILE *outfile,
-				 const char *str,
-				 struct list_head *head,
-				 const struct process_trailer_options *opts)
+/*
+ * Parse trailers in "str", populating the trailer info and "head"
+ * linked list structure.
+ */
+static void parse_trailers(struct trailer_info *info,
+			     const char *str,
+			     struct list_head *head,
+			     const struct process_trailer_options *opts)
 {
-	struct trailer_info info;
 	struct strbuf tok = STRBUF_INIT;
 	struct strbuf val = STRBUF_INIT;
 	size_t i;
 
-	trailer_info_get(&info, str, opts);
-
-	/* Print lines before the trailers as is */
-	if (!opts->only_trailers)
-		fwrite(str, 1, info.trailer_start - str, outfile);
+	trailer_info_get(info, str, opts);
 
-	if (!opts->only_trailers && !info.blank_line_before_trailer)
-		fprintf(outfile, "\n");
-
-	for (i = 0; i < info.trailer_nr; i++) {
+	for (i = 0; i < info->trailer_nr; i++) {
 		int separator_pos;
-		char *trailer = info.trailers[i];
+		char *trailer = info->trailers[i];
 		if (trailer[0] == comment_line_char)
 			continue;
 		separator_pos = find_separator(trailer, separators);
@@ -1002,10 +998,6 @@ static size_t process_input_file(FILE *outfile,
 					 strbuf_detach(&val, NULL));
 		}
 	}
-
-	trailer_info_release(&info);
-
-	return info.trailer_end - str;
 }
 
 static void free_all(struct list_head *head)
@@ -1054,6 +1046,7 @@ void process_trailers(const char *file,
 {
 	LIST_HEAD(head);
 	struct strbuf sb = STRBUF_INIT;
+	struct trailer_info info;
 	size_t trailer_end;
 	FILE *outfile = stdout;
 
@@ -1064,8 +1057,16 @@ void process_trailers(const char *file,
 	if (opts->in_place)
 		outfile = create_in_place_tempfile(file);
 
+	parse_trailers(&info, sb.buf, &head, opts);
+	trailer_end = info.trailer_end - sb.buf;
+
 	/* Print the lines before the trailers */
-	trailer_end = process_input_file(outfile, sb.buf, &head, opts);
+	if (!opts->only_trailers)
+		fwrite(sb.buf, 1, info.trailer_start - sb.buf, outfile);
+
+	if (!opts->only_trailers && !info.blank_line_before_trailer)
+		fprintf(outfile, "\n");
+
 
 	if (!opts->only_input) {
 		LIST_HEAD(arg_head);
@@ -1076,6 +1077,7 @@ void process_trailers(const char *file,
 	print_all(outfile, &head, opts);
 
 	free_all(&head);
+	trailer_info_release(&info);
 
 	/* Print the lines after the trailers as is */
 	if (!opts->only_trailers)
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v2 3/6] trailer: split process_command_line_args into separate functions
From: Linus Arver via GitGitGadget @ 2023-09-09  6:16 UTC (permalink / raw)
  To: git; +Cc: Glen Choo, Christian Couder, Phillip Wood, Linus Arver,
	Linus Arver
In-Reply-To: <pull.1563.v2.git.1694240177.gitgitgadget@gmail.com>

From: Linus Arver <linusa@google.com>

Previously, process_command_line_args did two things:

    (1) parse trailers from the configuration, and
    (2) parse trailers defined on the command line.

Separate (1) outside to a new function, parse_trailers_from_config.
Rename the remaining logic to parse_trailers_from_command_line_args.

Signed-off-by: Linus Arver <linusa@google.com>
---
 trailer.c | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/trailer.c b/trailer.c
index 2c56cbc4a2e..b6de5d9cb2d 100644
--- a/trailer.c
+++ b/trailer.c
@@ -711,30 +711,35 @@ static void add_arg_item(struct list_head *arg_head, char *tok, char *val,
 	list_add_tail(&new_item->list, arg_head);
 }
 
-static void process_command_line_args(struct list_head *arg_head,
-				      struct list_head *new_trailer_head)
+static void parse_trailers_from_config(struct list_head *config_head)
 {
 	struct arg_item *item;
-	struct strbuf tok = STRBUF_INIT;
-	struct strbuf val = STRBUF_INIT;
-	const struct conf_info *conf;
 	struct list_head *pos;
 
-	/*
-	 * In command-line arguments, '=' is accepted (in addition to the
-	 * separators that are defined).
-	 */
-	char *cl_separators = xstrfmt("=%s", separators);
-
 	/* Add an arg item for each configured trailer with a command */
 	list_for_each(pos, &conf_head) {
 		item = list_entry(pos, struct arg_item, list);
 		if (item->conf.command)
-			add_arg_item(arg_head,
+			add_arg_item(config_head,
 				     xstrdup(token_from_item(item, NULL)),
 				     xstrdup(""),
 				     &item->conf, NULL);
 	}
+}
+
+static void parse_trailers_from_command_line_args(struct list_head *arg_head,
+						  struct list_head *new_trailer_head)
+{
+	struct strbuf tok = STRBUF_INIT;
+	struct strbuf val = STRBUF_INIT;
+	const struct conf_info *conf;
+	struct list_head *pos;
+
+	/*
+	 * In command-line arguments, '=' is accepted (in addition to the
+	 * separators that are defined).
+	 */
+	char *cl_separators = xstrfmt("=%s", separators);
 
 	/* Add an arg item for each trailer on the command line */
 	list_for_each(pos, new_trailer_head) {
@@ -1069,8 +1074,11 @@ void process_trailers(const char *file,
 
 
 	if (!opts->only_input) {
+		LIST_HEAD(config_head);
 		LIST_HEAD(arg_head);
-		process_command_line_args(&arg_head, new_trailer_head);
+		parse_trailers_from_config(&config_head);
+		parse_trailers_from_command_line_args(&arg_head, new_trailer_head);
+		list_splice(&config_head, &arg_head);
 		process_trailers_lists(&head, &arg_head);
 	}
 
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v2 4/6] trailer: teach find_patch_start about --no-divider
From: Linus Arver via GitGitGadget @ 2023-09-09  6:16 UTC (permalink / raw)
  To: git; +Cc: Glen Choo, Christian Couder, Phillip Wood, Linus Arver,
	Linus Arver
In-Reply-To: <pull.1563.v2.git.1694240177.gitgitgadget@gmail.com>

From: Linus Arver <linusa@google.com>

Currently, find_patch_start only finds the start of the patch part of
the input (by looking at the "---" divider) for cases where the
"--no-divider" flag has not been provided. If the user provides this
flag, we do not rely on find_patch_start at all and just call strlen()
directly on the input.

Instead, make find_patch_start aware of "--no-divider" and make it
handle that case as well. This means we no longer need to call strlen at
all and can just rely on the existing code in find_patch_start. By
forcing callers to consider this important option, we avoid the kind of
mistake described in be3d654343 (commit: pass --no-divider to
interpret-trailers, 2023-06-17).

This patch will make unit testing a bit more pleasant in this area in
the future when we adopt a unit testing framework, because we would not
have to test multiple functions to check how finding the start of a
patch part works (we would only need to test find_patch_start).

Signed-off-by: Linus Arver <linusa@google.com>
---
 trailer.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/trailer.c b/trailer.c
index b6de5d9cb2d..f646e484a23 100644
--- a/trailer.c
+++ b/trailer.c
@@ -812,14 +812,14 @@ static ssize_t last_line(const char *buf, size_t len)
  * Return the position of the start of the patch or the length of str if there
  * is no patch in the message.
  */
-static size_t find_patch_start(const char *str)
+static size_t find_patch_start(const char *str, int no_divider)
 {
 	const char *s;
 
 	for (s = str; *s; s = next_line(s)) {
 		const char *v;
 
-		if (skip_prefix(s, "---", &v) && isspace(*v))
+		if (!no_divider && skip_prefix(s, "---", &v) && isspace(*v))
 			return s - str;
 	}
 
@@ -1109,11 +1109,7 @@ void trailer_info_get(struct trailer_info *info, const char *str,
 
 	ensure_configured();
 
-	if (opts->no_divider)
-		patch_start = strlen(str);
-	else
-		patch_start = find_patch_start(str);
-
+	patch_start = find_patch_start(str, opts->no_divider);
 	trailer_end = find_trailer_end(str, patch_start);
 	trailer_start = find_trailer_start(str, trailer_end);
 
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v2 6/6] trailer: use offsets for trailer_start/trailer_end
From: Linus Arver via GitGitGadget @ 2023-09-09  6:16 UTC (permalink / raw)
  To: git; +Cc: Glen Choo, Christian Couder, Phillip Wood, Linus Arver,
	Linus Arver
In-Reply-To: <pull.1563.v2.git.1694240177.gitgitgadget@gmail.com>

From: Linus Arver <linusa@google.com>

Previously these fields in the trailer_info struct were of type "const
char *" and pointed to positions in the input string directly (to the
start and end positions of the trailer block).

Use offsets to make the intended usage less ambiguous. We only need to
reference the input string in format_trailer_info(), so update that
function to take a pointer to the input.

Signed-off-by: Linus Arver <linusa@google.com>
---
 trailer.c | 17 ++++++++---------
 trailer.h |  7 +++----
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/trailer.c b/trailer.c
index 6ad2fbca942..00326720e81 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1055,7 +1055,6 @@ void process_trailers(const char *file,
 	LIST_HEAD(head);
 	struct strbuf sb = STRBUF_INIT;
 	struct trailer_info info;
-	size_t trailer_end;
 	FILE *outfile = stdout;
 
 	ensure_configured();
@@ -1066,11 +1065,10 @@ void process_trailers(const char *file,
 		outfile = create_in_place_tempfile(file);
 
 	parse_trailers(&info, sb.buf, &head, opts);
-	trailer_end = info.trailer_end - sb.buf;
 
 	/* Print the lines before the trailers */
 	if (!opts->only_trailers)
-		fwrite(sb.buf, 1, info.trailer_start - sb.buf, outfile);
+		fwrite(sb.buf, 1, info.trailer_start, outfile);
 
 	if (!opts->only_trailers && !info.blank_line_before_trailer)
 		fprintf(outfile, "\n");
@@ -1092,7 +1090,7 @@ void process_trailers(const char *file,
 
 	/* Print the lines after the trailers as is */
 	if (!opts->only_trailers)
-		fwrite(sb.buf + trailer_end, 1, sb.len - trailer_end, outfile);
+		fwrite(sb.buf + info.trailer_end, 1, sb.len - info.trailer_end, outfile);
 
 	if (opts->in_place)
 		if (rename_tempfile(&trailers_tempfile, file))
@@ -1104,7 +1102,7 @@ void process_trailers(const char *file,
 void trailer_info_get(struct trailer_info *info, const char *str,
 		      const struct process_trailer_options *opts)
 {
-	int patch_start, trailer_end, trailer_start;
+	size_t patch_start, trailer_end = 0, trailer_start = 0;
 	struct strbuf **trailer_lines, **ptr;
 	char **trailer_strings = NULL;
 	size_t nr = 0, alloc = 0;
@@ -1139,8 +1137,8 @@ void trailer_info_get(struct trailer_info *info, const char *str,
 
 	info->blank_line_before_trailer = ends_with_blank_line(str,
 							       trailer_start);
-	info->trailer_start = str + trailer_start;
-	info->trailer_end = str + trailer_end;
+	info->trailer_start = trailer_start;
+	info->trailer_end = trailer_end;
 	info->trailers = trailer_strings;
 	info->trailer_nr = nr;
 }
@@ -1155,6 +1153,7 @@ void trailer_info_release(struct trailer_info *info)
 
 static void format_trailer_info(struct strbuf *out,
 				const struct trailer_info *info,
+				const char *msg,
 				const struct process_trailer_options *opts)
 {
 	size_t origlen = out->len;
@@ -1164,7 +1163,7 @@ static void format_trailer_info(struct strbuf *out,
 	if (!opts->only_trailers && !opts->unfold && !opts->filter &&
 	    !opts->separator && !opts->key_only && !opts->value_only &&
 	    !opts->key_value_separator) {
-		strbuf_add(out, info->trailer_start,
+		strbuf_add(out, msg + info->trailer_start,
 			   info->trailer_end - info->trailer_start);
 		return;
 	}
@@ -1219,7 +1218,7 @@ void format_trailers_from_commit(struct strbuf *out, const char *msg,
 	struct trailer_info info;
 
 	trailer_info_get(&info, msg, opts);
-	format_trailer_info(out, &info, opts);
+	format_trailer_info(out, &info, msg, opts);
 	trailer_info_release(&info);
 }
 
diff --git a/trailer.h b/trailer.h
index a689d768c79..13fbf0dcd12 100644
--- a/trailer.h
+++ b/trailer.h
@@ -37,11 +37,10 @@ struct trailer_info {
 	int blank_line_before_trailer;
 
 	/*
-	 * Pointers to the start and end of the trailer block found. If there
-	 * is no trailer block found, these 2 pointers point to the end of the
-	 * input string.
+	 * Offsets to the trailer block start and end positions in the input
+	 * string. If no trailer block is found, these are set to 0.
 	 */
-	const char *trailer_start, *trailer_end;
+	size_t trailer_start, trailer_end;
 
 	/*
 	 * Array of trailers found.
-- 
gitgitgadget

^ permalink raw reply related

* [PATCH v2 5/6] trailer: rename *_DEFAULT enums to *_UNSPECIFIED
From: Linus Arver via GitGitGadget @ 2023-09-09  6:16 UTC (permalink / raw)
  To: git; +Cc: Glen Choo, Christian Couder, Phillip Wood, Linus Arver,
	Linus Arver
In-Reply-To: <pull.1563.v2.git.1694240177.gitgitgadget@gmail.com>

From: Linus Arver <linusa@google.com>

Do not use *_DEFAULT as a suffix to the enums, because the word
"default" is overloaded. The following are two examples of the ambiguity
of the word "default":

(1) "Default" can mean using the "default" values that are hardcoded
    in trailer.c as

        default_conf_info.where = WHERE_END;
        default_conf_info.if_exists = EXISTS_ADD_IF_DIFFERENT_NEIGHBOR;
        default_conf_info.if_missing = MISSING_ADD;

    in ensure_configured(). These values are referred to as "the
    default" in the docs for interpret-trailers. These defaults are used
    if no "trailer.*" configurations are defined.

(2) "Default" can also mean the "trailer.*" configurations themselves,
    because these configurations are used by "default" (ahead of the
    hardcoded defaults in (1)) if no command line arguments are
    provided. This concept of defaulting back to the configurations was
    introduced in 0ea5292e6b (interpret-trailers: add options for
    actions, 2017-08-01).

In addition, the corresponding *_DEFAULT values are chosen when the user
provides the "--no-where", "--no-if-exists", or "--no-if-missing" flags
on the command line. These "--no-*" flags are used to clear previously
provided flags of the form "--where", "--if-exists", and "--if-missing".
Using these "--no-*" flags undoes the specifying of these flags (if
any), so using the word "UNSPECIFIED" is more natural here.

So instead of using "*_DEFAULT", use "*_UNSPECIFIED" because this
signals to the reader that the *_UNSPECIFIED value by itself carries no
meaning (it's a zero value and by itself does not "default" to anything,
necessitating the need to have some other way of getting to a useful
value).

Signed-off-by: Linus Arver <linusa@google.com>
---
 trailer.c | 17 ++++++++++-------
 trailer.h |  6 +++---
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/trailer.c b/trailer.c
index f646e484a23..6ad2fbca942 100644
--- a/trailer.c
+++ b/trailer.c
@@ -388,7 +388,7 @@ static void process_trailers_lists(struct list_head *head,
 int trailer_set_where(enum trailer_where *item, const char *value)
 {
 	if (!value)
-		*item = WHERE_DEFAULT;
+		*item = WHERE_UNSPECIFIED;
 	else if (!strcasecmp("after", value))
 		*item = WHERE_AFTER;
 	else if (!strcasecmp("before", value))
@@ -405,7 +405,7 @@ int trailer_set_where(enum trailer_where *item, const char *value)
 int trailer_set_if_exists(enum trailer_if_exists *item, const char *value)
 {
 	if (!value)
-		*item = EXISTS_DEFAULT;
+		*item = EXISTS_UNSPECIFIED;
 	else if (!strcasecmp("addIfDifferent", value))
 		*item = EXISTS_ADD_IF_DIFFERENT;
 	else if (!strcasecmp("addIfDifferentNeighbor", value))
@@ -424,7 +424,7 @@ int trailer_set_if_exists(enum trailer_if_exists *item, const char *value)
 int trailer_set_if_missing(enum trailer_if_missing *item, const char *value)
 {
 	if (!value)
-		*item = MISSING_DEFAULT;
+		*item = MISSING_UNSPECIFIED;
 	else if (!strcasecmp("doNothing", value))
 		*item = MISSING_DO_NOTHING;
 	else if (!strcasecmp("add", value))
@@ -586,7 +586,10 @@ static void ensure_configured(void)
 	if (configured)
 		return;
 
-	/* Default config must be setup first */
+	/*
+	 * Default config must be setup first. These defaults are used if there
+	 * are no "trailer.*" or "trailer.<token>.*" options configured.
+	 */
 	default_conf_info.where = WHERE_END;
 	default_conf_info.if_exists = EXISTS_ADD_IF_DIFFERENT_NEIGHBOR;
 	default_conf_info.if_missing = MISSING_ADD;
@@ -701,11 +704,11 @@ static void add_arg_item(struct list_head *arg_head, char *tok, char *val,
 	new_item->value = val;
 	duplicate_conf(&new_item->conf, conf);
 	if (new_trailer_item) {
-		if (new_trailer_item->where != WHERE_DEFAULT)
+		if (new_trailer_item->where != WHERE_UNSPECIFIED)
 			new_item->conf.where = new_trailer_item->where;
-		if (new_trailer_item->if_exists != EXISTS_DEFAULT)
+		if (new_trailer_item->if_exists != EXISTS_UNSPECIFIED)
 			new_item->conf.if_exists = new_trailer_item->if_exists;
-		if (new_trailer_item->if_missing != MISSING_DEFAULT)
+		if (new_trailer_item->if_missing != MISSING_UNSPECIFIED)
 			new_item->conf.if_missing = new_trailer_item->if_missing;
 	}
 	list_add_tail(&new_item->list, arg_head);
diff --git a/trailer.h b/trailer.h
index ab2cd017567..a689d768c79 100644
--- a/trailer.h
+++ b/trailer.h
@@ -5,14 +5,14 @@
 #include "strbuf.h"
 
 enum trailer_where {
-	WHERE_DEFAULT,
+	WHERE_UNSPECIFIED,
 	WHERE_END,
 	WHERE_AFTER,
 	WHERE_BEFORE,
 	WHERE_START
 };
 enum trailer_if_exists {
-	EXISTS_DEFAULT,
+	EXISTS_UNSPECIFIED,
 	EXISTS_ADD_IF_DIFFERENT_NEIGHBOR,
 	EXISTS_ADD_IF_DIFFERENT,
 	EXISTS_ADD,
@@ -20,7 +20,7 @@ enum trailer_if_exists {
 	EXISTS_DO_NOTHING
 };
 enum trailer_if_missing {
-	MISSING_DEFAULT,
+	MISSING_UNSPECIFIED,
 	MISSING_ADD,
 	MISSING_DO_NOTHING
 };
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 0/2] diff-merges: introduce '-d' option
From: Sergey Organov @ 2023-09-09 12:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Sergey Organov

This new convenience option requests full diff with respect to first
parent, so that

  git log -d

will output diff with respect to first parent for every commit,
universally, no matter how many parents the commit turns out to have.

It's implemented as pure synonym for

  --diff-merges=first-parent --patch

The first commit in the series tweaks diff-merges documentation a bit,
and is valuable by itself. It's put here as '-d' implementation commit
depends on it in its documentation part.

Note: the need for this new convenience option mostly emerged from
denial by the community of patches that modify '-m' behavior to imply
'-p' as the rest of similar options (such as --cc) do.

Sergey Organov (2):
  diff-merges: improve --diff-merges documentation
  diff-merges: introduce '-d' option

 Documentation/diff-options.txt | 101 +++++++++++++++++++--------------
 Documentation/git-log.txt      |   4 +-
 diff-merges.c                  |   3 +
 t/t4013-diff-various.sh        |   8 +++
 4 files changed, 71 insertions(+), 45 deletions(-)

-- 
2.25.1


^ permalink raw reply

* [PATCH 2/2] diff-merges: introduce '-d' option
From: Sergey Organov @ 2023-09-09 12:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Sergey Organov
In-Reply-To: <20230909125446.142715-1-sorganov@gmail.com>

This option provides a shortcut to request diff with respect to first
parent for any kind of commit, universally. It's implemented as pure
synonym for "--diff-merges=first-parent --patch".

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 Documentation/diff-options.txt | 4 ++++
 Documentation/git-log.txt      | 2 +-
 diff-merges.c                  | 3 +++
 t/t4013-diff-various.sh        | 8 ++++++++
 4 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index f93aa3e46a52..d773dafcb10a 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -51,6 +51,10 @@ ifdef::git-log[]
 Note: This option not implying `-p` is legacy feature that is
 preserved for the sake of backward compatibility.
 
+-d::
+	Produce diff with respect to first parent.
+	Shortcut for '--diff-merges=first-parent -p'.
+
 -c::
 	Produce combined diff output for merge commits.
 	Shortcut for '--diff-merges=combined -p'.
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 9b7ec96e767a..59bd74a1a596 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -120,7 +120,7 @@ By default, `git log` does not generate any diff output. The options
 below can be used to show the changes made by each commit.
 
 Note that unless one of `--diff-merges` variants (including short
-`-m`, `-c`, and `--cc` options) is explicitly given, merge commits
+`-d`, `-m`, `-c`, and `--cc` options) is explicitly given, merge commits
 will not show a diff, even if a diff format like `--patch` is
 selected, nor will they match search options like `-S`. The exception
 is when `--first-parent` is in use, in which case `first-parent` is
diff --git a/diff-merges.c b/diff-merges.c
index ec97616db1df..6eb72e6fc28a 100644
--- a/diff-merges.c
+++ b/diff-merges.c
@@ -125,6 +125,9 @@ int diff_merges_parse_opts(struct rev_info *revs, const char **argv)
 	if (!suppress_m_parsing && !strcmp(arg, "-m")) {
 		set_to_default(revs);
 		revs->merges_need_diff = 0;
+	} else if (!strcmp(arg, "-d")) {
+		set_first_parent(revs);
+		revs->merges_imply_patch = 1;
 	} else if (!strcmp(arg, "-c")) {
 		set_combined(revs);
 		revs->merges_imply_patch = 1;
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 5de1d190759f..a07d6eb6dd97 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -473,6 +473,14 @@ test_expect_success 'log --diff-merges=on matches --diff-merges=separate' '
 	test_cmp expected actual
 '
 
+test_expect_success 'log -d matches --diff-merges=1 -p' '
+	git log --diff-merges=1 -p master >result &&
+	process_diffs result >expected &&
+	git log -d master >result &&
+	process_diffs result >actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'deny wrong log.diffMerges config' '
 	test_config log.diffMerges wrong-value &&
 	test_expect_code 128 git log
-- 
2.25.1


^ permalink raw reply related

* [PATCH 1/2] diff-merges: improve --diff-merges documentation
From: Sergey Organov @ 2023-09-09 12:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Sergey Organov
In-Reply-To: <20230909125446.142715-1-sorganov@gmail.com>

* Put descriptions of convenience shortcuts first, so they are the
  first things reader observes, not lengthy stuff.

* Add explanation note on '-m' not implying '-p' unlike similar
  options.

* Get rid of very long line containing all the --diff-merges formats
  by replacing them with <format>, and putting each supported format
  on its own line.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 Documentation/diff-options.txt | 97 +++++++++++++++++++---------------
 Documentation/git-log.txt      |  2 +-
 2 files changed, 55 insertions(+), 44 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 9f33f887711d..f93aa3e46a52 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -43,66 +43,77 @@ endif::git-diff[]
 endif::git-format-patch[]
 
 ifdef::git-log[]
---diff-merges=(off|none|on|first-parent|1|separate|m|combined|c|dense-combined|cc|remerge|r)::
+-m::
+	Show diffs for merge commits in the default format. This is
+	similar to '--diff-merges=on' (which see) except `-m` will
+	produce no output unless `-p` is given as well.
++
+Note: This option not implying `-p` is legacy feature that is
+preserved for the sake of backward compatibility.
+
+-c::
+	Produce combined diff output for merge commits.
+	Shortcut for '--diff-merges=combined -p'.
+
+--cc::
+	Produce dense combined diff output for merge commits.
+	Shortcut for '--diff-merges=dense-combined -p'.
+
+--remerge-diff::
+	Produce diff against re-merge.
+	Shortcut for '--diff-merges=remerge -p'.
+
 --no-diff-merges::
+	Synonym for '--diff-merges=off'.
+
+--diff-merges=<format>::
 	Specify diff format to be used for merge commits. Default is
-	{diff-merges-default} unless `--first-parent` is in use, in which case
-	`first-parent` is the default.
+	{diff-merges-default} unless `--first-parent` is in use, in
+	which case `first-parent` is the default.
 +
---diff-merges=(off|none):::
---no-diff-merges:::
+The following formats are supported:
++
+--
+off, none::
 	Disable output of diffs for merge commits. Useful to override
 	implied value.
 +
---diff-merges=on:::
---diff-merges=m:::
--m:::
-	This option makes diff output for merge commits to be shown in
-	the default format. `-m` will produce the output only if `-p`
-	is given as well. The default format could be changed using
+on, m::
+	Make diff output for merge commits to be shown in the default
+	format. The default format could be changed using
 	`log.diffMerges` configuration parameter, which default value
 	is `separate`.
 +
---diff-merges=first-parent:::
---diff-merges=1:::
-	This option makes merge commits show the full diff with
-	respect to the first parent only.
+first-parent, 1::
+	Show full diff with respect to first parent. This is the same
+	format as `--patch` produces for non-merge commits.
 +
---diff-merges=separate:::
-	This makes merge commits show the full diff with respect to
-	each of the parents. Separate log entry and diff is generated
-	for each parent.
+separate::
+	Show full diff with respect to each of parents.
+	Separate log entry and diff is generated for each parent.
 +
---diff-merges=remerge:::
---diff-merges=r:::
---remerge-diff:::
-	With this option, two-parent merge commits are remerged to
-	create a temporary tree object -- potentially containing files
-	with conflict markers and such.  A diff is then shown between
-	that temporary tree and the actual merge commit.
+remerge, r::
+	Remerge two-parent merge commits to create a temporary tree
+	object--potentially containing files with conflict markers
+	and such.  A diff is then shown between that temporary tree
+	and the actual merge commit.
 +
 The output emitted when this option is used is subject to change, and
 so is its interaction with other options (unless explicitly
 documented).
 +
---diff-merges=combined:::
---diff-merges=c:::
--c:::
-	With this option, diff output for a merge commit shows the
-	differences from each of the parents to the merge result
-	simultaneously instead of showing pairwise diff between a
-	parent and the result one at a time. Furthermore, it lists
-	only files which were modified from all parents. `-c` implies
-	`-p`.
+combined, c::
+	Show differences from each of the parents to the merge
+	result simultaneously instead of showing pairwise diff between
+	a parent and the result one at a time. Furthermore, it lists
+	only files which were modified from all parents.
 +
---diff-merges=dense-combined:::
---diff-merges=cc:::
---cc:::
-	With this option the output produced by
-	`--diff-merges=combined` is further compressed by omitting
-	uninteresting hunks whose contents in the parents have only
-	two variants and the merge result picks one of them without
-	modification.  `--cc` implies `-p`.
+dense-combined, cc::
+	Further compress output produced by `--diff-merges=combined`
+	by omitting uninteresting hunks whose contents in the parents
+	have only two variants and the merge result picks one of them
+	without modification.
+--
 
 --combined-all-paths::
 	This flag causes combined diffs (used for merge commits) to
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 2a66cf888074..9b7ec96e767a 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -124,7 +124,7 @@ Note that unless one of `--diff-merges` variants (including short
 will not show a diff, even if a diff format like `--patch` is
 selected, nor will they match search options like `-S`. The exception
 is when `--first-parent` is in use, in which case `first-parent` is
-the default format.
+the default format for merge commits.
 
 :git-log: 1
 :diff-merges-default: `off`
-- 
2.25.1


^ permalink raw reply related

* Re: [RFC][PATCH 0/32] SHA256 and SHA1 interoperability
From: Eric W. Biederman @ 2023-09-09 12:58 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, brian m. carlson
In-Reply-To: <87sf7ol0z3.fsf@email.froward.int.ebiederm.org>


I forgot to mention the patches are against 2.42.

Eric

^ permalink raw reply

* [PATCH] completion: improve doc for complex aliases
From: Philippe Blain via GitGitGadget @ 2023-09-09 15:49 UTC (permalink / raw)
  To: git; +Cc: Steffen Prohaska, Philippe Blain, Philippe Blain

From: Philippe Blain <levraiphilippeblain@gmail.com>

The completion code can be told to use a particular completion for
aliases that shell out by using ': git <cmd> ;' as the first command of
the alias. This only works if <cmd> and the semicolon are separated by a
space. The examples have that space but it's not clear if it's just for
style or if it's mandatory.

Explicitely mention it.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
    completion: improve doc for complex aliases
    
    The completion code can be told to use a particular completion for
    aliases that shell out by using ': git ;' as the first command of the
    alias. This only works if and the semicolon are separated by a space.
    The examples have that space but it's not clear if it's just for style
    or if it's mandatory.
    
    Explicitely mention it.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1585%2Fphil-blain%2Fcompletion-shell-aliases-doc-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1585/phil-blain/completion-shell-aliases-doc-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1585

 contrib/completion/git-completion.bash | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index dc95c34cc85..659df570496 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -28,6 +28,7 @@
 # completion style.  For example '!f() { : git commit ; ... }; f' will
 # tell the completion to use commit completion.  This also works with aliases
 # of form "!sh -c '...'".  For example, "!sh -c ': git commit ; ... '".
+# Be sure to add a space between the command name and the ';'.
 #
 # If you have a command that is not part of git, but you would still
 # like completion, you can use __git_complete:

base-commit: 23c56f7bd5f1667f8b793d796bf30e39545920f6
-- 
gitgitgadget

^ permalink raw reply related

* Re: diff alias which used to work in version 2.27, failing from version 2.28 onwards
From: Mikael Magnusson @ 2023-09-09 18:45 UTC (permalink / raw)
  To: Reverdell Auriane; +Cc: git@vger.kernel.org
In-Reply-To: <31866645d2ac4cb1be17e35fae742cb2@cscs.ch>

On 8/24/23, Reverdell  Auriane <auriane.reverdell@cscs.ch> wrote:
> Hi,
>
> To output the diff of a specific commit, I created the alias dici (see
> below), git dici 12345 or git dici to output the diff of the HEAD:
>
> [alias]
>     dici = !git diff ${1:-HEAD}^..${1:-HEAD}
>
> a trace for this alias gives (same trace for both git 2.27 and 2.28!):
>
> 14:38:45.275964 git.c:439               trace: built-in: git diff
> a2028e7b^..a2028e7b a2028e7b
>
> The commit number is put again at the end of the command (the wanted alias
> is supposed to output only `git diff a2028e7b^..a2028e7b`), this was
> accepted by version 2.27 but not by the version 2.28. For now, my temporary
> fix is adding a bash comment at the end of the alias:
>
> [alias]
>     dici = "!git diff ${1:-HEAD}^..${1:-HEAD} #"
>
> Note: the first alias still doesn't work with the latest git versions
> (tested with 2.40.0, 2.39.2, 2.37.5).
>
> Is that expected behavior? if yes, how is the right/clean way to discard the
> command line argument of the alias?

If you want to interpolate arguments in the command string, the safest
way is something like
dici = !sh -c 'git diff ${1:-HEAD}~..${1:-HEAD}' sh

(but why not just use git show?)

you can also use the ^! syntax instead:
dici = !sh -c 'git diff ${1:-HEAD}^!' sh

-- 
Mikael Magnusson

^ permalink raw reply

* [PATCH v2] status: fix branch shown when not only bisecting
From: Rubén Justo @ 2023-09-09 20:12 UTC (permalink / raw)
  To: Git List, Junio C Hamano
In-Reply-To: <48745298-f12b-8efb-4e48-90d2c22a8349@gmail.com>

In 83c750acde (wt-status.*: better advice for git status added,
2012-06-05), git-status received new informative messages to describe
the ongoing work in a worktree.

These messages were enhanced in 0722c805d6 (status: show the branch name
if possible in in-progress info, 2013-02-03), to show, if possible, the
branch where the operation was initiated.

Since then, we show incorrect information when several operations are in
progress and one of them is bisect:

   $ git checkout -b foo
   $ GIT_SEQUENCE_EDITOR='echo break >' git rebase -i HEAD~
   $ git checkout -b bar
   $ git bisect start
   $ git status
   ...

   You are currently editing a commit while rebasing branch 'bar' on '...'.

   You are currently bisecting, started from branch 'bar'.

   ...

Note that we erroneously say "while rebasing branch 'bar'" when we
should be referring to "foo".

This must have gone unnoticed for so long because it must be unusual to
start a bisection while another operation is in progress.  And even less
usual to involve different branches.

It caught my attention reviewing a leak introduced in 8b87cfd000
(wt-status: move strbuf into read_and_strip_branch(), 2013-03-16).

A simple change to deal with this situation can be to record in struct
wt_status_state, the branch where the bisect starts separately from the
branch related to other operations.

Let's do it and so we'll be able to display correct information and
we'll avoid the leak as well.

Signed-off-by: Rubén Justo <rjusto@gmail.com>

---

Let's try again.


 branch.c               |  4 ++--
 ref-filter.c           |  2 +-
 t/t7512-status-help.sh | 28 ++++++++++++++++++++++++++++
 worktree.c             |  4 ++--
 wt-status.c            |  7 ++++---
 wt-status.h            |  1 +
 6 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/branch.c b/branch.c
index 06f7af9dd4..534594f7f8 100644
--- a/branch.c
+++ b/branch.c
@@ -420,9 +420,9 @@ static void prepare_checked_out_branches(void)
 		wt_status_state_free_buffers(&state);
 
 		if (wt_status_check_bisect(wt, &state) &&
-		    state.branch) {
+		    state.bisecting_from) {
 			struct strbuf ref = STRBUF_INIT;
-			strbuf_addf(&ref, "refs/heads/%s", state.branch);
+			strbuf_addf(&ref, "refs/heads/%s", state.bisecting_from);
 			old = strmap_put(&current_checked_out_branches,
 					 ref.buf,
 					 xstrdup(wt->path));
diff --git a/ref-filter.c b/ref-filter.c
index 1bfaf20fbf..7fd5548e93 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -2124,7 +2124,7 @@ char *get_head_description(void)
 				    state.detached_from);
 	} else if (state.bisect_in_progress)
 		strbuf_addf(&desc, _("(no branch, bisect started on %s)"),
-			    state.branch);
+			    state.bisecting_from);
 	else if (state.detached_from) {
 		if (state.detached_at)
 			strbuf_addf(&desc, _("(HEAD detached at %s)"),
diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index c2ab8a444a..802f8f704c 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -692,6 +692,34 @@ EOF
 '
 
 
+test_expect_success 'status when bisecting while rebasing' '
+	git reset --hard main &&
+	test_when_finished "git rebase --abort" &&
+	ONTO=$(git rev-parse --short HEAD^) &&
+	FAKE_LINES="break" git rebase -i HEAD^ &&
+	test_when_finished "git checkout -" &&
+	git checkout -b bisect_while_rebasing &&
+	test_when_finished "git bisect reset" &&
+	git bisect start &&
+	cat >expected <<EOF &&
+On branch bisect_while_rebasing
+Last command done (1 command done):
+   break
+No commands remaining.
+You are currently editing a commit while rebasing branch '\''bisect'\'' on '\''$ONTO'\''.
+  (use "git commit --amend" to amend the current commit)
+  (use "git rebase --continue" once you are satisfied with your changes)
+
+You are currently bisecting, started from branch '\''bisect_while_rebasing'\''.
+  (use "git bisect reset" to get back to the original branch)
+
+nothing to commit (use -u to show untracked files)
+EOF
+	git status --untracked-files=no >actual &&
+	test_cmp expected actual
+'
+
+
 test_expect_success 'status when rebase --apply conflicts with statushints disabled' '
 	git reset --hard main &&
 	git checkout -b statushints_disabled &&
diff --git a/worktree.c b/worktree.c
index b8cf29e6a1..360e2b1866 100644
--- a/worktree.c
+++ b/worktree.c
@@ -395,9 +395,9 @@ int is_worktree_being_bisected(const struct worktree *wt,
 
 	memset(&state, 0, sizeof(state));
 	found_bisect = wt_status_check_bisect(wt, &state) &&
-		       state.branch &&
+		       state.bisecting_from &&
 		       skip_prefix(target, "refs/heads/", &target) &&
-		       !strcmp(state.branch, target);
+		       !strcmp(state.bisecting_from, target);
 	wt_status_state_free_buffers(&state);
 	return found_bisect;
 }
diff --git a/wt-status.c b/wt-status.c
index d03dfab9e4..dec28e8124 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -861,6 +861,7 @@ void wt_status_state_free_buffers(struct wt_status_state *state)
 	FREE_AND_NULL(state->branch);
 	FREE_AND_NULL(state->onto);
 	FREE_AND_NULL(state->detached_from);
+	FREE_AND_NULL(state->bisecting_from);
 }
 
 static void wt_longstatus_print_unmerged(struct wt_status *s)
@@ -1569,10 +1570,10 @@ static void show_revert_in_progress(struct wt_status *s,
 static void show_bisect_in_progress(struct wt_status *s,
 				    const char *color)
 {
-	if (s->state.branch)
+	if (s->state.bisecting_from)
 		status_printf_ln(s, color,
 				 _("You are currently bisecting, started from branch '%s'."),
-				 s->state.branch);
+				 s->state.bisecting_from);
 	else
 		status_printf_ln(s, color,
 				 _("You are currently bisecting."));
@@ -1733,7 +1734,7 @@ int wt_status_check_bisect(const struct worktree *wt,
 
 	if (!stat(worktree_git_path(wt, "BISECT_LOG"), &st)) {
 		state->bisect_in_progress = 1;
-		state->branch = get_branch(wt, "BISECT_START");
+		state->bisecting_from = get_branch(wt, "BISECT_START");
 		return 1;
 	}
 	return 0;
diff --git a/wt-status.h b/wt-status.h
index ab9cc9d8f0..819dcad723 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -94,6 +94,7 @@ struct wt_status_state {
 	char *branch;
 	char *onto;
 	char *detached_from;
+	char *bisecting_from;
 	struct object_id detached_oid;
 	struct object_id revert_head_oid;
 	struct object_id cherry_pick_head_oid;
-- 
2.40.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox