Skip to content

Commit

Permalink
Synthesized lockfile targets should never err on missing lockfiles. (p…
Browse files Browse the repository at this point in the history
  • Loading branch information
kaos authored Mar 8, 2023
1 parent 89a1458 commit 376433a
Show file tree
Hide file tree
Showing 8 changed files with 73 additions and 43 deletions.
4 changes: 2 additions & 2 deletions src/python/pants/backend/python/dependency_inference/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
from pants.backend.python.util_rules import ancestor_files, pex
from pants.backend.python.util_rules.ancestor_files import AncestorFiles, AncestorFilesRequest
from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints
from pants.base.glob_match_error_behavior import GlobMatchErrorBehavior
from pants.core import target_types
from pants.core.target_types import AllAssetTargets, AllAssetTargetsByPath, AllAssetTargetsRequest
from pants.core.util_rules import stripped_source_files
Expand All @@ -57,7 +58,6 @@
Targets,
)
from pants.engine.unions import UnionRule
from pants.option.global_options import OwnersNotFoundBehavior
from pants.source.source_root import SourceRoot, SourceRootRequest
from pants.util.docutil import doc_url
from pants.util.strutil import bullet_list, softwrap
Expand Down Expand Up @@ -564,7 +564,7 @@ async def infer_python_conftest_dependencies(
owners = await MultiGet(
# NB: Because conftest.py files effectively always have content, we require an
# owning target.
Get(Owners, OwnersRequest((f,), OwnersNotFoundBehavior.error))
Get(Owners, OwnersRequest((f,), GlobMatchErrorBehavior.error))
for f in conftest_files.snapshot.files
)

Expand Down
15 changes: 14 additions & 1 deletion src/python/pants/core/target_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
FileDigest,
FileEntry,
MergeDigests,
PathGlobs,
RemovePrefix,
Snapshot,
)
Expand All @@ -48,6 +49,7 @@
HydrateSourcesRequest,
InvalidFieldTypeException,
MultipleSourcesField,
OptionalSingleSourceField,
OverridesField,
SingleSourceField,
SourcesField,
Expand All @@ -60,6 +62,7 @@
generate_multiple_sources_field_help_message,
)
from pants.engine.unions import UnionRule
from pants.option.global_options import UnmatchedBuildFileGlobs
from pants.util.docutil import bin_name
from pants.util.frozendict import FrozenDict
from pants.util.logging import LogLevel
Expand Down Expand Up @@ -858,9 +861,19 @@ async def package_archive_target(field_set: ArchiveFieldSet) -> BuiltPackage:
# -----------------------------------------------------------------------------------------------


class LockfileSourceField(SingleSourceField):
class LockfileSourceField(OptionalSingleSourceField):
"""Source field for synthesized `_lockfile` targets.
It is special in that it always ignores any missing files, regardless of the global
`--unmatched-build-file-globs` option.
"""

uses_source_roots = False
required = True
value: str

def path_globs(self, unmatched_build_file_globs: UnmatchedBuildFileGlobs) -> PathGlobs: # type: ignore[misc]
return super().path_globs(UnmatchedBuildFileGlobs.ignore())


class LockfileDependenciesField(Dependencies):
Expand Down
19 changes: 18 additions & 1 deletion src/python/pants/core/target_types_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
FilesGeneratorTarget,
FileSourceField,
FileTarget,
LockfileSourceField,
RelocatedFiles,
RelocateFilesViaCodegenRequest,
ResourceTarget,
Expand All @@ -33,14 +34,15 @@
from pants.core.util_rules import archive, source_files, system_binaries
from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest
from pants.engine.addresses import Address
from pants.engine.fs import EMPTY_SNAPSHOT, DigestContents, FileContent
from pants.engine.fs import EMPTY_SNAPSHOT, DigestContents, FileContent, GlobMatchErrorBehavior
from pants.engine.platform import Platform
from pants.engine.target import (
GeneratedSources,
SourcesField,
TransitiveTargets,
TransitiveTargetsRequest,
)
from pants.option.global_options import UnmatchedBuildFileGlobs
from pants.testutil.rule_runner import QueryRule, RuleRunner, mock_console


Expand Down Expand Up @@ -431,3 +433,18 @@ def test_http_source_filename(url, expected):
def test_invalid_http_source(kwargs, exc_match):
with exc_match:
http_source(**kwargs)


