Skip to content

Commit

Permalink
Allow pants to select targets by file(s) (#5930)
Browse files Browse the repository at this point in the history
Fixes #5912

Problem
There should be an option to accept literal files, and then select the targets that own those files. Similar to how the --changed-parent triggers a diff and the targets are selected based on the result.
The proposed syntax is something like:

$ ./pants \
  --owner-of=this/is/a/file/name.java \    # flag triggers owner lookup
  --owner-of=this/is/a/file/name/too.py \  # flag triggers owner lookup
  compile                                  # goal
Solution
I've created a global option --owner-of= that takes a list of files as a parameter, and created a OwnerCalculator class to handle the logic similar to how ChangeCalculator works with the --changed-* subsystem.

Result
Now users will be able to run goals on files without needing to know which target owns those files.
Also, the list-owners goal can be deprecated in favor of --owner-of=some/file.py list

It is important to note that multiple target selection methods are not allowed, so it fails when more than one of --changed-*, --owner-of, or target specs are supplied.
e.g. this fails:

$ ./pants \
  --owner-of=this/is/a/file/name.java \    # flag triggers owner lookup
  --owner-of=this/is/a/file/name/too.py \  # flag triggers owner lookup
  compile
  <another target>
  • Loading branch information
alanbato authored and kwlzn committed Jun 9, 2018
1 parent 80b658e commit 5971e20
Show file tree
Hide file tree
Showing 6 changed files with 117 additions and 6 deletions.
5 changes: 5 additions & 0 deletions src/python/pants/backend/graph_info/tasks/list_owners.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import json

from pants.base.deprecated import deprecated
from pants.base.exceptions import TaskError
from pants.build_graph.source_mapper import LazySourceMapper
from pants.task.console_task import ConsoleTask
Expand Down Expand Up @@ -51,3 +52,7 @@ def console_output(self, targets):
if owner_info.values():
for address_spec in owner_info.values()[0]:
yield address_spec

@deprecated("1.10.0.dev0", "Run './pants --owner-of=<file> list' instead")
def execute(self):
super(ListOwners, self).execute()
56 changes: 50 additions & 6 deletions src/python/pants/init/target_roots_calculator.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# coding=utf-8
# Copyright 2016 Pants project contributors (see CONTRIBUTORS.md).
# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
Expand Down Expand Up @@ -128,27 +128,45 @@ def create(cls, options, session, symbol_table, build_root=None):
changed_options = options.for_scope('changed')
changed_request = ChangedRequest.from_options(changed_options)

# Determine the `--owner-of=` arguments provided from the global options
owned_files = options.for_global_scope().owner_of

logger.debug('spec_roots are: %s', spec_roots)
logger.debug('changed_request is: %s', changed_request)
logger.debug('owned_files are: %s', owned_files)
scm = get_scm()
change_calculator = ChangeCalculator(session, symbol_table, scm) if scm else None
owner_calculator = OwnerCalculator(session, symbol_table) if owned_files else None
targets_specified = sum(1 for item
in (changed_request.is_actionable(), owned_files, spec_roots)
if item)

if targets_specified > 1:
# We've been provided a more than one of: a change request, an owner request, or spec roots.
raise InvalidSpecConstraint(
'Multiple target selection methods provided. Please use only one of '
'--changed-*, --owner-of, or target specs'
)

if change_calculator and changed_request.is_actionable():
if spec_roots:
# We've been provided spec roots (e.g. `./pants list ::`) AND a changed request. Error out.
raise InvalidSpecConstraint('cannot provide changed parameters and target specs!')

# We've been provided no spec roots (e.g. `./pants list`) AND a changed request. Compute
# alternate target roots.
changed_addresses = change_calculator.changed_target_addresses(changed_request)
logger.debug('changed addresses: %s', changed_addresses)
return TargetRoots(tuple(SingleAddress(a.spec_path, a.target_name) for a in changed_addresses))

if owner_calculator and owned_files:
# We've been provided no spec roots (e.g. `./pants list`) AND a owner request. Compute
# alternate target roots.
owner_addresses = owner_calculator.owner_target_addresses(owned_files)
logger.debug('owner addresses: %s', owner_addresses)
return TargetRoots(tuple(SingleAddress(a.spec_path, a.target_name) for a in owner_addresses))

return TargetRoots(spec_roots)


class ChangeCalculator(object):
"""A ChangeCalculator that find the target addresses of changed files based on scm."""
"""A ChangeCalculator that finds the target addresses of changed files based on scm."""

def __init__(self, scheduler, symbol_table, scm, workspace=None, changes_since=None,
diffspec=None):
Expand Down Expand Up @@ -211,3 +229,29 @@ def iter_changed_target_addresses(self, changed_request):

def changed_target_addresses(self, changed_request):
return list(self.iter_changed_target_addresses(changed_request))


class OwnerCalculator(object):
"""An OwnerCalculator that finds the target addresses of the files passed down as arguments
to --owner-of
"""

def __init__(self, scheduler, symbol_table):
"""
:param scheduler: The `Scheduler` instance to use for computing file to target mapping
:param symbol_table: The symbol table.
"""
self._scheduler = scheduler
self._symbol_table = symbol_table
self._mapper = EngineSourceMapper(self._scheduler)

def iter_owner_target_addresses(self, owned_files):
"""Given an list of owned files, compute and yield all affected target addresses"""
owner_addresses = set(address
for address
in self._mapper.iter_target_addresses_for_sources(owned_files))
for address in owner_addresses:
yield address

def owner_target_addresses(self, owner_request):
return list(self.iter_owner_target_addresses(owner_request))
5 changes: 5 additions & 0 deletions src/python/pants/option/global_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,11 @@ def register_bootstrap_options(cls, register):
register('--subproject-roots', type=list, advanced=True, fromfile=True, default=[],
help='Paths that correspond with build roots for any subproject that this '
'project depends on.')
register('--owner-of', type=list, default=[], metavar='<path>',
help='Select the targets that own these files. '
'This is the third target calculation strategy along with the --changed '
'options and specifying the targets directly. These three types of target '
'selection are mutually exclusive.')

# These logging options are registered in the bootstrap phase so that plugins can log during
# registration and not so that their values can be interpolated in configs.
Expand Down
9 changes: 9 additions & 0 deletions tests/python/pants_test/engine/legacy/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,15 @@ python_tests(
timeout = 600,
)

python_tests(
name = 'owners_integration',
sources = [ 'test_owners_integration.py' ],
dependencies = [
'tests/python/pants_test:int-test',
],
tags = {'integration'},
)

python_tests(
name = 'dependees_integration',
sources = ['test_dependees_integration.py'],
Expand Down
1 change: 1 addition & 0 deletions tests/python/pants_test/engine/legacy/test_graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class GraphTestBase(unittest.TestCase):
def _make_setup_args(self, specs):
options = mock.Mock(target_specs=specs)
options.for_scope.return_value = mock.Mock(diffspec=None, changes_since=None)
options.for_global_scope.return_value = mock.Mock(owner_of=None)
return options

def _default_build_config(self, build_file_aliases=None):
Expand Down
47 changes: 47 additions & 0 deletions tests/python/pants_test/engine/legacy/test_owners_integration.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# coding=utf-8
# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
unicode_literals, with_statement)

from pants_test.pants_run_integration_test import PantsRunIntegrationTest


class ListOwnersIntegrationTest(PantsRunIntegrationTest):
def test_owner_of_success(self):
pants_run = self.do_command('--owner-of=testprojects/tests/python/pants/dummies/test_pass.py',
'list',
success=True)
self.assertEquals(
pants_run.stdout_data.strip(),
'testprojects/tests/python/pants/dummies:passing_target'
)

def test_owner_list_not_owned(self):
pants_run = self.do_command('--owner-of=testprojects/tests/python/pants/dummies/test_nonexistent.py',
'list',
success=True)
self.assertIn('WARNING: No targets were matched in', pants_run.stderr_data)

def test_owner_list_two_target_specs(self):
# Test that any of these combinations fail with the same error message.
expected_error = ('Multiple target selection methods provided. Please use only one of '
'--changed-*, --owner-of, or target specs')
pants_run_1 = self.do_command('--owner-of=testprojects/tests/python/pants/dummies/test_pass.py',
'--changed-parent=master',
'list',
success=False)
self.assertIn(expected_error, pants_run_1.stderr_data)

pants_run_2 = self.do_command('--owner-of=testprojects/tests/python/pants/dummies/test_pass.py',
'list',
'testprojects/tests/python/pants/dummies:passing_target',
success=False)
self.assertIn(expected_error, pants_run_2.stderr_data)

pants_run_3 = self.do_command('--changed-parent=master',
'list',
'testprojects/tests/python/pants/dummies:passing_target',
success=False)
self.assertIn(expected_error, pants_run_3.stderr_data)

0 comments on commit 5971e20

Please sign in to comment.