Starter project - version of 'globs/rglobs' that uses regular python regular expression syntax

[ericzundel]

'rglobs()' just barely works, you can't specify a complex expression (like a prefix before the '*') and expect it to work, see twitter-archive/commons#380

Ideally we'd fix the broken underlying function but I've tried it and its not simple! See https://rbcommons.com/s/twitter/r/2351/

But I was thinking that it should be pretty easy to make a new Fileset type that respected python regular expression syntax. Then you could be reasonably certain that any type of expression would work. You could make a version of globs:

a_target(name='foo',
  sources=search(r'<regexp>'),
...
)

and a version of rglobs:

a_target(name='foo',
  sources=rsearch(r'<regexp>'), 
  ...
)

[jsirois]

Does zglobs do what you want with prefixes?

[stuhood]

Also, please note that we've completely re-implemented these for the new engine, and will be trying to switch the implementation over very shortly.

See https://github.com/pantsbuild/pants/blob/master/tests/python/pants_test/engine/exp/test_fs.py#L61

[ericzundel]

@stuhood zglobs wouldn't help, it is powered by the same broken fnmatch_translate_extended() method.

I think this may still be broken in the new engine. I created an RB to demonstrate it:

https://rbcommons.com/s/twitter/r/3840/

[stuhood]

@ericzundel : Thanks! @JieGhost noticed that during his most recent review; it is fixed there: https://rbcommons.com/s/twitter/r/3828/

[ericzundel]

Terriffic! So you are planning to cherry pick this into the 1.0.x branch?

[stuhood]

No, this will definitely not go in the 1.0.x branch. The daemon might make
it into 1.1.x, and it should definitely be in 1.2.x. Even then, it will
likely be experimental for a while.
On May 8, 2016 2:40 AM, "Eric Ayers" [email protected] wrote:

Terriffic! So you are planning to cherry pick this into the 1.0.x branch?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#3341 (comment)

[kwlzn]

for posterity sake:

I (misguidedly) took a whack at a glob->regex translator as part of converting --pants-ignore values to watchman file events (which consume PCRE regex). the implementation I came up with seems simpler and more idiomatic than others I've (since) seen, so I thought I'd share that here:

master...twitter:kwlzn/pantsd/watchman_ignore#diff-25fffac73b9eb494eb3f5cb839ebfc91R43

with a basic passing test suite here:

master...twitter:kwlzn/pantsd/watchman_ignore#diff-d1037a3870180a48a65be900f0aba836R44

the spec itself may need a little more work, but I think we could easily get this to our own custom, ~perfect version of fnmatch.translate which would let us eventually inline the 3rdparty dep on pathspec.

[ericzundel]

Fixing up the fnmatch.translate would be nice if we are going to stick with
the legacy implementation much longer. I don't think we (Square) needs to
get it into the 1.0.x branch though. Will it make it into the 1.1.x branch?

On Thu, May 26, 2016 at 12:42 AM Kris Wilson [email protected]
wrote:

for posterity sake:

I (misguidedly) took a whack at a glob->regex translator as part of
converting --pants-ignore values to watchman file events (which consume
PCRE regex). the implementation I came up with seems simpler and more
idiomatic than others I've (since) seen, so I thought I'd share that here:

master...twitter:kwlzn/pantsd/watchman_ignore
#diff-25fffac73b9eb494eb3f5cb839ebfc91R43
master...twitter:kwlzn/pantsd/watchman_ignore#diff-25fffac73b9eb494eb3f5cb839ebfc91R43

with a basic passing test suite here:

master...twitter:kwlzn/pantsd/watchman_ignore
#diff-d1037a3870180a48a65be900f0aba836R44
master...twitter:kwlzn/pantsd/watchman_ignore#diff-d1037a3870180a48a65be900f0aba836R44

the spec itself may need a little more work, but I think we could easily
get this to our own custom, ~perfect version of fnmatch.translate which
would let us eventually inline the 3rdparty dep on pathspec.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#3341 (comment)

[stuhood]

The implementation of globs has been completely overhauled and normalized in the v2 engine (now the default in master), and it no longer uses twitter.commons at all.

Conversion from rglobs (and zglobs) to globs happens here, and then globs have a normalized impl on the rust side.