Skip to content

Commit

Permalink
Rid all targets of sources_rel_path parameters. [API]
Browse files Browse the repository at this point in the history
These paramaters were largely unused or else redundantly used/
misused.  The existing external users of this parameter agreed
they can adjust during the next pants upgrade cycle they have
so this change kills the parameters outright.

Fixup GoLocalSource globbing to be lazy and relative to the
spec_path so that it can employ a standard call to
`self.create_sources_field` when setting up its sources payload field.

Testing Done:
CI went green here:
  https://travis-ci.org/pantsbuild/pants/builds/78098070

Bugs closed: 2101

Reviewed at https://rbcommons.com/s/twitter/r/2738/
  • Loading branch information
jsirois committed Aug 31, 2015
1 parent 5e92228 commit 13d0bc1
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 27 deletions.
11 changes: 2 additions & 9 deletions contrib/cpp/src/python/pants/contrib/cpp/targets/cpp_target.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,23 +12,16 @@
class CppTarget(Target):
"""A base class for all cpp targets."""

def __init__(self,
address=None,
payload=None,
sources_rel_path=None,
sources=None,
**kwargs):
def __init__(self, address=None, payload=None, sources=None, **kwargs):
"""
:param sources: Source code files to build. Paths are relative to the BUILD
file's directory.
:type sources: ``Fileset`` (from globs or rglobs) or list of strings
"""
if sources_rel_path is None:
sources_rel_path = address.spec_path
payload = payload or Payload()
payload.add_fields({
'sources': self.create_sources_field(sources=sources,
sources_rel_path=sources_rel_path,
sources_rel_path=address.spec_path,
key_arg='sources'),
})
super(CppTarget, self).__init__(address=address, payload=payload, **kwargs)
2 changes: 2 additions & 0 deletions contrib/go/src/python/pants/contrib/go/targets/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ python_library(
name='go_local_source',
sources=['go_local_source.py'],
dependencies=[
'3rdparty/python/twitter/commons:twitter.common.dirutil',
':go_target',
'src/python/pants/base:build_environment',
'src/python/pants/base:payload',
],
)
Expand Down
15 changes: 12 additions & 3 deletions contrib/go/src/python/pants/contrib/go/targets/go_local_source.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@
unicode_literals, with_statement)

import os
from glob import glob

from pants.base.build_environment import get_buildroot
from pants.base.payload import Payload
from twitter.common.dirutil.fileset import Fileset

from pants.contrib.go.targets.go_target import GoTarget

Expand All @@ -32,11 +33,19 @@ def local_import_path(cls, source_root, address):
return cls.package_path(source_root, address.spec_path)

def __init__(self, address=None, payload=None, **kwargs):
sources = glob(os.path.join(address.spec_path, '*.go'))
# TODO(John Sirois): Make pants.backend.core.wrapped_globs.Globs in the core backend
# constructable with just a rel_path. Right now it violates the Law of Demeter and
# fundamentally takes a ParseContext, which it doesn't actually need and which other backend
# consumers should not need to know about or create.
# Here we depend on twitter/commons which is less than ideal in core pants and even worse in a
# plugin. We depend on it here to ensure the globbing is lazy and skipped if the target is
# never fingerprinted (eg: when running `./pants list`).
sources = Fileset.globs('*.go', root=os.path.join(get_buildroot(), address.spec_path))

payload = payload or Payload()
payload.add_fields({
'sources': self.create_sources_field(sources=sources,
sources_rel_path='',
sources_rel_path=address.spec_path,
key_arg='sources'),
})
super(GoLocalSource, self).__init__(address=address, payload=payload, **kwargs)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,7 @@ def execute(self):
for target in targets:
java_synthetic_name = '{0}-{1}'.format(target.id, 'java')
java_sources_rel_path = os.path.relpath(self.namespace_out, get_buildroot())
java_spec_path = java_sources_rel_path
java_synthetic_address = Address(java_spec_path, java_synthetic_name)
java_synthetic_address = Address(java_sources_rel_path, java_synthetic_name)
java_generated_sources = [
os.path.join(os.path.dirname(source), 'java_{0}.java'.format(os.path.basename(source)))
for source in self.sources_generated_by_target(target)
Expand All @@ -140,7 +139,6 @@ def execute(self):
target_type=JavaLibrary,
dependencies=[dep.address for dep in self.synthetic_target_extra_dependencies],
derived_from=target,
sources_rel_path=java_sources_rel_path,
sources=java_relative_generated_sources,
)
java_synthetic_target = build_graph.get_target(java_synthetic_address)
Expand All @@ -161,8 +159,7 @@ def execute(self):

