Subject: Re: [PATCH v3 2/3] safe.directory: normalize the configured path

From: Junio C Hamano <[email-masked]>
Date: Tue, 30 Jul 2024 09:03:35 -0700

Jeff King <[email-masked]> writes:

>> +			if (!is_absolute_path(check) && strcmp(check, ".")) {
>> +				warning(_("safe.directory '%s' not absolute"),
>> +					check);
>> +				goto next;
>> +			}
>
> This is_absolute_path() check is redundant, isn't it? If we are checking
> for a literal ".", then we know the path must be non-absolute.

What I meant was "If it is not absolute, that is an error, but if
the thing is a dot, that is allowed as an exception".

Is the lack of "!" confusing, I wonder?   We could rewrite it to
make it more explicit:

	if (is_absolute_path(check) || !strcmp(check, ".")) {
		; /* OK */
	} else {
		warning(_("not absolute %s"), check);
		goto next;
	}

My earlier draft for v3 had the check for dot a lot earlier in the
function, i.e.

	-	} else if (!strcmp(value, "*")) {
	+	} else if (!strcmp(value, "*") || !strcmp(value, ".")) {
			data->is_safe = 1;

and this part said "If not absolute, that is an error" without
anything about dot.

But then I changed my mind and made it unsafe to do this:

	cd .git/refs && git -c safe.directory=. foo

as safe.directory=. means "A repository at the current directory of
the process is allowed" and the repository in this case is not at "."
but at "..", meaning "." is a lot stricter than "*".