From 07b2ce007a57fd621b15a0be58249c64f87f2a65 Mon Sep 17 00:00:00 2001 From: Yi Cheng Date: Sat, 15 Dec 2018 15:54:21 -0800 Subject: [PATCH 1/9] reest --- .../pants/contrib/googlejavaformat/googlejavaformat.py | 1 + src/python/pants/backend/jvm/tasks/rewrite_base.py | 4 +++- src/python/pants/task/target_restriction_mixins.py | 7 +++++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/contrib/googlejavaformat/src/python/pants/contrib/googlejavaformat/googlejavaformat.py b/contrib/googlejavaformat/src/python/pants/contrib/googlejavaformat/googlejavaformat.py index fa15cf0b042..05201fbb56c 100644 --- a/contrib/googlejavaformat/src/python/pants/contrib/googlejavaformat/googlejavaformat.py +++ b/contrib/googlejavaformat/src/python/pants/contrib/googlejavaformat/googlejavaformat.py @@ -11,6 +11,7 @@ from pants.java.jar.jar_dependency import JarDependency from pants.task.fmt_task_mixin import FmtTaskMixin from pants.task.lint_task_mixin import LintTaskMixin +from pants.task.target_restriction_mixins import HasSkipByTargetTagMixin class GoogleJavaFormatBase(RewriteBase): diff --git a/src/python/pants/backend/jvm/tasks/rewrite_base.py b/src/python/pants/backend/jvm/tasks/rewrite_base.py index 7ae52a42475..fab24cca620 100644 --- a/src/python/pants/backend/jvm/tasks/rewrite_base.py +++ b/src/python/pants/backend/jvm/tasks/rewrite_base.py @@ -13,12 +13,13 @@ from pants.base.exceptions import TaskError from pants.option.custom_types import dir_option from pants.process.xargs import Xargs +from pants.task.target_restriction_mixins import HasSkipByTargetTagMixin from pants.util.dirutil import fast_relpath, safe_mkdir_for_all from pants.util.memo import memoized_property from pants.util.meta import AbstractClass -class RewriteBase(NailgunTask, AbstractClass): +class RewriteBase(NailgunTask, AbstractClass, HasSkipByTargetTagMixin): """Abstract base class for JVM-based tools that check/rewrite sources.""" @classmethod @@ -57,6 +58,7 @@ def cache_target_dirs(self): def execute(self): """Runs the tool on all source files that are located.""" + self.filter_by_tag() relevant_targets = self._get_non_synthetic_targets(self.get_targets()) if self.sideeffecting: diff --git a/src/python/pants/task/target_restriction_mixins.py b/src/python/pants/task/target_restriction_mixins.py index 87c4018989c..487dba5af03 100644 --- a/src/python/pants/task/target_restriction_mixins.py +++ b/src/python/pants/task/target_restriction_mixins.py @@ -41,6 +41,13 @@ def register_options(cls, register): "If true, act on the transitive dependency closure of those targets.") +class HasSkipByTargetTagMixin(HasTransitiveOptionMixin): + + def filter_by_tag(self): + + print(self.get_targets()) + + class HasSkipOptionMixin(object): """A mixin for tasks that have a --skip option. From 8dc895cefd77b87f86af16c103bb9f42a05583e1 Mon Sep 17 00:00:00 2001 From: Yi Cheng Date: Sun, 16 Dec 2018 18:13:14 -0800 Subject: [PATCH 2/9] reset --- .../googlejavaformat/googlejavaformat.py | 1 - .../org/pantsbuild/example/hello/greet/BUILD | 1 + .../pants/backend/jvm/tasks/rewrite_base.py | 14 ++++++++--- src/python/pants/engine/round_engine.py | 4 +-- .../pants/task/target_restriction_mixins.py | 12 ++++----- src/python/pants/task/task.py | 25 +++++++++++++++++-- 6 files changed, 42 insertions(+), 15 deletions(-) diff --git a/contrib/googlejavaformat/src/python/pants/contrib/googlejavaformat/googlejavaformat.py b/contrib/googlejavaformat/src/python/pants/contrib/googlejavaformat/googlejavaformat.py index 05201fbb56c..fa15cf0b042 100644 --- a/contrib/googlejavaformat/src/python/pants/contrib/googlejavaformat/googlejavaformat.py +++ b/contrib/googlejavaformat/src/python/pants/contrib/googlejavaformat/googlejavaformat.py @@ -11,7 +11,6 @@ from pants.java.jar.jar_dependency import JarDependency from pants.task.fmt_task_mixin import FmtTaskMixin from pants.task.lint_task_mixin import LintTaskMixin -from pants.task.target_restriction_mixins import HasSkipByTargetTagMixin class GoogleJavaFormatBase(RewriteBase): diff --git a/examples/tests/java/org/pantsbuild/example/hello/greet/BUILD b/examples/tests/java/org/pantsbuild/example/hello/greet/BUILD index 25a7d05cbf5..d760fe3f33f 100644 --- a/examples/tests/java/org/pantsbuild/example/hello/greet/BUILD +++ b/examples/tests/java/org/pantsbuild/example/hello/greet/BUILD @@ -8,4 +8,5 @@ junit_tests( 'examples/src/java/org/pantsbuild/example/hello/greet', 'examples/src/resources/org/pantsbuild/example/hello', ], + tags = {'no-lint.google-java-format.skip'} ) diff --git a/src/python/pants/backend/jvm/tasks/rewrite_base.py b/src/python/pants/backend/jvm/tasks/rewrite_base.py index fab24cca620..399cf96b45a 100644 --- a/src/python/pants/backend/jvm/tasks/rewrite_base.py +++ b/src/python/pants/backend/jvm/tasks/rewrite_base.py @@ -13,13 +13,12 @@ from pants.base.exceptions import TaskError from pants.option.custom_types import dir_option from pants.process.xargs import Xargs -from pants.task.target_restriction_mixins import HasSkipByTargetTagMixin from pants.util.dirutil import fast_relpath, safe_mkdir_for_all from pants.util.memo import memoized_property from pants.util.meta import AbstractClass -class RewriteBase(NailgunTask, AbstractClass, HasSkipByTargetTagMixin): +class RewriteBase(NailgunTask, AbstractClass): """Abstract base class for JVM-based tools that check/rewrite sources.""" @classmethod @@ -58,9 +57,18 @@ def cache_target_dirs(self): def execute(self): """Runs the tool on all source files that are located.""" - self.filter_by_tag() relevant_targets = self._get_non_synthetic_targets(self.get_targets()) + # force_run_tag = 'no-{}.skip'.format(self.options_scope) + # force_run_targets = set(t for t in relevant_targets if force_run_tag in t.tags) + # force_skip_tag = '{}.skip'.format(self.options_scope) + # force_skip_targets = set(t for t in relevant_targets if force_skip_tag in t.tags) + # + # final_targets = set(relevant_targets).union(force_run_targets).difference(force_skip_targets) + # + # self.context.log.debug("Force skipping targets by tag: {}\n{}".format(force_skip_tag, force_skip_targets)) + # self.context.log.debug("Force running targets by tag: {}\n{}".format(force_run_tag, force_run_targets)) + if self.sideeffecting: # Always execute sideeffecting tasks without invalidation. self._execute_for(relevant_targets) diff --git a/src/python/pants/engine/round_engine.py b/src/python/pants/engine/round_engine.py index 95e6de80622..8c1fb66f4ce 100644 --- a/src/python/pants/engine/round_engine.py +++ b/src/python/pants/engine/round_engine.py @@ -43,8 +43,8 @@ def attempt(self, explain): with self._context.new_workunit(name=name, labels=[WorkUnitLabel.TASK], log_config=log_config): if explain: self._context.log.debug('Skipping execution of {} in explain mode'.format(name)) - elif task.skip_execution: - self._context.log.info('Skipping {}'.format(name)) + # elif task.skip_execution: + # self._context.log.info('Skipping {}'.format(name)) else: task.execute() diff --git a/src/python/pants/task/target_restriction_mixins.py b/src/python/pants/task/target_restriction_mixins.py index 487dba5af03..bd8a4e85e96 100644 --- a/src/python/pants/task/target_restriction_mixins.py +++ b/src/python/pants/task/target_restriction_mixins.py @@ -41,13 +41,6 @@ def register_options(cls, register): "If true, act on the transitive dependency closure of those targets.") -class HasSkipByTargetTagMixin(HasTransitiveOptionMixin): - - def filter_by_tag(self): - - print(self.get_targets()) - - class HasSkipOptionMixin(object): """A mixin for tasks that have a --skip option. @@ -62,6 +55,11 @@ class HasSkipOptionMixin(object): def skip_execution(self): return self.get_options().skip + @property + def supports_skipping_target_by_tag(self): + return True + + class SkipOptionRegistrar(object): """Registrar of --skip.""" diff --git a/src/python/pants/task/task.py b/src/python/pants/task/task.py index f588a571a19..7f4d755eaec 100644 --- a/src/python/pants/task/task.py +++ b/src/python/pants/task/task.py @@ -222,6 +222,10 @@ def act_transitively(self): """ return True + @property + def supports_skipping_target_by_tag(self): + return False + def get_targets(self, predicate=None): """Returns the candidate targets this task should act on. @@ -237,8 +241,25 @@ 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))) + relevant_targets = self.context.targets(predicate) if self.act_transitively else list( + filter(predicate, self.context.target_roots)) + if not self.supports_skipping_target_by_tag: + return relevant_targets + + force_run_tag = 'no-{}.skip'.format(self.options_scope) + force_run_targets = set(t for t in relevant_targets if force_run_tag in t.tags) + + force_skip_tag = '{}.skip'.format(self.options_scope) + force_skip_targets = set(t for t in relevant_targets if force_skip_tag in t.tags) + + if force_run_targets: + self.context.log.debug("Force skipping targets by tag: {}\n{}".format(force_skip_tag, force_skip_targets)) + if force_skip_targets: + self.context.log.debug("Force running targets by tag: {}\n{}".format(force_run_tag, force_run_targets)) + + final_targets = set(relevant_targets).union(force_run_targets).difference(force_skip_targets) + + return final_targets @memoized_property def workdir(self): From e402adc4c84e0f65e477abd6067b9ebf54f5d7a0 Mon Sep 17 00:00:00 2001 From: Yi Cheng Date: Sun, 16 Dec 2018 18:47:10 -0800 Subject: [PATCH 3/9] force skip/run by tag --- .../pants/backend/jvm/tasks/rewrite_base.py | 10 -------- src/python/pants/engine/round_engine.py | 2 -- .../pants/task/target_restriction_mixins.py | 1 - src/python/pants/task/task.py | 24 +++++++++++++------ 4 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/python/pants/backend/jvm/tasks/rewrite_base.py b/src/python/pants/backend/jvm/tasks/rewrite_base.py index 399cf96b45a..7ae52a42475 100644 --- a/src/python/pants/backend/jvm/tasks/rewrite_base.py +++ b/src/python/pants/backend/jvm/tasks/rewrite_base.py @@ -59,16 +59,6 @@ def execute(self): """Runs the tool on all source files that are located.""" relevant_targets = self._get_non_synthetic_targets(self.get_targets()) - # force_run_tag = 'no-{}.skip'.format(self.options_scope) - # force_run_targets = set(t for t in relevant_targets if force_run_tag in t.tags) - # force_skip_tag = '{}.skip'.format(self.options_scope) - # force_skip_targets = set(t for t in relevant_targets if force_skip_tag in t.tags) - # - # final_targets = set(relevant_targets).union(force_run_targets).difference(force_skip_targets) - # - # self.context.log.debug("Force skipping targets by tag: {}\n{}".format(force_skip_tag, force_skip_targets)) - # self.context.log.debug("Force running targets by tag: {}\n{}".format(force_run_tag, force_run_targets)) - if self.sideeffecting: # Always execute sideeffecting tasks without invalidation. self._execute_for(relevant_targets) diff --git a/src/python/pants/engine/round_engine.py b/src/python/pants/engine/round_engine.py index 8c1fb66f4ce..dc340432a4d 100644 --- a/src/python/pants/engine/round_engine.py +++ b/src/python/pants/engine/round_engine.py @@ -43,8 +43,6 @@ def attempt(self, explain): with self._context.new_workunit(name=name, labels=[WorkUnitLabel.TASK], log_config=log_config): if explain: self._context.log.debug('Skipping execution of {} in explain mode'.format(name)) - # elif task.skip_execution: - # self._context.log.info('Skipping {}'.format(name)) else: task.execute() diff --git a/src/python/pants/task/target_restriction_mixins.py b/src/python/pants/task/target_restriction_mixins.py index bd8a4e85e96..774c84091e0 100644 --- a/src/python/pants/task/target_restriction_mixins.py +++ b/src/python/pants/task/target_restriction_mixins.py @@ -60,7 +60,6 @@ def supports_skipping_target_by_tag(self): return True - class SkipOptionRegistrar(object): """Registrar of --skip.""" diff --git a/src/python/pants/task/task.py b/src/python/pants/task/task.py index 7f4d755eaec..0a56a20002b 100644 --- a/src/python/pants/task/task.py +++ b/src/python/pants/task/task.py @@ -210,6 +210,15 @@ def skip_execution(self): """ return False + @property + def supports_skipping_target_by_tag(self): + """ + Whether this task supports forced skip/non-skip on targets based on tags. + + :API: public + """ + return False + @property def act_transitively(self): """Whether this task should act on the transitive closure of the target roots. @@ -222,10 +231,6 @@ def act_transitively(self): """ return True - @property - def supports_skipping_target_by_tag(self): - return False - def get_targets(self, predicate=None): """Returns the candidate targets this task should act on. @@ -249,15 +254,20 @@ def get_targets(self, predicate=None): force_run_tag = 'no-{}.skip'.format(self.options_scope) force_run_targets = set(t for t in relevant_targets if force_run_tag in t.tags) + if not force_run_targets and self.skip_execution: + self.context.log.info('Skipping by returning empty targets ' + 'because task level is skipped and there is no forced run targets') + return [] + force_skip_tag = '{}.skip'.format(self.options_scope) force_skip_targets = set(t for t in relevant_targets if force_skip_tag in t.tags) - if force_run_targets: - self.context.log.debug("Force skipping targets by tag: {}\n{}".format(force_skip_tag, force_skip_targets)) if force_skip_targets: + self.context.log.debug("Force skipping targets by tag: {}\n{}".format(force_skip_tag, force_skip_targets)) + if force_run_targets: self.context.log.debug("Force running targets by tag: {}\n{}".format(force_run_tag, force_run_targets)) - final_targets = set(relevant_targets).union(force_run_targets).difference(force_skip_targets) + final_targets = list(set(relevant_targets).union(force_run_targets).difference(force_skip_targets)) return final_targets From 1911841441d6ec87b80662d87b9d44f5c618b892 Mon Sep 17 00:00:00 2001 From: Yi Cheng Date: Mon, 24 Dec 2018 18:46:27 -0800 Subject: [PATCH 4/9] fix tests --- tests/python/pants_test/task/echo_plugin/register.py | 6 ++++-- .../task/test_goal_options_mixin_integration.py | 12 ++++++------ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/tests/python/pants_test/task/echo_plugin/register.py b/tests/python/pants_test/task/echo_plugin/register.py index 27adc2ffb6f..4e964b86295 100644 --- a/tests/python/pants_test/task/echo_plugin/register.py +++ b/tests/python/pants_test/task/echo_plugin/register.py @@ -19,8 +19,10 @@ class EchoTaskBase(HasSkipAndTransitiveGoalOptionsMixin, Task): prefix = None def execute(self): - with open(os.path.join(self.workdir, 'output'), 'w') as fp: - fp.write('\n'.join(t.address.spec for t in self.get_targets())) + relevant_targets = self.get_targets() + if relevant_targets: + with open(os.path.join(self.workdir, 'output'), 'w') as fp: + fp.write('\n'.join(t.address.spec for t in relevant_targets)) class EchoOne(EchoTaskBase): diff --git a/tests/python/pants_test/task/test_goal_options_mixin_integration.py b/tests/python/pants_test/task/test_goal_options_mixin_integration.py index afd53f643c1..0aadb06d7d2 100644 --- a/tests/python/pants_test/task/test_goal_options_mixin_integration.py +++ b/tests/python/pants_test/task/test_goal_options_mixin_integration.py @@ -43,21 +43,21 @@ def get_echo(which): return None else: with open(path, 'r') as fp: - return [os.path.basename(x.strip()) for x in fp.readlines()] + return {os.path.basename(x.strip()) for x in fp.readlines()} self.assertEqual(expected_one, get_echo('one')) self.assertEqual(expected_two, get_echo('two')) def test_defaults(self): - self._do_test_goal_options([], ['foo:a', 'foo:b'], ['foo:a', 'foo:b']) + self._do_test_goal_options([], {'foo:a', 'foo:b'}, {'foo:a', 'foo:b'}) def test_set_at_goal_level(self): self._do_test_goal_options(['--skip'], None, None) def test_set_at_task_level(self): - self._do_test_goal_options(['--echo-one-skip'], None, ['foo:a', 'foo:b']) - self._do_test_goal_options(['--no-echo-two-transitive'], ['foo:a', 'foo:b'], ['foo:a']) + self._do_test_goal_options(['--echo-one-skip'], None, {'foo:a', 'foo:b'}) + self._do_test_goal_options(['--no-echo-two-transitive'], {'foo:a', 'foo:b'}, {'foo:a'}) def test_set_at_goal_and_task_level(self): - self._do_test_goal_options(['--skip', '--no-echo-one-skip'], ['foo:a', 'foo:b'], None) + self._do_test_goal_options(['--skip', '--no-echo-one-skip'], {'foo:a', 'foo:b'}, None) self._do_test_goal_options(['--no-transitive', '--echo-two-transitive'], - ['foo:a'], ['foo:a', 'foo:b']) + {'foo:a'}, {'foo:a', 'foo:b'}) From 2742ca2b36ec5cedcc3751de304377f34ed9ec87 Mon Sep 17 00:00:00 2001 From: Yi Cheng Date: Mon, 24 Dec 2018 19:14:25 -0800 Subject: [PATCH 5/9] linty stuff --- examples/tests/java/org/pantsbuild/example/hello/greet/BUILD | 1 - 1 file changed, 1 deletion(-) diff --git a/examples/tests/java/org/pantsbuild/example/hello/greet/BUILD b/examples/tests/java/org/pantsbuild/example/hello/greet/BUILD index d760fe3f33f..25a7d05cbf5 100644 --- a/examples/tests/java/org/pantsbuild/example/hello/greet/BUILD +++ b/examples/tests/java/org/pantsbuild/example/hello/greet/BUILD @@ -8,5 +8,4 @@ junit_tests( 'examples/src/java/org/pantsbuild/example/hello/greet', 'examples/src/resources/org/pantsbuild/example/hello', ], - tags = {'no-lint.google-java-format.skip'} ) From a6e03ba9c7e002960d7b35ef5d499205d9e0521b Mon Sep 17 00:00:00 2001 From: Yi Cheng Date: Mon, 24 Dec 2018 22:31:44 -0800 Subject: [PATCH 6/9] add tests --- src/python/pants/task/task.py | 13 +++++---- .../test_goal_options_mixin_integration.py | 27 ++++++++++++++----- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/python/pants/task/task.py b/src/python/pants/task/task.py index 0a56a20002b..a7bd01919f6 100644 --- a/src/python/pants/task/task.py +++ b/src/python/pants/task/task.py @@ -213,7 +213,7 @@ def skip_execution(self): @property def supports_skipping_target_by_tag(self): """ - Whether this task supports forced skip/non-skip on targets based on tags. + Whether this task supports forced skip/no-skip on targets based on tags. :API: public """ @@ -254,10 +254,13 @@ def get_targets(self, predicate=None): force_run_tag = 'no-{}.skip'.format(self.options_scope) force_run_targets = set(t for t in relevant_targets if force_run_tag in t.tags) - if not force_run_targets and self.skip_execution: - self.context.log.info('Skipping by returning empty targets ' - 'because task level is skipped and there is no forced run targets') - return [] + if self.skip_execution: + if not force_run_targets: + self.context.log.info('Skipping by returning empty targets ' + 'because task level is skipped and there is no forced run targets') + return [] + else: + return force_run_targets force_skip_tag = '{}.skip'.format(self.options_scope) force_skip_targets = set(t for t in relevant_targets if force_skip_tag in t.tags) diff --git a/tests/python/pants_test/task/test_goal_options_mixin_integration.py b/tests/python/pants_test/task/test_goal_options_mixin_integration.py index 0aadb06d7d2..4863e690c32 100644 --- a/tests/python/pants_test/task/test_goal_options_mixin_integration.py +++ b/tests/python/pants_test/task/test_goal_options_mixin_integration.py @@ -18,15 +18,15 @@ class TestGoalOptionsMixinIntegration(PantsRunIntegrationTest): def hermetic(cls): return True - def _do_test_goal_options(self, flags, expected_one, expected_two): + def _do_test_goal_options(self, flags, expected_one, expected_two, a_tag='{}', b_tag='{}'): with temporary_dir(root_dir=get_buildroot()) as src_dir: foo_dir = os.path.join(src_dir, 'foo') os.mkdir(foo_dir) with open(os.path.join(foo_dir, 'BUILD'), 'w') as fp: fp.write(textwrap.dedent(""" - target(name='a', dependencies=[':b']) - target(name='b') - """)) + target(name='a', dependencies=[':b'], tags={}) + target(name='b', tags={}) + """).format(a_tag, b_tag)) config = { 'GLOBAL': { @@ -44,8 +44,11 @@ def get_echo(which): else: with open(path, 'r') as fp: return {os.path.basename(x.strip()) for x in fp.readlines()} - self.assertEqual(expected_one, get_echo('one')) - self.assertEqual(expected_two, get_echo('two')) + + self.assertEqual(expected_one, get_echo('one'), + 'one expected {}, but got {}'.format(expected_one, get_echo('one'))) + self.assertEqual(expected_two, get_echo('two'), + 'two expected {}, but got {}'.format(expected_two, get_echo('two'))) def test_defaults(self): self._do_test_goal_options([], {'foo:a', 'foo:b'}, {'foo:a', 'foo:b'}) @@ -61,3 +64,15 @@ def test_set_at_goal_and_task_level(self): self._do_test_goal_options(['--skip', '--no-echo-one-skip'], {'foo:a', 'foo:b'}, None) self._do_test_goal_options(['--no-transitive', '--echo-two-transitive'], {'foo:a'}, {'foo:a', 'foo:b'}) + + def test_task_skip_but_target_tag_no_skip(self): + self._do_test_goal_options(['--echo-one-skip', '--echo-two-skip'], + expected_one={'foo:a'}, + expected_two={'foo:a'}, + a_tag='{"no-echo.one.skip", "no-echo.two.skip"}') + + def test_task_no_skip_but_target_tag_skip(self): + self._do_test_goal_options(['--no-echo-one-skip', '--no-echo-two-skip'], + expected_one={'foo:b'}, + expected_two={'foo:b'}, + a_tag='{"echo.one.skip", "echo.two.skip"}') From 57506f2ade69992dde752b4701d5e7675172a61c Mon Sep 17 00:00:00 2001 From: Yi Cheng Date: Mon, 24 Dec 2018 22:37:21 -0800 Subject: [PATCH 7/9] add one more --- .../task/test_goal_options_mixin_integration.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/python/pants_test/task/test_goal_options_mixin_integration.py b/tests/python/pants_test/task/test_goal_options_mixin_integration.py index 4863e690c32..0d6f740e1e9 100644 --- a/tests/python/pants_test/task/test_goal_options_mixin_integration.py +++ b/tests/python/pants_test/task/test_goal_options_mixin_integration.py @@ -71,8 +71,14 @@ def test_task_skip_but_target_tag_no_skip(self): expected_two={'foo:a'}, a_tag='{"no-echo.one.skip", "no-echo.two.skip"}') - def test_task_no_skip_but_target_tag_skip(self): + def test_task_no_skip_but_target_tag_skip_a(self): self._do_test_goal_options(['--no-echo-one-skip', '--no-echo-two-skip'], expected_one={'foo:b'}, expected_two={'foo:b'}, a_tag='{"echo.one.skip", "echo.two.skip"}') + + def test_task_no_skip_but_target_tag_skip_b(self): + self._do_test_goal_options(['--no-echo-one-skip', '--no-echo-two-skip'], + expected_one={'foo:a'}, + expected_two={'foo:a'}, + b_tag='{"echo.one.skip", "echo.two.skip"}') From 3e0f335dfc3040c6aa89a7548462a1695ca5bb56 Mon Sep 17 00:00:00 2001 From: Yi Cheng Date: Mon, 24 Dec 2018 22:46:47 -0800 Subject: [PATCH 8/9] simplify --- src/python/pants/task/task.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/python/pants/task/task.py b/src/python/pants/task/task.py index a7bd01919f6..818b874e2de 100644 --- a/src/python/pants/task/task.py +++ b/src/python/pants/task/task.py @@ -256,11 +256,10 @@ def get_targets(self, predicate=None): if self.skip_execution: if not force_run_targets: - self.context.log.info('Skipping by returning empty targets ' - 'because task level is skipped and there is no forced run targets') - return [] - else: - return force_run_targets + self.context.log.debug('Skipping by returning empty targets ' + 'because task level is skipped and there is no forced run targets') + + return force_run_targets force_skip_tag = '{}.skip'.format(self.options_scope) force_skip_targets = set(t for t in relevant_targets if force_skip_tag in t.tags) From 26bf0230aa4b952da6ee1f89a6b17218c605d204 Mon Sep 17 00:00:00 2001 From: Yi Cheng Date: Tue, 25 Dec 2018 02:28:17 -0800 Subject: [PATCH 9/9] use set literal --- .../task/test_goal_options_mixin_integration.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/python/pants_test/task/test_goal_options_mixin_integration.py b/tests/python/pants_test/task/test_goal_options_mixin_integration.py index 0d6f740e1e9..175828eb722 100644 --- a/tests/python/pants_test/task/test_goal_options_mixin_integration.py +++ b/tests/python/pants_test/task/test_goal_options_mixin_integration.py @@ -18,7 +18,7 @@ class TestGoalOptionsMixinIntegration(PantsRunIntegrationTest): def hermetic(cls): return True - def _do_test_goal_options(self, flags, expected_one, expected_two, a_tag='{}', b_tag='{}'): + def _do_test_goal_options(self, flags, expected_one, expected_two, a_tag=set(), b_tag=set()): with temporary_dir(root_dir=get_buildroot()) as src_dir: foo_dir = os.path.join(src_dir, 'foo') os.mkdir(foo_dir) @@ -69,16 +69,16 @@ def test_task_skip_but_target_tag_no_skip(self): self._do_test_goal_options(['--echo-one-skip', '--echo-two-skip'], expected_one={'foo:a'}, expected_two={'foo:a'}, - a_tag='{"no-echo.one.skip", "no-echo.two.skip"}') + a_tag={"no-echo.one.skip", "no-echo.two.skip"}) def test_task_no_skip_but_target_tag_skip_a(self): self._do_test_goal_options(['--no-echo-one-skip', '--no-echo-two-skip'], expected_one={'foo:b'}, expected_two={'foo:b'}, - a_tag='{"echo.one.skip", "echo.two.skip"}') + a_tag={"echo.one.skip", "echo.two.skip"}) def test_task_no_skip_but_target_tag_skip_b(self): self._do_test_goal_options(['--no-echo-one-skip', '--no-echo-two-skip'], expected_one={'foo:a'}, expected_two={'foo:a'}, - b_tag='{"echo.one.skip", "echo.two.skip"}') + b_tag={"echo.one.skip", "echo.two.skip"})