Junio C Hamano <gitster@xxxxxxxxx> writes: > Siddharth Agarwal <sid0@xxxxxx> writes: > >> With git-next, the --max-pack-size option to git repack doesn't work. >> >> With git at b139ac2, `git repack --max-pack-size=1g` says >> >> error: option `max-pack-size' expects a numerical value > > Thanks, Siddharth. > > It seems that we have a hand-crafted parser outside parse-options > framework in pack-objects, and the scripted git-repack used to pass > this option without interpreting itself. > > We probably should lift the OPT_ULONG() implementation out of > builtin/pack-objects.c and move it to parse-options.[ch] and use it > in the reimplementation of repack. > > And probably use it in other places where these "integers in > human-friendly units" may make sense, but that is a separate topic. The first step may look something like this (totally untested).. builtin/pack-objects.c | 22 +++------------------- parse-options.c | 17 +++++++++++++++++ parse-options.h | 3 +++ 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 541667f..b264f1f 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -2416,23 +2416,6 @@ static int option_parse_unpack_unreachable(const struct option *opt, 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; @@ -2462,8 +2445,9 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) N_("ignore packed objects")), OPT_INTEGER(0, "window", &window, N_("limit pack window by objects")), - OPT_ULONG(0, "window-memory", &window_memory_limit, - N_("limit pack window by memory in addition to object limit")), + { OPTION_ULONG, 0, "window-memory", &window_memory_limit, N_("n"), + N_("limit pack window by memory in addition to object limit"), + PARSE_OPT_HUMAN_NUMBER }, OPT_INTEGER(0, "depth", &depth, N_("maximum length of delta chain allowed in the resulting pack")), OPT_BOOL(0, "reuse-delta", &reuse_delta, diff --git a/parse-options.c b/parse-options.c index 7b8d3fa..3a47538 100644 --- a/parse-options.c +++ b/parse-options.c @@ -180,6 +180,23 @@ static int get_value(struct parse_opt_ctx_t *p, return opterror(opt, "expects a numerical value", flags); return 0; + case OPTION_ULONG: + if (unset) { + *(unsigned long *)opt->value = 0; + } else if (opt->flags & PARSE_OPT_OPTARG && !p->opt) { + *(unsigned long *)opt->value = opt->defval; + } else if (get_arg(p, opt, flags, &arg)) { + return -1; + } else if (opt->flags & PARSE_OPT_HUMAN_NUMBER) { + if (!git_parse_ulong(arg, (unsigned long *)opt->value)) + return opterror(opt, "expects a numerical value", flags); + } else { + *(unsigned long *)opt->value = strtoul(arg, (char **)&s, 10); + if (*s) + return opterror(opt, "expects a numerical value", flags); + } + return 0; + default: die("should not happen, someone must be hit on the forehead"); } diff --git a/parse-options.h b/parse-options.h index d670cb9..6a3cce0 100644 --- a/parse-options.h +++ b/parse-options.h @@ -17,6 +17,7 @@ enum parse_opt_type { /* options with arguments (usually) */ OPTION_STRING, OPTION_INTEGER, + OPTION_ULONG, OPTION_CALLBACK, OPTION_LOWLEVEL_CALLBACK, OPTION_FILENAME @@ -38,6 +39,7 @@ enum parse_opt_option_flags { PARSE_OPT_LASTARG_DEFAULT = 16, PARSE_OPT_NODASH = 32, PARSE_OPT_LITERAL_ARGHELP = 64, + PARSE_OPT_HUMAN_NUMBER = 128, PARSE_OPT_SHELL_EVAL = 256 }; @@ -133,6 +135,7 @@ struct option { #define OPT_CMDMODE(s, l, v, h, i) { OPTION_CMDMODE, (s), (l), (v), NULL, \ (h), PARSE_OPT_NOARG|PARSE_OPT_NONEG, NULL, (i) } #define OPT_INTEGER(s, l, v, h) { OPTION_INTEGER, (s), (l), (v), N_("n"), (h) } +#define OPT_ULONG(s, l, v, h) { OPTION_ULONG, (s), (l), (v), N_("n"), (h) } #define OPT_STRING(s, l, v, a, h) { OPTION_STRING, (s), (l), (v), (a), (h) } #define OPT_STRING_LIST(s, l, v, a, h) \ { OPTION_CALLBACK, (s), (l), (v), (a), \ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html