Merge branch 'nd/pack-objects-parseopt'

* nd/pack-objects-parseopt:
  pack-objects: convert to use parse_options()
  pack-objects: remove bogus comment
  pack-objects: do not accept "--index-version=version,"

Author: gitster

builtin/pack-objects.c

@@ -18,16 +18,11 @@
 #include "refs.h"
 #include "thread-utils.h"
 
-static const char pack_usage[] =
-  "git pack-objects [ -q | --progress | --all-progress ]\n"
-  "        [--all-progress-implied]\n"
-  "        [--max-pack-size=<n>] [--local] [--incremental]\n"
-  "        [--window=<n>] [--window-memory=<n>] [--depth=<n>]\n"
-  "        [--no-reuse-delta] [--no-reuse-object] [--delta-base-offset]\n"
-  "        [--threads=<n>] [--non-empty] [--revs [--unpacked | --all]]\n"
-  "        [--reflog] [--stdout | base-name] [--include-tag]\n"
-  "        [--keep-unreachable | --unpack-unreachable]\n"
-  "        [< ref-list | < object-list]";
+static const char *pack_usage[] = {
+	"git pack-objects --stdout [options...] [< ref-list | < object-list]",
+	"git pack-objects [options...] base-name [< ref-list | < object-list]",
+	NULL
+};
 
 struct object_entry {
 	struct pack_idx_entry idx;
@@ -2305,204 +2300,159 @@ static void get_object_list(int ac, const char **av)
 		loosen_unused_packed_objects(&revs);
 }
 
