diff mbox series

[v3,30/39] builtin/verify-pack: implement an --object-format option

Message ID 20200723010943.2329634-31-sandals@crustytoothpaste.net (mailing list archive)
State New, archived
Headers show
Series SHA-256, part 3/3 | expand

Commit Message

brian m. carlson July 23, 2020, 1:09 a.m. UTC 
A recently added test in t5702 started using git verify-pack outside of
a repository.  While this poses no problems with SHA-1, with SHA-256 we
implicitly rely on the setup of the repository to initialize our hash
algorithm settings.

Since we're not in a repository here, we need to provide git verify-pack
help to set things up properly.  git index-pack already knows an
--object-format option, so let's accept one as well and pass it down to
our git index-pack invocation.  Since this argument is optional, let's
dynamically determine the proper location to insert it into the array.
Finally, let's make t5702 pass the proper argument on down to its git
verify-pack caller.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/verify-pack.c  | 25 +++++++++++++++++--------
 t/t5702-protocol-v2.sh |  2 +-
 2 files changed, 18 insertions(+), 9 deletions(-)

Comments

Eric Sunshine July 23, 2020, 4:54 a.m. UTC | #1 
On Wed, Jul 22, 2020 at 9:10 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> Since we're not in a repository here, we need to provide git verify-pack
> help to set things up properly.  git index-pack already knows an
> --object-format option, so let's accept one as well and pass it down to
> our git index-pack invocation.  Since this argument is optional, let's
> dynamically determine the proper location to insert it into the array.
> Finally, let's make t5702 pass the proper argument on down to its git
> verify-pack caller.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> diff --git a/builtin/verify-pack.c b/builtin/verify-pack.c
> @@ -7,21 +7,27 @@
> +static int verify_one_pack(const char *path, unsigned int flags, const char *hash_algo)
>  {
> -       const char *argv[] = {"index-pack", NULL, NULL, NULL };
> +       const char *argv[] = {"index-pack", NULL, NULL, NULL, NULL };
> +       int argno = 1;
>
> +       if (hash_algo) {
> +               strbuf_addf(&hash_arg, "--object-format=%s", hash_algo);
> +               argv[argno++] = hash_arg.buf;
> +       }

This seems like a good candidate for 'struct argv_array' (but perhaps
that's too significant a change or out of scope of this patch?).
diff mbox series

Patch

diff --git a/builtin/verify-pack.c b/builtin/verify-pack.c
index c2a1a5c504..c4090102a9 100644
--- a/builtin/verify-pack.c
+++ b/builtin/verify-pack.c
@@ -7,21 +7,27 @@ 
 #define VERIFY_PACK_VERBOSE 01
 #define VERIFY_PACK_STAT_ONLY 02
 
-static int verify_one_pack(const char *path, unsigned int flags)
+static int verify_one_pack(const char *path, unsigned int flags, const char *hash_algo)
 {
 	struct child_process index_pack = CHILD_PROCESS_INIT;
-	const char *argv[] = {"index-pack", NULL, NULL, NULL };
-	struct strbuf arg = STRBUF_INIT;
+	const char *argv[] = {"index-pack", NULL, NULL, NULL, NULL };
+	struct strbuf arg = STRBUF_INIT, hash_arg = STRBUF_INIT;
 	int verbose = flags & VERIFY_PACK_VERBOSE;
 	int stat_only = flags & VERIFY_PACK_STAT_ONLY;
 	int err;
+	int argno = 1;
 
 	if (stat_only)
-		argv[1] = "--verify-stat-only";
+		argv[argno++] = "--verify-stat-only";
 	else if (verbose)
-		argv[1] = "--verify-stat";
+		argv[argno++] = "--verify-stat";
 	else
-		argv[1] = "--verify";
+		argv[argno++] = "--verify";
+
+	if (hash_algo) {
+		strbuf_addf(&hash_arg, "--object-format=%s", hash_algo);
+		argv[argno++] = hash_arg.buf;
+	}
 
 	/*
 	 * In addition to "foo.pack" we accept "foo.idx" and "foo";
@@ -31,7 +37,7 @@  static int verify_one_pack(const char *path, unsigned int flags)
 	if (strbuf_strip_suffix(&arg, ".idx") ||
 	    !ends_with(arg.buf, ".pack"))
 		strbuf_addstr(&arg, ".pack");
-	argv[2] = arg.buf;
+	argv[argno++] = arg.buf;
 
 	index_pack.argv = argv;
 	index_pack.git_cmd = 1;
@@ -60,12 +66,15 @@  int cmd_verify_pack(int argc, const char **argv, const char *prefix)
 {
 	int err = 0;
 	unsigned int flags = 0;
+	const char *object_format = NULL;
 	int i;
 	const struct option verify_pack_options[] = {
 		OPT_BIT('v', "verbose", &flags, N_("verbose"),
 			VERIFY_PACK_VERBOSE),
 		OPT_BIT('s', "stat-only", &flags, N_("show statistics only"),
 			VERIFY_PACK_STAT_ONLY),
+		OPT_STRING(0, "object-format", &object_format, N_("hash"),
+			   N_("specify the hash algorithm to use")),
 		OPT_END()
 	};
 
@@ -75,7 +84,7 @@  int cmd_verify_pack(int argc, const char **argv, const char *prefix)
 	if (argc < 1)
 		usage_with_options(verify_pack_usage, verify_pack_options);
 	for (i = 0; i < argc; i++) {
-		if (verify_one_pack(argv[i], flags))
+		if (verify_one_pack(argv[i], flags, object_format))
 			err = 1;
 	}
 
diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
index 1b54c35b01..7fc22171e7 100755
--- a/t/t5702-protocol-v2.sh
+++ b/t/t5702-protocol-v2.sh
@@ -829,7 +829,7 @@  test_expect_success 'part of packfile response provided as URI' '
 	# Ensure that my-blob and other-blob are in separate packfiles.
 	for idx in http_child/.git/objects/pack/*.idx
 	do
-		git verify-pack --verbose $idx >out &&
+		git verify-pack --object-format=$(test_oid algo) --verbose $idx >out &&
 		{
 			grep "^[0-9a-f]\{16,\} " out || :
 		} >out.objectlist &&