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

Re: [PATCH] replay: replace the_repository with repo parameter passed to cmd_replay()

From
Junio C Hamano <gitster@pobox.com>
Date
May 14, 2025, 22:10 UTC
Message-ID
<xmqqplga7sij.fsf@gitster.g>
In-Reply-To
<pull.1921.git.1747254806067.gitgitgadget@gmail.com>
"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>     require it and have no current alternatives. However, I still think it's
>     worthwhile to stop using the_repository everywhere while ignoring the
>     repo parameter explicitly passed in.
Sensible.
Show 16 quoted lines
> diff --git a/builtin/replay.c b/builtin/replay.c
> index 032c172b65e..225cef08807 100644
> --- a/builtin/replay.c
> +++ b/builtin/replay.c
> @@ -20,21 +20,22 @@
>  #include <oidset.h>
>  #include <tree.h>
>  
> -static const char *short_commit_name(struct commit *commit)
> +static const char *short_commit_name(struct repository *repo,
> +				     struct commit *commit)
>  {
> -	return repo_find_unique_abbrev(the_repository, &commit->object.oid,
> +	return repo_find_unique_abbrev(repo, &commit->object.oid,
>  				       DEFAULT_ABBREV);
>  }

I do not mind this, but I do have to wonder if it is simpler to make the two callers of this "helper" (which is not quite helping anything) to make these calls themselves.

Show 14 quoted lines
>  int cmd_replay(int argc,
>  	       const char **argv,
>  	       const char *prefix,
> -	       struct repository *repo UNUSED)
> +	       struct repository *repo)
>  {
>  	const char *advance_name_opt = NULL;
>  	char *advance_name = NULL;
> @@ -329,7 +334,7 @@ int cmd_replay(int argc,
>  		    "--advance", "--contained");
>  	advance_name = xstrdup_or_null(advance_name_opt);
>  
> -	repo_init_revisions(the_repository, &revs, prefix);
> +	repo_init_revisions(repo, &revs, prefix);

OK, since this command is marked as RUN_SETUP, it is safe to unconditionally use repo here. The only situation where it is called with repo==NULL is when somebody said "git replay -h" outside a repository, which would have made parse_options() to do the right thing and exited already without reaching this code, so we should be able to trust "repo" to be usable.

Will queue.  Thanks.
Previous: Elijah Newren via GitGitGadgetNext: Patrick Steinhardt
Message 2 of 3 in “replay: replace the_repository with repo parameter passed to cmd_replay()”
  1. replay: replace the_repository with repo parameter passed to cmd_replay()Elijah Newren via GitGitGadget, May 14, 2025
  2. Junio C HamanoMay 14, 2025
  3. Patrick SteinhardtMay 15, 2025

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.