Skip to content

Commit

Permalink
Fix whitelisting of files in pants_ignore (pantsbuild#4357)
Browse files Browse the repository at this point in the history
### Problem

Currently, files whitelisted (via `!`) in `pants_ignore` are still ignored, which caused a test that used a `.babelrc` file to fail in pantsbuild#4355.

### Solution

Properly handle the `ignore::Match::Whitelisted` case as a file that is _not_ ignored, and add a test for ignore/whitelist directly to `test_fs.py`.

### Result

Fixes pantsbuild#4355.
  • Loading branch information
Stu Hood authored Mar 21, 2017
1 parent 52e2648 commit 645dd3e
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 13 deletions.
1 change: 1 addition & 0 deletions pants.ini
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ backend_packages: +[
pants_ignore: +[
# venv directories under build-support.
'/build-support/*.venv/',
'!.babelrc',
]


Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/engine/subsystem/native_engine_version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
b2d27ac1631410208297c5a6e3912425e1ac945a
1749afda2a8990fd6371be4423dca0f37ad65379
9 changes: 8 additions & 1 deletion src/rust/engine/src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ pub struct PosixFS {
build_root: Dir,
// The pool needs to be reinitialized after a fork, so it is protected by a lock.
pool: RwLock<CpuPool>,
pub ignore: Gitignore,
ignore: Gitignore,
}

impl PosixFS {
Expand Down Expand Up @@ -354,6 +354,13 @@ impl PosixFS {
*pool = PosixFS::create_pool();
}

pub fn ignore<P: AsRef<Path>>(&self, path: P, is_dir: bool) -> bool {
match self.ignore.matched(path, is_dir) {
ignore::Match::None | ignore::Match::Whitelist(_) => false,
ignore::Match::Ignore(_) => true,
}
}

pub fn read_link(&self, link: &Link) -> BoxFuture<PathBuf, io::Error> {
let link_parent = link.0.parent().map(|p| p.to_owned());
let link_abs = self.build_root.0.join(link.0.as_path()).to_owned();
Expand Down
2 changes: 1 addition & 1 deletion src/rust/engine/src/nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ impl VFS<Failure> for Context {
}

fn ignore<P: AsRef<Path>>(&self, path: P, is_dir: bool) -> bool {
!self.core.vfs.ignore.matched(path, is_dir).is_none()
self.core.vfs.ignore(path, is_dir)
}

fn mk_error(msg: &str) -> Failure {
Expand Down
4 changes: 2 additions & 2 deletions tests/python/pants_test/engine/scheduler_test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class SchedulerTestBase(object):

_native = init_native()

def mk_fs_tree(self, build_root_src=None):
def mk_fs_tree(self, build_root_src=None, ignore_patterns=None):
"""Create a temporary FilesystemProjectTree.
:param build_root_src: Optional directory to pre-populate from; otherwise, empty.
Expand All @@ -46,7 +46,7 @@ def mk_fs_tree(self, build_root_src=None):
shutil.copytree(build_root_src, build_root, symlinks=True)
else:
os.mkdir(build_root)
return FileSystemProjectTree(build_root)
return FileSystemProjectTree(build_root, ignore_patterns=ignore_patterns)

def mk_scheduler(self,
tasks=None,
Expand Down
36 changes: 28 additions & 8 deletions tests/python/pants_test/engine/test_fs.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ class FSTest(unittest.TestCase, SchedulerTestBase, AbstractClass):
_original_src = os.path.join(os.path.dirname(__file__), 'examples/fs_test/fs_test.tar')

@contextmanager
def mk_project_tree(self):
def mk_project_tree(self, ignore_patterns=None):
"""Construct a ProjectTree for the given src path."""
project_tree = self.mk_fs_tree()
project_tree = self.mk_fs_tree(ignore_patterns=ignore_patterns)
with tarfile.open(self._original_src) as tar:
tar.extractall(project_tree.build_root)
yield project_tree
Expand All @@ -40,14 +40,14 @@ def mk_project_tree(self):
def specs(relative_to, *filespecs):
return PathGlobs.create(relative_to, include=filespecs)

def assert_walk_dirs(self, filespecs, paths):
self.assert_walk_snapshot('dirs', filespecs, paths)
def assert_walk_dirs(self, filespecs, paths, ignore_patterns=None):
self.assert_walk_snapshot('dirs', filespecs, paths, ignore_patterns=ignore_patterns)

def assert_walk_files(self, filespecs, paths):
self.assert_walk_snapshot('files', filespecs, paths)
def assert_walk_files(self, filespecs, paths, ignore_patterns=None):
self.assert_walk_snapshot('files', filespecs, paths, ignore_patterns=ignore_patterns)

def assert_walk_snapshot(self, field, filespecs, paths):
with self.mk_project_tree() as project_tree:
def assert_walk_snapshot(self, field, filespecs, paths, ignore_patterns=None):
with self.mk_project_tree(ignore_patterns=ignore_patterns) as project_tree:
scheduler = self.mk_scheduler(project_tree=project_tree)
result = self.execute(scheduler, Snapshot, self.specs('', *filespecs))[0]
self.assertEquals(sorted([p.path for p in getattr(result, field)]), sorted(paths))
Expand Down Expand Up @@ -136,6 +136,26 @@ def test_walk_recursive_all(self):
'd.ln/b/1.txt',
'd.ln/b/2'])

def test_walk_ignore(self):
# Ignore '*.ln' suffixed items at the root.
self.assert_walk_files(['**'],
['4.txt',
'a/3.txt',
'a/4.txt.ln',
'a/b/1.txt',
'a/b/2',],
ignore_patterns=['/*.ln'])
# Whitelist one entry.
self.assert_walk_files(['**'],
['4.txt',
'a/3.txt',
'a/4.txt.ln',
'a/b/1.txt',
'a/b/2',
'c.ln/1.txt',
'c.ln/2',],
ignore_patterns=['/*.ln', '!c.ln'])

def test_walk_recursive_trailing_doublestar(self):
self.assert_walk_files(['a/**'], ['a/3.txt',
'a/4.txt.ln',
Expand Down

0 comments on commit 645dd3e

Please sign in to comment.