Re: [PATCH v2 1/4] pathspec: use is_dir_sep() to check for trailing slashes

[Johannes Sixt <j6t@kdbg.org>]

Am 12.09.2013 21:24, schrieb John Keeping:
> This allows us to correctly removing trailing backslashes on Windows
> when checking for submodules.
> 
> Signed-off-by: John Keeping <john@keeping.me.uk>
> ---
>  pathspec.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/pathspec.c b/pathspec.c
> index ad1a9f5..7c6963b 100644
> --- a/pathspec.c
> +++ b/pathspec.c
> @@ -252,7 +252,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
>  	item->prefix = prefixlen;
>  
>  	if ((flags & PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP) &&
> -	    (item->len >= 1 && item->match[item->len - 1] == '/') &&
> +	    (item->len >= 1 && is_dir_sep(item->match[item->len - 1])) &&
>  	    (i = cache_name_pos(item->match, item->len - 1)) >= 0 &&
>  	    S_ISGITLINK(active_cache[i]->ce_mode)) {
>  		item->len--;
> @@ -267,7 +267,8 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
>  			if (!S_ISGITLINK(ce->ce_mode))
>  				continue;
>  
> -			if (item->len <= ce_len || match[ce_len] != '/' ||
> +			if (item->len <= ce_len ||
> +			    !is_dir_sep(match[ce_len]) ||
>  			    memcmp(ce->name, match, ce_len))
>  				continue;
>  			if (item->len == ce_len + 1) {

A design decisions to keep in mind:

Paths in the index *ALWAYS* use the slash, even on Windows. On Windows,
pathspec that are user input must undergo backslash-to-slash
transformation at a very early stage so that later processing that
compares the user input to index contents need not do it on the fly. The
backslash-to-slash transformation used to happen in get_pathspec() via
prefix_path() and normalize_path_copy().

If, at this point, the contents of 'match' is still being parsed for
pathspec magic, then it is likely correct to use is_dir_sep().

On the other hand, if at this point the contents of 'match' are used to
execute pathspec magic, then it is not correct to use is_dir_sep(); the
conversion of backslash to slash should have happened earlier, and no
backslashes should be present anymore.

(Yes, this means that on Windows we cannot escape glob characters
because, e.g., 'a\*.c' was turned into 'a/*.c'.)

-- Hannes