[PATCH 1/4] t/helper: teach pack-deltas to list delta entries

[PATCH 1/4] t/helper: teach pack-deltas to list delta entries

[Taylor Blau]

In the following commit(s), some tests will need to distinguish between
`REF_DELTA`s and `OFS_DELTA`s to exercise a new '--no-ref-delta' option
for 'pack-objects'.

Existing tools report delta relationships, but not how their bases are
represented in the pack.

Teach 'test-tool pack-deltas' a '--list-deltas' mode. For each delta
entry, print the object ID, its REF_DELTA or OFS_DELTA type, and the
base object ID or pack offset, respectively. This lets tests inspect
pack headers without open-coding a parser.

Signed-off-by: Taylor Blau <ttaylorr@openai.com>
---
 t/helper/test-pack-deltas.c | 69 +++++++++++++++++++++++++++++++++++++
 t/t5300-pack-object.sh      |  8 +++--
 2 files changed, 75 insertions(+), 2 deletions(-)

diff --git a/t/helper/test-pack-deltas.c b/t/helper/test-pack-deltas.c
index 840797cf0d..4ba6fe2dd3 100644
--- a/t/helper/test-pack-deltas.c
+++ b/t/helper/test-pack-deltas.c
@@ -7,6 +7,7 @@
 #include "hash.h"
 #include "hex.h"
 #include "pack.h"
+#include "packfile.h"
 #include "pack-objects.h"
 #include "parse-options.h"
 #include "setup.h"
@@ -15,6 +16,7 @@
 
 static const char *usage_str[] = {
 	"test-tool pack-deltas --num-objects <num-objects>",
+	"test-tool pack-deltas --list-deltas <pack>.idx",
 	NULL
 };
 
@@ -80,19 +82,86 @@ static void write_ref_delta(struct hashfile *f,
 	free(delta_buf);
 }
 
