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

[Junio C Hamano <gitster@pobox.com>]

Clemens Buchacher <drizzd@aon.at> writes:

> 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.