| Message ID | patch-15.17-c8ff241844a-20230317T152725Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 035c7de9e9ea11d26df5f9e4bb117f91ed11a9fd |
| Headers | show |
| Series | cocci: remove "the_index" wrapper macros | expand |
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > diff --git a/contrib/coccinelle/the_repository.pending.cocci b/contrib/coccinelle/the_repository.pending.cocci > deleted file mode 100644 > index 1190a3312bd..00000000000 > --- a/contrib/coccinelle/the_repository.pending.cocci > +++ /dev/null > @@ -1,14 +0,0 @@ > -// This file is used for the ongoing refactoring of > -// bringing the index or repository struct in all of > -// our code base. Now that we've deleted this file, I wanted to get a sense of where this series lands us in the the_repository migration. ISTR that we'd consider ourselves "done" when we stop referencing "the_repository" in non-builtins, so presumably we aren't there yet ;) Inspecting all of the ".h" files, we can see that the only remaining function/macro of this sort is "the_hash_algo". Because you expanded the search to cover cases not in "NO_THE_REPOSITORY_COMPATIBILITY_MACROS", you've actually achieved more than what your CL says. Hooray! We can't go so far as to say that we've removed all implicit references to "the_repository", though, since we still have functions that reference "the_repository" in their implementations. But, I don't think this ".cocci" file would help us with those cases anyway, since this was targeted specifically at functions/macros that were passing "the_repository" to functions that accepted a "struct repository" arg. Thanks for the cleanup, this is great!
Glen Choo <chooglen@google.com> writes: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> diff --git a/contrib/coccinelle/the_repository.pending.cocci b/contrib/coccinelle/the_repository.pending.cocci >> deleted file mode 100644 >> index 1190a3312bd..00000000000 >> --- a/contrib/coccinelle/the_repository.pending.cocci >> +++ /dev/null >> @@ -1,14 +0,0 @@ >> -// This file is used for the ongoing refactoring of >> -// bringing the index or repository struct in all of >> -// our code base. > We can't go so far as to say that we've removed all implicit references > to "the_repository", though, since we still have functions that > reference "the_repository" in their implementations. But, I don't think > this ".cocci" file would help us with those cases anyway, since this was > targeted specifically at functions/macros that were passing > "the_repository" to functions that accepted a "struct repository" arg. For these implicitly-the_repository functions, (e.g. git_path) we'd presumably refactor them into repo_* versions and then apply the same sorts of changes we did in this series? I guess we'd make those changes in contrib/coccinelle/the_repository.cocci, so we don't need the *.pending* one. On that note, I'm curious what contrib/coccinelle/the_repository.cocci is doing for us after this series. By definition, all of the macros have been fully migrated, so they're all a noop. Would this slow down `make coccicheck`?
On Wed, Mar 22 2023, Glen Choo wrote: > Glen Choo <chooglen@google.com> writes: > >> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: >> >>> diff --git a/contrib/coccinelle/the_repository.pending.cocci b/contrib/coccinelle/the_repository.pending.cocci >>> deleted file mode 100644 >>> index 1190a3312bd..00000000000 >>> --- a/contrib/coccinelle/the_repository.pending.cocci >>> +++ /dev/null >>> @@ -1,14 +0,0 @@ >>> -// This file is used for the ongoing refactoring of >>> -// bringing the index or repository struct in all of >>> -// our code base. >> We can't go so far as to say that we've removed all implicit references >> to "the_repository", though, since we still have functions that >> reference "the_repository" in their implementations. But, I don't think >> this ".cocci" file would help us with those cases anyway, since this was >> targeted specifically at functions/macros that were passing >> "the_repository" to functions that accepted a "struct repository" arg. > > For these implicitly-the_repository functions, (e.g. git_path) we'd > presumably refactor them into repo_* versions and then apply the same > sorts of changes we did in this series? I guess we'd make those changes > in contrib/coccinelle/the_repository.cocci, so we don't need the > *.pending* one. Whether we add it to a "pending" or not is just a question of whether the migration is done right away, or left for later. > On that note, I'm curious what contrib/coccinelle/the_repository.cocci > is doing for us after this series. By definition, all of the macros have > been fully migrated, so they're all a noop. I left them for the benefit of any in-flight conflicts, or semantic conflicts with out-of-tree. I.e. in such a case you'd keep the other side, then apply the cocci rule, and the result is a semantically correct merge of the two. > Would this slow down `make coccicheck`? A bit. It doesn't slow it down by much, as these rules are the simplest to execute, spatch can skip the file if the relevant tokens aren't found in it.
diff --git a/add-interactive.c b/add-interactive.c index 00a0f6f96f3..33f41d65a47 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -551,7 +551,7 @@ static int get_modified_files(struct repository *r, opt.def = is_initial ? empty_tree_oid_hex() : oid_to_hex(&head_oid); - init_revisions(&rev, NULL); + repo_init_revisions(the_repository, &rev, NULL); setup_revisions(0, NULL, &rev, &opt); rev.diffopt.output_format = DIFF_FORMAT_CALLBACK; diff --git a/builtin/bisect.c b/builtin/bisect.c index 34ba7b18f69..0f35361bd16 100644 --- a/builtin/bisect.c +++ b/builtin/bisect.c @@ -568,7 +568,7 @@ static int prepare_revs(struct bisect_terms *terms, struct rev_info *revs) * sets up a revision walk. */ reset_revision_walk(); - init_revisions(revs, NULL); + repo_init_revisions(the_repository, revs, NULL); setup_revisions(0, NULL, revs, NULL); for_each_glob_ref_in(add_bisect_ref, bad, "refs/bisect/", &cb); cb.object_flags = UNINTERESTING; @@ -1095,7 +1095,7 @@ static enum bisect_error bisect_skip(struct bisect_terms *terms, int argc, struct rev_info revs; struct commit *commit; - init_revisions(&revs, NULL); + repo_init_revisions(the_repository, &revs, NULL); setup_revisions(2, argv + i - 1, &revs, NULL); if (prepare_revision_walk(&revs)) diff --git a/builtin/stash.c b/builtin/stash.c index 2017c278df2..888ffa07288 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -901,7 +901,7 @@ static int show_stash(int argc, const char **argv, const char *prefix) init_diff_ui_defaults(); git_config(git_diff_ui_config, NULL); - init_revisions(&rev, prefix); + repo_init_revisions(the_repository, &rev, prefix); argc = parse_options(argc, argv, prefix, options, git_stash_show_usage, PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN_OPT | @@ -1090,7 +1090,7 @@ static int check_changes_tracked_files(const struct pathspec *ps) if (repo_read_index(the_repository) < 0) return -1; - init_revisions(&rev, NULL); + repo_init_revisions(the_repository, &rev, NULL); copy_pathspec(&rev.prune_data, ps); rev.diffopt.flags.quick = 1; @@ -1277,7 +1277,7 @@ static int stash_working_tree(struct stash_info *info, const struct pathspec *ps struct strbuf diff_output = STRBUF_INIT; struct index_state istate = INDEX_STATE_INIT(the_repository); - init_revisions(&rev, NULL); + repo_init_revisions(the_repository, &rev, NULL); copy_pathspec(&rev.prune_data, ps); set_alternate_index_output(stash_index_path.buf); diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 25c70f415f1..32b8e498172 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -1108,7 +1108,7 @@ static int compute_summary_module_list(struct object_id *head_oid, strvec_pushv(&diff_args, info->argv); git_config(git_diff_basic_config, NULL); - init_revisions(&rev, info->prefix); + repo_init_revisions(the_repository, &rev, info->prefix); rev.abbrev = 0; precompose_argv_prefix(diff_args.nr, diff_args.v, NULL); setup_revisions(diff_args.nr, diff_args.v, &rev, &opt); diff --git a/contrib/coccinelle/the_repository.cocci b/contrib/coccinelle/the_repository.cocci index 1d1ac7d4fc5..765ad689678 100644 --- a/contrib/coccinelle/the_repository.cocci +++ b/contrib/coccinelle/the_repository.cocci @@ -113,6 +113,10 @@ | - rerere + repo_rerere +// revision.h +| +- init_revisions ++ repo_init_revisions ) ( + the_repository, diff --git a/contrib/coccinelle/the_repository.pending.cocci b/contrib/coccinelle/the_repository.pending.cocci deleted file mode 100644 index 1190a3312bd..00000000000 --- a/contrib/coccinelle/the_repository.pending.cocci +++ /dev/null @@ -1,14 +0,0 @@ -// This file is used for the ongoing refactoring of -// bringing the index or repository struct in all of -// our code base. - -@@ -@@ -( -// revision.h -- init_revisions -+ repo_init_revisions -) - ( -+ the_repository, - ...) diff --git a/range-diff.c b/range-diff.c index 15d0bc35a87..ff5d19f8add 100644 --- a/range-diff.c +++ b/range-diff.c @@ -588,7 +588,7 @@ int is_range_diff_range(const char *arg) int i, positive = 0, negative = 0; struct rev_info revs; - init_revisions(&revs, NULL); + repo_init_revisions(the_repository, &revs, NULL); if (setup_revisions(3, argv, &revs, NULL) == 1) { for (i = 0; i < revs.pending.nr; i++) if (revs.pending.objects[i].item->flags & UNINTERESTING) diff --git a/revision.h b/revision.h index 30febad09a1..eeb91677d07 100644 --- a/revision.h +++ b/revision.h @@ -415,9 +415,6 @@ struct rev_info { void repo_init_revisions(struct repository *r, struct rev_info *revs, const char *prefix); -#ifndef NO_THE_REPOSITORY_COMPATIBILITY_MACROS -#define init_revisions(revs, prefix) repo_init_revisions(the_repository, revs, prefix) -#endif /** * Parse revision information, filling in the `rev_info` structure, and
Apply the part of "the_repository.pending.cocci" pertaining to "revision.h". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- add-interactive.c | 2 +- builtin/bisect.c | 4 ++-- builtin/stash.c | 6 +++--- builtin/submodule--helper.c | 2 +- contrib/coccinelle/the_repository.cocci | 4 ++++ contrib/coccinelle/the_repository.pending.cocci | 14 -------------- range-diff.c | 2 +- revision.h | 3 --- 8 files changed, 12 insertions(+), 25 deletions(-) delete mode 100644 contrib/coccinelle/the_repository.pending.cocci