From 27d9b2c5c45e67a75d6bc9ed611859e14ae6e854 Mon Sep 17 00:00:00 2001 From: codealchemy Date: Thu, 21 Feb 2019 13:37:29 -0800 Subject: [PATCH 1/8] Add subsystem for filtering targets by tasks This subsystem is responsible for handling options meant to exclude targets from specific tasks The application of the logic itself is contained in the TargetFiltering class - which currently only handles excluding targets with provided tags and can be expanded upon for additional filtering options. --- .../build_graph/target_filter_subsystem.py | 41 +++++++++++++++++ tests/python/pants_test/build_graph/BUILD | 10 +++++ .../test_target_filter_subsystem.py | 45 +++++++++++++++++++ 3 files changed, 96 insertions(+) create mode 100644 src/python/pants/build_graph/target_filter_subsystem.py create mode 100644 tests/python/pants_test/build_graph/test_target_filter_subsystem.py diff --git a/src/python/pants/build_graph/target_filter_subsystem.py b/src/python/pants/build_graph/target_filter_subsystem.py new file mode 100644 index 00000000000..541d5f5c7fd --- /dev/null +++ b/src/python/pants/build_graph/target_filter_subsystem.py @@ -0,0 +1,41 @@ +# coding=utf-8 +# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +from __future__ import absolute_import, division, print_function, unicode_literals + +import logging + +from pants.subsystem.subsystem import Subsystem + + +logger = logging.getLogger(__name__) + + +class TargetFilter(Subsystem): + """Filter out targets matching given options. + + :API: public + """ + + options_scope = 'target-filter' + + @classmethod + def register_options(cls, register): + super(TargetFilter, cls).register_options(register) + + register('--exclude-tags', type=list, + default=[], + help='Skip targets with given tag(s).') + + def apply(self, targets): + exclude_tags = set(self.get_options().exclude_tags) + return TargetFiltering.apply_tag_blacklist(exclude_tags, targets) + + +class TargetFiltering(object): + """Apply filtering logic against targets.""" + + @staticmethod + def apply_tag_blacklist(exclude_tags, targets): + return [t for t in targets if not exclude_tags.intersection(t.tags)] diff --git a/tests/python/pants_test/build_graph/BUILD b/tests/python/pants_test/build_graph/BUILD index 534c499179f..047ebcd3edc 100644 --- a/tests/python/pants_test/build_graph/BUILD +++ b/tests/python/pants_test/build_graph/BUILD @@ -146,3 +146,13 @@ python_tests( 'tests/python/pants_test:test_base', ] ) + +python_tests( + name = 'target_filter_subsystem', + sources = ['test_target_filter_subsystem.py'], + dependencies = [ + '3rdparty/python:future', + 'src/python/pants/build_graph', + 'tests/python/pants_test:task_test_base', + ] +) diff --git a/tests/python/pants_test/build_graph/test_target_filter_subsystem.py b/tests/python/pants_test/build_graph/test_target_filter_subsystem.py new file mode 100644 index 00000000000..d6bf071a896 --- /dev/null +++ b/tests/python/pants_test/build_graph/test_target_filter_subsystem.py @@ -0,0 +1,45 @@ +# coding=utf-8 +# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +from __future__ import absolute_import, division, print_function, unicode_literals + +from pants.build_graph.target_filter_subsystem import TargetFiltering +from pants.task.task import Task +from pants_test.task_test_base import TaskTestBase + + +class TestTargetFilter(TaskTestBase): + + class DummyTask(Task): + options_scope = 'dummy' + + def execute(self): pass + + @classmethod + def task_type(cls): + return cls.DummyTask + + def test_filtering_single_tag(self): + a = self.make_target('a', tags=[]) + b = self.make_target('b', tags=['skip-me']) + c = self.make_target('c', tags=['tag1', 'skip-me']) + + filtered_targets = TargetFiltering.apply_tag_blacklist({'skip-me'}, [a, b, c]) + self.assertEqual([a], filtered_targets) + + def test_filtering_multiple_tags(self): + a = self.make_target('a', tags=['tag1', 'skip-me']) + b = self.make_target('b', tags=['tag1', 'tag2', 'skip-me']) + c = self.make_target('c', tags=['tag2']) + + filtered_targets = TargetFiltering.apply_tag_blacklist({'skip-me', 'tag2'}, [a, b, c]) + self.assertEqual([], filtered_targets) + + def test_filtering_no_tags(self): + a = self.make_target('a', tags=['tag1']) + b = self.make_target('b', tags=['tag1', 'tag2']) + c = self.make_target('c', tags=['tag2']) + + filtered_targets = TargetFiltering.apply_tag_blacklist(set(), [a, b, c]) + self.assertEqual([a, b, c], filtered_targets) From 2a4ca6a7d4c087ea9cdbae324c86b4179ddb7ce8 Mon Sep 17 00:00:00 2001 From: codealchemy Date: Thu, 21 Feb 2019 13:51:18 -0800 Subject: [PATCH 2/8] Add TargetFilter as a subsystem dependency of Task Hooks the filtering into the v1 engine --- src/python/pants/task/task.py | 8 +++++--- .../test_target_filter_subsystem.py | 18 ++++++++++++++++-- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/python/pants/task/task.py b/src/python/pants/task/task.py index d7b5e28dc90..4315e2a3552 100644 --- a/src/python/pants/task/task.py +++ b/src/python/pants/task/task.py @@ -18,6 +18,7 @@ from pants.base.worker_pool import Work from pants.cache.artifact_cache import UnreadableArtifact, call_insert, call_use_cached_files from pants.cache.cache_setup import CacheSetup +from pants.build_graph.target_filter_subsystem import TargetFilter from pants.invalidation.build_invalidator import (BuildInvalidator, CacheKeyGenerator, UncacheableCacheKeyGenerator) from pants.invalidation.cache_manager import InvalidationCacheManager, InvalidationCheck @@ -96,7 +97,7 @@ def _compute_stable_name(cls): @classmethod def subsystem_dependencies(cls): return (super(TaskBase, cls).subsystem_dependencies() + - (CacheSetup.scoped(cls), BuildInvalidator.Factory, SourceRootConfig)) + (CacheSetup.scoped(cls), TargetFilter.scoped(cls), BuildInvalidator.Factory, SourceRootConfig)) @classmethod def product_types(cls): @@ -237,8 +238,9 @@ def get_targets(self, predicate=None): :API: public """ - return (self.context.targets(predicate) if self.act_transitively - else list(filter(predicate, self.context.target_roots))) + return TargetFilter.scoped_instance(self).apply( + (self.context.targets(predicate) if self.act_transitively + else list(filter(predicate, self.context.target_roots)))) @memoized_property def workdir(self): diff --git a/tests/python/pants_test/build_graph/test_target_filter_subsystem.py b/tests/python/pants_test/build_graph/test_target_filter_subsystem.py index d6bf071a896..eda5f8399d1 100644 --- a/tests/python/pants_test/build_graph/test_target_filter_subsystem.py +++ b/tests/python/pants_test/build_graph/test_target_filter_subsystem.py @@ -4,7 +4,7 @@ from __future__ import absolute_import, division, print_function, unicode_literals -from pants.build_graph.target_filter_subsystem import TargetFiltering +from pants.build_graph.target_filter_subsystem import TargetFilter, TargetFiltering from pants.task.task import Task from pants_test.task_test_base import TaskTestBase @@ -14,12 +14,26 @@ class TestTargetFilter(TaskTestBase): class DummyTask(Task): options_scope = 'dummy' - def execute(self): pass + def execute(self): + self.context.products.safe_create_data('task_targets', self.get_targets) @classmethod def task_type(cls): return cls.DummyTask + def test_task_execution_with_filter(self): + a = self.make_target('a', tags=['skip-me']) + b = self.make_target('b', dependencies=[a], tags=[]) + + context = self.context(for_task_types=[self.DummyTask], for_subsystems=[TargetFilter], target_roots=[b], options={ + TargetFilter.options_scope: { + 'exclude_tags': ['skip-me'] + } + }) + + self.create_task(context).execute() + self.assertEqual([b], context.products.get_data('task_targets')) + def test_filtering_single_tag(self): a = self.make_target('a', tags=[]) b = self.make_target('b', tags=['skip-me']) From 8c174ce187459737d9459c356cceb543e783d79f Mon Sep 17 00:00:00 2001 From: codealchemy Date: Thu, 21 Feb 2019 14:27:24 -0800 Subject: [PATCH 3/8] Add logging around excluded targets if any are present Show the count as info if any targets are excluded, and show each skipped target in debug mode --- src/python/pants/task/task.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/python/pants/task/task.py b/src/python/pants/task/task.py index 4315e2a3552..a461f6cd202 100644 --- a/src/python/pants/task/task.py +++ b/src/python/pants/task/task.py @@ -238,9 +238,18 @@ def get_targets(self, predicate=None): :API: public """ - return TargetFilter.scoped_instance(self).apply( - (self.context.targets(predicate) if self.act_transitively - else list(filter(predicate, self.context.target_roots)))) + initial_targets = (self.context.targets(predicate) if self.act_transitively + else list(filter(predicate, self.context.target_roots))) + + included_targets = TargetFilter.scoped_instance(self).apply(initial_targets) + excluded_targets = set(initial_targets).difference(included_targets) + + if excluded_targets: + self.context.log.info("{} target(s) excluded".format(len(excluded_targets))) + for target in excluded_targets: + self.context.log.debug("{} excluded".format(target.address.spec)) + + return included_targets @memoized_property def workdir(self): From 496c1fdea7bd2fd0a548ee30abefc94698432131 Mon Sep 17 00:00:00 2001 From: codealchemy Date: Thu, 21 Feb 2019 16:01:58 -0800 Subject: [PATCH 4/8] Lint feedback - import sort order fix --- src/python/pants/task/task.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/pants/task/task.py b/src/python/pants/task/task.py index a461f6cd202..eed4c7d90e6 100644 --- a/src/python/pants/task/task.py +++ b/src/python/pants/task/task.py @@ -16,9 +16,9 @@ from pants.base.exceptions import TaskError from pants.base.worker_pool import Work +from pants.build_graph.target_filter_subsystem import TargetFilter from pants.cache.artifact_cache import UnreadableArtifact, call_insert, call_use_cached_files from pants.cache.cache_setup import CacheSetup -from pants.build_graph.target_filter_subsystem import TargetFilter from pants.invalidation.build_invalidator import (BuildInvalidator, CacheKeyGenerator, UncacheableCacheKeyGenerator) from pants.invalidation.cache_manager import InvalidationCacheManager, InvalidationCheck From 0b37c8ab0d629f9faeb7df6c5680f7950a0f737f Mon Sep 17 00:00:00 2001 From: codealchemy Date: Fri, 22 Feb 2019 09:05:57 -0800 Subject: [PATCH 5/8] Minor tweaks from feedback - Update docstring for filtering class - Fix test dependencies --- src/python/pants/build_graph/target_filter_subsystem.py | 2 +- tests/python/pants_test/build_graph/BUILD | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/python/pants/build_graph/target_filter_subsystem.py b/src/python/pants/build_graph/target_filter_subsystem.py index 541d5f5c7fd..f64b964627e 100644 --- a/src/python/pants/build_graph/target_filter_subsystem.py +++ b/src/python/pants/build_graph/target_filter_subsystem.py @@ -13,7 +13,7 @@ class TargetFilter(Subsystem): - """Filter out targets matching given options. + """Filter targets matching configured criteria. :API: public """ diff --git a/tests/python/pants_test/build_graph/BUILD b/tests/python/pants_test/build_graph/BUILD index 047ebcd3edc..e1fdf849dd4 100644 --- a/tests/python/pants_test/build_graph/BUILD +++ b/tests/python/pants_test/build_graph/BUILD @@ -151,8 +151,8 @@ python_tests( name = 'target_filter_subsystem', sources = ['test_target_filter_subsystem.py'], dependencies = [ - '3rdparty/python:future', 'src/python/pants/build_graph', + 'src/python/pants/task', 'tests/python/pants_test:task_test_base', ] ) From 3e62727289cdce5def9c0d10d3805723ff4bf5f9 Mon Sep 17 00:00:00 2001 From: codealchemy Date: Fri, 22 Feb 2019 09:06:20 -0800 Subject: [PATCH 6/8] Fingerprint exclude-tags option --- src/python/pants/build_graph/target_filter_subsystem.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/pants/build_graph/target_filter_subsystem.py b/src/python/pants/build_graph/target_filter_subsystem.py index f64b964627e..8d326f447a0 100644 --- a/src/python/pants/build_graph/target_filter_subsystem.py +++ b/src/python/pants/build_graph/target_filter_subsystem.py @@ -25,7 +25,7 @@ def register_options(cls, register): super(TargetFilter, cls).register_options(register) register('--exclude-tags', type=list, - default=[], + default=[], fingerprint=True, help='Skip targets with given tag(s).') def apply(self, targets): From b0117f15051505e1812b53560ed16d37fbfcd31e Mon Sep 17 00:00:00 2001 From: codealchemy Date: Fri, 22 Feb 2019 09:15:55 -0800 Subject: [PATCH 7/8] Update filtering class to hold state for targets / tags to exclude --- .../pants/build_graph/target_filter_subsystem.py | 11 +++++++---- .../build_graph/test_target_filter_subsystem.py | 6 +++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/python/pants/build_graph/target_filter_subsystem.py b/src/python/pants/build_graph/target_filter_subsystem.py index 8d326f447a0..15d79e24828 100644 --- a/src/python/pants/build_graph/target_filter_subsystem.py +++ b/src/python/pants/build_graph/target_filter_subsystem.py @@ -30,12 +30,15 @@ def register_options(cls, register): def apply(self, targets): exclude_tags = set(self.get_options().exclude_tags) - return TargetFiltering.apply_tag_blacklist(exclude_tags, targets) + return TargetFiltering(targets, exclude_tags).apply_tag_blacklist() class TargetFiltering(object): """Apply filtering logic against targets.""" - @staticmethod - def apply_tag_blacklist(exclude_tags, targets): - return [t for t in targets if not exclude_tags.intersection(t.tags)] + def __init__(self, targets, exclude_tags): + self.targets = targets + self.exclude_tags = exclude_tags + + def apply_tag_blacklist(self): + return [t for t in self.targets if not self.exclude_tags.intersection(t.tags)] diff --git a/tests/python/pants_test/build_graph/test_target_filter_subsystem.py b/tests/python/pants_test/build_graph/test_target_filter_subsystem.py index eda5f8399d1..81ffcaf064e 100644 --- a/tests/python/pants_test/build_graph/test_target_filter_subsystem.py +++ b/tests/python/pants_test/build_graph/test_target_filter_subsystem.py @@ -39,7 +39,7 @@ def test_filtering_single_tag(self): b = self.make_target('b', tags=['skip-me']) c = self.make_target('c', tags=['tag1', 'skip-me']) - filtered_targets = TargetFiltering.apply_tag_blacklist({'skip-me'}, [a, b, c]) + filtered_targets = TargetFiltering([a, b, c], {'skip-me'}).apply_tag_blacklist() self.assertEqual([a], filtered_targets) def test_filtering_multiple_tags(self): @@ -47,7 +47,7 @@ def test_filtering_multiple_tags(self): b = self.make_target('b', tags=['tag1', 'tag2', 'skip-me']) c = self.make_target('c', tags=['tag2']) - filtered_targets = TargetFiltering.apply_tag_blacklist({'skip-me', 'tag2'}, [a, b, c]) + filtered_targets = TargetFiltering([a, b, c], {'skip-me', 'tag2'}).apply_tag_blacklist() self.assertEqual([], filtered_targets) def test_filtering_no_tags(self): @@ -55,5 +55,5 @@ def test_filtering_no_tags(self): b = self.make_target('b', tags=['tag1', 'tag2']) c = self.make_target('c', tags=['tag2']) - filtered_targets = TargetFiltering.apply_tag_blacklist(set(), [a, b, c]) + filtered_targets = TargetFiltering([a, b, c], set()).apply_tag_blacklist() self.assertEqual([a, b, c], filtered_targets) From 04b6435e36f85f04930e2bef305858ae87622888 Mon Sep 17 00:00:00 2001 From: codealchemy Date: Fri, 22 Feb 2019 11:30:57 -0800 Subject: [PATCH 8/8] Add a few more appropriate imports and dependencies --- src/python/pants/build_graph/target_filter_subsystem.py | 1 + src/python/pants/task/task.py | 2 +- tests/python/pants_test/build_graph/BUILD | 1 + .../pants_test/build_graph/test_target_filter_subsystem.py | 2 ++ 4 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/python/pants/build_graph/target_filter_subsystem.py b/src/python/pants/build_graph/target_filter_subsystem.py index 15d79e24828..af6d4b766fa 100644 --- a/src/python/pants/build_graph/target_filter_subsystem.py +++ b/src/python/pants/build_graph/target_filter_subsystem.py @@ -5,6 +5,7 @@ from __future__ import absolute_import, division, print_function, unicode_literals import logging +from builtins import object, set from pants.subsystem.subsystem import Subsystem diff --git a/src/python/pants/task/task.py b/src/python/pants/task/task.py index eed4c7d90e6..037e917a2e3 100644 --- a/src/python/pants/task/task.py +++ b/src/python/pants/task/task.py @@ -7,7 +7,7 @@ import os import sys from abc import abstractmethod -from builtins import filter, map, object, str, zip +from builtins import filter, map, object, set, str, zip from contextlib import contextmanager from hashlib import sha1 from itertools import repeat diff --git a/tests/python/pants_test/build_graph/BUILD b/tests/python/pants_test/build_graph/BUILD index e1fdf849dd4..e8ab092819e 100644 --- a/tests/python/pants_test/build_graph/BUILD +++ b/tests/python/pants_test/build_graph/BUILD @@ -151,6 +151,7 @@ python_tests( name = 'target_filter_subsystem', sources = ['test_target_filter_subsystem.py'], dependencies = [ + '3rdparty/python:future', 'src/python/pants/build_graph', 'src/python/pants/task', 'tests/python/pants_test:task_test_base', diff --git a/tests/python/pants_test/build_graph/test_target_filter_subsystem.py b/tests/python/pants_test/build_graph/test_target_filter_subsystem.py index 81ffcaf064e..3c9fd41c0d1 100644 --- a/tests/python/pants_test/build_graph/test_target_filter_subsystem.py +++ b/tests/python/pants_test/build_graph/test_target_filter_subsystem.py @@ -4,6 +4,8 @@ from __future__ import absolute_import, division, print_function, unicode_literals +from builtins import set + from pants.build_graph.target_filter_subsystem import TargetFilter, TargetFiltering from pants.task.task import Task from pants_test.task_test_base import TaskTestBase