Re: [PATCH 05/16] path: refactor `repo_submodule_path()` family of functions

[Patrick Steinhardt <ps@pks.im>]

On Thu, Feb 06, 2025 at 04:05:13AM -0800, Karthik Nayak wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> [snip]
> 
> > diff --git a/path.c b/path.c
> > index d918d0409e..d721507be8 100644
> > --- a/path.c
> > +++ b/path.c
> > @@ -560,14 +560,15 @@ const char *repo_worktree_path_replace(const struct repository *repo,
> >  }
> >
> >  /* Returns 0 on success, negative on failure. */
> > -static int do_submodule_path(struct strbuf *buf, const char *path,
> > +static int do_submodule_path(struct repository *repo,
> > +			     struct strbuf *buf, const char *path,
> >  			     const char *fmt, va_list args)
> >  {
> >  	struct strbuf git_submodule_common_dir = STRBUF_INIT;
> >  	struct strbuf git_submodule_dir = STRBUF_INIT;
> >  	int ret;
> >
> > -	ret = submodule_to_gitdir(the_repository, &git_submodule_dir, path);
> > +	ret = submodule_to_gitdir(repo, &git_submodule_dir, path);
> >  	if (ret)
> >  		goto cleanup;
> >
> > @@ -586,13 +587,14 @@ static int do_submodule_path(struct strbuf *buf, const char *path,
> >  	return ret;
> >  }
> >
> > -char *git_pathdup_submodule(const char *path, const char *fmt, ...)
> > +char *repo_submodule_path(struct repository *repo,
> 
> To stay consistent with the other repo_* functions, should we change
> `struct repository *repo` to `const struct repository *repo`?

Somebody noticed :) But no, we cannot, we need to internally pass the
repo to functions that expect a non-const pointer. This is because deep
down in the callstack we end up calling `repo_read_gitmodules()`, which
modifies the repository.

I'll add a comment to the commit message.

Patrick