@pytest.mark.parametrize(
"error_behavior", [GlobMatchErrorBehavior.warn, GlobMatchErrorBehavior.error]
)
def test_lockfile_glob_match_error_behavior(
error_behavior: GlobMatchErrorBehavior,
) -> None:
lockfile_source = LockfileSourceField("test.lock", Address("", target_name="lockfile-test"))
assert (
GlobMatchErrorBehavior.ignore
== lockfile_source.path_globs(
UnmatchedBuildFileGlobs(error_behavior)
).glob_match_error_behavior
)
16 changes: 6 additions & 10 deletions src/python/pants/engine/internals/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,7 @@
_generate_file_level_targets,
)
from pants.engine.unions import UnionMembership, UnionRule
from pants.option.global_options import (
GlobalOptions,
OwnersNotFoundBehavior,
UnmatchedBuildFileGlobs,
)
from pants.option.global_options import GlobalOptions, UnmatchedBuildFileGlobs
from pants.util.docutil import bin_name, doc_url
from pants.util.frozendict import FrozenDict
from pants.util.logging import LogLevel
Expand Down Expand Up @@ -741,7 +737,7 @@ async def coarsened_targets(

def _log_or_raise_unmatched_owners(
file_paths: Sequence[PurePath],
owners_not_found_behavior: OwnersNotFoundBehavior,
owners_not_found_behavior: GlobMatchErrorBehavior,
ignore_option: str | None = None,
) -> None:
option_msg = (
Expand All @@ -767,7 +763,7 @@ def _log_or_raise_unmatched_owners(
f"{doc_url('create-initial-build-files')}.{option_msg}"
)

if owners_not_found_behavior == OwnersNotFoundBehavior.warn:
if owners_not_found_behavior == GlobMatchErrorBehavior.warn:
logger.warning(msg)
else:
raise ResolveError(msg)
Expand All @@ -782,7 +778,7 @@ class OwnersRequest:
"""

sources: tuple[str, ...]
owners_not_found_behavior: OwnersNotFoundBehavior = OwnersNotFoundBehavior.ignore
owners_not_found_behavior: GlobMatchErrorBehavior = GlobMatchErrorBehavior.ignore
filter_by_global_options: bool = False
match_if_owning_build_file_included_in_sources: bool = False

Expand Down Expand Up @@ -890,7 +886,7 @@ def create_live_and_deleted_gets(

if (
unmatched_sources
and owners_request.owners_not_found_behavior != OwnersNotFoundBehavior.ignore
and owners_request.owners_not_found_behavior != GlobMatchErrorBehavior.ignore
):
_log_or_raise_unmatched_owners(
[PurePath(path) for path in unmatched_sources], owners_request.owners_not_found_behavior
Expand All @@ -908,7 +904,7 @@ def create_live_and_deleted_gets(
def extract_unmatched_build_file_globs(
global_options: GlobalOptions,
) -> UnmatchedBuildFileGlobs:
return global_options.unmatched_build_file_globs
return UnmatchedBuildFileGlobs(global_options.unmatched_build_file_globs)


class AmbiguousCodegenImplementationsException(Exception):
Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/engine/target.py
Original file line number Diff line number Diff line change
Expand Up @@ -2073,7 +2073,7 @@ def path_globs(self, unmatched_build_file_globs: UnmatchedBuildFileGlobs) -> Pat
# Use fields default error behavior if defined, if we use default globs else the provided
# error behavior.
error_behavior = (
unmatched_build_file_globs.to_glob_match_error_behavior()
unmatched_build_file_globs.error_behavior
if conjunction == GlobExpansionConjunction.all_match
or self.default_glob_match_error_behavior is None
else self.default_glob_match_error_behavior
Expand Down Expand Up @@ -2888,7 +2888,7 @@ def relativize_glob(glob: str) -> str:
return tuple(
PathGlobs(
[relativize_glob(glob)],
glob_match_error_behavior=unmatched_build_file_globs.to_glob_match_error_behavior(),
glob_match_error_behavior=unmatched_build_file_globs.error_behavior,
description_of_origin=f"the `overrides` field for {address}",
)
for glob in overrides_keys
Expand Down
6 changes: 3 additions & 3 deletions src/python/pants/engine/target_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1047,7 +1047,7 @@ class TestMultipleSourcesField(MultipleSourcesField):
default_glob_match_error_behavior = GlobMatchErrorBehavior.ignore

sources = TestMultipleSourcesField(field_value, Address("test"))
actual = sources.path_globs(UnmatchedBuildFileGlobs.warn)
actual = sources.path_globs(UnmatchedBuildFileGlobs.warn())
for attr, expect in zip(expected._fields, expected):
if expect is not SKIP:
assert getattr(actual, attr) == expect
Expand Down Expand Up @@ -1107,7 +1107,7 @@ class TestSingleSourceField(SingleSourceField):

sources = TestSingleSourceField(field_value, Address("test"))

actual = sources.path_globs(UnmatchedBuildFileGlobs.warn)
actual = sources.path_globs(UnmatchedBuildFileGlobs.warn())
for attr, expect in zip(expected._fields, expected):
if expect is not SKIP:
assert getattr(actual, attr) == expect
Expand Down Expand Up @@ -1428,7 +1428,7 @@ def test_overrides_field_normalization() -> None:
{"foo.ext": tgt1_override, ("foo.ext", "bar*.ext"): tgt2_override}, Address("dir")
)
globs = OverridesField.to_path_globs(
Address("dir"), path_field.flatten(), UnmatchedBuildFileGlobs.error
Address("dir"), path_field.flatten(), UnmatchedBuildFileGlobs.error()
)
assert [path_globs.globs for path_globs in globs] == [
("dir/foo.ext",),
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/init/specs_calculator.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def calculate_specs(
) -> Specs:
"""Determine the specs for a given Pants run."""
global_options = options.for_global_scope()
unmatched_cli_globs = global_options.unmatched_cli_globs.to_glob_match_error_behavior()
unmatched_cli_globs = global_options.unmatched_cli_globs
specs = SpecsParser().parse_specs(
options.specs,
description_of_origin="CLI arguments",
Expand Down
50 changes: 27 additions & 23 deletions src/python/pants/option/global_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from datetime import datetime
from enum import Enum
from pathlib import Path, PurePath
from typing import Any, Callable, Type, cast
from typing import Any, Callable, Type, TypeVar, cast

from pants.base.build_environment import (
get_buildroot,
Expand Down Expand Up @@ -73,36 +73,40 @@ class DynamicUIRenderer(Enum):
experimental_prodash = "experimental-prodash"


class UnmatchedBuildFileGlobs(Enum):
"""What to do when globs do not match in BUILD files."""
_G = TypeVar("_G", bound="_GlobMatchErrorBehaviorOptionBase")

warn = "warn"
error = "error"

def to_glob_match_error_behavior(self) -> GlobMatchErrorBehavior:
return GlobMatchErrorBehavior(self.value)
@dataclass(frozen=True)
class _GlobMatchErrorBehaviorOptionBase:
"""This class exists to have dedicated types per global option of the `GlobMatchErrorBehavior`
so we can extract the relevant option in a rule to limit the scope of downstream rules to avoid
depending on the entire global options data."""

error_behavior: GlobMatchErrorBehavior

class UnmatchedCliGlobs(Enum):
"""What to do when globs do not match in CLI args."""
@classmethod
def ignore(cls: type[_G]) -> _G:
return cls(GlobMatchErrorBehavior.ignore)

ignore = "ignore"
warn = "warn"
error = "error"
@classmethod
def warn(cls: type[_G]) -> _G:
return cls(GlobMatchErrorBehavior.warn)

@classmethod
def error(cls: type[_G]) -> _G:
return cls(GlobMatchErrorBehavior.error)

def to_glob_match_error_behavior(self) -> GlobMatchErrorBehavior:
return GlobMatchErrorBehavior(self.value)

class UnmatchedBuildFileGlobs(_GlobMatchErrorBehaviorOptionBase):
"""What to do when globs do not match in BUILD files."""

class OwnersNotFoundBehavior(Enum):
"""What to do when a file argument cannot be mapped to an owning target."""

ignore = "ignore"
warn = "warn"
error = "error"
class UnmatchedCliGlobs(_GlobMatchErrorBehaviorOptionBase):
"""What to do when globs do not match in CLI args."""


def to_glob_match_error_behavior(self) -> GlobMatchErrorBehavior:
return GlobMatchErrorBehavior(self.value)
class OwnersNotFoundBehavior(_GlobMatchErrorBehaviorOptionBase):
"""What to do when a file argument cannot be mapped to an owning target."""


@enum.unique
Expand Down Expand Up @@ -1577,7 +1581,7 @@ class GlobalOptions(BootstrapOptions, Subsystem):
)

unmatched_build_file_globs = EnumOption(
default=UnmatchedBuildFileGlobs.warn,
default=GlobMatchErrorBehavior.warn,
help=softwrap(
"""
What to do when files and globs specified in BUILD files, such as in the
Expand All @@ -1591,7 +1595,7 @@ class GlobalOptions(BootstrapOptions, Subsystem):
advanced=True,
)
unmatched_cli_globs = EnumOption(
default=UnmatchedCliGlobs.error,
default=GlobMatchErrorBehavior.error,
help=softwrap(
"""
What to do when command line arguments, e.g. files and globs like `dir::`, cannot be
Expand Down

0 comments on commit 376433a

Please sign in to comment.