Re: [PATCH] Demonstrate breakage: checkout overwrites untracked symlink with directory

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 9, 2011, 23:48 UTC
Message-ID: <7vpqr0n51f.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20110205183351.GA25717@localhost>

Clemens Buchacher <drizzd@aon.at> writes:

Show 13 quoted lines

> diff --git a/symlinks.c b/symlinks.c
> index 3cacebd..034943b 100644
> --- a/symlinks.c
> +++ b/symlinks.c
> @@ -223,7 +223,7 @@ int check_leading_path(const char *name, int len)
>         int flags;
>         int match_len = lstat_cache_matchlen(cache, name, len, &flags,
>                            FL_SYMLINK|FL_NOENT|FL_DIR, USE_ONLY_LSTAT);
> -       if (flags & (FL_SYMLINK|FL_NOENT))
> +       if (flags & FL_NOENT)
>                 return 0;
>         else if (flags & FL_DIR)
>                 return -1;

This function used to be named has-symlink-or-noent-leading-path before f66caaf (do not overwrite files in leading path, 2010-10-09) and was used to check for exactly that condition. For example, verify_absent_1() used it to verify that a path A/B/C/D is not on the filesystem (e.g. in preparation for checking it out) by making sure that none of A, A/B, or A/B/C exists -- or is an untracked symlink. If one of them is a symlink leading elsewhere, even if lstat("A/B/C/D") said the path exists, A/B/C/D is not something we have in the work tree, and we decide that we can check out the path by possibly removing intermediate symbolic link and running mkdir.

Now you are changing the semantics of the function, so that we cannot clobber intermediate symbolic links when we check out a path. It may probably be a good change.

Can we rename this function to fix the naming regression introduced in f66caaf, by the way? "check_leading_path()" is a horrible name for a function that takes some parameters and returns a boolean, as the boolness of the function already says enough that it is about "check", giving the first part of the name 0-bit of information, and the remainder of the name doesn't say much either: what aspect of leading-path is the function about? Should the pathcomponents exist, should they not exist, why should the caller care?

A name that explains for what purpose the caller is expected to call it is probably the best kind of name. As long as the purpose does not change, even though the implementation and the semantics are changed later, the name can stay the same without losing its meaning. The second best kind is a name that explains what it does. The old name of this function was of this kind, until f66caaf renamed it to a meaningless name.

Perhaps can-clobber-to-checkout would be a good candidate.