From: D. Ben Knoble <ben.knoble@gmail.com>
Date: Thu, 20 Nov 2025 22:15:46 GMT
Subject: Re: [PATCH] config: really pretend missing :(optional) value is not there
Message-ID: <CALnO6CC0HU60F47yoE45ei7_K2_MeLRS7fihMPn+f8top7Jr7w@mail.gmail.com>
In-Reply-To: <xmqqms4g7b1h.fsf@gitster.g>

On Thu, Nov 20, 2025 at 2:35 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Earlier we added support for a value spelled as ":(optional)path"
> for configuration variables whose values are of type "path", with
> the documented semantics "if the path is missing, behave as if such
> a variable definition is not even there."
>
> This has worked OK for code paths that reads configuration files and
> stores the configured value as a string, where NULL in such a string
> is treated as if the setting is not there, left as the default.
>
> However, there are other code paths that do not _ignore_ such NULL
> values and misbehave.  "git config get --path" is one of them.
>
> When git_config_pathname() helper function finds that the value of
> the variable is an optional path *and* the path is missing, it
> leaves the destination pointer intact (which usually is left to
> NULL) and returns 0 to signal a success.  format_config() helper
> however assumed that the destination pointer always gets a string,
> which no longer is the case, and segfaulted.
>
> Make sure that git_config_pathname() clears the destination pointer
> in such a case, and teach format_config() to react to the condition
> by returning 1 (which is different from 0 that is a normal success
> and negative that is an error) to its callers.  Adjust the callers
> to react to this new return value that tells them to pretend as if
> they did not even see this partcular <key, value> pair.
>
> Reported-by: Han Jiang <jhcarl0814@gmail.com>
> Helped-by: Jeff King <peff@peff.net>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>
>  * This is only about "git config get --path".  Another patch for
>    the rest of the callers of git_config_pathname() will follow in a
>    separate message.
>
>  builtin/config.c           | 45 ++++++++++++++++++++++++++++++--------
>  config.c                   |  1 +
>  t/t1311-config-optional.sh | 36 ++++++++++++++++++++++++++++++
>  3 files changed, 73 insertions(+), 9 deletions(-)

This needs a tweak to Meson, probably in t/meson.build, for the new
test script. Otherwise Meson-based packages (like Gentoo) won't build.

-- 
D. Ben Knoble