git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Elijah Newren <newren@gmail.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 17/17] libs: use "struct repository *" argument, not "the_repository"
Date: Sat, 18 Mar 2023 23:19:50 -0700	[thread overview]
Message-ID: <CABPp-BEdpTO=DRjLq_p+dgX68M0HUVB--3yQR4Sdp8rnFYeyfA@mail.gmail.com> (raw)
In-Reply-To: <patch-17.17-6f86a34bf8b-20230317T152725Z-avarab@gmail.com>

On Fri, Mar 17, 2023 at 8:55 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
> As can easily be seen from grepping in our sources, we had these uses
> of "the_repository" in various library code in cases where the
> function in question was already getting a "struct repository *"
> argument. Let's use that argument instead.
>
> Out of these changes only the changes to "cache-tree.c",
> "commit-reach.c", "shallow.c" and "upload-pack.c" would have cleanly
> applied before the migration away from the "repo_*()" wrapper macros
> in the preceding commits.
>
> The rest aren't new, as we'd previously implicitly refer to
> "the_repository", but it's now more obvious that we were doing the
> wrong thing all along, and should have used the parameter instead.

YES!  I love seeing fixes like this.  I noticed some a while back in
the merge code, and had some patches somewhere that I think I never
got around to submitting, but which have been in the back of my mind
bugging me.  Nice to see lots of these kinds of cases getting fixed.

> The change to change "get_index_format_default(the_repository)" in
> "read-cache.c" to use the "r" variable instead should arguably have
> been part of [1], or in the subsequent cleanup in [2]. Let's do it
> here, as can be seen from the initial code in [3] it's not important
> that we use "the_repository" there, but would prefer to always use the
> current repository.
>
> This change excludes the "the_repository" use in "upload-pack.c"'s
> upload_pack_advertise(), as the in-flight [4] makes that change.
>
> 1. ee1f0c242ef (read-cache: add index.skipHash config option,
>    2023-01-06)
> 2. 6269f8eaad0 (treewide: always have a valid "index_state.repo"
>    member, 2023-01-17)
> 3. 7211b9e7534 (repo-settings: consolidate some config settings,
>    2019-08-13)
> 4. <Y/hbUsGPVNAxTdmS@coredump.intra.peff.net>
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  add-interactive.c |   2 +-
>  branch.c          |   8 ++--
>  builtin/replace.c |   2 +-
>  cache-tree.c      |   4 +-
>  combine-diff.c    |   2 +-
>  commit-graph.c    |   2 +-
>  commit-reach.c    |   2 +-
>  merge-recursive.c |   2 +-

Doh, doesn't fix up the ones I had.  I don't remember the exact issue
and can't find the patches right now, but looking around a little I
see a write_object_file() which should be a repo_write_object_file(),
but it's in a function that doesn't take a repository argument.
However, the caller has access to a repository argument, in the form
of opt->repo, so we've got some low-hanging fruit that could be fixed
up.  Doesn't need to be part of your series, but it's nice that your
series has done some of the groundwork to make these issues more
obvious.

