Merge branch 'jk/cat-file-batch-all-wo-replace'

"git cat-file --batch" with the "--batch-all-objects" option is
supposed to iterate over all the objects found in a repository, but
it used to translate these object names using the replace mechanism,
which defeats the point of enumerating all objects in the repository.
This has been corrected.

* jk/cat-file-batch-all-wo-replace:
  cat-file: use packed_object_info() for --batch-all-objects
  cat-file: split ordered/unordered batch-all-objects callbacks
  cat-file: disable refs/replace with --batch-all-objects
  cat-file: mention --unordered along with --batch-all-objects
  t1006: clean up broken objects

Documentation/git-cat-file.txt

@@ -94,8 +94,10 @@ OPTIONS
 	Instead of reading a list of objects on stdin, perform the
 	requested batch operation on all objects in the repository and
 	any alternate object stores (not just reachable objects).
-	Requires `--batch` or `--batch-check` be specified. Note that
+	Requires `--batch` or `--batch-check` be specified. By default,
-	the objects are visited in order sorted by their hashes.
+	the objects are visited in order sorted by their hashes; see
+	also `--unordered` below. Objects are presented as-is, without
+	respecting the "replace" mechanism of linkgit:git-replace[1].
 
 --buffer::
 	Normally batch output is flushed after each object is output, so

builtin/cat-file.c

@@ -355,18 +355,34 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d
 	}
 }
 
