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

Re: [PATCH 2/8] refs.c: upgrade for_each_replace_ref to be a each_repo_ref_fn callback

From
Junio C Hamano <gitster@pobox.com>
Date
Jul 18, 2018, 18:32 UTC
Message-ID
<xmqqd0vkjsx3.fsf@gitster-ct.c.googlers.com>
In-Reply-To
<22dc9ce8364dd20876ab5eb626ecc6a7396d3b3e.1531926932.git.gitgitgadget@gmail.com>
"Stefan Beller via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 68 quoted lines
> From: Stefan Beller <sbeller@google.com>
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  builtin/replace.c | 8 ++++----
>  refs.c            | 9 ++++-----
>  refs.h            | 2 +-
>  replace-object.c  | 5 +++--
>  4 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/builtin/replace.c b/builtin/replace.c
> index ef22d724b..d0b1cdb06 100644
> --- a/builtin/replace.c
> +++ b/builtin/replace.c
> @@ -39,7 +39,8 @@ struct show_data {
>  	enum replace_format format;
>  };
>  
> -static int show_reference(const char *refname, const struct object_id *oid,
> +static int show_reference(struct repository *r, const char *refname,
> +			  const struct object_id *oid,
>  			  int flag, void *cb_data)
>  {
>  	struct show_data *data = cb_data;
> @@ -56,9 +57,8 @@ static int show_reference(const char *refname, const struct object_id *oid,
>  			if (get_oid(refname, &object))
>  				return error("Failed to resolve '%s' as a valid ref.", refname);
>  
> -			obj_type = oid_object_info(the_repository, &object,
> -						   NULL);
> -			repl_type = oid_object_info(the_repository, oid, NULL);
> +			obj_type = oid_object_info(r, &object, NULL);
> +			repl_type = oid_object_info(r, oid, NULL);
>  
>  			printf("%s (%s) -> %s (%s)\n", refname, type_name(obj_type),
>  			       oid_to_hex(oid), type_name(repl_type));
> diff --git a/refs.c b/refs.c
> index 2513f77ac..5700cd468 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1478,12 +1478,11 @@ int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,
>  	return do_for_each_ref(refs, prefix, fn, 0, flag, cb_data);
>  }
>  
> -int for_each_replace_ref(struct repository *r, each_ref_fn fn, void *cb_data)
> +int for_each_replace_ref(struct repository *r, each_repo_ref_fn fn, void *cb_data)
>  {
> -	return do_for_each_ref(get_main_ref_store(r),
> -			       git_replace_ref_base, fn,
> -			       strlen(git_replace_ref_base),
> -			       DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
> +	return do_for_each_repo_ref(r, git_replace_ref_base, fn,
> +				    strlen(git_replace_ref_base),
> +				    DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
>  }
>  
>  int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
> diff --git a/refs.h b/refs.h
> index 80eec8bbc..a0a18223a 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -317,7 +317,7 @@ int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data,
>  int for_each_tag_ref(each_ref_fn fn, void *cb_data);
>  int for_each_branch_ref(each_ref_fn fn, void *cb_data);
>  int for_each_remote_ref(each_ref_fn fn, void *cb_data);
> -int for_each_replace_ref(struct repository *r, each_ref_fn fn, void *cb_data);
> +int for_each_replace_ref(struct repository *r, each_repo_ref_fn fn, void *cb_data);

With a signature change like this, any change that introduces new call to for_each_replace_ref() using eac_ref_fn() would get compilation error, so this is minimally correct.

Two things that bothersome are that
 - for_each_tag/branch/remote_ref() and for_each_replace_ref() now
   work and look quite differently.
 - existing users of for_each_replace_ref() who were all happy
   working in the_repository have to pass it explicitly, even
   thought they do not have any need to.

In this case, even if you introduced for_each_replace_ref_in_repo(), making for_each_replace_ref() a thin wrapper that always uses the_repository is a bit more cumbersome than just a simple macro.

But it *is* doable (you'd need to use a wrapping structure around cb_data), and a developer who case about maintainability during API transition would have taken pains to do so. A bit dissapointing.

Show 26 quoted lines
>  int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data);
>  int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,
>  			 const char *prefix, void *cb_data);
> diff --git a/replace-object.c b/replace-object.c
> index 801b5c167..017f02f8e 100644
> --- a/replace-object.c
> +++ b/replace-object.c
> @@ -6,7 +6,8 @@
>  #include "repository.h"
>  #include "commit.h"
>  
> -static int register_replace_ref(const char *refname,
> +static int register_replace_ref(struct repository *r,
> +				const char *refname,
>  				const struct object_id *oid,
>  				int flag, void *cb_data)
>  {
> @@ -25,7 +26,7 @@ static int register_replace_ref(const char *refname,
>  	oidcpy(&repl_obj->replacement, oid);
>  
>  	/* Register new object */
> -	if (oidmap_put(the_repository->objects->replace_map, repl_obj))
> +	if (oidmap_put(r->objects->replace_map, repl_obj))
>  		die("duplicate replace ref: %s", refname);
>  
>  	return 0;
Previous: Stefan Beller via GitGitGadgetNext: Stefan Beller
Message 4 of 47 in “Clarify commit-graph and grafts/replace/shallow incompatibilities”
  1. 0/8 Clarify commit-graph and grafts/replace/shallow incompatibilitiesDerrick Stolee via GitGitGadget, Jul 18, 2018
  2. 1/8 refs.c: migrate internal ref iteration to pass thru repository argumentStefan Beller via GitGitGadget, Jul 18, 2018
  3. 2/8 refs.c: upgrade for_each_replace_ref to be a each_repo_ref_fn callbackStefan Beller via GitGitGadget, Jul 18, 2018
  4. Junio C HamanoJul 18, 2018
  5. Stefan BellerJul 18, 2018
  6. Junio C HamanoJul 18, 2018
  7. 3/8 commit-graph: update design documentDerrick Stolee via GitGitGadget, Jul 18, 2018
  8. Jakub NarebskiJul 29, 2018
  9. 4/8 test-repository: properly init repoDerrick Stolee via GitGitGadget, Jul 18, 2018
  10. Jakub NarebskiJul 29, 2018
  11. 5/8 commit-graph: not compatible with replace objectsDerrick Stolee via GitGitGadget, Jul 18, 2018
  12. Jeff KingJul 18, 2018
  13. Jeff KingJul 18, 2018
  14. Derrick StoleeJul 18, 2018
  15. Derrick StoleeJul 20, 2018
  16. Stefan BellerJul 20, 2018
  17. Derrick StoleeJul 20, 2018
  18. Jakub NarebskiJul 29, 2018
  19. 6/8 commit-graph: not compatible with graftsDerrick Stolee via GitGitGadget, Jul 18, 2018
  20. Jakub NarebskiJul 29, 2018
  21. 7/8 commit-graph: not compatible with uninitialized repoDerrick Stolee via GitGitGadget, Jul 18, 2018
  22. Jakub NarebskiJul 29, 2018
  23. 8/8 commit-graph: close_commit_graph before shallow walkDerrick Stolee via GitGitGadget, Jul 18, 2018
  24. Jakub NarebskiJul 30, 2018
  25. DO-NOT-MERGE: write and read commit-graph alwaysDerrick Stolee, Jul 18, 2018
  26. Jakub NarebskiJul 30, 2018
  27. Stefan BellerJul 30, 2018
  28. Jakub NarebskiJul 31, 2018
  29. Elijah NewrenJul 31, 2018
  30. Jakub NarebskiJul 29, 2018
  31. Eric SunshineJul 29, 2018
  32. 0/8 Clarify commit-graph and grafts/replace/shallow incompatibilitiesDerrick Stolee, Aug 20, 2018
  33. 6/8 commit-graph: not compatible with graftsDerrick Stolee, Aug 20, 2018
  34. 7/8 commit-graph: not compatible with uninitialized repoDerrick Stolee, Aug 20, 2018
  35. 2/8 refs.c: upgrade for_each_replace_ref to be a each_repo_ref_fn callbackDerrick Stolee, Aug 20, 2018
  36. 3/8 commit-graph: update design documentDerrick Stolee, Aug 20, 2018
  37. 4/8 test-repository: properly init repoDerrick Stolee, Aug 20, 2018
  38. 5/8 commit-graph: not compatible with replace objectsDerrick Stolee, Aug 20, 2018
  39. Junio C HamanoAug 21, 2018
  40. Derrick StoleeAug 21, 2018
  41. 8/8 commit-graph: close_commit_graph before shallow walkDerrick Stolee, Aug 20, 2018
  42. 1/8 refs.c: migrate internal ref iteration to pass thru repository argumentDerrick Stolee, Aug 20, 2018
  43. Stefan BellerAug 20, 2018
  44. Junio C HamanoAug 21, 2018
  45. Derrick StoleeAug 21, 2018
  46. Stefan BellerAug 20, 2018
  47. Derrick StoleeAug 20, 2018

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.