+static int list_delta(const struct object_id *oid,
+		      struct packed_git *p,
+		      uint32_t pos,
+		      void *_w_curs)
+{
+	struct pack_window **w_curs = _w_curs;
+	off_t obj_offset = nth_packed_object_offset(p, pos);
+	off_t cur = obj_offset;
+	size_t size;
+	enum object_type type = unpack_object_header(p, w_curs, &cur,
+						      &size);
+
+	if (type < 0)
+		die("unable to parse object at position %"PRIu32, pos);
+	if (type != OBJ_REF_DELTA && type != OBJ_OFS_DELTA)
+		return 0;
+
+	if (type == OBJ_REF_DELTA) {
+		struct object_id base_oid;
+		const unsigned char *base = use_pack(p, w_curs, cur,
+						     NULL);
+
+		oidread(&base_oid, base, p->repo->hash_algo);
+		printf("%s REF_DELTA %s\n", oid_to_hex(oid),
+		       oid_to_hex(&base_oid));
+	} else {
+		off_t base_offset = get_delta_base(p, w_curs, &cur,
+						   type, obj_offset);
+
+		if (!base_offset)
+			die("unable to read base of object %s", oid_to_hex(oid));
+		printf("%s OFS_DELTA %"PRIuMAX"\n", oid_to_hex(oid),
+		       (uintmax_t)base_offset);
+	}
+
+	return 0;
+}
+
+static void list_deltas(const char *idx_name)
+{
+	struct packed_git *p;
+	struct pack_window *w_curs = NULL;
+
+	p = add_packed_git(the_repository, idx_name, strlen(idx_name), 1);
+	if (!p || open_pack_index(p))
+		die("unable to open pack index %s", idx_name);
+
+	if (for_each_object_in_pack(p, list_delta, &w_curs,
+				    ODB_FOR_EACH_OBJECT_PACK_ORDER))
+		die("unable to iterate over objects in %s", idx_name);
+
+	unuse_pack(&w_curs);
+	close_pack(p);
+	free(p);
+}
+
 int cmd__pack_deltas(int argc, const char **argv)
 {
 	int num_objects = -1;
+	int list_deltas_mode = 0;
 	struct hashfile *f;
 	struct strbuf line = STRBUF_INIT;
 	struct option options[] = {
 		OPT_INTEGER('n', "num-objects", &num_objects, N_("the number of objects to write")),
+		OPT_BOOL(0, "list-deltas", &list_deltas_mode,
+			 N_("list REF_DELTA and OFS_DELTA entries")),
 		OPT_END()
 	};
 
 	argc = parse_options(argc, argv, NULL,
 			     options, usage_str, 0);
 
+	if (list_deltas_mode) {
+		if (argc != 1 || num_objects >= 0)
+			usage_with_options(usage_str, options);
+		setup_git_directory(the_repository);
+		list_deltas(argv[0]);
+		return 0;
+	}
+
 	if (argc || num_objects < 0)
 		usage_with_options(usage_str, options);
 
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index 73445782e7..4bee490ff6 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -190,7 +190,9 @@ test_expect_success 'unpack without delta (core.fsyncmethod=batch)' '
 
 test_expect_success 'pack with REF_DELTA' '
 	packname_2=$(git pack-objects --progress test-2 <obj-list 2>stderr) &&
-	check_deltas stderr -gt 0
+	check_deltas stderr -gt 0 &&
+	test-tool pack-deltas --list-deltas test-2-$packname_2.idx >deltas &&
+	test_grep " REF_DELTA " deltas
 '
 
 test_expect_success 'unpack with REF_DELTA' '
@@ -204,7 +206,9 @@ test_expect_success 'unpack with REF_DELTA (core.fsyncmethod=batch)' '
 test_expect_success 'pack with OFS_DELTA' '
 	packname_3=$(git pack-objects --progress --delta-base-offset test-3 \
 			<obj-list 2>stderr) &&
-	check_deltas stderr -gt 0
+	check_deltas stderr -gt 0 &&
+	test-tool pack-deltas --list-deltas test-3-$packname_3.idx >deltas &&
+	test_grep " OFS_DELTA " deltas
 '
 
 test_expect_success 'unpack with OFS_DELTA' '
-- 
2.55.0

[PATCH 2/4] pack-objects: introduce `--no-ref-delta`

[Taylor Blau]

Some consumers of 'pack-objects' may wish to avoid packs which contain
`REF_DELTA` entries. For instance, a 'receive-pack' implementation which
retains the resulting pack without building an index of object IDs may
prefer every delta base to be discoverable from an earlier entry in the
same pack.

Teach 'pack-objects' a new `--no-ref-delta` option to avoid writing
`REF_DELTA` entries, without changing whether `OFS_DELTA` is allowed.

When used without `--delta-base-offset`, no delta representation
remains, so avoid delta search entirely. Otherwise, allow new deltas
whose bases appear earlier in the same pack.

For now, disable delta- and bitmap-reuse under `--no-ref-delta`, since
either may copy an existing `REF_DELTA` entry. This is overly
pessimistic, but simplifies the changes in this commit. The next commit
re-enables reuse in the cases which do not require `REF_DELTA`.

Signed-off-by: Taylor Blau <ttaylorr@openai.com>
---
 Documentation/git-pack-objects.adoc |  8 ++++-
 builtin/pack-objects.c              | 16 ++++++---
 t/t5300-pack-object.sh              | 52 +++++++++++++++++++++++++++++
 3 files changed, 71 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-pack-objects.adoc b/Documentation/git-pack-objects.adoc
index 65cd00c152..5e42e4429d 100644
--- a/Documentation/git-pack-objects.adoc
+++ b/Documentation/git-pack-objects.adoc
@@ -10,7 +10,8 @@ SYNOPSIS
 --------
 [verse]
 'git pack-objects' [-q | --progress | --all-progress] [--all-progress-implied]
-		   [--no-reuse-delta] [--delta-base-offset] [--non-empty]
+		   [--no-reuse-delta] [--delta-base-offset] [--no-ref-delta]
+		   [--non-empty]
 		   [--local] [--incremental] [--window=<n>] [--depth=<n>]
 		   [--revs [--unpacked | --all]] [--keep-pack=<pack-name>]
 		   [--cruft] [--cruft-expiration=<time>]
@@ -297,6 +298,11 @@ Note: Porcelain commands such as `git gc` (see linkgit:git-gc[1]),
 in modern Git when they put objects in your repository into pack files.
 So does `git bundle` (see linkgit:git-bundle[1]) when it creates a bundle.
 
+--no-ref-delta::
+	Do not emit deltas which represent their base by their literal
+	object ID. This is independent of `--delta-base-offset`;
+	without that option, no deltas are emitted.
+
 --threads=<n>::
 	Specifies the number of threads to spawn when searching for best
 	delta matches.  This requires that pack-objects be compiled with
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index e3760b3492..c3574fcb8a 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -190,7 +190,8 @@ static inline void oe_set_delta_size(struct packing_data *pack,
 
 static const char *const pack_usage[] = {
 	N_("git pack-objects [-q | --progress | --all-progress] [--all-progress-implied]\n"
-	   "                 [--no-reuse-delta] [--delta-base-offset] [--non-empty]\n"
+	   "                 [--no-reuse-delta] [--delta-base-offset] [--no-ref-delta]\n"
+	   "                 [--non-empty]\n"
 	   "                 [--local] [--incremental] [--window=<n>] [--depth=<n>]\n"
 	   "                 [--revs [--unpacked | --all]] [--keep-pack=<pack-name>]\n"
 	   "                 [--cruft] [--cruft-expiration=<time>]\n"
@@ -221,6 +222,7 @@ static int ignore_packed_keep_in_core;
 static int ignore_packed_keep_in_core_open;
 static int ignore_packed_keep_in_core_has_cruft;
 static int allow_ofs_delta;
+static int allow_ref_delta = 1;
 static struct pack_idx_option pack_idx_opts;
 static const char *base_name;
 static int progress = 1;
@@ -3405,6 +3407,9 @@ static int should_attempt_deltas(struct object_entry *entry)
 	if (entry->no_try_delta)
 		return 0;
 
+	if (entry->preferred_base && !allow_ref_delta)
+		return 0;
+
 	if (!entry->preferred_base) {
 		if (oe_type(entry) < 0)
 			die(_("unable to get type of object %s"),
@@ -3647,7 +3652,8 @@ static void prepare_pack(int window, int depth)
 	if (!pack_to_stdout)
 		do_check_packed_object_crc = 1;
 
-	if (!to_pack.nr_objects || !window || !depth)
+	if (!to_pack.nr_objects || !window || !depth ||
+	    (!allow_ref_delta && !allow_ofs_delta))
 		return;
 
 	if (path_walk)
@@ -4662,7 +4668,7 @@ static int pack_options_allow_reuse(void)
 	       !ignore_packed_keep_on_disk &&
 	       !ignore_packed_keep_in_core &&
 	       (!local || !have_non_local_packs) &&
-	       !incremental;
+	       !incremental && allow_ref_delta;
 }
 
 static int get_object_list_from_bitmap(struct rev_info *revs)
@@ -5111,6 +5117,8 @@ int cmd_pack_objects(int argc,
 			 N_("reuse existing objects")),
 		OPT_BOOL(0, "delta-base-offset", &allow_ofs_delta,
 			 N_("use OFS_DELTA objects")),
+		OPT_BOOL(0, "ref-delta", &allow_ref_delta,
+			 N_("use REF_DELTA objects")),
 		OPT_INTEGER(0, "threads", &delta_search_threads,
 			    N_("use threads when searching for best delta matches")),
 		OPT_BOOL(0, "non-empty", &non_empty,
@@ -5309,7 +5317,7 @@ int cmd_pack_objects(int argc,
 	if (unpack_unreachable || keep_unreachable || pack_loose_unreachable)
 		use_internal_rev_list = 1;
 
-	if (!reuse_object)
+	if (!reuse_object || !allow_ref_delta)
 		reuse_delta = 0;
 	if (cfg->pack_compression_level == -1)
 		cfg->pack_compression_level = Z_DEFAULT_COMPRESSION;
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index 4bee490ff6..b9e36044b9 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -211,6 +211,58 @@ test_expect_success 'pack with OFS_DELTA' '
 	test_grep " OFS_DELTA " deltas
 '
 
+test_expect_success 'pack without REF_DELTA' '
+	git pack-objects --no-ref-delta --stdout <obj-list >no-ref.pack &&
+	git index-pack -o no-ref.idx no-ref.pack &&
+
+	test-tool pack-deltas --list-deltas no-ref.idx >deltas &&
+	test_must_be_empty deltas
+'
+
+test_expect_success 'pack without REF_DELTA with OFS_DELTA' '
+	git pack-objects --delta-base-offset --no-ref-delta --stdout \
+		<obj-list >no-ref-ofs.pack &&
+	git index-pack -o no-ref-ofs.idx no-ref-ofs.pack &&
+
+	test-tool pack-deltas --list-deltas no-ref-ofs.idx >deltas &&
+	test_grep " OFS_DELTA " deltas &&
+	test_grep ! " REF_DELTA " deltas
+'
+
+test_expect_success 'pack without REF_DELTA skips excluded delta bases' '
+	test_when_finished "git read-tree $tree" &&
+
+	echo bar >>d &&
+	git update-index --add d &&
+	thin_tree=$(git write-tree) &&
+	thin_commit=$(git commit-tree $thin_tree -p $commit </dev/null) &&
+
+	{
+		echo $thin_commit &&
+		echo ^$commit
+	} >thin-revs &&
+
+	# Each type appears only once in the output, so any delta must
+	# use an excluded base and therefore be a REF_DELTA.
+	git pack-objects --thin --stdout --revs \
+		<thin-revs >thin.pack &&
+	git index-pack --fix-thin --stdin thin-fixed.pack \
+		<thin.pack >/dev/null &&
+
+	test-tool pack-deltas --list-deltas thin-fixed.idx >deltas &&
+	test_grep ! " OFS_DELTA " deltas &&
+	test_grep " REF_DELTA " deltas &&
+
+	git pack-objects --thin --stdout --revs \
+		--delta-base-offset --no-ref-delta \
+		<thin-revs >no-ref-thin.pack &&
+	git index-pack --fix-thin --stdin no-ref-thin-fixed.pack \
+		<no-ref-thin.pack >/dev/null &&
+
+	test-tool pack-deltas --list-deltas no-ref-thin-fixed.idx >deltas &&
+	test_must_be_empty deltas
+'
+
 test_expect_success 'unpack with OFS_DELTA' '
 	check_unpack test-3-${packname_3} obj-list
 '
-- 
2.55.0

[PATCH 3/4] pack-objects: support reuse with `--no-ref-delta`

[Taylor Blau]

The previous commit disables delta- and bitmap-reuse entirely whenever
pack-objects is given '--no-ref-delta' for the sake of simplicity. This
is overly pessimistic.

When '--delta-base-offset' is also given, delta reuse can remain
enabled. A reused delta whose base is written earlier in the output can
be encoded as an `OFS_DELTA`, even when its source copy was encoded as a
`REF_DELTA`.

Preferred bases and external thin-pack bases are different: neither
appears in the output, so deltas against either still require encoding
the object as a `REF_DELTA`, and thus cannot be reused.

Without '--delta-base-offset', delta reuse remains disabled, since no
delta representation remains.

Bitmap reuse follows a different path, since selected entries may be
copied without passing through the code which chooses a delta
representation. When given '--no-ref-delta', we must inspect candidate
objects individually, and leave `REF_DELTA` entries to the normal object
path outside of pack-reuse.

We must likewise avoid the special-case for reusing either the single or
preferred pack corresponding to the bitmap by whole `eword_t`'s at a
time.

Signed-off-by: Taylor Blau <ttaylorr@openai.com>
---
 builtin/pack-objects.c      | 17 +++++++++++++----
 pack-bitmap.c               | 30 ++++++++++++++++++++----------
 pack-bitmap.h               |  3 ++-
 t/t5300-pack-object.sh      | 21 ++++++++++++++++++++-
 t/t5332-multi-pack-reuse.sh | 16 ++++++++++++++++
 5 files changed, 71 insertions(+), 16 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index c3574fcb8a..43cd4be2e5 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2207,6 +2207,13 @@ static int can_reuse_delta(const struct object_id *base_oid,
 	 */
 	base = packlist_find(&to_pack, base_oid);
 	if (base) {
+		/*
+		 * A preferred base is omitted from the resulting pack, so it
+		 * can only be referenced by object ID.
+		 */
+		if (base->preferred_base && !allow_ref_delta)
+			return 0;
+
 		if (!in_same_island(&delta->idx.oid, &base->idx.oid))
 			return 0;
 		*base_out = base;
@@ -2218,7 +2225,8 @@ static int can_reuse_delta(const struct object_id *base_oid,
 	 * even if it was buried too deep in history to make it into the
 	 * packing list.
 	 */
-	if (thin && bitmap_has_oid_in_uninteresting(bitmap_git, base_oid)) {
+	if (allow_ref_delta && thin &&
+	    bitmap_has_oid_in_uninteresting(bitmap_git, base_oid)) {
 		if (use_delta_islands) {
 			if (!in_same_island(&delta->idx.oid, base_oid))
 				return 0;
@@ -4668,7 +4676,7 @@ static int pack_options_allow_reuse(void)
 	       !ignore_packed_keep_on_disk &&
 	       !ignore_packed_keep_in_core &&
 	       (!local || !have_non_local_packs) &&
-	       !incremental && allow_ref_delta;
+	       !incremental && (allow_ref_delta || allow_ofs_delta);
 }
 
 static int get_object_list_from_bitmap(struct rev_info *revs)
@@ -4690,7 +4698,8 @@ static int get_object_list_from_bitmap(struct rev_info *revs)
 						   &reuse_packfiles,
 						   &reuse_packfiles_nr,
 						   &reuse_packfile_bitmap,
-						   allow_pack_reuse == MULTI_PACK_REUSE);
+						   allow_pack_reuse == MULTI_PACK_REUSE,
+						   allow_ref_delta);
 
 	if (reuse_packfiles) {
 		reuse_packfile_objects = bitmap_popcount(reuse_packfile_bitmap);
@@ -5317,7 +5326,7 @@ int cmd_pack_objects(int argc,
 	if (unpack_unreachable || keep_unreachable || pack_loose_unreachable)
 		use_internal_rev_list = 1;
 
-	if (!reuse_object || !allow_ref_delta)
+	if (!reuse_object || (!allow_ref_delta && !allow_ofs_delta))
 		reuse_delta = 0;
 	if (cfg->pack_compression_level == -1)
 		cfg->pack_compression_level = Z_DEFAULT_COMPRESSION;
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 83eb47a28b..36cb02e374 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -2267,7 +2267,8 @@ static int try_partial_reuse(struct bitmap_index *bitmap_git,
 			     uint32_t pack_pos,
 			     off_t offset,
 			     struct bitmap *reuse,
-			     struct pack_window **w_curs)
+			     struct pack_window **w_curs,
+			     int allow_ref_delta)
 {
 	off_t delta_obj_offset;
 	enum object_type type;
@@ -2286,6 +2287,9 @@ static int try_partial_reuse(struct bitmap_index *bitmap_git,
 		uint32_t base_pos;
 		uint32_t base_bitmap_pos;
 
+		if (type == OBJ_REF_DELTA && !allow_ref_delta)
+			return 0;
+
 		/*
 		 * Find the position of the base object so we can look it up
 		 * in our bitmaps. If we can't come up with an offset, or if
@@ -2358,20 +2362,19 @@ static int try_partial_reuse(struct bitmap_index *bitmap_git,
 
 static void reuse_partial_packfile_from_bitmap_1(struct bitmap_index *bitmap_git,
 						 struct bitmapped_pack *pack,
-						 struct bitmap *reuse)
+						 struct bitmap *reuse,
+						 int allow_ref_delta)
 {
 	struct bitmap *result = bitmap_git->result;
 	struct pack_window *w_curs = NULL;
 	size_t pos = pack->bitmap_pos / BITS_IN_EWORD;
 
-	if (!pack->bitmap_pos) {
+	if (allow_ref_delta && !pack->bitmap_pos) {
 		/*
 		 * If we're processing the first (in the case of a MIDX, the
 		 * preferred pack) or the only (in the case of single-pack
-		 * bitmaps) pack, then we can reuse whole words at a time.
-		 *
-		 * This is because we know that any deltas in this range *must*
-		 * have their bases chosen from the same pack, since:
+		 * bitmaps) pack, then any delta in this range must have its
+		 * base chosen from the same pack:
 		 *
 		 * - In the single pack case, there is no other pack to choose
 		 *   them from.
@@ -2380,6 +2383,10 @@ static void reuse_partial_packfile_from_bitmap_1(struct bitmap_index *bitmap_git
 		 *   all ties are broken in favor of that pack (i.e. the one
 		 *   we're currently processing). So any duplicate bases will be
 		 *   resolved in favor of the pack we're processing.
+		 *
+		 * When REF_DELTAs are allowed, we can therefore reuse whole
+		 * words at a time without inspecting object headers. Otherwise,
+		 * inspect each object below to avoid reusing a REF_DELTA entry.
 		 */
 		while (pos < result->word_alloc &&
 		       pos < pack->bitmap_nr / BITS_IN_EWORD &&
@@ -2429,7 +2436,8 @@ static void reuse_partial_packfile_from_bitmap_1(struct bitmap_index *bitmap_git
 			}
 
 			if (try_partial_reuse(bitmap_git, pack, bit_pos,
-					      pack_pos, ofs, reuse, &w_curs) < 0) {
+					      pack_pos, ofs, reuse, &w_curs,
+					      allow_ref_delta) < 0) {
 				/*
 				 * try_partial_reuse indicated we couldn't reuse
 				 * any bits, so there is no point in trying more
@@ -2464,7 +2472,8 @@ void reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,
 					struct bitmapped_pack **packs_out,
 					size_t *packs_nr_out,
 					struct bitmap **reuse_out,
-					int multi_pack_reuse)
+					int multi_pack_reuse,
+					int allow_ref_delta)
 {
 	struct repository *r = bitmap_repo(bitmap_git);
 	struct bitmapped_pack *packs = NULL;
@@ -2559,7 +2568,8 @@ void reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,
 	reuse = bitmap_word_alloc(word_alloc);
 
 	for (i = 0; i < packs_nr; i++)
-		reuse_partial_packfile_from_bitmap_1(bitmap_git, &packs[i], reuse);
+		reuse_partial_packfile_from_bitmap_1(bitmap_git, &packs[i], reuse,
+						     allow_ref_delta);
 
 	if (bitmap_is_empty(reuse)) {
 		free(packs);
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 19a8655457..39b6309736 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -116,7 +116,8 @@ void reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,
 					struct bitmapped_pack **packs_out,
 					size_t *packs_nr_out,
 					struct bitmap **reuse_out,
-					int multi_pack_reuse);
+					int multi_pack_reuse,
+					int allow_ref_delta);
 int rebuild_existing_bitmaps(struct bitmap_index *, struct packing_data *mapping,
 			     kh_oid_map_t *reused_bitmaps, int show_progress);
 void free_bitmap_index(struct bitmap_index *);
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index b9e36044b9..02c09e3f7d 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -229,6 +229,20 @@ test_expect_success 'pack without REF_DELTA with OFS_DELTA' '
 	test_grep ! " REF_DELTA " deltas
 '
 
+test_expect_success 'pack without REF_DELTA reuses deltas as OFS_DELTA' '
+	# Install the REF_DELTA pack above and disable delta search, so any
+	# output delta must be a reused REF_DELTA rewritten as OFS_DELTA.
+	test_when_finished "rm -f .git/objects/pack/pack-$packname_2.*" &&
+	git index-pack --stdin <test-2-${packname_2}.pack >/dev/null &&
+
+	git pack-objects --window=0 --delta-base-offset \
+		--no-ref-delta --stdout <obj-list >reused.pack &&
+	git index-pack -o reused.idx reused.pack &&
+	test-tool pack-deltas --list-deltas reused.idx >deltas &&
+	test_grep " OFS_DELTA " deltas &&
+	test_grep ! " REF_DELTA " deltas
+'
+
 test_expect_success 'pack without REF_DELTA skips excluded delta bases' '
 	test_when_finished "git read-tree $tree" &&
 
@@ -253,7 +267,12 @@ test_expect_success 'pack without REF_DELTA skips excluded delta bases' '
 	test_grep ! " OFS_DELTA " deltas &&
 	test_grep " REF_DELTA " deltas &&
 
-	git pack-objects --thin --stdout --revs \
+	# Store the REF_DELTA entries above and disable delta search below,
+	# so any output delta would have to reuse an excluded-base
+	# REF_DELTA.
+	git index-pack --stdin <thin-fixed.pack >/dev/null &&
+
+	git pack-objects --thin --window=0 --stdout --revs \
 		--delta-base-offset --no-ref-delta \
 		<thin-revs >no-ref-thin.pack &&
 	git index-pack --fix-thin --stdin no-ref-thin-fixed.pack \
diff --git a/t/t5332-multi-pack-reuse.sh b/t/t5332-multi-pack-reuse.sh
index 881ce668e1..bc479653ec 100755
--- a/t/t5332-multi-pack-reuse.sh
+++ b/t/t5332-multi-pack-reuse.sh
@@ -111,6 +111,22 @@ test_expect_success 'reuse all objects from all packs' '
 	test_pack_objects_reused_all 9 3
 '
 
+test_expect_success '--no-ref-delta reuses REF_DELTA-free bitmapped packs' '
+	# Whole-word reuse is unavailable under --no-ref-delta, so reusing
+	# every object below exercises the per-object bitmap path.
+	: >trace2.txt &&
+	GIT_TRACE2_EVENT="$PWD/trace2.txt" \
+		git pack-objects --stdout --revs --all --delta-base-offset \
+		--no-ref-delta >got.pack &&
+
+	test_pack_reused 9 <trace2.txt &&
+	test_packs_reused 3 <trace2.txt &&
+
+	git index-pack --strict -o got.idx got.pack &&
+	test-tool pack-deltas --list-deltas got.idx >deltas &&
+	test_grep ! " REF_DELTA " deltas
+'
+
 test_expect_success 'reuse objects from first pack with middle gap' '
 	for i in D E F
 	do
-- 
2.55.0

[PATCH 4/4] send-pack: honor `no-ref-delta` capability

[Taylor Blau]

Add a 'no-ref-delta' receive-pack capability and teach send-pack to pass
'--no-ref-delta' to 'pack-objects' when the server advertises it.

Keep this separate from 'ofs-delta' so that a server may request that
`send-pack` omit `REF_DELTA` without also accepting `OFS_DELTA`.

Signed-off-by: Taylor Blau <ttaylorr@openai.com>
---
 Documentation/gitprotocol-capabilities.adoc | 17 ++++++++++++++---
 builtin/receive-pack.c                      |  5 +++++
 send-pack.c                                 |  4 ++++
 send-pack.h                                 |  1 +
 t/t5516-fetch-push.sh                       | 14 ++++++++++++++
 5 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/Documentation/gitprotocol-capabilities.adoc b/Documentation/gitprotocol-capabilities.adoc
index 2cf7735be4..bbe88defdf 100644
--- a/Documentation/gitprotocol-capabilities.adoc
+++ b/Documentation/gitprotocol-capabilities.adoc
@@ -34,9 +34,9 @@ were sent.  Server MUST NOT ignore capabilities that client requested
 and server advertised.  As a consequence of these rules, server MUST
 NOT advertise capabilities it does not understand.
 
-The 'atomic', 'report-status', 'report-status-v2', 'delete-refs', 'quiet',
-and 'push-cert' capabilities are sent and recognized by the receive-pack
-(push to server) process.
+The 'atomic', 'report-status', 'report-status-v2', 'delete-refs',
+'no-ref-delta', 'quiet', and 'push-cert' capabilities are sent and
+recognized by the receive-pack (push to server) process.
 
 The 'ofs-delta' and 'side-band-64k' capabilities are sent and recognized
 by both upload-pack and receive-pack protocols.  The 'agent' and 'session-id'
@@ -174,6 +174,17 @@ The server can send, and the client can understand, PACKv2 with delta referring
 its base by position in pack rather than by an obj-id.  That is, they can
 send/read OBJ_OFS_DELTA (aka type 6) in a packfile.
 
+no-ref-delta
+------------
+
+The receive-pack server can request, and the client can send, PACKv2
+without deltas referring to their bases by an obj-id. That is, the
+client MUST NOT send OBJ_REF_DELTA (aka type 7) in a packfile when the
+server advertises this capability.
+
+This does not imply that the server understands OBJ_OFS_DELTA entries;
+that is negotiated separately with the 'ofs-delta' capability.
+
 agent
 -----
 
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 19eb6a1b61..1c516cbdc6 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -66,6 +66,7 @@ static struct strbuf fsck_msg_types = STRBUF_INIT;
 static int receive_unpack_limit = -1;
 static int transfer_unpack_limit = -1;
 static int advertise_atomic_push = 1;
+static int advertise_no_ref_delta;
 static int advertise_push_options;
 static int advertise_sid;
 static int unpack_limit = 100;
@@ -290,6 +291,8 @@ static void show_ref(const char *path, const struct object_id *oid)
 			strbuf_addstr(&cap, " atomic");
 		if (prefer_ofs_delta)
 			strbuf_addstr(&cap, " ofs-delta");
+		if (advertise_no_ref_delta)
+			strbuf_addstr(&cap, " no-ref-delta");
 		if (push_cert_nonce)
 			strbuf_addf(&cap, " push-cert=%s", push_cert_nonce);
 		if (advertise_push_options)
@@ -2631,6 +2634,8 @@ int cmd_receive_pack(int argc,
 		OPT_HIDDEN_BOOL(0, "http-backend-info-refs", &advertise_refs, NULL),
 		OPT_ALIAS(0, "advertise-refs", "http-backend-info-refs"),
 		OPT_HIDDEN_BOOL(0, "reject-thin-pack-for-testing", &reject_thin, NULL),
+		OPT_HIDDEN_BOOL(0, "advertise-no-ref-delta-for-testing",
+				&advertise_no_ref_delta, NULL),
 		OPT_END()
 	};
 
diff --git a/send-pack.c b/send-pack.c
index 3bb5afc687..2beb1c4be9 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -80,6 +80,8 @@ static int pack_objects(struct repository *r,
 		strvec_push(&po.args, "--thin");
 	if (args->use_ofs_delta)
 		strvec_push(&po.args, "--delta-base-offset");
+	if (args->no_ref_delta)
+		strvec_push(&po.args, "--no-ref-delta");
 	if (args->quiet || !args->progress)
 		strvec_push(&po.args, "-q");
 	if (args->progress)
@@ -570,6 +572,8 @@ int send_pack(struct repository *r,
 		allow_deleting_refs = 1;
 	if (server_supports("ofs-delta"))
 		args->use_ofs_delta = 1;
+	if (server_supports("no-ref-delta"))
+		args->no_ref_delta = 1;
 	if (server_supports("side-band-64k"))
 		use_sideband = 1;
 	if (server_supports("quiet"))
diff --git a/send-pack.h b/send-pack.h
index 13850c98bb..30be2be0f2 100644
--- a/send-pack.h
+++ b/send-pack.h
@@ -28,6 +28,7 @@ struct send_pack_args {
 		force_update:1,
 		use_thin_pack:1,
 		use_ofs_delta:1,
+		no_ref_delta:1,
 		dry_run:1,
 		/* One of the SEND_PACK_PUSH_CERT_* constants. */
 		push_cert:2,
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 1b986349a8..c00074afe8 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1548,6 +1548,20 @@ EOF
 	git push --no-thin --receive-pack="$rcvpck" no-thin/.git refs/heads/main:refs/heads/foo
 '
 
+test_expect_success 'push honors no-ref-delta capability' '
+	test_commit no-ref-delta &&
+
+	rcvpck="git receive-pack --advertise-no-ref-delta-for-testing" &&
+
+	GIT_TRACE2_EVENT="$PWD/no-ref-delta" \
+	git push --receive-pack="$rcvpck" no-thin/.git \
+		refs/heads/main:refs/heads/bar &&
+
+	test_subcommand git pack-objects --all-progress-implied --revs \
+		--stdout --thin --delta-base-offset --no-ref-delta -q \
+		<no-ref-delta
+'
+
 test_expect_success 'pushing a tag pushes the tagged object' '
 	blob=$(echo unreferenced | git hash-object -w --stdin) &&
 	git tag -m foo tag-of-blob $blob &&
-- 
2.55.0