fix glob_to_regex

[wisechengyi]

1. checkout branch regex_repo from https://github.com/wisechengyi/pants
2. ./pants invalidate compile contrib/scrooge/tests/thrift/org/pantsbuild/contrib/scrooge/scrooge_gen:good-thrift, which will fail because

u'.pants.d/gen/scrooge/9f0b049b44d7/contrib.scrooge.tests.thrift.org.pantsbuild.contrib.scrooge.scrooge_gen.good-thrift/current/all/your/base/thriftscala/MyService$FinagleClient.scala'

is NOT matching with

[u'.pants.d/gen/scrooge/9f0b049b44d7/contrib.scrooge.tests.thrift.org.pantsbuild.contrib.scrooge.scrooge_gen.good-thrift/current/all/your/base/thriftscala/MyService$FinagleClient.scala', 
u'.pants.d/gen/scrooge/9f0b049b44d7/contrib.scrooge.tests.thrift.org.pantsbuild.contrib.scrooge.scrooge_gen.good-thrift/current/all/your/base/thriftscala/MyService$FinagleService.scala', 
u'.pants.d/gen/scrooge/9f0b049b44d7/contrib.scrooge.tests.thrift.org.pantsbuild.contrib.scrooge.scrooge_gen.good-thrift/current/all/your/base/thriftscala/MyService.scala']

in https://github.com/wisechengyi/pants/blob/227aa8c279fa754f0790e405f16839d6920fe990/src/python/pants/source/filespec.py#L11

workaround:
https://github.com/wisechengyi/pants/blob/227aa8c279fa754f0790e405f16839d6920fe990/src/python/pants/source/filespec.py#L49-L50

[stuhood]

Hmmmm. I'm pretty sure we have another implementation of this somewhere? We use https://pypi.python.org/pypi/pathspec for this in ProjectTree, for example. And the v2 engine uses the rust impl from https://crates.io/crates/glob

[wisechengyi]

that's good news. i'll try it out

[stuhood]

Oh, I see what's going on (although I have no idea why we're even trying to match files this way... shouldn't the SourcesField matching be enough?). The implementation needs to escape all regex characters.

[JieGhost]

This was added in 8565b63.
I think this is what sources payload field uses for matching. The old logic uses fnmatch which has issues.

So the issue here is '$' has to be escaped right?
Are there any possible regex chars other than '.' and '$' in filenames?
Or do we want to play safe and exclude everything that is meaningful in regex?

[wisechengyi]

It does seem that escaping dollar sign should do the trick. There are special ones to keep such as **, so it may be good just to do the incremental fix. I'm on it with #4350 (not ready yet)

…

On Mar 21, 2017, at 8:15 AM, JieGhost ***@***.***> wrote: This was added in ***@***.*** I think this is what sources payload field uses for matching. The old logic uses fnmatch which has issues. So the issue here is '$' has to be escaped right? Are there any possible regex chars other than '.' and '$' in filenames? Or do we want to play safe and exclude everything that is meaningful in regex? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[JieGhost]

OK, thanks!

[wisechengyi]

closed with #4350