Re: [PATCH v2 10/21] environment: make `get_git_namespace()` self-contained

From: Justin Tobler <jltobler@gmail.com>
Date: Sep 11, 2024, 16:21 UTC
Message-ID: <otvkxgywps6smoib32x4gqiwc4klbjl6qbh66bh6wwwfvb2b2p@22gqczmx3p3e>
In-Reply-To: <f0d3794dfc44cd4393fd79fab9f60b73cf33db89.1725008898.git.ps@pks.im>

On 24/08/30 11:09AM, Patrick Steinhardt wrote:

Show 11 quoted lines

> The logic to set up and retrieve `git_namespace` is distributed across
> different functions which communicate with each other via a global
> environment variable. This is rather pointless though, as the value is
> always derived from an environment variable, and this environment
> variable does not change after we have parsed global options.
> 
> Convert the function to be fully self-contained such that it lazily
> populates once called.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---

[snip]

Show 15 quoted lines

> @@ -229,9 +204,35 @@ int have_git_dir(void)
>  
>  const char *get_git_namespace(void)
>  {
> -	if (!git_namespace)
> -		BUG("git environment hasn't been setup");
> -	return git_namespace;
> +	static const char *namespace;
> +
> +	struct strbuf buf = STRBUF_INIT;
> +	struct strbuf **components, **c;
> +	const char *raw_namespace;
> +
> +	if (namespace)
> +		return namespace;

We only initialize `namespace` once and do it lazily when `get_git_namespace()` is invoked for the first time. This is nice because we can simplify and remove the `git_namespace` global.

Makes sense to me.

Show 27 quoted lines

> +
> +	raw_namespace = getenv(GIT_NAMESPACE_ENVIRONMENT);
> +	if (!raw_namespace || !*raw_namespace) {
> +		namespace = "";
> +		return namespace;
> +	}
> +
> +	strbuf_addstr(&buf, raw_namespace);
> +	components = strbuf_split(&buf, '/');
> +	strbuf_reset(&buf);
> +	for (c = components; *c; c++)
> +		if (strcmp((*c)->buf, "/") != 0)
> +			strbuf_addf(&buf, "refs/namespaces/%s", (*c)->buf);
> +	strbuf_list_free(components);
> +	if (check_refname_format(buf.buf, 0))
> +		die(_("bad git namespace path \"%s\""), raw_namespace);
> +	strbuf_addch(&buf, '/');
> +
> +	namespace = strbuf_detach(&buf, NULL);
> +
> +	return namespace;
>  }
>  
>  const char *strip_namespace(const char *namespaced_ref)
> -- 
> 2.46.0.421.g159f2d50e7.dirty
>