Skip to content
This repository has been archived by the owner on Dec 10, 2020. It is now read-only.

Commit

Permalink
Allow targets to have sensible defaults for sources=.
Browse files Browse the repository at this point in the history
95% of the time, a java_library() target will have
sources=globs('*.java'), and it seems unnecessary to require this
boilerplate to be repeated everywhere.

This change provides a way for target to say "if no explicit sources
are specified, use this glob".

The change is gated behind an option, because it can have unintended
consequences.  For example, in the Pants codebase itself we had a few
places where we were using a python_library() with no sources as a
dependency aggregator, when we should have been using a target().

This change fixes such quirks in our own codebase.

It also modifies a couple of our example targets to take advantage
of this functionality, as proof-of-concept.

This change required modifying quite a few tests that were creating
library targets with no sources: These tests didn't init the relevant
subsystem, and so couldn't consult the new option to see what to do
when no sources= were provided.  These tests were fixed in one of
two ways: either by initializing the subsystem, or by providing
explicit sources=[], whichever made more sense.

Note that instead of creating a new subsystem, I added this option
to the Target.UnknownArguments subsystem.  This implied an expanded
role for that subsystem, so its name was no longer applicable.
I renamed it to Target.Arguments, and deprecated its scope.

Finally, note that the defaults for library and test targets make
it easy to have tests live side-by-side with the code they test,
instead of in a separate source tree, which is a really useful
idiom IMO.

Testing Done:
CI passes: https://travis-ci.org/pantsbuild/pants/builds/168486240

Reviewed at https://rbcommons.com/s/twitter/r/4300/
  • Loading branch information
benjyw committed Oct 18, 2016
1 parent 6b34ffa commit 127867b
Show file tree
Hide file tree
Showing 45 changed files with 267 additions and 124 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,21 @@ java_library(
)

java_library(
name='no_sources',
sources=[],
dependencies=[
':low',
':normal',
':high',
]
)

target(
name='all',
dependencies=[
':none',
':low',
':normal',
':high'
':high',
]
)
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ def test_error(self):
self.assertIn('Bugs: 1 (High: 1, Normal: 0, Low: 0)', pants_run.stdout_data)

def test_transitive(self):
cmd = ['compile', 'contrib/findbugs/tests/java/org/pantsbuild/contrib/findbugs:all']
cmd = ['compile', 'contrib/findbugs/tests/java/org/pantsbuild/contrib/findbugs:no_sources']
pants_ini_config = {'compile.findbugs': {'transitive': False}}
pants_run = self.run_pants(cmd, config=pants_ini_config)
self.assert_success(pants_run)
Expand Down
2 changes: 2 additions & 0 deletions contrib/go/src/python/pants/contrib/go/targets/go_library.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
class GoLibrary(GoLocalSource):
"""A local Go package."""

default_sources_globs = '*.go'

@classmethod
def alias(cls):
return 'go_library'
3 changes: 2 additions & 1 deletion examples/src/java/org/pantsbuild/example/hello/greet/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@

# Note that the target has no explicit name, so it defaults to the name
# of the directory, in this case 'greet'.
# It also has no explicit sources, so it defaults to the sources implied
# by the target type, in this case "globs('*.java')".
java_library(
dependencies = [], # A more realistic example would depend on other libs,
# but this "hello world" is pretty simple.
sources = globs('*.java'),
provides = artifact(org='org.pantsbuild.example',
name='hello-greet',
repo=public,),
Expand Down
6 changes: 5 additions & 1 deletion examples/src/python/example/hello/greet/BUILD
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
# Copyright 2014 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).


# Note that the target has no explicit name, so it defaults to the name
# of the directory, in this case 'greet'.
# It also has no explicit sources, so it defaults to the sources implied
# by the target type, in this case "globs('*.py')".
python_library(
dependencies=[
'3rdparty/python:ansicolors',
],
sources=globs('*.py'),
)
2 changes: 2 additions & 0 deletions migrations/options/src/python/migrate_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
# E.g.:
#('backends', 'packages'): ('DEFAULT', 'backend_packages'),
#('unknown-arguments', 'ignored'): None,

('unknown-arguments', 'ignored'): ('target-arguments', 'ignored'),
}


Expand Down
8 changes: 8 additions & 0 deletions pants.ini
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,14 @@ pants_ignore: +[
]


[target-arguments]
implicit_sources: True


[jvm]
options: ["-Xmx1g", "-XX:MaxPermSize=256m"]


[cache]
# Caching is on globally by default, but we disable it here for development purposes.
# It is explicitly re-enabled below for [cache.bootstrap] only.
Expand Down Expand Up @@ -262,14 +267,17 @@ resolver_cache_dir: %(pants_bootstrapdir)s/python_cache/requirements
# You can change this to point to a config.ini holding the definition of your keys.
keystore_config_location: %(pants_configdir)s/android/keystore/default_config.ini


[test.pytest]
timeouts: true
timeout_default: 60


[test.junit]
timeouts: true
timeout_default: 60


[buildgen.go]
materialize: True
remote: True
Expand Down
26 changes: 21 additions & 5 deletions src/docs/first_tutorial.md
Original file line number Diff line number Diff line change
Expand Up @@ -272,10 +272,12 @@ Here, `java_library` is the target's *type*. Different target types
support different arguments. The following arguments are pretty common:

