diff mbox series

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

Message ID f0d3794dfc44cd4393fd79fab9f60b73cf33db89.1725008898.git.ps@pks.im (mailing list archive)
State Superseded
Headers show
Series environment: guard reliance on `the_repository` | expand

Commit Message

Patrick Steinhardt Aug. 30, 2024, 9:09 a.m. UTC 
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>
---
 environment.c | 57 ++++++++++++++++++++++++++-------------------------
 1 file changed, 29 insertions(+), 28 deletions(-)

Comments

Justin Tobler Sept. 11, 2024, 4:21 p.m. UTC | #1 
On 24/08/30 11:09AM, Patrick Steinhardt wrote:
> 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]
> @@ -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.

> +
> +	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
>
diff mbox series

Patch

diff --git a/environment.c b/environment.c
index f337efd1dd5..49844418419 100644
--- a/environment.c
+++ b/environment.c
@@ -122,8 +122,6 @@  int core_preload_index = 1;
 /* This is set by setup_git_dir_gently() and/or git_default_config() */
 char *git_work_tree_cfg;
 
-static char *git_namespace;
-
 /*
  * Repository-local GIT_* environment variables; see environment.h for details.
  */
@@ -146,27 +144,6 @@  const char * const local_repo_env[] = {
 	NULL
 };
 
-static char *expand_namespace(const char *raw_namespace)
-{
-	struct strbuf buf = STRBUF_INIT;
-	struct strbuf **components, **c;
-
-	if (!raw_namespace || !*raw_namespace)
-		return xstrdup("");
-
-	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, '/');
-	return strbuf_detach(&buf, NULL);
-}
-
 const char *getenv_safe(struct strvec *argv, const char *name)
 {
 	const char *value = getenv(name);
@@ -205,8 +182,6 @@  void setup_git_env(const char *git_dir)
 							  : "refs/replace/");
 	update_ref_namespace(NAMESPACE_REPLACE, git_replace_ref_base);
 
-	free(git_namespace);
-	git_namespace = expand_namespace(getenv(GIT_NAMESPACE_ENVIRONMENT));
 	shallow_file = getenv(GIT_SHALLOW_FILE_ENVIRONMENT);
 	if (shallow_file)
 		set_alternate_shallow_file(the_repository, shallow_file, 0);
@@ -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;
+
+	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)