From: Patrick Steinhardt <ps@pks.im>
Date: Wed, 11 Feb 2026 12:13:18 GMT
Subject: Re: [PATCH 2/5] parse: add git_parse_maybe_pathname()
Message-ID: <aYxyXlH5Z0toWgPj@pks.im>
In-Reply-To: <8d3a6a8265714c5e4bae0f2e5a587ea46a6adddc.1770698579.git.gitgitgadget@gmail.com>

On Tue, Feb 10, 2026 at 04:42:56AM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <stolee@gmail.com>
> 
> This extraction of logic from config.c's git_config_pathname() allows
> for parsing a fully-qualified path from a relative path along with
> validation of the existence of the path without failing with a die().

That sentence is quite something. I had to read it thrice to understand
what it wants to say :)

> diff --git a/parse.c b/parse.c
> index 48313571aa..3f37f0b93a 100644
> --- a/parse.c
> +++ b/parse.c
> @@ -209,3 +210,26 @@ unsigned long git_env_ulong(const char *k, unsigned long val)
>  		die(_("failed to parse %s"), k);
>  	return val;
>  }
> +
> +int git_parse_maybe_pathname(const char *value, char **dest)
> +{
> +	bool is_optional;
> +	char *path;
> +
> +	if (!value)
> +		return -1;
> +
> +	is_optional = skip_prefix(value, ":(optional)", &value);
> +	path = interpolate_path(value, 0);
> +	if (!path)
> +		return -1;
> +
> +	if (is_optional && is_missing_file(path)) {
> +		free(path);
> +		*dest = NULL;
> +		return 0;
> +	}
> +
> +	*dest = path;
> +	return 0;
> +}

Okay. So the difference is that this function here doesn't cause us to
die in case the path is not marked as optional and missing. Makes sense.

> diff --git a/parse.h b/parse.h
> index ea32de9a91..4f97c3727a 100644
> --- a/parse.h
> +++ b/parse.h
> @@ -19,4 +19,6 @@ int git_parse_maybe_bool_text(const char *value);
>  int git_env_bool(const char *, int);
>  unsigned long git_env_ulong(const char *, unsigned long);
>  
> +int git_parse_maybe_pathname(const char *value, char **dest);

I think this function could use some explanation what it actually does,
as the behaviour is non-trivial:

  - I think the ":(optional)" part needs to be documented properly to
    say that we return successfully with a NULL string in case the
    target path doesn't exist.

  - We should document that it expands "~" and "%(prefix)" (even though
    the latter feels somewhat coincidental to me).

  - The path is not resolved to an absolute path.

Thanks!

Patrick