git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 4/5] cocci: apply rules to rewrite callers of "refs" interfaces

From
Taylor Blau <me@ttaylorr.com>
Date
May 3, 2024, 18:48 UTC
Message-ID
<ZjUxlLyyvmHNifDp@nand.local>
In-Reply-To
<5109468b356637b8b4521a3e96ba228622804ca0.1714717057.git.ps@pks.im>
On Fri, May 03, 2024 at 08:28:14AM +0200, Patrick Steinhardt wrote:
Show 98 quoted lines
> Apply the rules that rewrite callers of "refs" interfaces to explicitly
> pass `struct ref_store`. The resulting patch has been applied with the
> `--whitespace=fix` option.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  add-interactive.c           | 17 +++++++----
>  bisect.c                    | 25 ++++++++++-----
>  blame.c                     |  4 +--
>  branch.c                    |  5 +--
>  builtin/am.c                | 38 ++++++++++++++---------
>  builtin/bisect.c            | 44 +++++++++++++++-----------
>  builtin/blame.c             |  4 +--
>  builtin/branch.c            | 49 ++++++++++++++++-------------
>  builtin/checkout.c          | 35 +++++++++++++--------
>  builtin/clone.c             | 36 +++++++++++++---------
>  builtin/describe.c          |  3 +-
>  builtin/fast-import.c       | 11 ++++---
>  builtin/fetch.c             | 20 ++++++++----
>  builtin/fsck.c              | 11 +++++--
>  builtin/gc.c                |  3 +-
>  builtin/log.c               |  6 ++--
>  builtin/merge.c             | 34 +++++++++++++--------
>  builtin/name-rev.c          |  5 +--
>  builtin/notes.c             | 26 +++++++++-------
>  builtin/pack-objects.c      | 10 ++++--
>  builtin/pull.c              |  2 +-
>  builtin/rebase.c            | 18 ++++++-----
>  builtin/receive-pack.c      | 15 ++++++---
>  builtin/reflog.c            | 25 ++++++++-------
>  builtin/remote.c            | 37 +++++++++++++---------
>  builtin/repack.c            |  7 +++--
>  builtin/replace.c           |  9 +++---
>  builtin/reset.c             | 13 +++++---
>  builtin/rev-parse.c         | 25 ++++++++++-----
>  builtin/show-branch.c       | 22 ++++++++-----
>  builtin/show-ref.c          | 19 ++++++++----
>  builtin/stash.c             | 23 ++++++++------
>  builtin/submodule--helper.c |  7 +++--
>  builtin/symbolic-ref.c      | 13 +++++---
>  builtin/tag.c               | 11 ++++---
>  builtin/update-index.c      |  2 +-
>  builtin/update-ref.c        | 21 ++++++++-----
>  builtin/worktree.c          | 19 +++++++-----
>  bundle-uri.c                | 12 +++++---
>  bundle.c                    |  2 +-
>  commit-graph.c              |  3 +-
>  commit.c                    |  3 +-
>  config.c                    |  3 +-
>  delta-islands.c             |  3 +-
>  fetch-pack.c                |  6 ++--
>  fmt-merge-msg.c             |  4 ++-
>  help.c                      |  5 +--
>  http-backend.c              | 13 +++++---
>  log-tree.c                  |  9 ++++--
>  ls-refs.c                   | 10 +++---
>  midx-write.c                |  3 +-
>  negotiator/default.c        |  3 +-
>  negotiator/skipping.c       |  3 +-
>  notes-cache.c               |  6 ++--
>  notes-merge.c               |  2 +-
>  notes-utils.c               |  7 +++--
>  notes.c                     |  5 +--
>  reachable.c                 |  5 +--
>  ref-filter.c                | 35 +++++++++++++++------
>  reflog-walk.c               | 27 +++++++++++-----
>  reflog.c                    | 20 +++++++-----
>  refs.c                      |  9 ++++--
>  remote.c                    | 38 ++++++++++++++---------
>  reset.c                     | 29 ++++++++++--------
>  revision.c                  | 27 ++++++++++------
>  sequencer.c                 | 61 ++++++++++++++++++++-----------------
>  server-info.c               |  3 +-
>  setup.c                     |  2 +-
>  shallow.c                   | 16 ++++++----
>  submodule.c                 |  6 ++--
>  transport-helper.c          | 29 ++++++++++--------
>  transport.c                 | 16 ++++++----
>  upload-pack.c               | 20 +++++++-----
>  walker.c                    |  6 ++--
>  wt-status.c                 | 22 +++++++------
>  75 files changed, 711 insertions(+), 436 deletions(-)
>
> diff --git a/add-interactive.c b/add-interactive.c
> index e17602b5e4..b5d6cd689a 100644
> --- a/add-interactive.c
> +++ b/add-interactive.c
> @@ -532,8 +532,9 @@ static int get_modified_files(struct repository *r,
>  			      size_t *binary_count)
>  {
>  	struct object_id head_oid;
> -	int is_initial = !resolve_ref_unsafe("HEAD", RESOLVE_REF_READING,
> -					     &head_oid, NULL);
> +	int is_initial = !refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
> +						  "HEAD", RESOLVE_REF_READING,
> +						  &head_oid, NULL);
>  	struct collection_status s = { 0 };
>  	int i;

