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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Nov 15, 2010 at 7:14 PM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote:
> Nguyán ThÃi Ngác Duy wrote:
>
>> 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".

Hmm.. get_index_dtype() would return DT_DIR if the submodule exists in
index. If it does not it must be a directory in worktree, right?
Call flow: excluded_from_list() -> get_dtype() -> get_index_dtype()
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]