Skip to content

Commit

Permalink
Render a warning rather than failing list when no targets are match…
Browse files Browse the repository at this point in the history
…ed (pantsbuild#5598)

### Problem

The `v2` engine changed the behaviour of `./pants list` (with no specs) from "list everything", to "fail with an error". But due to the deprecation/removal of the `changed` goal, this also has the effect of failing with an error for `./pants --changed=.. list` calls that result in no matched targets, which is a behaviour change from `v1`.

### Solution

Rather than raising an error, trigger a warning to stderr which naturally handles both cases by indicating that their query wasn't an error: it just didn't match anything.
  • Loading branch information
Stu Hood authored and wisechengyi committed Mar 15, 2018
1 parent f8c6010 commit e53154c
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 10 deletions.
4 changes: 3 additions & 1 deletion src/python/pants/backend/graph_info/tasks/listtargets.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,9 @@ def print_documented(target):
print_fn = lambda target: target.address.spec

visited = set()
for target in self.determine_target_roots('list', lambda target: not target.is_synthetic):
for target in self.determine_target_roots('list'):
if target.is_synthetic:
continue
result = print_fn(target)
if result and result not in visited:
visited.add(result)
Expand Down
7 changes: 3 additions & 4 deletions src/python/pants/task/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
unicode_literals, with_statement)

import os
import sys
from abc import abstractmethod
from contextlib import contextmanager
from hashlib import sha1
Expand Down Expand Up @@ -648,15 +649,13 @@ def require_single_root_target(self):
.format(', '.join([repr(t) for t in target_roots])))
return target_roots[0]

def determine_target_roots(self, goal_name, predicate=None):
def determine_target_roots(self, goal_name):
"""Helper for tasks that scan for default target roots.
:param string goal_name: The goal name to use for any warning emissions.
:param callable predicate: The predicate to pass to `context.scan().targets(predicate=X)`.
"""
if not self.context.target_roots:
raise TaskError('Please specify one or more explicit target '
'specs (e.g. `./pants {0} ::`).'.format(goal_name))
print('WARNING: No targets were matched in goal `{}`.'.format(goal_name), file=sys.stderr)

# For the v2 path, e.g. `./pants list` is a functional no-op. This matches the v2 mode behavior
# of e.g. `./pants --changed-parent=HEAD list` (w/ no changes) returning an empty result.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
from pants.backend.jvm.scala_artifact import ScalaArtifact
from pants.backend.jvm.targets.java_library import JavaLibrary
from pants.backend.python.targets.python_library import PythonLibrary
from pants.base.exceptions import TaskError
from pants.build_graph.build_file_aliases import BuildFileAliases
from pants.build_graph.target import Target
from pants_test.tasks.task_test_base import ConsoleTaskTestBase
Expand All @@ -30,8 +29,9 @@ def task_type(cls):
class ListTargetsTestEmpty(BaseListTargetsTest):

def test_list_all_empty(self):
with self.assertRaises(TaskError):
self.assertEqual('', self.execute_task())
# NB: Also renders a warning to stderr, which is challenging to detect here but confirmed in:
# tests/python/pants_test/engine/legacy/test_list_integration.py
self.assertEqual('', self.execute_task())


class ListTargetsTest(BaseListTargetsTest):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ def test_list_all(self):
self.assertGreater(len(pants_run.stdout_data.strip().split()), 1)

def test_list_none(self):
pants_run = self.do_command('list', success=False)
self.assertIn('Please specify one or more explicit target', pants_run.stdout_data)
pants_run = self.do_command('list', success=True)
self.assertIn('WARNING: No targets were matched in', pants_run.stderr_data)

def test_list_invalid_dir(self):
pants_run = self.do_command('list', 'abcde::', success=False)
Expand Down

0 comments on commit e53154c

Please sign in to comment.