mbox series

[v2,0/5] refs: remove functions without ref store

Message ID cover.1715065736.git.ps@pks.im (mailing list archive)
Headers show
Series refs: remove functions without ref store | expand

Message

Patrick Steinhardt May 7, 2024, 7:11 a.m. UTC
Hi,

this is the second version of my patch series that aims to remove
functions from "refs.h" that implicitly rely on `the_repository` to
obtain the ref store.

There's only a single change compared to v1. As discussed, we want to
give in-flight patch series a bit more guidance when they add new calls
to the now-removed functions. This is done in the form of a new section
with ifdef'd function declarations for every removed function. These are
easily greppable and trivially show the author of the series how they
are supposed to adapt to the new world.

Thanks for all the feedback!

Patrick

Patrick Steinhardt (5):
  refs: introduce missing functions that accept a `struct ref_store`
  refs: add `exclude_patterns` parameter to `for_each_fullref_in()`
  cocci: introduce rules to transform "refs" to pass ref store
  cocci: apply rules to rewrite callers of "refs" interfaces
  refs: remove functions without ref store

 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 +-
 contrib/coccinelle/refs.cocci | 103 ++++++++++++
 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                        | 200 +++--------------------
 refs.h                        | 291 ++++++++++++++++++++++++++--------
 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 +--
 77 files changed, 1052 insertions(+), 680 deletions(-)
 create mode 100644 contrib/coccinelle/refs.cocci

