From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 08 Nov 2010 20:13:35 GMT
Subject: Re: [PATCH] dir.c: fix EXC_FLAG_MUSTBEDIR match in sparse checkout
Message-ID: <7vfwvbpo9c.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1289153098-15684-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Commit c84de70 (excluded_1(): support exclude files in index -
> 2009-08-20) tries to work around the fact that there is no
> directory/file information in index entries, therefore
> EXC_FLAG_MUSTBEDIR match would fail.
>
> Unfortunately the workaround is flawed. This fixes it.
>
> Reported-by: Thomas Rinderknecht <thomasr@sailguy.org>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---

Hmmm...

> diff --git a/dir.c b/dir.c
> index d1e5e5e..b2dfb69 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -360,7 +360,8 @@ int excluded_from_list(const char *pathname,
>  
>  			if (x->flags & EXC_FLAG_MUSTBEDIR) {
>  				if (!dtype) {
> -					if (!prefixcmp(pathname, exclude))
> +					if (!prefixcmp(pathname, exclude) &&
> +					    pathname[x->patternlen] == '/')

- Can pathname be much shorter than x->patternlen (it doesn't matter as
  prefixcmp will return false in that case)?

- Can pathname be equal to exclude (yes it can but in that case pathname
  is not a directory but is a regular file, symlink or a submodule)?

So it may be a tricky code, but I do not think it is a "workaround" in any
way.  Isn't it just a "correct solution"?

By the way, builtin/add.c calls excluded() with DT_UNKNOWN and relies on
the fact that the macro is accidentally defined as 0.  108da0d (git add:
Add the "--ignore-missing" option for the dry run, 2010-07-10).  If it
means NULL, it should spell it out as such.

Thanks.