| Message ID | 20240418184043.2900955-3-christian.couder@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | upload-pack: support a missing-action | expand |
Christian Couder <christian.couder@gmail.com> writes: > `git pack-objects` supports the `--missing=<missing-action>` option in > the same way as `git rev-list` except when '<missing-action>' is > "print", which `git pack-objects` doesn't support. > > As we want to refactor `git pack-objects` to use the same code from > "missing.{c,h}" as `git rev-list` for the `--missing=...` feature, let's > make it possible for that code to reject `--missing=print`. > > `git pack-objects` will then use that code in a following commit. > > Signed-off-by: Christian Couder <chriscool@tuxfamily.org> > --- > builtin/rev-list.c | 2 +- > missing.c | 4 ++-- > missing.h | 2 +- > 3 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/builtin/rev-list.c b/builtin/rev-list.c > index f71cc5ebe1..a712a6fd62 100644 > --- a/builtin/rev-list.c > +++ b/builtin/rev-list.c > @@ -539,7 +539,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix) > int res; > if (revs.exclude_promisor_objects) > die(_("options '%s' and '%s' cannot be used together"), "--exclude-promisor-objects", "--missing"); > - res = parse_missing_action_value(arg); > + res = parse_missing_action_value(arg, 1); Hmph, this smells like a horribly unscalable design, as we make the vocabulary of missing-action richer, you'd end up piling on "this choice is allowed in this call" parameters, wouldn't you? The first person who adds such an ad-hoc parameter would say "hey, what's just one extra parameter print_ok between friends", but the next person would say the same thing for their new choice and adds frotz_ok, and we'd be in chaos. Rather, shouldn't the _caller_ decide if the parsed value is something it does not like and barf? Alternatively, add a _single_ "reject" bitset and do something like int parse_missing_action_value(const char *value, unsigned reject) { ... if (!(reject & (1<<MA_ERROR) && !strcmp(value, "error"))) return MA_ERROR; if (!(reject & (1<<MA_PRINT) && !strcmp(value, "print"))) return MA_PRINT; ... which would scale better (but still my preference is to have the caller deal with only the values it recognises---do not make the caller say "if (res >= 0 && res != MA_PRINT)" as that will not scale when new choices that are accepted elsewhere are added.
diff --git a/builtin/rev-list.c b/builtin/rev-list.c index f71cc5ebe1..a712a6fd62 100644 --- a/builtin/rev-list.c +++ b/builtin/rev-list.c @@ -539,7 +539,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix) int res; if (revs.exclude_promisor_objects) die(_("options '%s' and '%s' cannot be used together"), "--exclude-promisor-objects", "--missing"); - res = parse_missing_action_value(arg); + res = parse_missing_action_value(arg, 1); if (res >= 0) { arg_missing_action = res; break; diff --git a/missing.c b/missing.c index 83e0c5e584..d306dea2d9 100644 --- a/missing.c +++ b/missing.c @@ -2,7 +2,7 @@ #include "missing.h" #include "object-file.h" -int parse_missing_action_value(const char *value) +int parse_missing_action_value(const char *value, int print_ok) { if (!strcmp(value, "error")) return MA_ERROR; @@ -12,7 +12,7 @@ int parse_missing_action_value(const char *value) return MA_ALLOW_ANY; } - if (!strcmp(value, "print")) { + if (!strcmp(value, "print") && print_ok) { fetch_if_missing = 0; return MA_PRINT; } diff --git a/missing.h b/missing.h index c8261dfe55..77906691e7 100644 --- a/missing.h +++ b/missing.h @@ -13,6 +13,6 @@ enum missing_action { if parsing failed. Also sets the fetch_if_missing global variable from "object-file.h". */ -int parse_missing_action_value(const char *value); +int parse_missing_action_value(const char *value, int print_ok); #endif /* MISSING_H */
`git pack-objects` supports the `--missing=<missing-action>` option in the same way as `git rev-list` except when '<missing-action>' is "print", which `git pack-objects` doesn't support. As we want to refactor `git pack-objects` to use the same code from "missing.{c,h}" as `git rev-list` for the `--missing=...` feature, let's make it possible for that code to reject `--missing=print`. `git pack-objects` will then use that code in a following commit. Signed-off-by: Christian Couder <chriscool@tuxfamily.org> --- builtin/rev-list.c | 2 +- missing.c | 4 ++-- missing.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-)