**name**<br>
We use a target's name to refer to the target. This argument isn't just
"pretty common," it's required. You use names on the command line to
specify which targets to operate on. You also use names in `BUILD` files
when one target refers to another, e.g. in `dependencies`:
We use a target's name to refer to the target. If you omit the name,
it defaults to the name of the directory the `BUILD` file is in.

You use names on the command line to specify which targets to operate on.
You also use names in `BUILD` files when one target refers to another,
e.g. in `dependencies`:

**dependencies**<br>
List of things this target depends upon. If this target's code imports
Expand All @@ -284,7 +286,21 @@ target imports code that "lives" in `.jar`s/`.egg`s from elsewhere,
refer to them here.

**sources**<br>
List of source files. The `globs` function is handy here.
List of source files, which must be under the directory tree rooted
at the BUILD file's directory.

You can provide an explicit list, e.g.,
`sources=['Foo.java', 'Bar.java']`, or use the `globs` function
to glob over files, e.g., `sources=globs('*.java')`. Similarly,
`rglobs` will apply the glob pattern recursively in all subdirectories
of the BUILD file's directory.

If you omit `sources` in a target, Pants will attempt to apply a
sensible default that depends on the target type.
E.g., `junit_tests` will default to globbing over `*Test.java`,
`java_library` will default to globbing over `*.java`, minus the
test files, and so on.


The Usual Commands
------------------
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/codegen/tasks/BUILD
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Copyright 2014 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

python_library(
target(
name = 'all',
dependencies = [
':antlr_gen',
Expand Down
3 changes: 2 additions & 1 deletion src/python/pants/backend/jvm/targets/BUILD
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

python_library(
target(
name = 'all',
dependencies = [
':jvm',
Expand Down Expand Up @@ -77,6 +77,7 @@ python_library(
],
dependencies = [
'3rdparty/python/twitter/commons:twitter.common.collections',
':java', # For a dep on java_tests.py, which we might want to move to a separate target.
':jvm',
'src/python/pants/backend/jvm/subsystems:scala_platform',
'src/python/pants/base:exceptions',
Expand Down
4 changes: 4 additions & 0 deletions src/python/pants/backend/jvm/targets/java_library.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
unicode_literals, with_statement)

from pants.backend.jvm.targets.exportable_jvm_library import ExportableJvmLibrary
from pants.backend.jvm.targets.java_tests import JavaTests


class JavaLibrary(ExportableJvmLibrary):
Expand All @@ -20,6 +21,9 @@ class JavaLibrary(ExportableJvmLibrary):
:API: public
"""

default_sources_globs = '*.java'
default_sources_exclude_globs = JavaTests.java_test_globs

@classmethod
def subsystems(cls):
return super(JavaLibrary, cls).subsystems()
Expand Down
6 changes: 6 additions & 0 deletions src/python/pants/backend/jvm/targets/java_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,18 @@
from pants.base.payload_field import PrimitiveField


# TODO: Rename this to JUnitTests. It isn't just for Java language tests.
class JavaTests(JvmTarget):
"""JUnit tests.
:API: public
"""

java_test_globs = ('*Test.java',)
scala_test_globs = ('*Test.scala', '*Spec.scala')

default_sources_globs = java_test_globs + scala_test_globs

CONCURRENCY_SERIAL = 'SERIAL'
CONCURRENCY_PARALLEL_CLASSES = 'PARALLEL_CLASSES'
CONCURRENCY_PARALLEL_METHODS = 'PARALLEL_METHODS'
Expand Down
4 changes: 4 additions & 0 deletions src/python/pants/backend/jvm/targets/scala_library.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

from pants.backend.jvm.subsystems.scala_platform import ScalaPlatform
from pants.backend.jvm.targets.exportable_jvm_library import ExportableJvmLibrary
from pants.backend.jvm.targets.java_tests import JavaTests
from pants.base.exceptions import TargetDefinitionException
from pants.build_graph.address import Address

Expand All @@ -23,6 +24,9 @@ class ScalaLibrary(ExportableJvmLibrary):
:API: public
"""

default_sources_globs = '*.scala'
default_sources_exclude_globs = JavaTests.scala_test_globs

@classmethod
def subsystems(cls):
return super(ScalaLibrary, cls).subsystems() + (ScalaPlatform, )
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/jvm/tasks/BUILD
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Copyright 2016 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

python_library(
target(
name = 'all',
dependencies = [
':benchmark_run',
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/project_info/tasks/BUILD
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Copyright 2015 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

python_library(
target(
name = 'all',
dependencies = [
':dependencies',
Expand Down
45 changes: 0 additions & 45 deletions src/python/pants/backend/python/python_egg.py

This file was deleted.

5 changes: 4 additions & 1 deletion src/python/pants/backend/python/targets/python_library.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,14 @@
unicode_literals, with_statement)

from pants.backend.python.targets.python_target import PythonTarget
from pants.backend.python.targets.python_tests import PythonTests


class PythonLibrary(PythonTarget):
"""A Python library.
:API: public
"""
pass

default_sources_globs = '*.py'
default_sources_exclude_globs = PythonTests.default_sources_globs
3 changes: 3 additions & 0 deletions src/python/pants/backend/python/targets/python_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ class PythonTests(PythonTarget):
:API: public
"""

# These are the patterns matched by pytest's test discovery.
default_sources_globs = ('test_*.py', '*_test.py')

def __init__(self, coverage=None, timeout=None, **kwargs):
"""
:param coverage: the module(s) whose coverage should be generated, e.g.
Expand Down
Loading

0 comments on commit 127867b

Please sign in to comment.