Skip to content

Commit

Permalink
Fix Single Address Exclude (#6366)
Browse files Browse the repository at this point in the history
### Problem

The git commit hook (and CI lint) will lint targets that own changed files. Transforming specs to single addresses is currently broken and causes a "did you mean error" to be thrown on specs that actually exist in an affected build file:

```
ResolveError: "main_py2" was not found in namespace "testprojects/src/python/interpreter_selection/python_3_selection_testing". Did you mean one of:
          :lib_py2
          :lib_py23
          :lib_py3
          :main_py2
          :main_py23
          :main_py3
          :test_py2
          :test_py23
```
This manifested after trying to make changes to a `testprojects` BUILD file, which is excluded in the CI/commit hook lint step:
`./pants -q --exclude-target-regexp='testprojects/.*' --changed-parent=master lint`

### Solution

Edit build_files.py to aggregate excluded addresses in addition to included ones and check that excluded addresses align with exclude logic so that the "did you mean" error only occurs when the spec actually doesn't exist.

### Result

Changes to files in testprojects will not throw any "did you mean" errors in CI/commit hook lint invoked by the following command:
`./pants -q --exclude-target-regexp='testprojects/.*' --changed-parent=master lint`
  • Loading branch information
CMLivingston authored and Stu Hood committed Aug 20, 2018
1 parent 7fb7c83 commit a03144b
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 2 deletions.
8 changes: 6 additions & 2 deletions src/python/pants/engine/build_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -235,13 +235,17 @@ def filter_for_tag(tag):
include_target = wrap_filters(create_filters(specs.tags if specs.tags else '', filter_for_tag))

addresses = []
addresses_to_exclude = []
included = set()
def include(address_families, predicate=None):
matched = False
for af in address_families:
for (a, t) in af.addressables.items():
if (predicate is None or predicate(a)):
if include_target(t) and (not exclude_address(a.spec)):
should_exclude_address = exclude_address(a.spec)
if should_exclude_address:
addresses_to_exclude.append(a)
if include_target(t) and (not should_exclude_address):
matched = True
if a not in included:
addresses.append(a)
Expand Down Expand Up @@ -270,7 +274,7 @@ def include(address_families, predicate=None):
# spec.name here is generally the root node specified on commandline. equality here implies
# a root node i.e. node specified on commandline.
if not include([address_family], predicate=lambda a: a.target_name == spec.name):
if len(addresses) == 0:
if len(addresses) == 0 and len(addresses_to_exclude) == 0:
_raise_did_you_mean(address_family, spec.name)
elif type(spec) is AscendantAddresses:
include(
Expand Down
18 changes: 18 additions & 0 deletions tests/python/pants_test/engine/test_build_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,24 @@ def test_exclude_pattern(self):
self.assertEqual(len(targets.dependencies), 1)
self.assertEqual(targets.dependencies[0].spec, 'root:not_me')

def test_exclude_pattern_with_single_address(self):
"""Test that single address targets are filtered based on exclude patterns."""
spec = SingleAddress('root', 'not_me')
address_mapper = AddressMapper(JsonParser(TestTable()))
snapshot = Snapshot(DirectoryDigest('xx', 2),
(Path('root/BUILD', File('root/BUILD')),))
address_family = AddressFamily('root',
{
'not_me': ('root/BUILD', TargetAdaptor()),
}
)
targets = run_rule(
addresses_from_address_families, address_mapper, Specs([spec], exclude_patterns=tuple(['root.*'])),{
(Snapshot, PathGlobs): lambda _: snapshot,
(AddressFamily, Dir): lambda _: address_family,
})
self.assertEqual(len(targets.dependencies), 0)


class ApacheThriftConfiguration(StructWithDeps):
# An example of a mixed-mode object - can be directly embedded without a name or else referenced
Expand Down

0 comments on commit a03144b

Please sign in to comment.