>  notes-cache.c     |   2 +-
>  notes-utils.c     |   2 +-
>  object-name.c     |   8 ++--
>  read-cache.c      |   4 +-
>  reset.c           |   2 +-
>  sequencer.c       | 102 ++++++++++++++++++++++------------------------
>  shallow.c         |   2 +-
>  tree.c            |   2 +-
>  wt-status.c       |  10 ++---
>  17 files changed, 76 insertions(+), 82 deletions(-)
>
> diff --git a/add-interactive.c b/add-interactive.c
> index 33f41d65a47..02bf6fc15cb 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);
>
> -               repo_init_revisions(the_repository, &rev, NULL);
> +               repo_init_revisions(r, &rev, NULL);
>                 setup_revisions(0, NULL, &rev, &opt);
>
>                 rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
> diff --git a/branch.c b/branch.c
> index a01e465b114..2b266e6fcb9 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -531,7 +531,7 @@ static void dwim_branch_start(struct repository *r, const char *start_name,
>                 explicit_tracking = 1;
>
>         real_ref = NULL;
> -       if (repo_get_oid_mb(the_repository, start_name, &oid)) {
> +       if (repo_get_oid_mb(r, start_name, &oid)) {
>                 if (explicit_tracking) {
>                         int code = die_message(_(upstream_missing), start_name);
>                         advise_if_enabled(ADVICE_SET_UPSTREAM_FAILURE,
> @@ -541,8 +541,8 @@ static void dwim_branch_start(struct repository *r, const char *start_name,
>                 die(_("not a valid object name: '%s'"), start_name);
>         }
>
> -       switch (repo_dwim_ref(the_repository, start_name, strlen(start_name),
> -                             &oid, &real_ref, 0)) {
> +       switch (repo_dwim_ref(r, start_name, strlen(start_name), &oid,
> +                             &real_ref, 0)) {
>         case 0:
>                 /* Not branching from any existing branch */
>                 if (explicit_tracking)
> @@ -773,7 +773,7 @@ void create_branches_recursively(struct repository *r, const char *name,
>                             name);
>         }
>
> -       create_branch(the_repository, name, start_commitish, force, 0, reflog, quiet,
> +       create_branch(r, name, start_commitish, force, 0, reflog, quiet,
>                       BRANCH_TRACK_NEVER, dry_run);
>         if (dry_run)
>                 return;
> diff --git a/builtin/replace.c b/builtin/replace.c
> index 85f0b79c924..bd67107a7c7 100644
> --- a/builtin/replace.c
> +++ b/builtin/replace.c
> @@ -54,7 +54,7 @@ static int show_reference(struct repository *r, const char *refname,
>                         struct object_id object;
>                         enum object_type obj_type, repl_type;
>
> -                       if (repo_get_oid(the_repository, refname, &object))
> +                       if (repo_get_oid(r, refname, &object))
>                                 return error(_("failed to resolve '%s' as a valid ref"), refname);
>
>                         obj_type = oid_object_info(r, &object, NULL);
> diff --git a/cache-tree.c b/cache-tree.c
> index ab2cc1edf29..b0e490b5aa4 100644
> --- a/cache-tree.c
> +++ b/cache-tree.c
> @@ -814,14 +814,14 @@ void prime_cache_tree(struct repository *r,
>  {
>         struct strbuf tree_path = STRBUF_INIT;
>
> -       trace2_region_enter("cache-tree", "prime_cache_tree", the_repository);
> +       trace2_region_enter("cache-tree", "prime_cache_tree", r);
>         cache_tree_free(&istate->cache_tree);
>         istate->cache_tree = cache_tree();
>
>         prime_cache_tree_rec(r, istate->cache_tree, tree, &tree_path);
>         strbuf_release(&tree_path);
>         istate->cache_changed |= CACHE_TREE_CHANGED;
> -       trace2_region_leave("cache-tree", "prime_cache_tree", the_repository);
> +       trace2_region_leave("cache-tree", "prime_cache_tree", r);
>  }
>
>  /*
> diff --git a/combine-diff.c b/combine-diff.c
> index 20f8a03924f..fa5967a5be6 100644
> --- a/combine-diff.c
> +++ b/combine-diff.c
> @@ -332,7 +332,7 @@ static char *grab_blob(struct repository *r,
>                 *size = fill_textconv(r, textconv, df, &blob);
>                 free_filespec(df);
>         } else {
> -               blob = repo_read_object_file(the_repository, oid, &type, size);
> +               blob = repo_read_object_file(r, oid, &type, size);
>                 if (type != OBJ_BLOB)
>                         die("object '%s' is not a blob!", oid_to_hex(oid));
>         }
> diff --git a/commit-graph.c b/commit-graph.c
> index 268410e913c..035816f4d8c 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -2549,7 +2549,7 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g, int flags)
>
>                 graph_commit = lookup_commit(r, &cur_oid);
>                 odb_commit = (struct commit *)create_object(r, &cur_oid, alloc_commit_node(r));
> -               if (repo_parse_commit_internal(the_repository, odb_commit, 0, 0)) {
> +               if (repo_parse_commit_internal(r, odb_commit, 0, 0)) {
>                         graph_report(_("failed to parse commit %s from object database for commit-graph"),
>                                      oid_to_hex(&cur_oid));
>                         continue;
> diff --git a/commit-reach.c b/commit-reach.c
> index c88faf7e7b2..01eb11b5957 100644
> --- a/commit-reach.c
> +++ b/commit-reach.c
> @@ -448,7 +448,7 @@ int repo_is_descendant_of(struct repository *r,
>         if (!with_commit)
>                 return 1;
>
> -       if (generation_numbers_enabled(the_repository)) {
> +       if (generation_numbers_enabled(r)) {
>                 struct commit_list *from_list = NULL;
>                 int result;
>                 commit_list_insert(commit, &from_list);
> diff --git a/merge-recursive.c b/merge-recursive.c
> index d83ce19bb1f..20aa18e2203 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -3798,7 +3798,7 @@ static struct commit *get_ref(struct repository *repo,
>                 return make_virtual_commit(repo, (struct tree*)object, name);
>         if (object->type != OBJ_COMMIT)
>                 return NULL;
> -       if (repo_parse_commit(the_repository, (struct commit *)object))
> +       if (repo_parse_commit(repo, (struct commit *)object))
>                 return NULL;
>         return (struct commit *)object;
>  }
> diff --git a/notes-cache.c b/notes-cache.c
> index e5e9092d367..1a6e214ea79 100644
> --- a/notes-cache.c
> +++ b/notes-cache.c
> @@ -23,7 +23,7 @@ static int notes_cache_match_validity(struct repository *r,
>                 return 0;
>
>         memset(&pretty_ctx, 0, sizeof(pretty_ctx));
> -       repo_format_commit_message(the_repository, commit, "%s", &msg,
> +       repo_format_commit_message(r, commit, "%s", &msg,
>                                    &pretty_ctx);
>         strbuf_trim(&msg);
>
> diff --git a/notes-utils.c b/notes-utils.c
> index 0550cfded86..4be0aaa6de9 100644
> --- a/notes-utils.c
> +++ b/notes-utils.c
> @@ -23,7 +23,7 @@ void create_notes_commit(struct repository *r,
>                 struct object_id parent_oid;
>                 if (!read_ref(t->ref, &parent_oid)) {
>                         struct commit *parent = lookup_commit(r, &parent_oid);
> -                       if (repo_parse_commit(the_repository, parent))
> +                       if (repo_parse_commit(r, parent))
>                                 die("Failed to find/parse commit %s", t->ref);
>                         commit_list_insert(parent, &parents);
>                 }
> diff --git a/object-name.c b/object-name.c
> index df31f192ce3..c7bc311dcef 100644
> --- a/object-name.c
> +++ b/object-name.c
> @@ -1038,7 +1038,7 @@ static enum get_oid_result get_parent(struct repository *r,
>         if (ret)
>                 return ret;
>         commit = lookup_commit_reference(r, &oid);
> -       if (repo_parse_commit(the_repository, commit))
> +       if (repo_parse_commit(r, commit))
>                 return MISSING_OBJECT;
>         if (!idx) {
>                 oidcpy(result, &commit->object.oid);
> @@ -1072,7 +1072,7 @@ static enum get_oid_result get_nth_ancestor(struct repository *r,
>                 return MISSING_OBJECT;
>
>         while (generation--) {
> -               if (repo_parse_commit(the_repository, commit) || !commit->parents)
> +               if (repo_parse_commit(r, commit) || !commit->parents)
>                         return MISSING_OBJECT;
>                 commit = commit->parents->item;
>         }
> @@ -1363,10 +1363,10 @@ static int get_oid_oneline(struct repository *r,
>                 commit = pop_most_recent_commit(&list, ONELINE_SEEN);
>                 if (!parse_object(r, &commit->object.oid))
>                         continue;
> -               buf = repo_get_commit_buffer(the_repository, commit, NULL);
> +               buf = repo_get_commit_buffer(r, commit, NULL);
>                 p = strstr(buf, "\n\n");
>                 matches = negative ^ (p && !regexec(&regex, p + 2, 0, NULL, 0));
> -               repo_unuse_commit_buffer(the_repository, commit, buf);
> +               repo_unuse_commit_buffer(r, commit, buf);
>
>                 if (matches) {
>                         oidcpy(oid, &commit->object.oid);
> diff --git a/read-cache.c b/read-cache.c
> index 1548e886684..892fede6b41 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -2628,7 +2628,7 @@ int repo_index_has_changes(struct repository *repo,
>
>         if (tree)
>                 cmp = tree->object.oid;
> -       if (tree || !repo_get_oid_tree(the_repository, "HEAD", &cmp)) {
> +       if (tree || !repo_get_oid_tree(repo, "HEAD", &cmp)) {
>                 struct diff_options opt;
>
>                 repo_diff_setup(repo, &opt);
> @@ -2947,7 +2947,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
>         }
>
>         if (!istate->version)
> -               istate->version = get_index_format_default(the_repository);
> +               istate->version = get_index_format_default(r);
>
>         /* demote version 3 to version 2 when the latter suffices */
>         if (istate->version == 3 || istate->version == 2)
> diff --git a/reset.c b/reset.c
> index 3a9c6dfdd0a..65f8138c78b 100644
> --- a/reset.c
> +++ b/reset.c
> @@ -106,7 +106,7 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
>                 goto leave_reset_head;
>         }
>
> -       if (!repo_get_oid(the_repository, "HEAD", &head_oid)) {
> +       if (!repo_get_oid(r, "HEAD", &head_oid)) {
>                 head = &head_oid;
>         } else if (!oid || !reset_hard) {
>                 ret = error(_("could not determine HEAD revision"));
> diff --git a/sequencer.c b/sequencer.c
> index fb5a540fa04..f8f4e72112c 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -694,8 +694,8 @@ static int do_recursive_merge(struct repository *r,
>         o.show_rename_progress = 1;
>
>         head_tree = parse_tree_indirect(head);
> -       next_tree = next ? repo_get_commit_tree(the_repository, next) : empty_tree(r);
> -       base_tree = base ? repo_get_commit_tree(the_repository, base) : empty_tree(r);
> +       next_tree = next ? repo_get_commit_tree(r, next) : empty_tree(r);
> +       base_tree = base ? repo_get_commit_tree(r, base) : empty_tree(r);
>
>         for (i = 0; i < opts->xopts_nr; i++)
>                 parse_merge_opt(&o, opts->xopts[i]);
> @@ -773,7 +773,7 @@ static int is_index_unchanged(struct repository *r)
>          * the commit is invalid, repo_parse_commit() will complain.  So
>          * there is nothing for us to say here.  Just return failure.
>          */
> -       if (repo_parse_commit(the_repository, head_commit))
> +       if (repo_parse_commit(r, head_commit))
>                 return -1;
>
>         if (!(cache_tree_oid = get_cache_tree_oid(istate)))
> @@ -1338,15 +1338,15 @@ void print_commit_summary(struct repository *r,
>         commit = lookup_commit(r, oid);
>         if (!commit)
>                 die(_("couldn't look up newly created commit"));
> -       if (repo_parse_commit(the_repository, commit))
> +       if (repo_parse_commit(r, commit))
>                 die(_("could not parse newly created commit"));
>
>         strbuf_addstr(&format, "format:%h] %s");
>
> -       repo_format_commit_message(the_repository, commit, "%an <%ae>",
> -                                  &author_ident, &pctx);
> -       repo_format_commit_message(the_repository, commit, "%cn <%ce>",
> -                                  &committer_ident, &pctx);
> +       repo_format_commit_message(r, commit, "%an <%ae>", &author_ident,
> +                                  &pctx);
> +       repo_format_commit_message(r, commit, "%cn <%ce>", &committer_ident,
> +                                  &pctx);
>         if (strbuf_cmp(&author_ident, &committer_ident)) {
>                 strbuf_addstr(&format, "\n Author: ");
>                 strbuf_addbuf_percentquote(&format, &author_ident);
> @@ -1354,8 +1354,7 @@ void print_commit_summary(struct repository *r,
>         if (flags & SUMMARY_SHOW_AUTHOR_DATE) {
>                 struct strbuf date = STRBUF_INIT;
>
> -               repo_format_commit_message(the_repository, commit, "%ad",
> -                                          &date, &pctx);
> +               repo_format_commit_message(r, commit, "%ad", &date, &pctx);
>                 strbuf_addstr(&format, "\n Date: ");
>                 strbuf_addbuf_percentquote(&format, &date);
>                 strbuf_release(&date);
> @@ -1385,7 +1384,7 @@ void print_commit_summary(struct repository *r,
>         rev.diffopt.detect_rename = DIFF_DETECT_RENAME;
>         diff_setup_done(&rev.diffopt);
>
> -       refs = get_main_ref_store(the_repository);
> +       refs = get_main_ref_store(r);
>         head = refs_resolve_ref_unsafe(refs, "HEAD", 0, NULL, NULL);
>         if (!head)
>                 die(_("unable to resolve HEAD after creating commit"));
> @@ -1411,7 +1410,7 @@ static int parse_head(struct repository *r, struct commit **head)
>         struct commit *current_head;
>         struct object_id oid;
>
> -       if (repo_get_oid(the_repository, "HEAD", &oid)) {
> +       if (repo_get_oid(r, "HEAD", &oid)) {
>                 current_head = NULL;
>         } else {
>                 current_head = lookup_commit_reference(r, &oid);
> @@ -1421,7 +1420,7 @@ static int parse_head(struct repository *r, struct commit **head)
>                         warning(_("HEAD %s is not a commit!"),
>                                 oid_to_hex(&oid));
>                 }
> -               if (repo_parse_commit(the_repository, current_head))
> +               if (repo_parse_commit(r, current_head))
>                         return error(_("could not parse HEAD commit"));
>         }
>         *head = current_head;
> @@ -1464,9 +1463,8 @@ static int try_to_commit(struct repository *r,
>         if (flags & AMEND_MSG) {
>                 const char *exclude_gpgsig[] = { "gpgsig", "gpgsig-sha256", NULL };
>                 const char *out_enc = get_commit_output_encoding();
> -               const char *message = repo_logmsg_reencode(the_repository,
> -                                                          current_head, NULL,
> -                                                          out_enc);
> +               const char *message = repo_logmsg_reencode(r, current_head,
> +                                                          NULL, out_enc);
>
>                 if (!msg) {
>                         const char *orig_message = NULL;
> @@ -1477,7 +1475,7 @@ static int try_to_commit(struct repository *r,
>                         hook_commit = "HEAD";
>                 }
>                 author = amend_author = get_author(message);
> -               repo_unuse_commit_buffer(the_repository, current_head,
> +               repo_unuse_commit_buffer(r, current_head,
>                                          message);
>                 if (!author) {
>                         res = error(_("unable to parse commit author"));
> @@ -2004,18 +2002,18 @@ static int update_squash_messages(struct repository *r,
>                 struct commit *head_commit;
>                 const char *head_message, *body;
>
> -               if (repo_get_oid(the_repository, "HEAD", &head))
> +               if (repo_get_oid(r, "HEAD", &head))
>                         return error(_("need a HEAD to fixup"));
>                 if (!(head_commit = lookup_commit_reference(r, &head)))
>                         return error(_("could not read HEAD"));
> -               if (!(head_message = repo_logmsg_reencode(the_repository, head_commit, NULL, encoding)))
> +               if (!(head_message = repo_logmsg_reencode(r, head_commit, NULL,
> +                                                         encoding)))
>                         return error(_("could not read HEAD's commit message"));
>
>                 find_commit_subject(head_message, &body);
>                 if (command == TODO_FIXUP && !flag && write_message(body, strlen(body),
>                                                         rebase_path_fixup_msg(), 0) < 0) {
> -                       repo_unuse_commit_buffer(the_repository, head_commit,
> -                                                head_message);
> +                       repo_unuse_commit_buffer(r, head_commit, head_message);
>                         return error(_("cannot write '%s'"), rebase_path_fixup_msg());
>                 }
>                 strbuf_addf(&buf, "%c ", comment_line_char);
> @@ -2030,11 +2028,10 @@ static int update_squash_messages(struct repository *r,
>                 else
>                         strbuf_addstr(&buf, body);
>
> -               repo_unuse_commit_buffer(the_repository, head_commit,
> -                                        head_message);
> +               repo_unuse_commit_buffer(r, head_commit, head_message);
>         }
>
> -       if (!(message = repo_logmsg_reencode(the_repository, commit, NULL, encoding)))
> +       if (!(message = repo_logmsg_reencode(r, commit, NULL, encoding)))
>                 return error(_("could not read commit message of %s"),
>                              oid_to_hex(&commit->object.oid));
>         find_commit_subject(message, &body);
> @@ -2049,7 +2046,7 @@ static int update_squash_messages(struct repository *r,
>                 strbuf_add_commented_lines(&buf, body, strlen(body));
>         } else
>                 return error(_("unknown command: %d"), command);
> -       repo_unuse_commit_buffer(the_repository, commit, message);
> +       repo_unuse_commit_buffer(r, commit, message);
>
>         if (!res)
>                 res = write_message(buf.buf, buf.len, rebase_path_squash_msg(),
> @@ -2162,7 +2159,7 @@ static int do_pick_commit(struct repository *r,
>                 if (write_index_as_tree(&head, r->index, r->index_file, 0, NULL))
>                         return error(_("your index file is unmerged."));
>         } else {
> -               unborn = repo_get_oid(the_repository, "HEAD", &head);
> +               unborn = repo_get_oid(r, "HEAD", &head);
>                 /* Do we want to generate a root commit? */
>                 if (is_pick_or_similar(command) && opts->have_squash_onto &&
>                     oideq(&head, &opts->squash_onto)) {
> @@ -2224,7 +2221,7 @@ static int do_pick_commit(struct repository *r,
>                 msg_file = NULL;
>                 goto fast_forward_edit;
>         }
> -       if (parent && repo_parse_commit(the_repository, parent) < 0)
> +       if (parent && repo_parse_commit(r, parent) < 0)
>                 /* TRANSLATORS: The first %s will be a "todo" command like
>                    "revert" or "pick", the second %s a SHA1. */
>                 return error(_("%s: cannot parse parent commit %s"),
> @@ -2616,7 +2613,7 @@ static int parse_insn_line(struct repository *r, struct todo_item *item,
>         end_of_object_name = (char *) bol + strcspn(bol, " \t\n");
>         saved = *end_of_object_name;
>         *end_of_object_name = '\0';
> -       status = repo_get_oid(the_repository, bol, &commit_oid);
> +       status = repo_get_oid(r, bol, &commit_oid);
>         if (status < 0)
>                 error(_("could not parse '%s'"), bol); /* return later */
>         *end_of_object_name = saved;
> @@ -3531,12 +3528,12 @@ static int make_patch(struct repository *r,
>         strbuf_addf(&buf, "%s/message", get_dir(opts));
>         if (!file_exists(buf.buf)) {
>                 const char *encoding = get_commit_output_encoding();
> -               const char *commit_buffer = repo_logmsg_reencode(the_repository,
> +               const char *commit_buffer = repo_logmsg_reencode(r,
>                                                                  commit, NULL,
>                                                                  encoding);
>                 find_commit_subject(commit_buffer, &subject);
>                 res |= write_message(subject, strlen(subject), buf.buf, 1);
> -               repo_unuse_commit_buffer(the_repository, commit,
> +               repo_unuse_commit_buffer(r, commit,
>                                          commit_buffer);
>         }
>         strbuf_release(&buf);
> @@ -3716,7 +3713,7 @@ static int do_label(struct repository *r, const char *name, int len)
>         if (!transaction) {
>                 error("%s", err.buf);
>                 ret = -1;
> -       } else if (repo_get_oid(the_repository, "HEAD", &head_oid)) {
> +       } else if (repo_get_oid(r, "HEAD", &head_oid)) {
>                 error(_("could not read HEAD"));
>                 ret = -1;
>         } else if (ref_transaction_update(transaction, ref_name.buf, &head_oid,
> @@ -4004,8 +4001,7 @@ static int do_merge(struct repository *r,
>
>         if (commit) {
>                 const char *encoding = get_commit_output_encoding();
> -               const char *message = repo_logmsg_reencode(the_repository,
> -                                                          commit, NULL,
> +               const char *message = repo_logmsg_reencode(r, commit, NULL,
>                                                            encoding);
>                 const char *body;
>                 int len;
> @@ -4019,7 +4015,7 @@ static int do_merge(struct repository *r,
>                 find_commit_subject(message, &body);
>                 len = strlen(body);
>                 ret = write_message(body, len, git_path_merge_msg(r), 0);
> -               repo_unuse_commit_buffer(the_repository, commit, message);
> +               repo_unuse_commit_buffer(r, commit, message);
>                 if (ret) {
>                         error_errno(_("could not write '%s'"),
>                                     git_path_merge_msg(r));
> @@ -4119,8 +4115,7 @@ static int do_merge(struct repository *r,
>         }
>
>         merge_commit = to_merge->item;
> -       bases = repo_get_merge_bases(the_repository, head_commit,
> -                                    merge_commit);
> +       bases = repo_get_merge_bases(r, head_commit, merge_commit);
>         if (bases && oideq(&merge_commit->object.oid,
>                            &bases->item->object.oid)) {
>                 ret = 0;
> @@ -4475,7 +4470,7 @@ void create_autostash(struct repository *r, const char *path)
>                 if (capture_command(&stash, &buf, GIT_MAX_HEXSZ))
>                         die(_("Cannot autostash"));
>                 strbuf_trim_trailing_newline(&buf);
> -               if (repo_get_oid(the_repository, buf.buf, &oid))
> +               if (repo_get_oid(r, buf.buf, &oid))
>                         die(_("Unexpected stash response: '%s'"),
>                             buf.buf);
>                 strbuf_reset(&buf);
> @@ -4600,9 +4595,9 @@ static int stopped_at_head(struct repository *r)
>         struct commit *commit;
>         struct commit_message message;
>
> -       if (repo_get_oid(the_repository, "HEAD", &head) ||
> +       if (repo_get_oid(r, "HEAD", &head) ||
>             !(commit = lookup_commit(r, &head)) ||
> -           repo_parse_commit(the_repository, commit) || get_message(commit, &message))
> +           repo_parse_commit(r, commit) || get_message(commit, &message))
>                 fprintf(stderr, _("Stopped at HEAD\n"));
>         else {
>                 fprintf(stderr, _("Stopped at %s\n"), message.label);
> @@ -4750,7 +4745,7 @@ static int pick_commits(struct repository *r,
>                                  * otherwise we do not.
>                                  */
>                                 if (item->command == TODO_REWORD &&
> -                                   !repo_get_oid(the_repository, "HEAD", &oid) &&
> +                                   !repo_get_oid(r, "HEAD", &oid) &&
>                                     (oideq(&item->commit->object.oid, &oid) ||
>                                      (opts->have_squash_onto &&
>                                       oideq(&opts->squash_onto, &oid))))
> @@ -4839,7 +4834,7 @@ static int pick_commits(struct repository *r,
>                         struct object_id head, orig;
>                         int res;
>
> -                       if (repo_get_oid(the_repository, "HEAD", &head)) {
> +                       if (repo_get_oid(r, "HEAD", &head)) {
>                                 res = error(_("cannot read HEAD"));
>  cleanup_head_ref:
>                                 strbuf_release(&head_ref);
> @@ -4886,8 +4881,8 @@ static int pick_commits(struct repository *r,
>                         log_tree_opt.disable_stdin = 1;
>
>                         if (read_oneliner(&buf, rebase_path_orig_head(), 0) &&
> -                           !repo_get_oid(the_repository, buf.buf, &orig) &&
> -                           !repo_get_oid(the_repository, "HEAD", &head)) {
> +                           !repo_get_oid(r, buf.buf, &orig) &&
> +                           !repo_get_oid(r, "HEAD", &head)) {
>                                 diff_tree_oid(&orig, &head, "",
>                                               &log_tree_opt.diffopt);
>                                 log_tree_diff_flush(&log_tree_opt);
> @@ -4979,7 +4974,7 @@ static int commit_staged_changes(struct repository *r,
>                 struct strbuf rev = STRBUF_INIT;
>                 struct object_id head, to_amend;
>
> -               if (repo_get_oid(the_repository, "HEAD", &head))
> +               if (repo_get_oid(r, "HEAD", &head))
>                         return error(_("cannot amend non-existing commit"));
>                 if (!read_oneliner(&rev, rebase_path_amend(), 0))
>                         return error(_("invalid file: '%s'"), rebase_path_amend());
> @@ -5059,14 +5054,13 @@ static int commit_staged_changes(struct repository *r,
>                                 const char *encoding = get_commit_output_encoding();
>
>                                 if (parse_head(r, &commit) ||
> -                                   !(p = repo_logmsg_reencode(the_repository, commit, NULL, encoding)) ||
> +                                   !(p = repo_logmsg_reencode(r, commit, NULL, encoding)) ||
>                                     write_message(p, strlen(p), path, 0)) {
> -                                       repo_unuse_commit_buffer(the_repository,
> -                                                                commit, p);
> +                                       repo_unuse_commit_buffer(r, commit, p);
>                                         return error(_("could not write file: "
>                                                        "'%s'"), path);
>                                 }
> -                               repo_unuse_commit_buffer(the_repository,
> +                               repo_unuse_commit_buffer(r,
>                                                          commit, p);
>                         }
>                 }
> @@ -5206,7 +5200,7 @@ int sequencer_pick_revisions(struct repository *r,
>                 if (!strlen(name))
>                         continue;
>
> -               if (!repo_get_oid(the_repository, name, &oid)) {
> +               if (!repo_get_oid(r, name, &oid)) {
>                         if (!lookup_commit_reference_gently(r, &oid, 1)) {
>                                 enum object_type type = oid_object_info(r,
>                                                                         &oid,
> @@ -5249,7 +5243,7 @@ int sequencer_pick_revisions(struct repository *r,
>         if (walk_revs_populate_todo(&todo_list, opts) ||
>                         create_seq_dir(r) < 0)
>                 return -1;
> -       if (repo_get_oid(the_repository, "HEAD", &oid) && (opts->action == REPLAY_REVERT))
> +       if (repo_get_oid(r, "HEAD", &oid) && (opts->action == REPLAY_REVERT))
>                 return error(_("can't revert as initial commit"));
>         if (save_head(oid_to_hex(&oid)))
>                 return -1;
> @@ -5944,7 +5938,7 @@ static int skip_unnecessary_picks(struct repository *r,
>                         continue;
>                 if (item->command != TODO_PICK)
>                         break;
> -               if (repo_parse_commit(the_repository, item->commit)) {
> +               if (repo_parse_commit(r, item->commit)) {
>                         return error(_("could not parse commit '%s'"),
>                                 oid_to_hex(&item->commit->object.oid));
>                 }
> @@ -6115,7 +6109,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
>         struct object_id oid = onto->object.oid;
>         int res;
>
> -       repo_find_unique_abbrev_r(the_repository, shortonto, &oid,
> +       repo_find_unique_abbrev_r(r, shortonto, &oid,
>                                   DEFAULT_ABBREV);
>
>         if (buf->len == 0) {
> @@ -6395,8 +6389,8 @@ int sequencer_determine_whence(struct repository *r, enum commit_whence *whence)
>                 if (file_exists(git_path_seq_dir()))
>                         *whence = FROM_CHERRY_PICK_MULTI;
>                 if (file_exists(rebase_path()) &&
> -                   !repo_get_oid(the_repository, "REBASE_HEAD", &rebase_head) &&
> -                   !repo_get_oid(the_repository, "CHERRY_PICK_HEAD", &cherry_pick_head) &&
> +                   !repo_get_oid(r, "REBASE_HEAD", &rebase_head) &&
> +                   !repo_get_oid(r, "CHERRY_PICK_HEAD", &cherry_pick_head) &&
>                     oideq(&rebase_head, &cherry_pick_head))
>                         *whence = FROM_REBASE_PICK;
>                 else
> diff --git a/shallow.c b/shallow.c
> index 07278467e47..fbafc4b30e9 100644
> --- a/shallow.c
> +++ b/shallow.c
> @@ -30,7 +30,7 @@ int register_shallow(struct repository *r, const struct object_id *oid)
>  {
>         struct commit_graft *graft =
>                 xmalloc(sizeof(struct commit_graft));
> -       struct commit *commit = lookup_commit(the_repository, oid);
> +       struct commit *commit = lookup_commit(r, oid);
>
>         oidcpy(&graft->oid, oid);
>         graft->nr_parent = -1;
> diff --git a/tree.c b/tree.c
> index 1b403c4b5ce..8643b734642 100644
> --- a/tree.c
> +++ b/tree.c
> @@ -58,7 +58,7 @@ int read_tree_at(struct repository *r,
>                                     oid_to_hex(&entry.oid),
>                                     base->buf, entry.path);
>
> -                       if (repo_parse_commit(the_repository, commit))
> +                       if (repo_parse_commit(r, commit))
>                                 die("Invalid commit %s in submodule path %s%s",
>                                     oid_to_hex(&entry.oid),
>                                     base->buf, entry.path);
> diff --git a/wt-status.c b/wt-status.c
> index dcd1d0cee4c..28e35a2165a 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -1664,7 +1664,7 @@ static void wt_status_get_detached_from(struct repository *r,
>                 return;
>         }
>
> -       if (repo_dwim_ref(the_repository, cb.buf.buf, cb.buf.len, &oid, &ref,
> +       if (repo_dwim_ref(r, cb.buf.buf, cb.buf.len, &oid, &ref,
>                           1) == 1 &&
>             /* oid is a commit? match without further lookup */
>             (oideq(&cb.noid, &oid) ||
> @@ -1677,9 +1677,9 @@ static void wt_status_get_detached_from(struct repository *r,
>                 state->detached_from = xstrdup(from);
>         } else
>                 state->detached_from =
> -                       xstrdup(repo_find_unique_abbrev(the_repository, &cb.noid, DEFAULT_ABBREV));
> +                       xstrdup(repo_find_unique_abbrev(r, &cb.noid, DEFAULT_ABBREV));
>         oidcpy(&state->detached_oid, &cb.noid);
> -       state->detached_at = !repo_get_oid(the_repository, "HEAD", &oid) &&
> +       state->detached_at = !repo_get_oid(r, "HEAD", &oid) &&
>                              oideq(&oid, &state->detached_oid);
>
>         free(ref);
> @@ -1770,13 +1770,13 @@ void wt_status_get_state(struct repository *r,
>         } else if (wt_status_check_rebase(NULL, state)) {
>                 ;               /* all set */
>         } else if (refs_ref_exists(get_main_ref_store(r), "CHERRY_PICK_HEAD") &&
> -                  !repo_get_oid(the_repository, "CHERRY_PICK_HEAD", &oid)) {
> +                  !repo_get_oid(r, "CHERRY_PICK_HEAD", &oid)) {
>                 state->cherry_pick_in_progress = 1;
>                 oidcpy(&state->cherry_pick_head_oid, &oid);
>         }
>         wt_status_check_bisect(NULL, state);
>         if (refs_ref_exists(get_main_ref_store(r), "REVERT_HEAD") &&
> -           !repo_get_oid(the_repository, "REVERT_HEAD", &oid)) {
> +           !repo_get_oid(r, "REVERT_HEAD", &oid)) {
>                 state->revert_in_progress = 1;
>                 oidcpy(&state->revert_head_oid, &oid);
>         }
> --
> 2.40.0.rc1.1034.g5867a1b10c5

I started looking through it closely, but after a bit I realized that
the only kind of error you would likely to be make here would be the
kind caught by the compiler, so then I started just skimming over it.
But I think that's safe for this kind of change.  I'm a big fan of
these kinds of cleanups and fixups, though.

I'm not sure I can give a Reviewed-by since I just don't know cocci,
but it all looked pretty logical.  You definitely have my Acked-by on
the series, though.

  reply	other threads:[~2023-03-19  6:20 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-17 15:35 [PATCH 00/17] cocci: remove "the_index" wrapper macros Ævar Arnfjörð Bjarmason
2023-03-17 15:35 ` [PATCH 01/17] cocci: remove dead rule from "the_repository.pending.cocci" Ævar Arnfjörð Bjarmason
2023-03-17 16:14   ` Eric Sunshine
2023-03-19  5:53   ` Elijah Newren
2023-03-17 15:35 ` [PATCH 02/17] cocci: fix incorrect & verbose "the_repository" rules Ævar Arnfjörð Bjarmason
2023-03-19  5:55   ` Elijah Newren
2023-03-22 22:46   ` Glen Choo
2023-03-26  5:02     ` Ævar Arnfjörð Bjarmason
2023-03-17 15:35 ` [PATCH 03/17] cocci: sort "the_repository" rules by header Ævar Arnfjörð Bjarmason
2023-03-19  5:59   ` Elijah Newren
2023-03-17 15:35 ` [PATCH 04/17] cocci: add missing "the_repository" macros to "pending" Ævar Arnfjörð Bjarmason
2023-03-19  6:04   ` Elijah Newren
2023-03-17 15:35 ` [PATCH 05/17] cocci: apply the "cache.h" part of "the_repository.pending" Ævar Arnfjörð Bjarmason
2023-03-19  6:06   ` Elijah Newren
2023-03-22 23:17   ` Glen Choo
2023-03-17 15:35 ` [PATCH 06/17] cocci: apply the "commit-reach.h" " Ævar Arnfjörð Bjarmason
2023-03-17 15:35 ` [PATCH 07/17] cocci: apply the "commit.h" " Ævar Arnfjörð Bjarmason
2023-03-17 15:35 ` [PATCH 08/17] cocci: apply the "diff.h" " Ævar Arnfjörð Bjarmason
2023-03-17 15:35 ` [PATCH 09/17] cocci: apply the "object-store.h" " Ævar Arnfjörð Bjarmason
2023-03-17 15:35 ` [PATCH 10/17] cocci: apply the "pretty.h" " Ævar Arnfjörð Bjarmason
2023-03-17 15:35 ` [PATCH 11/17] cocci: apply the "packfile.h" " Ævar Arnfjörð Bjarmason
2023-03-17 15:35 ` [PATCH 12/17] cocci: apply the "promisor-remote.h" " Ævar Arnfjörð Bjarmason
2023-03-17 15:35 ` [PATCH 13/17] cocci: apply the "refs.h" " Ævar Arnfjörð Bjarmason
2023-03-17 15:35 ` [PATCH 14/17] cocci: apply the "rerere.h" " Ævar Arnfjörð Bjarmason
2023-03-17 15:35 ` [PATCH 15/17] cocci: apply the "revision.h" " Ævar Arnfjörð Bjarmason
2023-03-22 23:38   ` Glen Choo
2023-03-22 23:53     ` Glen Choo
2023-03-26  4:59       ` Ævar Arnfjörð Bjarmason
2023-03-17 15:35 ` [PATCH 16/17] post-cocci: adjust comments for recent repo_* migration Ævar Arnfjörð Bjarmason
2023-03-19  6:12   ` Elijah Newren
2023-03-17 15:35 ` [PATCH 17/17] libs: use "struct repository *" argument, not "the_repository" Ævar Arnfjörð Bjarmason
2023-03-19  6:19   ` Elijah Newren [this message]
2023-03-28 10:53     ` Ævar Arnfjörð Bjarmason
2023-03-17 19:22 ` [PATCH 00/17] cocci: remove "the_index" wrapper macros Junio C Hamano
2023-03-28 13:58 ` [PATCH v2 00/17] cocci: remove "the_repository" " Ævar Arnfjörð Bjarmason
2023-03-28 13:58   ` [PATCH v2 01/17] cocci: remove dead rule from "the_repository.pending.cocci" Ævar Arnfjörð Bjarmason
2023-03-28 13:58   ` [PATCH v2 02/17] cocci: fix incorrect & verbose "the_repository" rules Ævar Arnfjörð Bjarmason
2023-03-29 20:35     ` Taylor Blau
2023-03-28 13:58   ` [PATCH v2 03/17] cocci: sort "the_repository" rules by header Ævar Arnfjörð Bjarmason
2023-03-28 13:58   ` [PATCH v2 04/17] cocci: add missing "the_repository" macros to "pending" Ævar Arnfjörð Bjarmason
2023-03-28 13:58   ` [PATCH v2 05/17] cocci: apply the "cache.h" part of "the_repository.pending" Ævar Arnfjörð Bjarmason
2023-03-28 13:58   ` [PATCH v2 06/17] cocci: apply the "commit-reach.h" " Ævar Arnfjörð Bjarmason
2023-03-28 13:58   ` [PATCH v2 07/17] cocci: apply the "commit.h" " Ævar Arnfjörð Bjarmason
2023-03-28 13:58   ` [PATCH v2 08/17] cocci: apply the "diff.h" " Ævar Arnfjörð Bjarmason
2023-03-28 13:58   ` [PATCH v2 09/17] cocci: apply the "object-store.h" " Ævar Arnfjörð Bjarmason
2023-03-28 13:58   ` [PATCH v2 10/17] cocci: apply the "pretty.h" " Ævar Arnfjörð Bjarmason
2023-03-28 13:58   ` [PATCH v2 11/17] cocci: apply the "packfile.h" " Ævar Arnfjörð Bjarmason
2023-03-28 13:58   ` [PATCH v2 12/17] cocci: apply the "promisor-remote.h" " Ævar Arnfjörð Bjarmason
2023-03-28 13:58   ` [PATCH v2 13/17] cocci: apply the "refs.h" " Ævar Arnfjörð Bjarmason
2023-03-28 13:58   ` [PATCH v2 14/17] cocci: apply the "rerere.h" " Ævar Arnfjörð Bjarmason
2023-03-28 13:58   ` [PATCH v2 15/17] cocci: apply the "revision.h" " Ævar Arnfjörð Bjarmason
2023-03-28 13:58   ` [PATCH v2 16/17] post-cocci: adjust comments for recent repo_* migration Ævar Arnfjörð Bjarmason
2023-03-29 20:50     ` Taylor Blau
2023-03-28 13:58   ` [PATCH v2 17/17] libs: use "struct repository *" argument, not "the_repository" Ævar Arnfjörð Bjarmason
2023-03-28 14:09   ` [PATCH v2 00/17] cocci: remove "the_repository" wrapper macros Junio C Hamano
2023-03-28 14:34     ` Junio C Hamano
2023-03-29 19:02   ` Junio C Hamano
2023-03-29 22:04     ` Taylor Blau
2023-03-30  3:55     ` Elijah Newren
2023-03-30 18:08     ` Glen Choo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CABPp-BEdpTO=DRjLq_p+dgX68M0HUVB--3yQR4Sdp8rnFYeyfA@mail.gmail.com' \
    --to=newren@gmail.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).