Re: [PATCH 01/10] add: do not rely on dtype being NULL behavior

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Nov 15, 2010, 12:14 UTC
Message-ID: <20101115121415.GB14729@burratino>
In-Reply-To: <1289817410-32470-2-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy wrote:

Show 13 quoted lines

> Commit c84de70 (excluded_1(): support exclude files in index -
> 2009-08-20) added support for excluded() where dtype can be NULL. It
> was designed specifically for index matching because there was no
> other way to extract dtype information from index. It did not support
> wildcard matching (for example, "a*/" pattern would fail to match).
> 
> The code was probably misread when commit 108da0d (git add: Add the
> "--ignore-missing" option for the dry run - 2010-07-10) was made
> because DT_UNKNOWN happens to be zero (NULL) too.
> 
> Do not pass DT_UNKNOWN/NULL to excluded(), instead pass a pointer to a
> variable that contains DT_UNKNOWN. The real dtype will be extracted
> from worktree by excluded(), as expected.

Could you rephrase this in a way that contrasts current and desired behavior? Is it like this?

	The "git add --ignore-missing --dry-run" codepath is
	interpreting .gitignore incorrectly, unlike "git add".  For
	example:

		$ test -e foo || echo missing
		missing
		$ echo foo/ >>.gitignore
		$ mkdir bar
		$ git add --ignore-missing --dry-run foo; echo $?
		The following paths are ignored by one of your .gitignore files:
		foo/
		Use -f if you really want to add them.
		fatal: no files added
		128
		$ git add --ignore-missing --dry-run bar/foo; echo $?
		0

	In the original use case (preparing to add a submodule) the
	behavior of the first command is correct, second incorrect.
	If the entry to be added was a regular file, it would be the
	other way around.

	The cause: the --ignore-missing code passes DT_UNKNOWN as the
	dtype_ptr argument to excluded() which happens to equal zero
	(NULL) and accidentally triggers the "match pathspecs in index
	only" codepath (see c84de70, excluded_1(): support exclude
	files in index, 2009-08-20) that is unfortunately a bit
	primitive.

	Surely what was really wanted is to check paths against the
	index and work tree, defaulting to "regular file".

Wait --- that's not true. In the "git submodule add" case, we really want to default to (or even better, force) "directory".