Range-diff against v1:
1:  dba5df086d = 1:  dba5df086d refs: introduce missing functions that accept a `struct ref_store`
2:  4f34bb2e03 = 2:  4f34bb2e03 refs: add `exclude_patterns` parameter to `for_each_fullref_in()`
3:  ffe83f7482 = 3:  ffe83f7482 cocci: introduce rules to transform "refs" to pass ref store
4:  5109468b35 = 4:  5109468b35 cocci: apply rules to rewrite callers of "refs" interfaces
5:  47eb6aee92 ! 5:  773873f244 refs: remove functions without ref store
    @@ Commit message
         the respective variants without the ref store are now unused. Remove
         them.
     
    +    There are likely patch series in-flight that use the now-removed
    +    functions. To help the authors, the old implementations have been added
    +    to "refs.c" in an ifdef'd section as a reference for how to migrate each
    +    of the respective callers.
    +
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
      ## refs.c ##
    @@ refs.h: int refs_reflog_expire(struct ref_store *refs,
      
      struct ref_store *get_main_ref_store(struct repository *r);
      
    +@@ refs.h: void update_ref_namespace(enum ref_namespace namespace, char *ref);
    + int is_pseudoref(struct ref_store *refs, const char *refname);
    + int is_headref(struct ref_store *refs, const char *refname);
    + 
    ++/*
    ++ * The following functions have been removed in Git v2.45 in favor of functions
    ++ * that receive a `ref_store` as parameter. The intent of this section is
    ++ * merely to help patch authors of in-flight series to have a reference what
    ++ * they should be migrating to. The section will be removed in Git v2.46.
    ++ */
    ++#if 0
    ++static char *resolve_refdup(const char *refname, int resolve_flags,
    ++			    struct object_id *oid, int *flags)
    ++{
    ++	return refs_resolve_refdup(get_main_ref_store(the_repository),
    ++				   refname, resolve_flags,
    ++				   oid, flags);
    ++}
    ++
    ++static int read_ref_full(const char *refname, int resolve_flags,
    ++			 struct object_id *oid, int *flags)
    ++{
    ++	return refs_read_ref_full(get_main_ref_store(the_repository), refname,
    ++				  resolve_flags, oid, flags);
    ++}
    ++
    ++static int read_ref(const char *refname, struct object_id *oid)
    ++{
    ++	return refs_read_ref(get_main_ref_store(the_repository), refname, oid);
    ++}
    ++
    ++static int ref_exists(const char *refname)
    ++{
    ++	return refs_ref_exists(get_main_ref_store(the_repository), refname);
    ++}
    ++
    ++static int for_each_tag_ref(each_ref_fn fn, void *cb_data)
    ++{
    ++	return refs_for_each_tag_ref(get_main_ref_store(the_repository), fn, cb_data);
    ++}
    ++
    ++static int for_each_branch_ref(each_ref_fn fn, void *cb_data)
    ++{
    ++	return refs_for_each_branch_ref(get_main_ref_store(the_repository), fn, cb_data);
    ++}
    ++
    ++static int for_each_remote_ref(each_ref_fn fn, void *cb_data)
    ++{
    ++	return refs_for_each_remote_ref(get_main_ref_store(the_repository), fn, cb_data);
    ++}
    ++
    ++static int head_ref_namespaced(each_ref_fn fn, void *cb_data)
    ++{
    ++	return refs_head_ref_namespaced(get_main_ref_store(the_repository),
    ++					fn, cb_data);
    ++}
    ++
    ++static int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,
    ++				const char *prefix, void *cb_data)
    ++{
    ++	return refs_for_each_glob_ref_in(get_main_ref_store(the_repository),
    ++					 fn, pattern, prefix, cb_data);
    ++}
    ++
    ++static int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data)
    ++{
    ++	return refs_for_each_glob_ref(get_main_ref_store(the_repository),
    ++				      fn, pattern, cb_data);
    ++}
    ++
    ++static int delete_ref(const char *msg, const char *refname,
    ++		      const struct object_id *old_oid, unsigned int flags)
    ++{
    ++	return refs_delete_ref(get_main_ref_store(the_repository), msg, refname,
    ++			       old_oid, flags);
    ++}
    ++
    ++static struct ref_transaction *ref_transaction_begin(struct strbuf *err)
    ++{
    ++	return ref_store_transaction_begin(get_main_ref_store(the_repository), err);
    ++}
    ++
    ++static int update_ref(const char *msg, const char *refname,
    ++		      const struct object_id *new_oid,
    ++		      const struct object_id *old_oid,
    ++		      unsigned int flags, enum action_on_err onerr)
    ++{
    ++	return refs_update_ref(get_main_ref_store(the_repository), msg, refname, new_oid,
    ++			       old_oid, flags, onerr);
    ++}
    ++
    ++static char *shorten_unambiguous_ref(const char *refname, int strict)
    ++{
    ++	return refs_shorten_unambiguous_ref(get_main_ref_store(the_repository),
    ++					    refname, strict);
    ++}
    ++
    ++static int head_ref(each_ref_fn fn, void *cb_data)
    ++{
    ++	return refs_head_ref(get_main_ref_store(the_repository), fn, cb_data);
    ++}
    ++
    ++static int for_each_ref(each_ref_fn fn, void *cb_data)
    ++{
    ++	return refs_for_each_ref(get_main_ref_store(the_repository), fn, cb_data);
    ++}
    ++
    ++static int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
    ++{
    ++	return refs_for_each_ref_in(get_main_ref_store(the_repository), prefix, fn, cb_data);
    ++}
    ++
    ++static int for_each_fullref_in(const char *prefix,
    ++			       const char **exclude_patterns,
    ++			       each_ref_fn fn, void *cb_data)
    ++{
    ++	return refs_for_each_fullref_in(get_main_ref_store(the_repository),
    ++					prefix, exclude_patterns, fn, cb_data);
    ++}
    ++
    ++static int for_each_namespaced_ref(const char **exclude_patterns,
    ++				   each_ref_fn fn, void *cb_data)
    ++{
    ++	return refs_for_each_namespaced_ref(get_main_ref_store(the_repository),
    ++					    exclude_patterns, fn, cb_data);
    ++}
    ++
    ++static int for_each_rawref(each_ref_fn fn, void *cb_data)
    ++{
    ++	return refs_for_each_rawref(get_main_ref_store(the_repository), fn, cb_data);
    ++}
    ++
    ++static const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
    ++				      struct object_id *oid, int *flags)
    ++{
    ++	return refs_resolve_ref_unsafe(get_main_ref_store(the_repository), refname,
    ++				       resolve_flags, oid, flags);
    ++}
    ++
    ++static int create_symref(const char *ref_target, const char *refs_heads_master,
    ++			 const char *logmsg)
    ++{
    ++	return refs_create_symref(get_main_ref_store(the_repository), ref_target,
    ++				  refs_heads_master, logmsg);
    ++}
    ++
    ++static int for_each_reflog(each_reflog_fn fn, void *cb_data)
    ++{
    ++	return refs_for_each_reflog(get_main_ref_store(the_repository), fn, cb_data);
    ++}
    ++
    ++static int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn,
    ++				       void *cb_data)
    ++{
    ++	return refs_for_each_reflog_ent_reverse(get_main_ref_store(the_repository),
    ++						refname, fn, cb_data);
    ++}
    ++
    ++static int for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn,
    ++			       void *cb_data)
    ++{
    ++	return refs_for_each_reflog_ent(get_main_ref_store(the_repository), refname,
    ++					fn, cb_data);
    ++}
    ++
    ++static int reflog_exists(const char *refname)
    ++{
    ++	return refs_reflog_exists(get_main_ref_store(the_repository), refname);
    ++}
    ++
    ++static int safe_create_reflog(const char *refname, struct strbuf *err)
    ++{
    ++	return refs_create_reflog(get_main_ref_store(the_repository), refname,
    ++				  err);
    ++}
    ++
    ++static int delete_reflog(const char *refname)
    ++{
    ++	return refs_delete_reflog(get_main_ref_store(the_repository), refname);
    ++}
    ++
    ++static int reflog_expire(const char *refname,
    ++			 unsigned int flags,
    ++			 reflog_expiry_prepare_fn prepare_fn,
    ++			 reflog_expiry_should_prune_fn should_prune_fn,
    ++			 reflog_expiry_cleanup_fn cleanup_fn,
    ++			 void *policy_cb_data)
    ++{
    ++	return refs_reflog_expire(get_main_ref_store(the_repository),
    ++				  refname, flags,
    ++				  prepare_fn, should_prune_fn,
    ++				  cleanup_fn, policy_cb_data);
    ++}
    ++
    ++static int delete_refs(const char *msg, struct string_list *refnames,
    ++		       unsigned int flags)
    ++{
    ++	return refs_delete_refs(get_main_ref_store(the_repository), msg, refnames, flags);
    ++}
    ++
    ++static int rename_ref(const char *oldref, const char *newref, const char *logmsg)
    ++{
    ++	return refs_rename_ref(get_main_ref_store(the_repository), oldref, newref, logmsg);
    ++}
    ++
    ++static int copy_existing_ref(const char *oldref, const char *newref, const char *logmsg)
    ++{
    ++	return refs_copy_existing_ref(get_main_ref_store(the_repository), oldref, newref, logmsg);
    ++}
    ++#endif
    ++
    + #endif /* REFS_H */

Comments

Taylor Blau May 7, 2024, 5:27 p.m. UTC | #1
On Tue, May 07, 2024 at 09:11:34AM +0200, Patrick Steinhardt wrote:
> Hi,
>
> this is the second version of my patch series that aims to remove
> functions from "refs.h" that implicitly rely on `the_repository` to
> obtain the ref store.
>
> There's only a single change compared to v1. As discussed, we want to
> give in-flight patch series a bit more guidance when they add new calls
> to the now-removed functions. This is done in the form of a new section
> with ifdef'd function declarations for every removed function. These are
> easily greppable and trivially show the author of the series how they
> are supposed to adapt to the new world.
>
> Thanks for all the feedback!

Thanks for the range-diff, which is definitely helpful for reviewing
this round ;-).

Having followed the discussions in the previous round, particularly
Junio's comments about #ifdefing the declarations of the removed
funcitons, I think that this round is in good shape.

    Acked-by: Taylor Blau <me@ttaylorr.com>

Thanks,
Taylor