synthetic_name = '{0}-{1}'.format(target.id, 'scala')
sources_rel_path = os.path.relpath(self.namespace_out, get_buildroot())
spec_path = sources_rel_path
synthetic_address = Address(spec_path, synthetic_name)
synthetic_address = Address(sources_rel_path, synthetic_name)
generated_sources = [
'{0}.{1}'.format(source, 'scala')
for source in self.sources_generated_by_target(target)
Expand All @@ -173,7 +170,6 @@ def execute(self):
address=synthetic_address,
target_type=ScalaLibrary,
dependencies=self.synthetic_target_extra_dependencies,
sources_rel_path=sources_rel_path,
sources=relative_generated_sources,
derived_from=target,
java_sources=[java_synthetic_target.address.spec],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,9 +236,14 @@ def execute(self):
address=synthetic_address,
target_type=self.synthetic_target_type,
dependencies=self.synthetic_target_extra_dependencies(target),
sources_rel_path=sources_rel_path,
sources=relative_generated_sources,
derived_from=target,

# TODO(John Sirois): This assumes - currently, a JvmTarget or PythonTarget which both
# happen to have this attribute for carrying publish metadata but share no interface
# that defines this canonical property. Lift up an interface and check for it or else
# add a way for SimpleCodeGen subclasses to specify extra attribute names that should be
# copied over from the target to its derived target.
provides=target.provides,
)
synthetic_target = self.target
Expand Down
5 changes: 1 addition & 4 deletions src/python/pants/backend/jvm/targets/jvm_target.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ def subsystems(cls):
def __init__(self,
address=None,
payload=None,
sources_rel_path=None,
sources=None,
provides=None,
excludes=None,
Expand Down Expand Up @@ -59,13 +58,11 @@ def __init__(self,
by DistributionLocator.cached().version.
"""
self.address = address # Set in case a TargetDefinitionException is thrown early
if sources_rel_path is None:
sources_rel_path = address.spec_path
payload = payload or Payload()
excludes = ExcludesField(self.assert_list(excludes, expected_type=Exclude, key_arg='excludes'))
configurations = ConfigurationsField(self.assert_list(configurations, key_arg='configurations'))
payload.add_fields({
'sources': self.create_sources_field(sources, sources_rel_path, address, key_arg='sources'),
'sources': self.create_sources_field(sources, address.spec_path, key_arg='sources'),
'provides': provides,
'excludes': excludes,
'configurations': configurations,
Expand Down
5 changes: 1 addition & 4 deletions src/python/pants/backend/python/targets/python_target.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ class PythonTarget(Target):
def __init__(self,
address=None,
payload=None,
sources_rel_path=None,
sources=None,
resources=None, # Old-style resources (file list, Fileset).
resource_targets=None, # New-style resources (Resources target specs).
Expand Down Expand Up @@ -53,11 +52,9 @@ def __init__(self,
agnostic to interpreter class.
"""
self.address = address
if sources_rel_path is None:
sources_rel_path = address.spec_path
payload = payload or Payload()
payload.add_fields({
'sources': self.create_sources_field(sources, sources_rel_path, address, key_arg='sources'),
'sources': self.create_sources_field(sources, address.spec_path, key_arg='sources'),
'resources': self.create_sources_field(resources, address.spec_path, key_arg='resources'),
'provides': provides,
'compatibility': PrimitiveField(maybe_list(compatibility or ())),
Expand Down

0 comments on commit 13d0bc1

Please sign in to comment.