+static int option_parse_index_version(const struct option *opt,
+				      const char *arg, int unset)
+{
+	char *c;
+	const char *val = arg;
+	pack_idx_opts.version = strtoul(val, &c, 10);
+	if (pack_idx_opts.version > 2)
+		die(_("unsupported index version %s"), val);
+	if (*c == ',' && c[1])
+		pack_idx_opts.off32_limit = strtoul(c+1, &c, 0);
+	if (*c || pack_idx_opts.off32_limit & 0x80000000)
+		die(_("bad index version '%s'"), val);
+	return 0;
+}
+
+static int option_parse_ulong(const struct option *opt,
+			      const char *arg, int unset)
+{
+	if (unset)
+		die(_("option %s does not accept negative form"),
+		    opt->long_name);
+
+	if (!git_parse_ulong(arg, opt->value))
+		die(_("unable to parse value '%s' for option %s"),
+		    arg, opt->long_name);
+	return 0;
+}
+
+#define OPT_ULONG(s, l, v, h) \
+	{ OPTION_CALLBACK, (s), (l), (v), "n", (h),	\
+	  PARSE_OPT_NONEG, option_parse_ulong }
+
 int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 {
 	int use_internal_rev_list = 0;
 	int thin = 0;
 	int all_progress_implied = 0;
-	uint32_t i;
-	const char **rp_av;
-	int rp_ac_alloc = 64;
-	int rp_ac;
+	const char *rp_av[6];
+	int rp_ac = 0;
+	int rev_list_unpacked = 0, rev_list_all = 0, rev_list_reflog = 0;
+	struct option pack_objects_options[] = {
+		OPT_SET_INT('q', "quiet", &progress,
+			    "do not show progress meter", 0),
+		OPT_SET_INT(0, "progress", &progress,
+			    "show progress meter", 1),
+		OPT_SET_INT(0, "all-progress", &progress,
+			    "show progress meter during object writing phase", 2),
+		OPT_BOOL(0, "all-progress-implied",
+			 &all_progress_implied,
+			 "similar to --all-progress when progress meter is shown"),
+		{ OPTION_CALLBACK, 0, "index-version", NULL, "version[,offset]",
+		  "write the pack index file in the specified idx format version",
+		  0, option_parse_index_version },
+		OPT_ULONG(0, "max-pack-size", &pack_size_limit,
+			  "maximum size of each output pack file"),
+		OPT_BOOL(0, "local", &local,
+			 "ignore borrowed objects from alternate object store"),
+		OPT_BOOL(0, "incremental", &incremental,
+			 "ignore packed objects"),
+		OPT_INTEGER(0, "window", &window,
+			    "limit pack window by objects"),
+		OPT_ULONG(0, "window-memory", &window_memory_limit,
+			  "limit pack window by memory in addition to object limit"),
+		OPT_INTEGER(0, "depth", &depth,
+			    "maximum length of delta chain allowed in the resulting pack"),
+		OPT_BOOL(0, "reuse-delta", &reuse_delta,
+			 "reuse existing deltas"),
+		OPT_BOOL(0, "reuse-object", &reuse_object,
+			 "reuse existing objects"),
+		OPT_BOOL(0, "delta-base-offset", &allow_ofs_delta,
+			 "use OFS_DELTA objects"),
+		OPT_INTEGER(0, "threads", &delta_search_threads,
+			    "use threads when searching for best delta matches"),
+		OPT_BOOL(0, "non-empty", &non_empty,
+			 "do not create an empty pack output"),
+		OPT_BOOL(0, "revs", &use_internal_rev_list,
+			 "read revision arguments from standard input"),
+		{ OPTION_SET_INT, 0, "unpacked", &rev_list_unpacked, NULL,
+		  "limit the objects to those that are not yet packed",
+		  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
+		{ OPTION_SET_INT, 0, "all", &rev_list_all, NULL,
+		  "include objects reachable from any reference",
+		  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
+		{ OPTION_SET_INT, 0, "reflog", &rev_list_reflog, NULL,
+		  "include objects referred by reflog entries",
+		  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
+		OPT_BOOL(0, "stdout", &pack_to_stdout,
+			 "output pack to stdout"),
+		OPT_BOOL(0, "include-tag", &include_tag,
+			 "include tag objects that refer to objects to be packed"),
+		OPT_BOOL(0, "keep-unreachable", &keep_unreachable,
+			 "keep unreachable objects"),
+		OPT_BOOL(0, "unpack-unreachable", &unpack_unreachable,
+			 "unpack unreachable objects"),
+		OPT_BOOL(0, "thin", &thin,
+			 "create thin packs"),
+		OPT_BOOL(0, "honor-pack-keep", &ignore_packed_keep,
+			 "ignore packs that have companion .keep file"),
+		OPT_INTEGER(0, "compression", &pack_compression_level,
+			    "pack compression level"),
+		OPT_SET_INT(0, "keep-true-parents", &grafts_replace_parents,
+			    "do not hide commits by grafts", 0),
+		OPT_END(),
+	};
 
 	read_replace_refs = 0;
 
-	rp_av = xcalloc(rp_ac_alloc, sizeof(*rp_av));
-
-	rp_av[0] = "pack-objects";
-	rp_av[1] = "--objects"; /* --thin will make it --objects-edge */
-	rp_ac = 2;
-
 	reset_pack_idx_option(&pack_idx_opts);
 	git_config(git_pack_config, NULL);
 	if (!pack_compression_seen && core_compression_seen)
 		pack_compression_level = core_compression_level;
 
 	progress = isatty(2);
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
-
-		if (*arg != '-')
-			break;
+	argc = parse_options(argc, argv, prefix, pack_objects_options,
+			     pack_usage, 0);
 
-		if (!strcmp("--non-empty", arg)) {
-			non_empty = 1;
-			continue;
-		}
-		if (!strcmp("--local", arg)) {
-			local = 1;
-			continue;
-		}
-		if (!strcmp("--incremental", arg)) {
-			incremental = 1;
-			continue;
-		}
-		if (!strcmp("--honor-pack-keep", arg)) {
-			ignore_packed_keep = 1;
-			continue;
-		}
-		if (!prefixcmp(arg, "--compression=")) {
-			char *end;
-			int level = strtoul(arg+14, &end, 0);
-			if (!arg[14] || *end)
-				usage(pack_usage);
-			if (level == -1)
-				level = Z_DEFAULT_COMPRESSION;
-			else if (level < 0 || level > Z_BEST_COMPRESSION)
-				die("bad pack compression level %d", level);
-			pack_compression_level = level;
-			continue;
-		}
-		if (!prefixcmp(arg, "--max-pack-size=")) {
-			pack_size_limit_cfg = 0;
-			if (!git_parse_ulong(arg+16, &pack_size_limit))
-				usage(pack_usage);
-			continue;
-		}
-		if (!prefixcmp(arg, "--window=")) {
-			char *end;
-			window = strtoul(arg+9, &end, 0);
-			if (!arg[9] || *end)
-				usage(pack_usage);
-			continue;
-		}
-		if (!prefixcmp(arg, "--window-memory=")) {
-			if (!git_parse_ulong(arg+16, &window_memory_limit))
-				usage(pack_usage);
-			continue;
-		}
-		if (!prefixcmp(arg, "--threads=")) {
-			char *end;
-			delta_search_threads = strtoul(arg+10, &end, 0);
-			if (!arg[10] || *end || delta_search_threads < 0)
-				usage(pack_usage);
-#ifdef NO_PTHREADS
-			if (delta_search_threads != 1)
-				warning("no threads support, "
-					"ignoring %s", arg);
-#endif
-			continue;
-		}
-		if (!prefixcmp(arg, "--depth=")) {
-			char *end;
-			depth = strtoul(arg+8, &end, 0);
-			if (!arg[8] || *end)
-				usage(pack_usage);
-			continue;
-		}
-		if (!strcmp("--progress", arg)) {
-			progress = 1;
-			continue;
-		}
-		if (!strcmp("--all-progress", arg)) {
-			progress = 2;
-			continue;
-		}
-		if (!strcmp("--all-progress-implied", arg)) {
-			all_progress_implied = 1;
-			continue;
-		}
-		if (!strcmp("-q", arg)) {
-			progress = 0;
-			continue;
-		}
-		if (!strcmp("--no-reuse-delta", arg)) {
-			reuse_delta = 0;
-			continue;
-		}
-		if (!strcmp("--no-reuse-object", arg)) {
-			reuse_object = reuse_delta = 0;
-			continue;
-		}
-		if (!strcmp("--delta-base-offset", arg)) {
-			allow_ofs_delta = 1;
-			continue;
-		}
-		if (!strcmp("--stdout", arg)) {
-			pack_to_stdout = 1;
-			continue;
-		}
-		if (!strcmp("--revs", arg)) {
-			use_internal_rev_list = 1;
-			continue;
-		}
-		if (!strcmp("--keep-unreachable", arg)) {
-			keep_unreachable = 1;
-			continue;
-		}
-		if (!strcmp("--unpack-unreachable", arg)) {
-			unpack_unreachable = 1;
-			continue;
-		}
-		if (!strcmp("--include-tag", arg)) {
-			include_tag = 1;
-			continue;
-		}
-		if (!strcmp("--unpacked", arg) ||
-		    !strcmp("--reflog", arg) ||
-		    !strcmp("--all", arg)) {
-			use_internal_rev_list = 1;
-			if (rp_ac >= rp_ac_alloc - 1) {
-				rp_ac_alloc = alloc_nr(rp_ac_alloc);
-				rp_av = xrealloc(rp_av,
-						 rp_ac_alloc * sizeof(*rp_av));
-			}
-			rp_av[rp_ac++] = arg;
-			continue;
-		}
-		if (!strcmp("--thin", arg)) {
-			use_internal_rev_list = 1;
-			thin = 1;
-			rp_av[1] = "--objects-edge";
-			continue;
-		}
-		if (!prefixcmp(arg, "--index-version=")) {
-			char *c;
-			pack_idx_opts.version = strtoul(arg + 16, &c, 10);
-			if (pack_idx_opts.version > 2)
-				die("bad %s", arg);
-			if (*c == ',')
-				pack_idx_opts.off32_limit = strtoul(c+1, &c, 0);
-			if (*c || pack_idx_opts.off32_limit & 0x80000000)
-				die("bad %s", arg);
-			continue;
-		}
-		if (!strcmp(arg, "--keep-true-parents")) {
-			grafts_replace_parents = 0;
-			continue;
-		}
-		usage(pack_usage);
-	}
-
-	/* Traditionally "pack-objects [options] base extra" failed;
-	 * we would however want to take refs parameter that would
-	 * have been given to upstream rev-list ourselves, which means
-	 * we somehow want to say what the base name is.  So the
-	 * syntax would be:
-	 *
-	 * pack-objects [options] base <refs...>
-	 *
-	 * in other words, we would treat the first non-option as the
-	 * base_name and send everything else to the internal revision
-	 * walker.
-	 */
+	if (argc) {
+		base_name = argv[0];
+		argc--;
+	}
+	if (pack_to_stdout != !base_name || argc)
+		usage_with_options(pack_usage, pack_objects_options);
 
-	if (!pack_to_stdout)
-		base_name = argv[i++];
+	rp_av[rp_ac++] = "pack-objects";
+	if (thin) {
+		use_internal_rev_list = 1;
+		rp_av[rp_ac++] = "--objects-edge";
+	} else
+		rp_av[rp_ac++] = "--objects";
 
-	if (pack_to_stdout != !base_name)
-		usage(pack_usage);
+	if (rev_list_all) {
+		use_internal_rev_list = 1;
+		rp_av[rp_ac++] = "--all";
+	}
+	if (rev_list_reflog) {
+		use_internal_rev_list = 1;
+		rp_av[rp_ac++] = "--reflog";
+	}
+	if (rev_list_unpacked) {
+		use_internal_rev_list = 1;
+		rp_av[rp_ac++] = "--unpacked";
+	}
 
+	if (!reuse_object)
+		reuse_delta = 0;
+	if (pack_compression_level == -1)
+		pack_compression_level = Z_DEFAULT_COMPRESSION;
+	else if (pack_compression_level < 0 || pack_compression_level > Z_BEST_COMPRESSION)
+		die("bad pack compression level %d", pack_compression_level);
+#ifdef NO_PTHREADS
+	if (delta_search_threads != 1)
+		warning("no threads support, ignoring %s", arg);
+#endif
 	if (!pack_to_stdout && !pack_size_limit)
 		pack_size_limit = pack_size_limit_cfg;
 	if (pack_to_stdout && pack_size_limit)

t/t5300-pack-object.sh

@@ -38,6 +38,10 @@ test_expect_success \
     'pack without delta' \
     'packname_1=$(git pack-objects --window=0 test-1 <obj-list)'
 
+test_expect_success \
+    'pack-objects with bogus arguments' \
+    'test_must_fail git pack-objects --window=0 test-1 blah blah <obj-list'
+
 rm -fr .git2
 mkdir .git2
 

t/t5302-pack-index.sh

@@ -73,6 +73,10 @@ test_expect_success 'index-pack --verify on index version 2' '
 	git index-pack --verify "test-2-${pack2}.pack"
 '
 
+test_expect_success \
+    'pack-objects --index-version=2, is not accepted' \
+    'test_must_fail git pack-objects --index-version=2, test-3 <obj-list'
+
 test_expect_success \
     'index v2: force some 64-bit offsets with pack-objects' \
     'pack3=$(git pack-objects --index-version=2,0x40000 test-3 <obj-list)'