OK, so this is the patch that applies the Coccinelle script from the previous step.

This all makes sense, but I wonder if we should be more careful than blindly replacing these functions with the same set of arguments following a new

    get_main_ref_store(the_repository)

For instance, in this hunk, we have a 'struct repository *' in scope via the 'r' parameter in add-interactive.c::get_modified_files().

I don't think it's wrong per-se to use the_repository here, but it does create something to clean up in the future.

I haven't looked at other spots throughout this patch, just noticed this in the first hunk and thought that I'd say something about it. Otherwise the transformation seems obviously correct.

Thanks, Taylor

Previous: Patrick SteinhardtNext: Junio C Hamano
Message 8 of 32 in “refs: remove functions without ref store”
  1. 0/5 refs: remove functions without ref storePatrick Steinhardt, May 3, 2024
  2. 1/5 refs: introduce missing functions that accept a `struct ref_store`Patrick Steinhardt, May 3, 2024
  3. Junio C HamanoMay 3, 2024
  4. 2/5 refs: add `exclude_patterns` parameter to `for_each_fullref_in()`Patrick Steinhardt, May 3, 2024
  5. Taylor BlauMay 3, 2024
  6. 3/5 cocci: introduce rules to transform "refs" to pass ref storePatrick Steinhardt, May 3, 2024
  7. 4/5 cocci: apply rules to rewrite callers of "refs" interfacesPatrick Steinhardt, May 3, 2024
  8. Taylor BlauMay 3, 2024
  9. Junio C HamanoMay 3, 2024
  10. Patrick SteinhardtMay 6, 2024
  11. 5/5 refs: remove functions without ref storePatrick Steinhardt, May 3, 2024
  12. James LiuMay 6, 2024
  13. Junio C HamanoMay 3, 2024
  14. Jeff KingMay 3, 2024
  15. Junio C HamanoMay 3, 2024
  16. Patrick SteinhardtMay 6, 2024
  17. Junio C HamanoMay 6, 2024
  18. Patrick SteinhardtMay 7, 2024
  19. Junio C HamanoMay 7, 2024
  20. Patrick SteinhardtMay 7, 2024
  21. Junio C HamanoMay 7, 2024
  22. Jeff KingMay 9, 2024
  23. Patrick SteinhardtMay 10, 2024
  24. Taylor BlauMay 3, 2024
  25. Junio C HamanoMay 3, 2024
  26. 0/5 refs: remove functions without ref storePatrick Steinhardt, May 7, 2024
  27. 1/5 refs: introduce missing functions that accept a `struct ref_store`Patrick Steinhardt, May 7, 2024
  28. 2/5 refs: add `exclude_patterns` parameter to `for_each_fullref_in()`Patrick Steinhardt, May 7, 2024
  29. 3/5 cocci: introduce rules to transform "refs" to pass ref storePatrick Steinhardt, May 7, 2024
  30. 4/5 cocci: apply rules to rewrite callers of "refs" interfacesPatrick Steinhardt, May 7, 2024
  31. 5/5 refs: remove functions without ref storePatrick Steinhardt, May 7, 2024
  32. Taylor BlauMay 7, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.