Re: [Outreachy PATCH v2] checkout: pass the repository to unique_tracking_name()

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> Replace the last sentence with something like:
>
>     The callers still cannot call unique_tracking_name() with an
>     arbitrary repository parameter other than the_repository, until
>     the call in unique_tracking_name() to for_each_remote(), which
>     only works with the_repository, is updated to take an arbitrary
>     repository instance.
>
> perhaps?
>
> And explained that way, it becomes dubious if we chose a good place
> to stop.  The updated unique_tracking_name() gives an illusion that
> callers now can pass an arbitrary repository to it and expect it to
> work correctly with the given repository, but it does not.
>
> So, I dunno.

I think we need to improve the situation by doing something like the
attached patch.  We already have similar provisions in environment.h
(to clarify that have_git_dir() and friends only work with
'the_repository'), hash.h ('the_hash_algo'), hex.h (get_hash_hex(),
get_oid_hex(), etc.), path.h (GIT_PATH_FUNC() macro) and
repository.h ('the_repository' itself).

As we pass a 'struct repository' pointer to more functions, making
sure they do not call functions that still work only with
'the_repository' will become more important.

I suspect that in remote.h, there may be more symbols that need to
be hidden when 'USE_THE_REPOSITORY_VARIABLE' is not defined, to
ensure functions written without the macro can truly work with
an arbitrary repository instance.  The attached patch is just to
illustrate the direction.

This is totally outside your topic and meant for more senior
developers who help polish the foundation of the codebase.

Thanks.


 remote.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git i/remote.h w/remote.h
index bfa5218c6e..eb7d4481eb 100644
--- i/remote.h
+++ w/remote.h
@@ -145,8 +145,10 @@ int remote_is_configured(struct remote *remote, int in_repo);
 
 typedef int each_remote_fn(struct remote *remote, void *priv);
 
+#ifdef USE_THE_REPOSITORY_VARIABLE
 /* iterate through struct remotes */
 int for_each_remote(each_remote_fn fn, void *priv);
+#endif
 
 int remote_has_url(struct remote *remote, const char *url);
 struct strvec *push_url_of_remote(struct remote *remote);