Re: [PATCH 5/5] do not overwrite files in leading path

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 13, 2010, 21:57 UTC
Message-ID: <7v1v7tkb6g.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1286632380-7002-6-git-send-email-drizzd@aon.at>

Clemens Buchacher <drizzd@aon.at> writes:

Show 8 quoted lines

> If the work tree contains an untracked file x, and
> unpack-trees wants to checkout a path x/*, the
> file x is removed unconditionally.
>
> Instead, apply the same checks that are normally
> used for untracked files, and abort if the file
> cannot be removed.
>

Too short a line is also hard to read as too long a line.

Show 10 quoted lines

> diff --git a/cache.h b/cache.h
> index 2ef2fa3..f65d6f9 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -852,7 +852,7 @@ struct cache_def {
>  
>  extern int has_symlink_leading_path(const char *name, int len);
>  extern int threaded_has_symlink_leading_path(struct cache_def *, const char *, int);
> -extern int has_symlink_or_noent_leading_path(const char *name, int len);
> +extern int check_leading_path(const char *name, int len);

This is an API regression. "check" in function name "check-blah" does not tell what you are checking (is it checking if the path is Ok? is it checking if the path has symlinked components? is it checking if some part of the path overlaps with tracked content? is it checking if you have already run lstat(2) on some of the leading path components?), nor for what purpose the check can be used (it is Ok to overwrite it?, is it Ok to create a directory there? is it Ok to create a file there?).

At least the old name told us what it checked, didn't it?

The approach you took looks sound, though. What did you feel was "not exactly pretty" about it?

Thanks.