[engine] Proper implementation of `**` globs in the v2 engine

Review Request #4034 — Created June 29, 2016 and submitted

yujiec
pants
3413, 3589, 3608
pants-reviews
kwlzn, stuhood

We don't have a proper implementation of trailing ** matching in the new engine.
Currently "**" matches 1 or more levels of dirs, while in gitignore syntax and also in zsh spec, "**" should match 0 or more levels of dirs.

For example, we expect "**" behaves like following:
** -> matches everything, recursively.
dir/** -> matches everything under dir, recursively.
**/*.py -> matches every dir recursively, searching for files with a .py extension (even in cwd).

This review does following:
1. implement "**" matching logic as specified above.
2. add test cases in test_fs.py and test_path_globs.py.
3. add some missing dependencies in test BUILD files.

ci green:
https://travis-ci.org/pantsbuild/pants/builds/141203951

  • 0
  • 0
  • 1
  • 0
  • 1
Description From Last Updated
YU
ST
  1. Looks good! Thanks.

  2. src/python/pants/engine/fs.py (Diff revision 1)
     
     

    Please update the pydocs for this.

  3. src/python/pants/engine/fs.py (Diff revision 1)
     
     
     
     
     
     
     

    So it looks like PathDirWildcard.remainders is never actually a list anymore, and is just a literal? That's excellent: please convert remainders to just remainder.

  4. src/python/pants/engine/fs.py (Diff revision 1)
     
     

    no need to wrap in tuple here if you're turning it into a tuple below.

  5. tests/python/pants_test/engine/test_fs.py (Diff revision 1)
     
     
     

    These aren't trailing, technically.

  6. Should probably add a trailing ** case here.

  7. 
      
KW
  1. lgtm w/ a +1 to Stu's feedback and an added comment of my own along the same lines.

  2. src/python/pants/engine/fs.py (Diff revision 1)
     
     
     
     
     

    same comment here as Stu's comment above - don't need the 2x tuple().

    should be able to make path_globs here and above a generator comprehension (which is iterable) like so:

    path_globs = (... for ... in ...)
    return PathGlobs(tuple(chain.from_iterable(path_globs)))
    
  3. 
      
YU
IT
  1. Ship It!
  2. 
      
KW
  1. thanks Yujie! this is in @ c7cd71d25a7a1d1c499a2debc558c95fef352561

    please mark this RB as submitted when you get a chance.

  2. 
      
YU
Review request changed

Status: Closed (submitted)

Change Summary:

in c7cd71d25a7a1d1c499a2debc558c95fef352561. thanks, Kris, Stu and Ity!
Loading...