+/*
+ * If "pack" is non-NULL, then "offset" is the byte offset within the pack from
+ * which the object may be accessed (though note that we may also rely on
+ * data->oid, too). If "pack" is NULL, then offset is ignored.
+ */
 static void batch_object_write(const char *obj_name,
 			       struct strbuf *scratch,
 			       struct batch_options *opt,
-			       struct expand_data *data)
+			       struct expand_data *data,
+			       struct packed_git *pack,
+			       off_t offset)
 {
-	if (!data->skip_object_info &&
+	if (!data->skip_object_info) {
-	    oid_object_info_extended(the_repository, &data->oid, &data->info,
+		int ret;
-				     OBJECT_INFO_LOOKUP_REPLACE) < 0) {
+
-		printf("%s missing\n",
+		if (pack)
-		       obj_name ? obj_name : oid_to_hex(&data->oid));
+			ret = packed_object_info(the_repository, pack, offset,
-		fflush(stdout);
+						 &data->info);
-		return;
+		else
+			ret = oid_object_info_extended(the_repository,
+						       &data->oid, &data->info,
+						       OBJECT_INFO_LOOKUP_REPLACE);
+		if (ret < 0) {
+			printf("%s missing\n",
+			       obj_name ? obj_name : oid_to_hex(&data->oid));
+			fflush(stdout);
+			return;
+		}
 	}
 
 	strbuf_reset(scratch);
@@ -428,7 +444,7 @@ static void batch_one_object(const char *obj_name,
 		return;
 	}
 
-	batch_object_write(obj_name, scratch, opt, data);
+	batch_object_write(obj_name, scratch, opt, data, NULL, 0);
 }
 
 struct object_cb_data {
@@ -442,7 +458,8 @@ static int batch_object_cb(const struct object_id *oid, void *vdata)
 {
 	struct object_cb_data *data = vdata;
 	oidcpy(&data->expand->oid, oid);
-	batch_object_write(NULL, data->scratch, data->opt, data->expand);
+	batch_object_write(NULL, data->scratch, data->opt, data->expand,
+			   NULL, 0);
 	return 0;
 }
 
@@ -463,21 +480,26 @@ static int collect_packed_object(const struct object_id *oid,
 	return 0;
 }
 
-static int batch_unordered_object(const struct object_id *oid, void *vdata)
+static int batch_unordered_object(const struct object_id *oid,
+				  struct packed_git *pack, off_t offset,
+				  void *vdata)
 {
 	struct object_cb_data *data = vdata;
 
 	if (oidset_insert(data->seen, oid))
 		return 0;
 
-	return batch_object_cb(oid, data);
+	oidcpy(&data->expand->oid, oid);
+	batch_object_write(NULL, data->scratch, data->opt, data->expand,
+			   pack, offset);
+	return 0;
 }
 
 static int batch_unordered_loose(const struct object_id *oid,
 				 const char *path,
 				 void *data)
 {
-	return batch_unordered_object(oid, data);
+	return batch_unordered_object(oid, NULL, 0, data);
 }
 
 static int batch_unordered_packed(const struct object_id *oid,
@@ -485,7 +507,9 @@ static int batch_unordered_packed(const struct object_id *oid,
 				  uint32_t pos,
 				  void *data)
 {
-	return batch_unordered_object(oid, data);
+	return batch_unordered_object(oid, pack,
+				      nth_packed_object_offset(pack, pos),
+				      data);
 }
 
 static int batch_objects(struct batch_options *opt)
@@ -529,6 +553,8 @@ static int batch_objects(struct batch_options *opt)
 		if (has_promisor_remote())
 			warning("This repository uses promisor remotes. Some objects may not be loaded.");
 
+		read_replace_refs = 0;
+
 		cb.opt = opt;
 		cb.expand = &data;
 		cb.scratch = &output;

t/t1006-cat-file.sh

@@ -331,6 +331,11 @@ test_expect_success "Size of broken object is correct" '
 	git cat-file -s --allow-unknown-type $bogus_sha1 >actual &&
 	test_cmp expect actual
 '
+
+test_expect_success 'clean up broken object' '
+	rm .git/objects/$(test_oid_to_path $bogus_sha1)
+'
+
 bogus_type="abcdefghijklmnopqrstuvwxyz1234679"
 bogus_content="bogus"
 bogus_size=$(strlen "$bogus_content")
@@ -348,6 +353,10 @@ test_expect_success "Size of large broken object is correct when type is large"
 	test_cmp expect actual
 '
 
+test_expect_success 'clean up broken object' '
+	rm .git/objects/$(test_oid_to_path $bogus_sha1)
+'
+
 # Tests for git cat-file --follow-symlinks
 test_expect_success 'prep for symlink tests' '
 	echo_without_newline "$hello_content" >morx &&
@@ -608,4 +617,70 @@ test_expect_success 'cat-file --batch="batman" with --batch-all-objects will wor
 	cmp expect actual
 '
 
+test_expect_success 'set up replacement object' '
+	orig=$(git rev-parse HEAD) &&
+	git cat-file commit $orig >orig &&
+	{
+		cat orig &&
+		echo extra
+	} >fake &&
+	fake=$(git hash-object -t commit -w fake) &&
+	orig_size=$(git cat-file -s $orig) &&
+	fake_size=$(git cat-file -s $fake) &&
+	git replace $orig $fake
+'
+
+test_expect_success 'cat-file --batch respects replace objects' '
+	git cat-file --batch >actual <<-EOF &&
+	$orig
+	EOF
+	{
+		echo "$orig commit $fake_size" &&
+		cat fake &&
+		echo
+	} >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'cat-file --batch-check respects replace objects' '
+	git cat-file --batch-check >actual <<-EOF &&
+	$orig
+	EOF
+	echo "$orig commit $fake_size" >expect &&
+	test_cmp expect actual
+'
+
+# Pull the entry for object with oid "$1" out of the output of
+# "cat-file --batch", including its object content (which requires
+# parsing and reading a set amount of bytes, hence perl).
+extract_batch_output () {
+    perl -ne '
+	BEGIN { $oid = shift }
+	if (/^$oid \S+ (\d+)$/) {
+	    print;
+	    read STDIN, my $buf, $1;
+	    print $buf;
+	    print "\n";
+	}
+    ' "$@"
+}
+
+test_expect_success 'cat-file --batch-all-objects --batch ignores replace' '
+	git cat-file --batch-all-objects --batch >actual.raw &&
+	extract_batch_output $orig <actual.raw >actual &&
+	{
+		echo "$orig commit $orig_size" &&
+		cat orig &&
+		echo
+	} >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'cat-file --batch-all-objects --batch-check ignores replace' '
+	git cat-file --batch-all-objects --batch-check >actual.raw &&
+	grep ^$orig actual.raw >actual &&
+	echo "$orig commit $orig_size" >expect &&
+	test_cmp expect actual
+'
+
 test_done