Skip to content

Commit

Permalink
Remove support for globs(), rglobs(), and zglobs() (pantsbuild#…
Browse files Browse the repository at this point in the history
…9247)

In a followup, we will further simplify `wrapped_globs.py` and `engine/legacy/structs.py`, such as unifying `Files` and its abstract superclass `BaseGlobs`.
  • Loading branch information
Eric-Arellano authored Mar 7, 2020
1 parent 86f7173 commit c62f342
Show file tree
Hide file tree
Showing 34 changed files with 167 additions and 505 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ class CppTarget(Target):
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: :class:`pants.source.wrapped_globs.FilesetWithSpec` (from globs or rglobs) or
list of strings
:type sources: :class:`pants.source.wrapped_globs.FilesetWithSpec`
"""
payload = payload or Payload()
payload.add_field(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def _create_thrift_project(self, thrift_files):
"""
go_thrift_library(
name='fleem',
sources=globs('*.thrift'),
sources=['*.thrift'],
)
"""
).strip()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def __init__(
"""
:param sources: Javascript and other source code files that make up this module; paths are
relative to the BUILD file's directory.
:type sources: `globs`, `rglobs` or a list of strings
:type sources: a list of strings, with excludes prefixed by `!`
:param package_manager: choose among supported package managers (npm or yarn).
:param build_script: build script name as defined in package.json. All files that are needed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def setup_empty_directory_build_file(self) -> Iterator[None]:
"""\
node_module(
name='javascriptstyle-empty',
sources=rglobs('package.json', 'yarn.lock', '*.js', '.eslintignore'),
sources=['package.json', 'yarn.lock', '**/*.js', '.eslintignore'],
package_manager='yarn',
)
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def __init__(self, sources=None, address=None, payload=None, **kwargs):
"""
:param sources: Scala source that makes up this module; paths are relative to the BUILD
file's directory.
:type sources: `globs`, `rglobs` or a list of strings
:type sources: a list of strings, with excludes prefixed by `!`
"""
payload = payload or Payload()
payload.add_fields(
Expand Down
15 changes: 9 additions & 6 deletions examples/src/java/org/pantsbuild/example/3rdparty_jvm.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,14 @@ that matches the one used to publish the artifact, pants will always prefer the
local target definition to the published jar if it is in the context:

:::python
java_library(name='api',
sources = globs('*.java'),
provides = artifact(org='org.archie', name='api', repo=myrepo),
java_library(
name='api',
sources=['*.java'],
provides=artifact(org='org.archie', name='api', repo=myrepo),
)

jar_library(name='bin-dep',
jar_library(
name='bin-dep',
jars=[
jar(org='org.archie', name='consumer', rev='1.2.3'),
],
Expand Down Expand Up @@ -102,11 +104,12 @@ Rather than mark the `jar` intransitive, you can `exclude` some
transitive dependencies from JVM targets:

:::python
java_library(name = 'loadtest',
java_library(
name = 'loadtest',
dependencies = [
'3rdparty/storm:storm',
],
sources = globs('*.java'),
sources = ['*.java'],
excludes = [
exclude('org.sonatype.sisu.inject', 'cglib')
]
Expand Down
5 changes: 3 additions & 2 deletions examples/src/java/org/pantsbuild/example/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -346,8 +346,9 @@ jvm-platforms in pants.toml, and set what targets use which platforms. For examp
And then in a BUILD file:

:::python
java_library(name='my-library',
sources=globs('*.java'),
java_library(
name='my-library',
sources=['*.java'],
platform='java9',
)

Expand Down
2 changes: 1 addition & 1 deletion examples/src/java/org/pantsbuild/example/hello/greet/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
# 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')".
# by the target type, in this case "['*.java']".
java_library(
dependencies = [], # A more realistic example would depend on other libs,
# but this "hello world" is pretty simple.
Expand Down
2 changes: 1 addition & 1 deletion examples/src/python/example/hello/greet/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
# 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')".
# by the target type, in this case "['*.py']".
python_library(
dependencies=[
'3rdparty/python:ansicolors',
Expand Down
2 changes: 1 addition & 1 deletion examples/src/scala/org/pantsbuild/example/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ The referred-to

(If your circularly-referencing `*.scala` and `*.java` files are in the *same* directory, you don't
need separate `java_library` and `scala_library` targets. Instead, use
`scala_library(sources=globs('*.scala', '*.java'),...)`.)
`scala_library(sources=['*.scala', '*.java'],...)`.)

Scala Version
-------------
Expand Down
5 changes: 3 additions & 2 deletions examples/src/thrift/org/pantsbuild/example/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,13 @@ Thrift; it generates Java.

:::python
# Target defined in src/thrift/com/twitter/mybird/BUILD:
java_thrift_library(name='mybird',
java_thrift_library(
name='mybird',
# Specify dependencies for thrift IDL file includes.
dependencies=[
'src/thrift/com/twitter/otherbird',
],
sources=globs('*.thrift')
sources=['*.thrift']
)

Pants knows that before it compiles such a target, it must first
Expand Down
6 changes: 3 additions & 3 deletions src/docs/common_tasks/resources.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,23 @@ Create a `resources` target that tells Pants which files to include in the resou

## Discussion

A `resources` target definition must include a `name` and a list of `sources`, which can consist of a simple list of file names, multiple `globs` or `rglobs` definitions (more on that in [[Use globs and rglobs to Group Files|pants('src/docs/common_tasks:globs')]]), or any combination thereof. This would create a resource bundle with two files:
A `resources` target definition must include a `name` and a list of `sources`, which is a simple list of file names. This would create a resource bundle with two files:

::python
resources(
name='config'
sources=['server-config.yaml', 'logging-config.xml'],
)

This would include a glob of files to a resource bundle:
This would include a recursive glob of files to a resource bundle:

::python
resources(
name='templates',
sources=['**/*.mustache'],
)

This would include a mixture of globs, rglobs, and specific files:
This would include a mixture of globs, recursive globs, and specific files:

::python
resources(
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/jvm/targets/jvm_target.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def __init__(
transitive dependencies against.
: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
:type sources: a list of strings, with excludes prefixed by `!`
:param services: A dict mapping service interface names to the classes owned by this target
that implement them. Keys are fully qualified service class names, values are
lists of strings, each string the fully qualified class name of a class owned
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/jvm/tasks/jvm_dependency_usage.py
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ def _construct_edge(dep_tgt, products_used):
continue
# Create edge only for those direct or transitive dependencies in order to
# disqualify irrelevant targets that happen to share some file in sources,
# not uncommon when globs especially rglobs is used.
# not uncommon when globs (especially recursive globs) are used.
if derived_from not in transitive_deps:
continue
node.add_edge(_construct_edge(dep_tgt, products_used={product_dep}), derived_from)
Expand Down
8 changes: 4 additions & 4 deletions src/python/pants/build_graph/app_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,15 +111,15 @@ class Bundle:
and ``scripts`` directories: ::
bundles=[
bundle(fileset=[rglobs('config/*', 'scripts/*'), 'my.cfg']),
bundle(fileset=[config/**/*', 'scripts/**/*', 'my.cfg']),
]
To include files relative to some path component use the ``relative_to`` parameter.
The following places the contents of ``common/config`` in a ``config`` directory
in the bundle. ::
bundles=[
bundle(relative_to='common', fileset=globs('common/config/*'))
bundle(relative_to='common', fileset=['common/config/*'])
]
"""

