Skip to content

Conversation

@carlosmn
Copy link
Member

@carlosmn carlosmn commented Dec 5, 2014

git has odd ideas about what is possible to negate

foo
!foo/bar

does not negate foo/bar and

foo/*
!foo/bar
!foo/baz/quux

negates foo/bar but not foo/baz/quux. Reproduce these rules in libgit2.

Given

    top
    !top/foo

in an ignore file, we should not unignore top/foo. This is an
implementation detail of the git code leaking, but that's the behaviour
we should show.

A negation rule can only negate an exact rule it has seen before.
A rule can only negate something which was explicitly mentioned in the
rules before it. Change our parsing to ignore a negative rule which does
not negate something mentioned in the rules above it.

While here, fix a wrong allocator usage. The memory for the match string
comes from pool allocator. We must not free it with the general
allocator. We can instead simply forget the string and it will be
cleaned up.
@carlosmn
Copy link
Member Author

carlosmn commented Dec 5, 2014

I don't like how complex the does_negate_rule() became, but I can't think of a simpler way right now. The match struct it set out for the more generous p_fnpatch() match, which we don't want to do here, so we need to figure out what the original was.

@carlosmn
Copy link
Member Author

carlosmn commented Dec 5, 2014

BTW, this fixes #2579

@carlosmn carlosmn added this to the libgit2 v0.22 milestone Dec 6, 2014
@ethomson
Copy link
Member

ethomson commented Dec 7, 2014

Yowza. That is... unexpected. Thanks for taking care of this.

ethomson added a commit that referenced this pull request Dec 7, 2014
Fix negative ignores withing ignored dirs
@ethomson ethomson merged commit 19ae843 into master Dec 7, 2014
@carlosmn carlosmn deleted the cmn/neg-ignore-dir branch December 7, 2014 01:26
@carlosmn
Copy link
Member Author

carlosmn commented Dec 7, 2014

"unexpected" is one word for it. The leak in this abstraction is about the size of the reservoir.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants