On Mon, Jul 29, 2024 at 06:10:03PM -0700, Junio C Hamano wrote:
> Two and a half things to note, compared to the previous step to
> normalize the actual path of the suspected repository, are:
>
> - A configured safe.directory may be coming from .gitignore in the
> home directory that may be shared across machines. The path
> meant to match with an entry may not necessarily exist on all of
> such machines, so not being able to convert them to real path on
> this machine is *not* a condition that is worthy of warning.
> Hence, we ignore a path that cannot be converted to a real path.
Thanks, I think this addresses my concern. One small thing I noticed in
the patch:
> @@ -1236,14 +1236,43 @@ static int safe_directory_cb(const char *key, const char *value,
>
> if (!git_config_pathname(&allowed, key, value)) {
> const char *check = allowed ? allowed : value;
> - if (ends_with(check, "/*")) {
> - size_t len = strlen(check);
> - if (!fspathncmp(check, data->path, len - 1))
> + char *to_free = NULL;
> +
> + /*
> + * Setting safe.directory to a non-absolute path
> + * makes little sense---it won't be relative to
> + * the configuration file the item is defined in.
> + * Except for ".", which means "if we are at the top
> + * level of a repository, then it is OK", which is
> + * slightly tighter than "*" that allows discovery.
> + */
> + 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 patch must be non-absolute.
-Peff