Expand All @@ -134,8 +134,8 @@ def __call__(self, rel_path=None, mapper=None, relative_to=None, fileset=None):
the source tree, returns a path to use in the resulting bundle. By default, an identity
mapper.
:param string relative_to: Set up a simple mapping from source path to bundle path.
:param fileset: The set of files to include in the bundle. A string filename, or list of
filenames, or a Fileset object (e.g. globs()).
:param fileset: The set of files to include in the bundle. A string filename or a list of
filenames/globs.
E.g., ``relative_to='common'`` removes that prefix from all files in the application bundle.
"""

Expand Down
4 changes: 0 additions & 4 deletions src/python/pants/build_graph/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
from pants.build_graph.resources import Resources
from pants.build_graph.target import Target
from pants.build_graph.target_scopes import ScopedDependencyFactory
from pants.source.wrapped_globs import Globs, RGlobs, ZGlobs
from pants.util.netrc import Netrc

"""Register the elementary BUILD file constructs."""
Expand Down Expand Up @@ -46,11 +45,8 @@ def build_file_aliases():
objects={"get_buildroot": get_buildroot, "netrc": Netrc, "pants_version": pants_version,},
context_aware_object_factories={
"buildfile_path": BuildFilePath,
"globs": Globs,
"intransitive": IntransitiveDependencyFactory,
"provided": ProvidedDependencyFactory,
"rglobs": RGlobs,
"scoped": ScopedDependencyFactory,
"zglobs": ZGlobs,
},
)
78 changes: 19 additions & 59 deletions src/python/pants/engine/legacy/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,13 @@
import os
import tokenize
from io import StringIO
from pathlib import PurePath
from typing import Dict, Tuple

from pants.base.build_file_target_factory import BuildFileTargetFactory
from pants.base.deprecated import warn_or_error
from pants.base.exceptions import UnaddressableObjectError
from pants.base.parse_context import ParseContext
from pants.build_graph.build_file_aliases import BuildFileAliases
from pants.engine.legacy.structs import BundleAdaptor, Globs, RGlobs, TargetAdaptor, ZGlobs
from pants.engine.legacy.structs import BundleAdaptor, TargetAdaptor
from pants.engine.objects import Serializable
from pants.engine.parser import ParseError, Parser, SymbolTable
from pants.option.global_options import BuildFileImportsBehavior
Expand Down Expand Up @@ -110,48 +108,10 @@ def __call__(self, *args, **kwargs):
for target_type in target_macro_factory.target_types:
symbols[target_type] = Registrar(parse_context, alias, underlying_symbol)

# TODO: Replace builtins for paths with objects that will create wrapped PathGlobs objects.
# The strategy for https://github.com/pantsbuild/pants/issues/3560 should account for
# migrating these additional captured arguments to typed Sources.
class GlobWrapper:
def __init__(self, parse_context, glob_type):
self._parse_context = parse_context
self._glob_type = glob_type

def __call__(self, *args, **kwargs):
return self._glob_type(*args, spec_path=self._parse_context.rel_path, **kwargs)

symbols["globs"] = GlobWrapper(parse_context, Globs)
symbols["rglobs"] = GlobWrapper(parse_context, RGlobs)
symbols["zglobs"] = GlobWrapper(parse_context, ZGlobs)

symbols["bundle"] = BundleAdaptor

return symbols, parse_context

@staticmethod
def check_for_deprecated_globs_usage(token: str, filepath: str, lineno: int) -> None:
# We have this deprecation here, rather than in `engine/legacy/structs.py` where the
# `sources` field is parsed, so that we can refer to the line number and filename as that
# information is not passed to `structs.py`.
if token in ["globs", "rglobs", "zglobs"]:
script_instructions = (
"curl -L -o fix_deprecated_globs_usage.py 'https://git.io/JvOKD' && chmod +x "
"fix_deprecated_globs_usage.py && ./fix_deprecated_globs_usage.py "
f"{PurePath(filepath).parent}"
)
warning = (
f"Using deprecated `{token}` in {filepath} at line {lineno}. Instead, use a list "
f"of files and globs, like `sources=['f1.py', '*.java']`. Specify excludes by putting "
f"an `!` at the start of the value, like `!ignore.py`.\n\nWe recommend using our "
f"migration script by running `{script_instructions}`"
)
warn_or_error(
removal_version="1.27.0.dev0",
deprecated_entity_description="Using `globs`, `rglobs`, and `zglobs`",
hint=warning,
)

def parse(self, filepath: str, filecontent: bytes):
python = filecontent.decode()

Expand All @@ -168,29 +128,29 @@ def parse(self, filepath: str, filecontent: bytes):
# Note that this is incredibly poor sandboxing. There are many ways to get around it.
# But it's sufficient to tell most users who aren't being actively malicious that they're doing
# something wrong, and it has a low performance overhead.
if "globs" in python or "import" in python:
if "import" in python:
io_wrapped_python = StringIO(python)
for token in tokenize.generate_tokens(io_wrapped_python.readline):
token_str = token[1]
lineno, _ = token[2]

self.check_for_deprecated_globs_usage(token_str, filepath, lineno)
if token_str != "import":
continue

if token_str == "import":
if self._build_file_imports_behavior == BuildFileImportsBehavior.warn:
logger.warning(
f"Import used in {filepath} at line {lineno}. Import statements should "
f"be avoided in BUILD files because they can easily break Pants caching and lead to "
f"stale results. Instead, consider rewriting your code into a Pants plugin: "
f"https://www.pantsbuild.org/howto_plugin.html"
)
else:
raise ParseError(
f"Import used in {filepath} at line {lineno}. Import statements are banned in "
f"BUILD files in this repository and should generally be avoided because "
f"they can easily break Pants caching and lead to stale results. Instead, consider "
f"rewriting your code into a Pants plugin: "
f"https://www.pantsbuild.org/howto_plugin.html"
)
if self._build_file_imports_behavior == BuildFileImportsBehavior.warn:
logger.warning(
f"Import used in {filepath} at line {lineno}. Import statements should "
f"be avoided in BUILD files because they can easily break Pants caching and lead to "
f"stale results. Instead, consider rewriting your code into a Pants plugin: "
f"https://www.pantsbuild.org/howto_plugin.html"
)
else:
raise ParseError(
f"Import used in {filepath} at line {lineno}. Import statements are banned in "
f"BUILD files in this repository and should generally be avoided because "
f"they can easily break Pants caching and lead to stale results. Instead, consider "
f"rewriting your code into a Pants plugin: "
f"https://www.pantsbuild.org/howto_plugin.html"
)

return list(self._parse_context._storage.objects)
20 changes: 2 additions & 18 deletions src/python/pants/engine/legacy/structs.py
Original file line number Diff line number Diff line change
Expand Up @@ -441,8 +441,7 @@ class PantsPluginAdaptorWithOrigin(TargetAdaptorWithOrigin):
adaptor: PantsPluginAdaptor


# TODO: Remove all the subclasses once we remove globs et al. The only remaining subclass would be
# Files, which should simply be unified into BaseGlobs.
# TODO: Consolidate this with `Files`
class BaseGlobs(Locatable, metaclass=ABCMeta):
"""An adaptor class to allow BUILD file parsing from ContextAwareObjectFactories."""

Expand All @@ -461,7 +460,7 @@ def from_sources_field(
isinstance(s, str) for s in sources
):
return Files(*sources, spec_path=spec_path)
raise ValueError(f"Expected either a glob or list of literal sources. Got: {sources}")
raise ValueError(f"Expected a list of literal sources and globs. Got: {sources}")

@property
@abstractmethod
Expand Down Expand Up @@ -562,21 +561,6 @@ def __str__(self) -> str:
return f"[{', '.join(repr(p) for p in self._patterns)}]"


class Globs(BaseGlobs):
path_globs_kwarg = "globs"
legacy_globs_class = wrapped_globs.Globs


class RGlobs(BaseGlobs):
path_globs_kwarg = "rglobs"
legacy_globs_class = wrapped_globs.RGlobs


class ZGlobs(BaseGlobs):
path_globs_kwarg = "zglobs"
legacy_globs_class = wrapped_globs.ZGlobs


@dataclass(frozen=True)
class GlobsWithConjunction:
non_path_globs: BaseGlobs
Expand Down
Loading

0 comments on commit c62f342

Please sign in to comment.