{"thread":{"id":"29488","subject":"[PATCH] pack-objects: convert to use parse_options()","startedAt":"2012-01-31T13:48:22Z","lastAt":"2012-02-03T00:31:44Z","messageCount":8,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Nguyen Thai Ngoc Duy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"183412","messageId":"1328017702-14489-1-git-send-email-pclouds@gmail.com","threadId":"29488","inReplyTo":null,"subject":"[PATCH] pack-objects: convert to use parse_options()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-01-31T13:48:22Z","receivedAt":"2012-01-31T13:48:22Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n builtin/pack-objects.c |  317 +++++++++++++++++++++---------------------------\n 1 files changed, 141 insertions(+), 176 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 0f2e7b8..59fdb3f 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -18,16 +18,11 @@\n #include \"refs.h\"\n #include \"thread-utils.h\"\n \n-static const char pack_usage[] =\n-  \"git pack-objects [ -q | --progress | --all-progress ]\\n\"\n-  \"        [--all-progress-implied]\\n\"\n-  \"        [--max-pack-size=<n>] [--local] [--incremental]\\n\"\n-  \"        [--window=<n>] [--window-memory=<n>] [--depth=<n>]\\n\"\n-  \"        [--no-reuse-delta] [--no-reuse-object] [--delta-base-offset]\\n\"\n-  \"        [--threads=<n>] [--non-empty] [--revs [--unpacked | --all]]\\n\"\n-  \"        [--reflog] [--stdout | base-name] [--include-tag]\\n\"\n-  \"        [--keep-unreachable | --unpack-unreachable]\\n\"\n-  \"        [< ref-list | < object-list]\";\n+static const char *pack_usage[] = {\n+\t\"git pack-objects --stdout [options...] [< ref-list | < object-list]\",\n+\t\"git pack-objects [options...] base-name [< ref-list | < object-list]\",\n+\tNULL\n+};\n \n struct object_entry {\n \tstruct pack_idx_entry idx;\n@@ -2305,183 +2300,140 @@ static void get_object_list(int ac, const char **av)\n \t\tloosen_unused_packed_objects(&revs);\n }\n \n+static int option_parse_index_version(const struct option *opt,\n+\t\t\t\t      const char *arg, int unset)\n+{\n+\tchar *c;\n+\tconst char *val = arg;\n+\tpack_idx_opts.version = strtoul(val, &c, 10);\n+\tif (pack_idx_opts.version > 2)\n+\t\tdie(\"unsupported index version %s\", val);\n+\tif (*c == ',' && c[1])\n+\t\tpack_idx_opts.off32_limit = strtoul(c+1, &c, 0);\n+\tif (*c || pack_idx_opts.off32_limit & 0x80000000)\n+\t\tdie(\"bad index version %s\", val);\n+\treturn 0;\n+}\n+\n+static int option_parse_ulong(const struct option *opt,\n+\t\t\t      const char *arg, int unset)\n+{\n+\tif (unset)\n+\t\tdie(\"option %s does not accept negative form\",\n+\t\t    opt->long_name);\n+\n+\tif (!git_parse_ulong(arg, opt->value))\n+\t\tdie(\"unable to parse value '%s' for option %s\",\n+\t\t    arg, opt->long_name);\n+\treturn 0;\n+}\n+\n+#define OPT_ULONG(s, l, v, h) \\\n+\t{ OPTION_CALLBACK, (s), (l), (v), \"n\", (h),\t\\\n+\t  PARSE_OPT_NONEG, option_parse_ulong }\n+\n int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n {\n \tint use_internal_rev_list = 0;\n \tint thin = 0;\n \tint all_progress_implied = 0;\n-\tuint32_t i;\n-\tconst char **rp_av;\n-\tint rp_ac_alloc = 64;\n-\tint rp_ac;\n+\tconst char *rp_av[6];\n+\tint rp_ac = 0;\n+\tint rev_list_unpacked = 0, rev_list_all = 0, rev_list_reflog = 0;\n+\tstruct option pack_objects_options[] = {\n+\t\tOPT_SET_INT('q', \"quiet\", &progress,\n+\t\t\t    \"do not show progress meter\", 0),\n+\t\tOPT_SET_INT(0, \"progress\", &progress,\n+\t\t\t    \"show progress meter\", 1),\n+\t\tOPT_SET_INT(0, \"all-progress\", &progress,\n+\t\t\t    \"show progress meter during object writing phase\", 2),\n+\t\tOPT_BOOL(0, \"all-progress-implied\",\n+\t\t\t &all_progress_implied,\n+\t\t\t \"similar to --all-progress when progress meter is shown\"),\n+\t\t{ OPTION_CALLBACK, 0, \"index-version\", NULL, \"version\",\n+\t\t  \"force generating pack index at a particular version\",\n+\t\t  0, option_parse_index_version },\n+\t\tOPT_ULONG(0, \"max-pack-size\", &pack_size_limit,\n+\t\t\t  \"maximum size of each output pack file\"),\n+\t\tOPT_BOOL(0, \"local\", &local,\n+\t\t\t \"ignore borrowed objects from alternate object store\"),\n+\t\tOPT_BOOL(0, \"incremental\", &incremental,\n+\t\t\t \"ignore packed objects\"),\n+\t\tOPT_INTEGER(0, \"window\", &window,\n+\t\t\t    \"limit pack window by objects\"),\n+\t\tOPT_ULONG(0, \"window-memory\", &window_memory_limit,\n+\t\t\t  \"limit pack window by memory\"),\n+\t\tOPT_INTEGER(0, \"depth\", &depth,\n+\t\t\t    \"limit pack window by maximum delta depth\"),\n+\t\tOPT_BOOL(0, \"reuse-delta\", &reuse_delta,\n+\t\t\t \"reusing existing deltas\"),\n+\t\tOPT_BOOL(0, \"reuse-object\", &reuse_object,\n+\t\t\t \"reusing existing objects\"),\n+\t\tOPT_BOOL(0, \"delta-base-offset\", &allow_ofs_delta,\n+\t\t\t \"use OFS_DELTA objects\"),\n+\t\tOPT_INTEGER(0, \"threads\", &delta_search_threads,\n+\t\t\t    \"use threads when searching for best delta matches\"),\n+\t\tOPT_BOOL(0, \"non-empty\", &non_empty,\n+\t\t\t \"only create if it would contain at least one object\"),\n+\t\tOPT_BOOL(0, \"revs\", &use_internal_rev_list,\n+\t\t\t \"read revision arguments from standard output\"),\n+\t\t{ OPTION_SET_INT, 0, \"unpacked\", &rev_list_unpacked, NULL,\n+\t\t  \"limit the objects to those that are not already packed\",\n+\t\t  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },\n+\t\t{ OPTION_SET_INT, 0, \"all\", &rev_list_all, NULL,\n+\t\t  \"include all refs under $GIT_DIR/refs directory\",\n+\t\t  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },\n+\t\t{ OPTION_SET_INT, 0, \"reflog\", &rev_list_reflog, NULL,\n+\t\t  \"include objects referred by reflog entries\",\n+\t\t  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },\n+\t\tOPT_BOOL(0, \"stdout\", &pack_to_stdout,\n+\t\t\t \"output pack to stdout\"),\n+\t\tOPT_BOOL(0, \"include-tag\", &include_tag,\n+\t\t\t \"include unasked-for annotated tags\"),\n+\t\tOPT_BOOL(0, \"keep-unreachable\", &keep_unreachable,\n+\t\t\t \"keep unreachable objects\"),\n+\t\tOPT_BOOL(0, \"unpack-unreachable\", &unpack_unreachable,\n+\t\t\t \"unpack unreachable objects\"),\n+\t\tOPT_BOOL(0, \"thin\", &thin,\n+\t\t\t \"create thin packs\"),\n+\t\tOPT_BOOL(0, \"honor-pack-keep\", &ignore_packed_keep,\n+\t\t\t \"ignore packs that have companion .keep file\"),\n+\t\tOPT_INTEGER(0, \"compression\", &pack_compression_level,\n+\t\t\t    \"pack compression level\"),\n+\t\tOPT_SET_INT(0, \"keep-true-parents\", &grafts_replace_parents,\n+\t\t\t    \"do not hide commits by grafts\", 0),\n+\t\tOPT_END(),\n+\t};\n \n \tread_replace_refs = 0;\n \n-\trp_av = xcalloc(rp_ac_alloc, sizeof(*rp_av));\n-\n-\trp_av[0] = \"pack-objects\";\n-\trp_av[1] = \"--objects\"; /* --thin will make it --objects-edge */\n-\trp_ac = 2;\n-\n \treset_pack_idx_option(&pack_idx_opts);\n \tgit_config(git_pack_config, NULL);\n \tif (!pack_compression_seen && core_compression_seen)\n \t\tpack_compression_level = core_compression_level;\n \n \tprogress = isatty(2);\n-\tfor (i = 1; i < argc; i++) {\n-\t\tconst char *arg = argv[i];\n+\targc = parse_options(argc, argv, prefix, pack_objects_options,\n+\t\t\t     pack_usage, 0);\n \n-\t\tif (*arg != '-')\n-\t\t\tbreak;\n+\trp_av[rp_ac++] = \"pack-objects\";\n+\tif (thin) {\n+\t\tuse_internal_rev_list = 1;\n+\t\trp_av[rp_ac++] = \"--objects-edge\";\n+\t} else\n+\t\trp_av[rp_ac++] = \"--objects\";\n \n-\t\tif (!strcmp(\"--non-empty\", arg)) {\n-\t\t\tnon_empty = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--local\", arg)) {\n-\t\t\tlocal = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--incremental\", arg)) {\n-\t\t\tincremental = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--honor-pack-keep\", arg)) {\n-\t\t\tignore_packed_keep = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!prefixcmp(arg, \"--compression=\")) {\n-\t\t\tchar *end;\n-\t\t\tint level = strtoul(arg+14, &end, 0);\n-\t\t\tif (!arg[14] || *end)\n-\t\t\t\tusage(pack_usage);\n-\t\t\tif (level == -1)\n-\t\t\t\tlevel = Z_DEFAULT_COMPRESSION;\n-\t\t\telse if (level < 0 || level > Z_BEST_COMPRESSION)\n-\t\t\t\tdie(\"bad pack compression level %d\", level);\n-\t\t\tpack_compression_level = level;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!prefixcmp(arg, \"--max-pack-size=\")) {\n-\t\t\tpack_size_limit_cfg = 0;\n-\t\t\tif (!git_parse_ulong(arg+16, &pack_size_limit))\n-\t\t\t\tusage(pack_usage);\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!prefixcmp(arg, \"--window=\")) {\n-\t\t\tchar *end;\n-\t\t\twindow = strtoul(arg+9, &end, 0);\n-\t\t\tif (!arg[9] || *end)\n-\t\t\t\tusage(pack_usage);\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!prefixcmp(arg, \"--window-memory=\")) {\n-\t\t\tif (!git_parse_ulong(arg+16, &window_memory_limit))\n-\t\t\t\tusage(pack_usage);\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!prefixcmp(arg, \"--threads=\")) {\n-\t\t\tchar *end;\n-\t\t\tdelta_search_threads = strtoul(arg+10, &end, 0);\n-\t\t\tif (!arg[10] || *end || delta_search_threads < 0)\n-\t\t\t\tusage(pack_usage);\n-#ifdef NO_PTHREADS\n-\t\t\tif (delta_search_threads != 1)\n-\t\t\t\twarning(\"no threads support, \"\n-\t\t\t\t\t\"ignoring %s\", arg);\n-#endif\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!prefixcmp(arg, \"--depth=\")) {\n-\t\t\tchar *end;\n-\t\t\tdepth = strtoul(arg+8, &end, 0);\n-\t\t\tif (!arg[8] || *end)\n-\t\t\t\tusage(pack_usage);\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--progress\", arg)) {\n-\t\t\tprogress = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--all-progress\", arg)) {\n-\t\t\tprogress = 2;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--all-progress-implied\", arg)) {\n-\t\t\tall_progress_implied = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"-q\", arg)) {\n-\t\t\tprogress = 0;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--no-reuse-delta\", arg)) {\n-\t\t\treuse_delta = 0;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--no-reuse-object\", arg)) {\n-\t\t\treuse_object = reuse_delta = 0;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--delta-base-offset\", arg)) {\n-\t\t\tallow_ofs_delta = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--stdout\", arg)) {\n-\t\t\tpack_to_stdout = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--revs\", arg)) {\n-\t\t\tuse_internal_rev_list = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--keep-unreachable\", arg)) {\n-\t\t\tkeep_unreachable = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--unpack-unreachable\", arg)) {\n-\t\t\tunpack_unreachable = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--include-tag\", arg)) {\n-\t\t\tinclude_tag = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--unpacked\", arg) ||\n-\t\t    !strcmp(\"--reflog\", arg) ||\n-\t\t    !strcmp(\"--all\", arg)) {\n-\t\t\tuse_internal_rev_list = 1;\n-\t\t\tif (rp_ac >= rp_ac_alloc - 1) {\n-\t\t\t\trp_ac_alloc = alloc_nr(rp_ac_alloc);\n-\t\t\t\trp_av = xrealloc(rp_av,\n-\t\t\t\t\t\t rp_ac_alloc * sizeof(*rp_av));\n-\t\t\t}\n-\t\t\trp_av[rp_ac++] = arg;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--thin\", arg)) {\n-\t\t\tuse_internal_rev_list = 1;\n-\t\t\tthin = 1;\n-\t\t\trp_av[1] = \"--objects-edge\";\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!prefixcmp(arg, \"--index-version=\")) {\n-\t\t\tchar *c;\n-\t\t\tpack_idx_opts.version = strtoul(arg + 16, &c, 10);\n-\t\t\tif (pack_idx_opts.version > 2)\n-\t\t\t\tdie(\"bad %s\", arg);\n-\t\t\tif (*c == ',')\n-\t\t\t\tpack_idx_opts.off32_limit = strtoul(c+1, &c, 0);\n-\t\t\tif (*c || pack_idx_opts.off32_limit & 0x80000000)\n-\t\t\t\tdie(\"bad %s\", arg);\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"--keep-true-parents\")) {\n-\t\t\tgrafts_replace_parents = 0;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tusage(pack_usage);\n+\tif (rev_list_unpacked) {\n+\t\tuse_internal_rev_list = 1;\n+\t\trp_av[rp_ac++] = \"--unpacked\";\n+\t}\n+\tif (rev_list_all) {\n+\t\tuse_internal_rev_list = 1;\n+\t\trp_av[rp_ac++] = \"--all\";\n+\t}\n+\tif (rev_list_reflog) {\n+\t\tuse_internal_rev_list = 1;\n+\t\trp_av[rp_ac++] = \"--reflog\";\n \t}\n \n \t/* Traditionally \"pack-objects [options] base extra\" failed;\n@@ -2497,12 +2449,25 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t * walker.\n \t */\n \n-\tif (!pack_to_stdout)\n-\t\tbase_name = argv[i++];\n-\n-\tif (pack_to_stdout != !base_name)\n-\t\tusage(pack_usage);\n+\tif (!pack_to_stdout) {\n+\t\tif (!argc)\n+\t\t\tdie(\"base name required if --stdout is not given\");\n+\t\tbase_name = argv[0];\n+\t\targc--;\n+\t}\n+\tif (argc)\n+\t\tdie(\"base name or --stdout are mutually exclusive\");\n \n+\tif (!reuse_object)\n+\t\treuse_delta = 0;\n+\tif (pack_compression_level == -1)\n+\t\tpack_compression_level = Z_DEFAULT_COMPRESSION;\n+\telse if (pack_compression_level < 0 || pack_compression_level > Z_BEST_COMPRESSION)\n+\t\tdie(\"bad pack compression level %d\", pack_compression_level);\n+#ifdef NO_PTHREADS\n+\tif (delta_search_threads != 1)\n+\t\twarning(\"no threads support, ignoring %s\", arg);\n+#endif\n \tif (!pack_to_stdout && !pack_size_limit)\n \t\tpack_size_limit = pack_size_limit_cfg;\n \tif (pack_to_stdout && pack_size_limit)\n-- \n1.7.8.36.g69ee2\n"},{"id":"183443","messageId":"7v4nvb78om.fsf@alter.siamese.dyndns.org","threadId":"29488","inReplyTo":"1328017702-14489-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] pack-objects: convert to use parse_options()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-31T23:29:13Z","receivedAt":"2012-01-31T23:29:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> @@ -2305,183 +2300,140 @@ static void get_object_list(int ac, const char **av)\n>  \t\tloosen_unused_packed_objects(&revs);\n>  }\n>  \n> +static int option_parse_index_version(const struct option *opt,\n> +\t\t\t\t      const char *arg, int unset)\n> +{\n> +\tchar *c;\n> +\tconst char *val = arg;\n> +\tpack_idx_opts.version = strtoul(val, &c, 10);\n> +\tif (pack_idx_opts.version > 2)\n> +\t\tdie(\"unsupported index version %s\", val);\n\nDying may have been the appropriate error path in the custom option\nparser, but is it still so for a callback in the parse-options framework?\n\nOr should this be a 'return error(_(\"...\"), val)'?\n\n> +\tif (*c == ',' && c[1])\n> +\t\tpack_idx_opts.off32_limit = strtoul(c+1, &c, 0);\n\nI do not think the original had this extra \"&& c[1]\" check and I didn't\nsee any explanation of this change in the log message, either.\n\n> +static int option_parse_ulong(const struct option *opt,\n> +\t\t\t      const char *arg, int unset)\n> +{\n> +\tif (unset)\n> +\t\tdie(\"option %s does not accept negative form\",\n> +\t\t    opt->long_name);\n> +\n> +\tif (!git_parse_ulong(arg, opt->value))\n> +\t\tdie(\"unable to parse value '%s' for option %s\",\n> +\t\t    arg, opt->long_name);\n> +\treturn 0;\n> +}\n\nLikewise on \"die()\".\n\n> +#define OPT_ULONG(s, l, v, h) \\\n> +\t{ OPTION_CALLBACK, (s), (l), (v), \"n\", (h),\t\\\n> +\t  PARSE_OPT_NONEG, option_parse_ulong }\n> +\n>  int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n>  {\n>  \tint use_internal_rev_list = 0;\n>  \tint thin = 0;\n>  \tint all_progress_implied = 0;\n> -\tuint32_t i;\n> -\tconst char **rp_av;\n> -\tint rp_ac_alloc = 64;\n> -\tint rp_ac;\n> +\tconst char *rp_av[6];\n\nNice stackframe shrinkage.\n\n> +\tint rp_ac = 0;\n> +\tint rev_list_unpacked = 0, rev_list_all = 0, rev_list_reflog = 0;\n> +\tstruct option pack_objects_options[] = {\n> +\t\tOPT_SET_INT('q', \"quiet\", &progress,\n> +\t\t\t    \"do not show progress meter\", 0),\n> +\t\tOPT_SET_INT(0, \"progress\", &progress,\n> +\t\t\t    \"show progress meter\", 1),\n> +\t\tOPT_SET_INT(0, \"all-progress\", &progress,\n> +\t\t\t    \"show progress meter during object writing phase\", 2),\n\nSounds as if the progress is not shown during the counting phase.\nNothing \", too\" at the end could not fix, though.\n\n> +\t\tOPT_BOOL(0, \"all-progress-implied\",\n> +\t\t\t &all_progress_implied,\n> +\t\t\t \"similar to --all-progress when progress meter is shown\"),\n\nHrm, interesting wording.\n\n> +\t\t{ OPTION_CALLBACK, 0, \"index-version\", NULL, \"version\",\n> +\t\t  \"force generating pack index at a particular version\",\n> +\t\t  0, option_parse_index_version },\n\nSounds as if you can generate a pack index at HEAD~24, but that is not\nwhat you meant. \"version\" is too loaded a word that needs qualification.\n\nPerhaps \"write the pack index file in the specified idx format version\".\n\n\"index\" is also a loaded word, and our documentation tries to use \"idx\"\nwhen we talk about the pack index (we say \"index\" when we mean the\ndircache).\n\n> +\t\tOPT_INTEGER(0, \"window\", &window,\n> +\t\t\t    \"limit pack window by objects\"),\n> +\t\tOPT_ULONG(0, \"window-memory\", &window_memory_limit,\n> +\t\t\t  \"limit pack window by memory\"),\n> +\t\tOPT_INTEGER(0, \"depth\", &depth,\n> +\t\t\t    \"limit pack window by maximum delta depth\"),\n\nThese descriptions are somewhat interesting.\n\nThe \"pack window\" is limited by number of objects given by --window (and\nby default this is set to 10), but if you give --window-memory, it could\nfurther dynamically shrink it under memory pressure. If the logic were to\nuse this dynamic shrinkage without any limit on the number of objects in\nflight when --window-memory and no --window is given, then you could say\n\"limit by memory\" as if the \"pack window\" is counted in terms of number of\nbytes in the window instead of number of objects and the description would\nbe accurate. But that is not quite the case as far as I know.\n\nAnd --depth does not affect the size of the pack window at all. It rejects\nan object with deep enough delta chain as a delta-base candidate to limit\nthe run-time performance penalty for the user of the resulting pack, but\ndoes not stop the caller from trying the next object in the window.\n\"maximum length of delta chain allowed in the resulting pack\", perhaps?\n\n> +\t\tOPT_BOOL(0, \"reuse-delta\", &reuse_delta,\n> +\t\t\t \"reusing existing deltas\"),\n> +\t\tOPT_BOOL(0, \"reuse-object\", &reuse_object,\n> +\t\t\t \"reusing existing objects\"),\n\ns/reusing/reuse/???\n\n> +\t\tOPT_BOOL(0, \"delta-base-offset\", &allow_ofs_delta,\n> +\t\t\t \"use OFS_DELTA objects\"),\n> +\t\tOPT_INTEGER(0, \"threads\", &delta_search_threads,\n> +\t\t\t    \"use threads when searching for best delta matches\"),\n> +\t\tOPT_BOOL(0, \"non-empty\", &non_empty,\n> +\t\t\t \"only create if it would contain at least one object\"),\n\ncreate what? \"do not create an empty pack output\"?\n\n> +\t\tOPT_BOOL(0, \"revs\", &use_internal_rev_list,\n> +\t\t\t \"read revision arguments from standard output\"),\n\ns/output/input/???\n\n> +\t\t{ OPTION_SET_INT, 0, \"unpacked\", &rev_list_unpacked, NULL,\n> +\t\t  \"limit the objects to those that are not already packed\",\n> +\t\t  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },\n\ns/already/yet/???\n\n> +\t\t{ OPTION_SET_INT, 0, \"all\", &rev_list_all, NULL,\n> +\t\t  \"include all refs under $GIT_DIR/refs directory\",\n> +\t\t  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },\n> +\t\t{ OPTION_SET_INT, 0, \"reflog\", &rev_list_reflog, NULL,\n> +\t\t  \"include objects referred by reflog entries\",\n> +\t\t  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },\n\nThe latter is more correct---packfile does not pack refs, it packs\nobjects.\n\nPerhaps former should match: \"include objects reachable from any\nreference\". I would also prefer to avoid \"$GIT_DIR/refs _directory_\"\nto discourage people from running \"ls -R .git/refs/\".\n\n> +\t\tOPT_BOOL(0, \"stdout\", &pack_to_stdout,\n> +\t\t\t \"output pack to stdout\"),\n> +\t\tOPT_BOOL(0, \"include-tag\", &include_tag,\n> +\t\t\t \"include unasked-for annotated tags\"),\n\n\"include tag objects that refer to objects to be packed\"?\n\n> +\t\tOPT_BOOL(0, \"keep-unreachable\", &keep_unreachable,\n> +\t\t\t \"keep unreachable objects\"),\n\nWhat does this option mean these days (aka \"where are they kept\")?\n\nIIRC, this was meant to be used in conjunction with the --unpacked= and\nlater with --kept-pack-only (see 03a9683d22) and then was later further\nmodified by 4d6acb70411.\n\nI *think* this meant to include objects that are in packfiles that are not\nmarked with .keep even when they are not in the revision range to be\npacked, so that we won't lose them during gc.\n\nBut I think --unpack-unreachable introduced by ca11b21 (let pack-objects\ndo the writing of unreachable objects as loose objects, 2008-05-14) that\nwrites these unreachable objects out of the pack to loose form made this\noption unnecessary, and it remains unused ever since. It may not be a bad\nidea to deprecate this option.\n\nObviously it is outside of the scope of this patch.\n\n> +\t\tOPT_BOOL(0, \"unpack-unreachable\", &unpack_unreachable,\n> +\t\t\t \"unpack unreachable objects\"),\n> +\t\tOPT_BOOL(0, \"thin\", &thin,\n> +\t\t\t \"create thin packs\"),\n> +\t\tOPT_BOOL(0, \"honor-pack-keep\", &ignore_packed_keep,\n> +\t\t\t \"ignore packs that have companion .keep file\"),\n> +\t\tOPT_INTEGER(0, \"compression\", &pack_compression_level,\n> +\t\t\t    \"pack compression level\"),\n> +\t\tOPT_SET_INT(0, \"keep-true-parents\", &grafts_replace_parents,\n> +\t\t\t    \"do not hide commits by grafts\", 0),\n\nIt is noteworthy that this is truly \"do not hide\".\n\nA graft entry can also add extra commits, but this option will not prevent\nthe command from including them, so that the result of repacking will keep\nthe union of parents in the real history (i.e. in commit objects) and\nparents in the pretend history (i.e. added by grafts).\n\n> +\t\tOPT_END(),\n> +\t};\n>  \n>  \tread_replace_refs = 0;\n> ...\n> -\t\t\tcontinue;\n> -\t\t}\n> -\t\tusage(pack_usage);\n> +\tif (rev_list_unpacked) {\n> +\t\tuse_internal_rev_list = 1;\n> +\t\trp_av[rp_ac++] = \"--unpacked\";\n> +\t}\n> +\tif (rev_list_all) {\n> +\t\tuse_internal_rev_list = 1;\n> +\t\trp_av[rp_ac++] = \"--all\";\n> +\t}\n> +\tif (rev_list_reflog) {\n> +\t\tuse_internal_rev_list = 1;\n> +\t\trp_av[rp_ac++] = \"--reflog\";\n>  \t}\n\nI'd prefer to have \"unpacked\" the last, to match the order of parameters\nin which the typical caller, i.e. repack, calls us, but that is minor.\n\n> @@ -2497,12 +2449,25 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n>  \t * walker.\n>  \t */\n>  \n> -\tif (!pack_to_stdout)\n> -\t\tbase_name = argv[i++];\n> -\n> -\tif (pack_to_stdout != !base_name)\n> -\t\tusage(pack_usage);\n> +\tif (!pack_to_stdout) {\n> +\t\tif (!argc)\n> +\t\t\tdie(\"base name required if --stdout is not given\");\n> +\t\tbase_name = argv[0];\n> +\t\targc--;\n> +\t}\n> +\tif (argc)\n> +\t\tdie(\"base name or --stdout are mutually exclusive\");\n\nDoesn't this change regress the most basic, i.e.\n\n\t$ git pack-objects\n        usage: ...\n\nOther than that, thanks for a pleasant and thought-provoking read.\n"},{"id":"183473","messageId":"CACsJy8CZm96pVkavq_LvpX-MxdOtBw7XuwsQF61FJE8qk6ZZTg@mail.gmail.com","threadId":"29488","inReplyTo":"7v4nvb78om.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] pack-objects: convert to use parse_options()","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-02-01T14:30:02Z","receivedAt":"2012-02-01T14:30:02Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"I'm going to address your comments soon (thanks by the way, making up\nsummary line for each option from git-pack-objects.txt was painful)\nbut since you like thought-provoking, check this comment out:\n\n\t/* Traditionally \"pack-objects [options] base extra\" failed;\n\t * we would however want to take refs parameter that would\n\t * have been given to upstream rev-list ourselves, which means\n\t * we somehow want to say what the base name is.  So the\n\t * syntax would be:\n\t *\n\t * pack-objects [options] base <refs...>\n\t *\n\t * in other words, we would treat the first non-option as the\n\t * base_name and send everything else to the internal revision\n\t * walker.\n\t */\n\nThe last paragraph does not hold true anymore as far as I understand\nthe code. IOW it drops everything in argv after base name. Looking\nback in history, the change was made by 8d1d8f8 (pack-objects: further\nwork on internal rev-list logic. - 2006-09-06). Nothing is mentioned\nwhy it drops the remaining of argv. I suspect this is a regression and\nwe should reinstate the behavior as the comment says.\n-- \nDuy\n"},{"id":"183474","messageId":"CACsJy8DoMTfTGK+r2yHrPMxm08ROGxVHKM_PFkKe5s-XX=GJhQ@mail.gmail.com","threadId":"29488","inReplyTo":"CACsJy8CZm96pVkavq_LvpX-MxdOtBw7XuwsQF61FJE8qk6ZZTg@mail.gmail.com","subject":"Re: [PATCH] pack-objects: convert to use parse_options()","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-02-01T14:49:50Z","receivedAt":"2012-02-01T14:49:50Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Feb 1, 2012 at 9:30 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:\n>        /* Traditionally \"pack-objects [options] base extra\" failed;\n>         * we would however want to take refs parameter that would\n>         * have been given to upstream rev-list ourselves, which means\n>         * we somehow want to say what the base name is.  So the\n>         * syntax would be:\n>         *\n>         * pack-objects [options] base <refs...>\n>         *\n>         * in other words, we would treat the first non-option as the\n>         * base_name and send everything else to the internal revision\n>         * walker.\n>         */\n>\n> The last paragraph does not hold true anymore as far as I understand\n> the code. IOW it drops everything in argv after base name. Looking\n> back in history, the change was made by 8d1d8f8 (pack-objects: further\n> work on internal rev-list logic. - 2006-09-06). Nothing is mentioned\n> why it drops the remaining of argv. I suspect this is a regression and\n> we should reinstate the behavior as the comment says.\n\nNope, false alarm. The comment was introduced in b5d97e6\n(pack-objects: run rev-list equivalent internally. - 2006-09-04) and\nthe remaining of argv has been ignored ever since.\n-- \nDuy\n"},{"id":"183477","messageId":"1328109440-9510-1-git-send-email-pclouds@gmail.com","threadId":"29488","inReplyTo":"1328017702-14489-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 1/3] pack-objects: do not accept \"--index-version=version,\"","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-02-01T15:17:18Z","receivedAt":"2012-02-01T15:17:18Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n builtin/pack-objects.c |    2 +-\n t/t5302-pack-index.sh  |    4 ++++\n 2 files changed, 5 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 0f2e7b8..297f792 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -2471,7 +2471,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\t\tpack_idx_opts.version = strtoul(arg + 16, &c, 10);\n \t\t\tif (pack_idx_opts.version > 2)\n \t\t\t\tdie(\"bad %s\", arg);\n-\t\t\tif (*c == ',')\n+\t\t\tif (*c == ',' && c[1])\n \t\t\t\tpack_idx_opts.off32_limit = strtoul(c+1, &c, 0);\n \t\t\tif (*c || pack_idx_opts.off32_limit & 0x80000000)\n \t\t\t\tdie(\"bad %s\", arg);\ndiff --git a/t/t5302-pack-index.sh b/t/t5302-pack-index.sh\nindex f8fa924..fe82025 100755\n--- a/t/t5302-pack-index.sh\n+++ b/t/t5302-pack-index.sh\n@@ -74,6 +74,10 @@ test_expect_success 'index-pack --verify on index version 2' '\n '\n \n test_expect_success \\\n+    'pack-objects --index-version=2, is not accepted' \\\n+    'test_must_fail git pack-objects --index-version=2, test-3 <obj-list'\n+\n+test_expect_success \\\n     'index v2: force some 64-bit offsets with pack-objects' \\\n     'pack3=$(git pack-objects --index-version=2,0x40000 test-3 <obj-list)'\n \n-- \n1.7.8.36.g69ee2\n"},{"id":"183475","messageId":"1328109440-9510-2-git-send-email-pclouds@gmail.com","threadId":"29488","inReplyTo":"1328109440-9510-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 2/3] pack-objects: remove bogus comment","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-02-01T15:17:19Z","receivedAt":"2012-02-01T15:17:19Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The comment was introduced in b5d97e6 (pack-objects: run rev-list\nequivalent internally. - 2006-09-04), stating that\n\ngit pack-objects [options] base-name <refs...>\n\nis acceptable and refs should be passed into rev-list. But that's not\ntrue. All arguments after base-name are ignored.\n\nRemove the comment and reject this syntax (i.e. no more arguments after\nbase name)\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n builtin/pack-objects.c |   15 +--------------\n t/t5300-pack-object.sh |    4 ++++\n 2 files changed, 5 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 297f792..80e3114 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -2484,23 +2484,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\tusage(pack_usage);\n \t}\n \n-\t/* Traditionally \"pack-objects [options] base extra\" failed;\n-\t * we would however want to take refs parameter that would\n-\t * have been given to upstream rev-list ourselves, which means\n-\t * we somehow want to say what the base name is.  So the\n-\t * syntax would be:\n-\t *\n-\t * pack-objects [options] base <refs...>\n-\t *\n-\t * in other words, we would treat the first non-option as the\n-\t * base_name and send everything else to the internal revision\n-\t * walker.\n-\t */\n-\n \tif (!pack_to_stdout)\n \t\tbase_name = argv[i++];\n \n-\tif (pack_to_stdout != !base_name)\n+\tif (pack_to_stdout != !base_name || argv[i])\n \t\tusage(pack_usage);\n \n \tif (!pack_to_stdout && !pack_size_limit)\ndiff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\nindex 602806d..d9d856b 100755\n--- a/t/t5300-pack-object.sh\n+++ b/t/t5300-pack-object.sh\n@@ -38,6 +38,10 @@ test_expect_success \\\n     'pack without delta' \\\n     'packname_1=$(git pack-objects --window=0 test-1 <obj-list)'\n \n+test_expect_success \\\n+    'pack-objects with bogus arguments' \\\n+    'test_must_fail git pack-objects --window=0 test-1 blah blah <obj-list'\n+\n rm -fr .git2\n mkdir .git2\n \n-- \n1.7.8.36.g69ee2\n"},{"id":"183476","messageId":"1328109440-9510-3-git-send-email-pclouds@gmail.com","threadId":"29488","inReplyTo":"1328109440-9510-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 3/3] pack-objects: convert to use parse_options()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-02-01T15:17:20Z","receivedAt":"2012-02-01T15:17:20Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Regarding die() vs error() in option_parse_*, nope, using error()\n makes parse_options() print command usage, which is long and distracts\n users from the real error message. I'm going to stick with die().\n\n builtin/pack-objects.c |  315 +++++++++++++++++++++---------------------------\n 1 files changed, 139 insertions(+), 176 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 80e3114..e21e5af 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -18,16 +18,11 @@\n #include \"refs.h\"\n #include \"thread-utils.h\"\n \n-static const char pack_usage[] =\n-  \"git pack-objects [ -q | --progress | --all-progress ]\\n\"\n-  \"        [--all-progress-implied]\\n\"\n-  \"        [--max-pack-size=<n>] [--local] [--incremental]\\n\"\n-  \"        [--window=<n>] [--window-memory=<n>] [--depth=<n>]\\n\"\n-  \"        [--no-reuse-delta] [--no-reuse-object] [--delta-base-offset]\\n\"\n-  \"        [--threads=<n>] [--non-empty] [--revs [--unpacked | --all]]\\n\"\n-  \"        [--reflog] [--stdout | base-name] [--include-tag]\\n\"\n-  \"        [--keep-unreachable | --unpack-unreachable]\\n\"\n-  \"        [< ref-list | < object-list]\";\n+static const char *pack_usage[] = {\n+\t\"git pack-objects --stdout [options...] [< ref-list | < object-list]\",\n+\t\"git pack-objects [options...] base-name [< ref-list | < object-list]\",\n+\tNULL\n+};\n \n struct object_entry {\n \tstruct pack_idx_entry idx;\n@@ -2305,191 +2300,159 @@ static void get_object_list(int ac, const char **av)\n \t\tloosen_unused_packed_objects(&revs);\n }\n \n+static int option_parse_index_version(const struct option *opt,\n+\t\t\t\t      const char *arg, int unset)\n+{\n+\tchar *c;\n+\tconst char *val = arg;\n+\tpack_idx_opts.version = strtoul(val, &c, 10);\n+\tif (pack_idx_opts.version > 2)\n+\t\tdie(_(\"unsupported index version %s\"), val);\n+\tif (*c == ',' && c[1])\n+\t\tpack_idx_opts.off32_limit = strtoul(c+1, &c, 0);\n+\tif (*c || pack_idx_opts.off32_limit & 0x80000000)\n+\t\tdie(_(\"bad index version '%s'\"), val);\n+\treturn 0;\n+}\n+\n+static int option_parse_ulong(const struct option *opt,\n+\t\t\t      const char *arg, int unset)\n+{\n+\tif (unset)\n+\t\tdie(_(\"option %s does not accept negative form\"),\n+\t\t    opt->long_name);\n+\n+\tif (!git_parse_ulong(arg, opt->value))\n+\t\tdie(_(\"unable to parse value '%s' for option %s\"),\n+\t\t    arg, opt->long_name);\n+\treturn 0;\n+}\n+\n+#define OPT_ULONG(s, l, v, h) \\\n+\t{ OPTION_CALLBACK, (s), (l), (v), \"n\", (h),\t\\\n+\t  PARSE_OPT_NONEG, option_parse_ulong }\n+\n int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n {\n \tint use_internal_rev_list = 0;\n \tint thin = 0;\n \tint all_progress_implied = 0;\n-\tuint32_t i;\n-\tconst char **rp_av;\n-\tint rp_ac_alloc = 64;\n-\tint rp_ac;\n+\tconst char *rp_av[6];\n+\tint rp_ac = 0;\n+\tint rev_list_unpacked = 0, rev_list_all = 0, rev_list_reflog = 0;\n+\tstruct option pack_objects_options[] = {\n+\t\tOPT_SET_INT('q', \"quiet\", &progress,\n+\t\t\t    \"do not show progress meter\", 0),\n+\t\tOPT_SET_INT(0, \"progress\", &progress,\n+\t\t\t    \"show progress meter\", 1),\n+\t\tOPT_SET_INT(0, \"all-progress\", &progress,\n+\t\t\t    \"show progress meter during object writing phase\", 2),\n+\t\tOPT_BOOL(0, \"all-progress-implied\",\n+\t\t\t &all_progress_implied,\n+\t\t\t \"similar to --all-progress when progress meter is shown\"),\n+\t\t{ OPTION_CALLBACK, 0, \"index-version\", NULL, \"version[,offset]\",\n+\t\t  \"write the pack index file in the specified idx format version\",\n+\t\t  0, option_parse_index_version },\n+\t\tOPT_ULONG(0, \"max-pack-size\", &pack_size_limit,\n+\t\t\t  \"maximum size of each output pack file\"),\n+\t\tOPT_BOOL(0, \"local\", &local,\n+\t\t\t \"ignore borrowed objects from alternate object store\"),\n+\t\tOPT_BOOL(0, \"incremental\", &incremental,\n+\t\t\t \"ignore packed objects\"),\n+\t\tOPT_INTEGER(0, \"window\", &window,\n+\t\t\t    \"limit pack window by objects\"),\n+\t\tOPT_ULONG(0, \"window-memory\", &window_memory_limit,\n+\t\t\t  \"limit pack window by memory in addition to object limit\"),\n+\t\tOPT_INTEGER(0, \"depth\", &depth,\n+\t\t\t    \"maximum length of delta chain allowed in the resulting pack\"),\n+\t\tOPT_BOOL(0, \"reuse-delta\", &reuse_delta,\n+\t\t\t \"reuse existing deltas\"),\n+\t\tOPT_BOOL(0, \"reuse-object\", &reuse_object,\n+\t\t\t \"reuse existing objects\"),\n+\t\tOPT_BOOL(0, \"delta-base-offset\", &allow_ofs_delta,\n+\t\t\t \"use OFS_DELTA objects\"),\n+\t\tOPT_INTEGER(0, \"threads\", &delta_search_threads,\n+\t\t\t    \"use threads when searching for best delta matches\"),\n+\t\tOPT_BOOL(0, \"non-empty\", &non_empty,\n+\t\t\t \"do not create an empty pack output\"),\n+\t\tOPT_BOOL(0, \"revs\", &use_internal_rev_list,\n+\t\t\t \"read revision arguments from standard input\"),\n+\t\t{ OPTION_SET_INT, 0, \"unpacked\", &rev_list_unpacked, NULL,\n+\t\t  \"limit the objects to those that are not yet packed\",\n+\t\t  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },\n+\t\t{ OPTION_SET_INT, 0, \"all\", &rev_list_all, NULL,\n+\t\t  \"include objects reachable from any reference\",\n+\t\t  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },\n+\t\t{ OPTION_SET_INT, 0, \"reflog\", &rev_list_reflog, NULL,\n+\t\t  \"include objects referred by reflog entries\",\n+\t\t  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },\n+\t\tOPT_BOOL(0, \"stdout\", &pack_to_stdout,\n+\t\t\t \"output pack to stdout\"),\n+\t\tOPT_BOOL(0, \"include-tag\", &include_tag,\n+\t\t\t \"include tag objects that refer to objects to be packed\"),\n+\t\tOPT_BOOL(0, \"keep-unreachable\", &keep_unreachable,\n+\t\t\t \"keep unreachable objects\"),\n+\t\tOPT_BOOL(0, \"unpack-unreachable\", &unpack_unreachable,\n+\t\t\t \"unpack unreachable objects\"),\n+\t\tOPT_BOOL(0, \"thin\", &thin,\n+\t\t\t \"create thin packs\"),\n+\t\tOPT_BOOL(0, \"honor-pack-keep\", &ignore_packed_keep,\n+\t\t\t \"ignore packs that have companion .keep file\"),\n+\t\tOPT_INTEGER(0, \"compression\", &pack_compression_level,\n+\t\t\t    \"pack compression level\"),\n+\t\tOPT_SET_INT(0, \"keep-true-parents\", &grafts_replace_parents,\n+\t\t\t    \"do not hide commits by grafts\", 0),\n+\t\tOPT_END(),\n+\t};\n \n \tread_replace_refs = 0;\n \n-\trp_av = xcalloc(rp_ac_alloc, sizeof(*rp_av));\n-\n-\trp_av[0] = \"pack-objects\";\n-\trp_av[1] = \"--objects\"; /* --thin will make it --objects-edge */\n-\trp_ac = 2;\n-\n \treset_pack_idx_option(&pack_idx_opts);\n \tgit_config(git_pack_config, NULL);\n \tif (!pack_compression_seen && core_compression_seen)\n \t\tpack_compression_level = core_compression_level;\n \n \tprogress = isatty(2);\n-\tfor (i = 1; i < argc; i++) {\n-\t\tconst char *arg = argv[i];\n+\targc = parse_options(argc, argv, prefix, pack_objects_options,\n+\t\t\t     pack_usage, 0);\n \n-\t\tif (*arg != '-')\n-\t\t\tbreak;\n-\n-\t\tif (!strcmp(\"--non-empty\", arg)) {\n-\t\t\tnon_empty = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--local\", arg)) {\n-\t\t\tlocal = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--incremental\", arg)) {\n-\t\t\tincremental = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--honor-pack-keep\", arg)) {\n-\t\t\tignore_packed_keep = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!prefixcmp(arg, \"--compression=\")) {\n-\t\t\tchar *end;\n-\t\t\tint level = strtoul(arg+14, &end, 0);\n-\t\t\tif (!arg[14] || *end)\n-\t\t\t\tusage(pack_usage);\n-\t\t\tif (level == -1)\n-\t\t\t\tlevel = Z_DEFAULT_COMPRESSION;\n-\t\t\telse if (level < 0 || level > Z_BEST_COMPRESSION)\n-\t\t\t\tdie(\"bad pack compression level %d\", level);\n-\t\t\tpack_compression_level = level;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!prefixcmp(arg, \"--max-pack-size=\")) {\n-\t\t\tpack_size_limit_cfg = 0;\n-\t\t\tif (!git_parse_ulong(arg+16, &pack_size_limit))\n-\t\t\t\tusage(pack_usage);\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!prefixcmp(arg, \"--window=\")) {\n-\t\t\tchar *end;\n-\t\t\twindow = strtoul(arg+9, &end, 0);\n-\t\t\tif (!arg[9] || *end)\n-\t\t\t\tusage(pack_usage);\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!prefixcmp(arg, \"--window-memory=\")) {\n-\t\t\tif (!git_parse_ulong(arg+16, &window_memory_limit))\n-\t\t\t\tusage(pack_usage);\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!prefixcmp(arg, \"--threads=\")) {\n-\t\t\tchar *end;\n-\t\t\tdelta_search_threads = strtoul(arg+10, &end, 0);\n-\t\t\tif (!arg[10] || *end || delta_search_threads < 0)\n-\t\t\t\tusage(pack_usage);\n-#ifdef NO_PTHREADS\n-\t\t\tif (delta_search_threads != 1)\n-\t\t\t\twarning(\"no threads support, \"\n-\t\t\t\t\t\"ignoring %s\", arg);\n-#endif\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!prefixcmp(arg, \"--depth=\")) {\n-\t\t\tchar *end;\n-\t\t\tdepth = strtoul(arg+8, &end, 0);\n-\t\t\tif (!arg[8] || *end)\n-\t\t\t\tusage(pack_usage);\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--progress\", arg)) {\n-\t\t\tprogress = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--all-progress\", arg)) {\n-\t\t\tprogress = 2;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--all-progress-implied\", arg)) {\n-\t\t\tall_progress_implied = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"-q\", arg)) {\n-\t\t\tprogress = 0;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--no-reuse-delta\", arg)) {\n-\t\t\treuse_delta = 0;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--no-reuse-object\", arg)) {\n-\t\t\treuse_object = reuse_delta = 0;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--delta-base-offset\", arg)) {\n-\t\t\tallow_ofs_delta = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--stdout\", arg)) {\n-\t\t\tpack_to_stdout = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--revs\", arg)) {\n-\t\t\tuse_internal_rev_list = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--keep-unreachable\", arg)) {\n-\t\t\tkeep_unreachable = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--unpack-unreachable\", arg)) {\n-\t\t\tunpack_unreachable = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--include-tag\", arg)) {\n-\t\t\tinclude_tag = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--unpacked\", arg) ||\n-\t\t    !strcmp(\"--reflog\", arg) ||\n-\t\t    !strcmp(\"--all\", arg)) {\n-\t\t\tuse_internal_rev_list = 1;\n-\t\t\tif (rp_ac >= rp_ac_alloc - 1) {\n-\t\t\t\trp_ac_alloc = alloc_nr(rp_ac_alloc);\n-\t\t\t\trp_av = xrealloc(rp_av,\n-\t\t\t\t\t\t rp_ac_alloc * sizeof(*rp_av));\n-\t\t\t}\n-\t\t\trp_av[rp_ac++] = arg;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(\"--thin\", arg)) {\n-\t\t\tuse_internal_rev_list = 1;\n-\t\t\tthin = 1;\n-\t\t\trp_av[1] = \"--objects-edge\";\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!prefixcmp(arg, \"--index-version=\")) {\n-\t\t\tchar *c;\n-\t\t\tpack_idx_opts.version = strtoul(arg + 16, &c, 10);\n-\t\t\tif (pack_idx_opts.version > 2)\n-\t\t\t\tdie(\"bad %s\", arg);\n-\t\t\tif (*c == ',' && c[1])\n-\t\t\t\tpack_idx_opts.off32_limit = strtoul(c+1, &c, 0);\n-\t\t\tif (*c || pack_idx_opts.off32_limit & 0x80000000)\n-\t\t\t\tdie(\"bad %s\", arg);\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"--keep-true-parents\")) {\n-\t\t\tgrafts_replace_parents = 0;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tusage(pack_usage);\n+\tif (argc) {\n+\t\tbase_name = argv[0];\n+\t\targc--;\n \t}\n+\tif (pack_to_stdout != !base_name || argc)\n+\t\tusage_with_options(pack_usage, pack_objects_options);\n \n-\tif (!pack_to_stdout)\n-\t\tbase_name = argv[i++];\n+\trp_av[rp_ac++] = \"pack-objects\";\n+\tif (thin) {\n+\t\tuse_internal_rev_list = 1;\n+\t\trp_av[rp_ac++] = \"--objects-edge\";\n+\t} else\n+\t\trp_av[rp_ac++] = \"--objects\";\n \n-\tif (pack_to_stdout != !base_name || argv[i])\n-\t\tusage(pack_usage);\n+\tif (rev_list_all) {\n+\t\tuse_internal_rev_list = 1;\n+\t\trp_av[rp_ac++] = \"--all\";\n+\t}\n+\tif (rev_list_reflog) {\n+\t\tuse_internal_rev_list = 1;\n+\t\trp_av[rp_ac++] = \"--reflog\";\n+\t}\n+\tif (rev_list_unpacked) {\n+\t\tuse_internal_rev_list = 1;\n+\t\trp_av[rp_ac++] = \"--unpacked\";\n+\t}\n \n+\tif (!reuse_object)\n+\t\treuse_delta = 0;\n+\tif (pack_compression_level == -1)\n+\t\tpack_compression_level = Z_DEFAULT_COMPRESSION;\n+\telse if (pack_compression_level < 0 || pack_compression_level > Z_BEST_COMPRESSION)\n+\t\tdie(\"bad pack compression level %d\", pack_compression_level);\n+#ifdef NO_PTHREADS\n+\tif (delta_search_threads != 1)\n+\t\twarning(\"no threads support, ignoring %s\", arg);\n+#endif\n \tif (!pack_to_stdout && !pack_size_limit)\n \t\tpack_size_limit = pack_size_limit_cfg;\n \tif (pack_to_stdout && pack_size_limit)\n-- \n1.7.8.36.g69ee2\n"},{"id":"183681","messageId":"7v7h04vjtb.fsf@alter.siamese.dyndns.org","threadId":"29488","inReplyTo":"1328109440-9510-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v2 1/3] pack-objects: do not accept \"--index-version=version,\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-03T00:31:44Z","receivedAt":"2012-02-03T00:31:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks; will queue all three in the series.\n"}]}