{"thread":{"id":"49940","subject":"[PATCH v2] revisions.c: put promisor option in specialized struct","startedAt":"2018-12-03T22:10:44Z","lastAt":"2018-12-03T22:17:32Z","messageCount":2,"participants":["Matthew DeVore","Jeff King"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"364494","messageId":"20181203221019.237034-1-matvore@google.com","threadId":"49940","inReplyTo":null,"subject":"[PATCH v2] revisions.c: put promisor option in specialized struct","fromName":"Matthew DeVore","fromEmail":"matvore@google.com","sentAt":"2018-12-03T22:10:19Z","receivedAt":"2018-12-03T22:10:44Z","isPatch":true,"sender":{"key":"matvore@google.com","avatar":"https://avatars.githubusercontent.com/u/946637?v=4"},"body":"Put the allow_exclude_promisor_objects flag in setup_revision_opt. When\nit was in rev_info, it was unclear when it was used, since rev_info is\npassed to functions that don't use the flag. This resulted in\nunnecessary setting of the flag in prune.c, so fix that as well.\n\nSigned-off-by: Matthew DeVore <matvore@google.com>\n---\n builtin/pack-objects.c |  6 ++++--\n builtin/prune.c        |  1 -\n builtin/rev-list.c     |  6 ++++--\n revision.c             | 10 ++++++----\n revision.h             |  4 ++--\n 5 files changed, 16 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 24bba8147f..889df2c755 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -3084,14 +3084,16 @@ static void record_recent_commit(struct commit *commit, void *data)\n static void get_object_list(int ac, const char **av)\n {\n \tstruct rev_info revs;\n+\tstruct setup_revision_opt s_r_opt = {\n+\t\t.allow_exclude_promisor_objects = 1,\n+\t};\n \tchar line[1000];\n \tint flags = 0;\n \tint save_warning;\n \n \trepo_init_revisions(the_repository, &revs, NULL);\n \tsave_commit_buffer = 0;\n-\trevs.allow_exclude_promisor_objects_opt = 1;\n-\tsetup_revisions(ac, av, &revs, NULL);\n+\tsetup_revisions(ac, av, &revs, &s_r_opt);\n \n \t/* make sure shallows are read */\n \tis_repository_shallow(the_repository);\ndiff --git a/builtin/prune.c b/builtin/prune.c\nindex e42653b99c..1ec9ddd751 100644\n--- a/builtin/prune.c\n+++ b/builtin/prune.c\n@@ -120,7 +120,6 @@ int cmd_prune(int argc, const char **argv, const char *prefix)\n \tsave_commit_buffer = 0;\n \tread_replace_refs = 0;\n \tref_paranoia = 1;\n-\trevs.allow_exclude_promisor_objects_opt = 1;\n \trepo_init_revisions(the_repository, &revs, prefix);\n \n \targc = parse_options(argc, argv, prefix, options, prune_usage, 0);\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex 3a2c0c23b6..14ef659c12 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -362,6 +362,9 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n {\n \tstruct rev_info revs;\n \tstruct rev_list_info info;\n+\tstruct setup_revision_opt s_r_opt = {\n+\t\t.allow_exclude_promisor_objects = 1,\n+\t};\n \tint i;\n \tint bisect_list = 0;\n \tint bisect_show_vars = 0;\n@@ -375,7 +378,6 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \tgit_config(git_default_config, NULL);\n \trepo_init_revisions(the_repository, &revs, prefix);\n \trevs.abbrev = DEFAULT_ABBREV;\n-\trevs.allow_exclude_promisor_objects_opt = 1;\n \trevs.commit_format = CMIT_FMT_UNSPECIFIED;\n \trevs.do_not_die_on_missing_tree = 1;\n \n@@ -407,7 +409,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \n-\targc = setup_revisions(argc, argv, &revs, NULL);\n+\targc = setup_revisions(argc, argv, &revs, &s_r_opt);\n \n \tmemset(&info, 0, sizeof(info));\n \tinfo.revs = &revs;\ndiff --git a/revision.c b/revision.c\nindex 13e0519c02..f6b32e6a42 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1791,7 +1791,8 @@ static void add_message_grep(struct rev_info *revs, const char *pattern)\n }\n \n static int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,\n-\t\t\t       int *unkc, const char **unkv)\n+\t\t\t       int *unkc, const char **unkv,\n+\t\t\t       const struct setup_revision_opt* opt)\n {\n \tconst char *arg = argv[0];\n \tconst char *optarg;\n@@ -2151,7 +2152,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->limited = 1;\n \t} else if (!strcmp(arg, \"--ignore-missing\")) {\n \t\trevs->ignore_missing = 1;\n-\t} else if (revs->allow_exclude_promisor_objects_opt &&\n+\t} else if (opt && opt->allow_exclude_promisor_objects &&\n \t\t   !strcmp(arg, \"--exclude-promisor-objects\")) {\n \t\tif (fetch_if_missing)\n \t\t\tBUG(\"exclude_promisor_objects can only be used when fetch_if_missing is 0\");\n@@ -2173,7 +2174,7 @@ void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,\n \t\t\tconst char * const usagestr[])\n {\n \tint n = handle_revision_opt(revs, ctx->argc, ctx->argv,\n-\t\t\t\t    &ctx->cpidx, ctx->out);\n+\t\t\t\t    &ctx->cpidx, ctx->out, NULL);\n \tif (n <= 0) {\n \t\terror(\"unknown option `%s'\", ctx->argv[0]);\n \t\tusage_with_options(usagestr, options);\n@@ -2391,7 +2392,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t\t\t\tcontinue;\n \t\t\t}\n \n-\t\t\topts = handle_revision_opt(revs, argc - i, argv + i, &left, argv);\n+\t\t\topts = handle_revision_opt(revs, argc - i, argv + i,\n+\t\t\t\t\t\t   &left, argv, opt);\n \t\t\tif (opts > 0) {\n \t\t\t\ti += opts - 1;\n \t\t\t\tcontinue;\ndiff --git a/revision.h b/revision.h\nindex 7987bfcd2e..52e5a88ff5 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -161,7 +161,6 @@ struct rev_info {\n \t\t\tdo_not_die_on_missing_tree:1,\n \n \t\t\t/* for internal use only */\n-\t\t\tallow_exclude_promisor_objects_opt:1,\n \t\t\texclude_promisor_objects:1;\n \n \t/* Diff flags */\n@@ -297,7 +296,8 @@ struct setup_revision_opt {\n \tconst char *def;\n \tvoid (*tweak)(struct rev_info *, struct setup_revision_opt *);\n \tconst char *submodule;\t/* TODO: drop this and use rev_info->repo */\n-\tint assume_dashdash;\n+\tunsigned int\tassume_dashdash:1,\n+\t\t\tallow_exclude_promisor_objects:1;\n \tunsigned revarg_opt;\n };\n \n-- \n2.20.0.rc1.387.gf8505762e3-goog\n\n"},{"id":"364496","messageId":"20181203221729.GC11883@sigill.intra.peff.net","threadId":"49940","inReplyTo":"20181203221019.237034-1-matvore@google.com","subject":"Re: [PATCH v2] revisions.c: put promisor option in specialized struct","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-12-03T22:17:29Z","receivedAt":"2018-12-03T22:17:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 03, 2018 at 02:10:19PM -0800, Matthew DeVore wrote:\n\n> Put the allow_exclude_promisor_objects flag in setup_revision_opt. When\n> it was in rev_info, it was unclear when it was used, since rev_info is\n> passed to functions that don't use the flag. This resulted in\n> unnecessary setting of the flag in prune.c, so fix that as well.\n> \n> Signed-off-by: Matthew DeVore <matvore@google.com>\n> ---\n>  builtin/pack-objects.c |  6 ++++--\n>  builtin/prune.c        |  1 -\n>  builtin/rev-list.c     |  6 ++++--\n>  revision.c             | 10 ++++++----\n>  revision.h             |  4 ++--\n>  5 files changed, 16 insertions(+), 11 deletions(-)\n\nThanks, this version looks good to me.\n\nOne style nit that I don't think is worth a re-roll, but that Junio\nmight want to tweak while applying:\n\n> diff --git a/revision.c b/revision.c\n> index 13e0519c02..f6b32e6a42 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -1791,7 +1791,8 @@ static void add_message_grep(struct rev_info *revs, const char *pattern)\n>  }\n>  \n>  static int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,\n> -\t\t\t       int *unkc, const char **unkv)\n> +\t\t\t       int *unkc, const char **unkv,\n> +\t\t\t       const struct setup_revision_opt* opt)\n\nWe keep the \"*\" with the variable name, not the type.\n\n-Peff\n"}]}