Skip to content

Commit

Permalink
Remove deprecated use_deprecated_directory_cli_args_semantics option (
Browse files Browse the repository at this point in the history
#16630)

Directories on the CLI now never mean the target `dir:dir`.

[ci skip-rust]
[ci skip-build-wheels]
  • Loading branch information
Eric-Arellano authored Aug 25, 2022
1 parent c35629a commit a75ab14
Show file tree
Hide file tree
Showing 15 changed files with 27 additions and 216 deletions.
4 changes: 1 addition & 3 deletions src/python/pants/backend/explorer/graphql/query/targets.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,7 @@ async def targets(self, info: Info, query: Optional[TargetsQuery] = None) -> Lis
req = GraphQLContext.request_state_from_info(info).product_request
specs = (
specs_parser.parse_specs(
query.specs,
convert_dir_literal_to_address_literal=False,
description_of_origin="GraphQL targets `query.specs`",
query.specs, description_of_origin="GraphQL targets `query.specs`"
)
if query is not None and query.specs
else None
Expand Down
12 changes: 7 additions & 5 deletions src/python/pants/backend/project_info/count_loc_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,15 @@ def test_count_loc(rule_runner: RuleRunner) -> None:
{
f"{py_dir}/foo.py": '# A comment.\n\nprint("some code")\n# Another comment.',
f"{py_dir}/bar.py": '# A comment.\n\nprint("some more code")',
f"{py_dir}/BUILD": "python_sources()",
f"{py_dir}/BUILD": "python_sources(name='lib')",
f"{elixir_dir}/foo.ex": 'IO.puts("Some elixir")\n# A comment',
f"{elixir_dir}/ignored.ex": "# We do not expect this file to appear in counts.",
f"{elixir_dir}/BUILD": "elixir(sources=['foo.ex'])",
f"{elixir_dir}/BUILD": "elixir(name='lib', sources=['foo.ex'])",
}
)
result = rule_runner.run_goal_rule(CountLinesOfCode, args=[py_dir, elixir_dir])
result = rule_runner.run_goal_rule(
CountLinesOfCode, args=[f"{py_dir}:lib", f"{elixir_dir}:lib"]
)
assert result.exit_code == 0
assert_counts(result.stdout, "Python", num_files=2, blank=2, comment=3, code=2)
assert_counts(result.stdout, "Elixir", comment=1, code=1)
Expand Down Expand Up @@ -98,6 +100,6 @@ def test_files_without_owners(rule_runner: RuleRunner) -> None:

def test_no_sources_exits_gracefully(rule_runner: RuleRunner) -> None:
py_dir = "src/py/foo"
rule_runner.write_files({f"{py_dir}/BUILD": "python_sources()"})
result = rule_runner.run_goal_rule(CountLinesOfCode, args=[py_dir])
rule_runner.write_files({f"{py_dir}/BUILD": "python_sources(name='lib')"})
result = rule_runner.run_goal_rule(CountLinesOfCode, args=[f"{py_dir}:lib"])
assert result == GoalRuleResult.noop()
4 changes: 2 additions & 2 deletions src/python/pants/backend/project_info/dependencies_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ def test_python_dependencies(rule_runner: RuleRunner) -> None:
assert_deps = partial(assert_dependencies, rule_runner)

assert_deps(
specs=["some/other/target"],
specs=["some/other/target:target"],
transitive=False,
expected=["some/other/target/a.py"],
)
Expand All @@ -146,7 +146,7 @@ def test_python_dependencies(rule_runner: RuleRunner) -> None:
expected=["3rdparty/python:req2", "some/target/a.py"],
)
assert_deps(
specs=["some/other/target"],
specs=["some/other/target:target"],
transitive=True,
expected=[
"3rdparty/python:req1",
Expand Down
8 changes: 1 addition & 7 deletions src/python/pants/backend/project_info/paths.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
TransitiveTargets,
TransitiveTargetsRequest,
)
from pants.option.global_options import GlobalOptions
from pants.option.option_types import StrOption


Expand Down Expand Up @@ -80,9 +79,7 @@ def find_paths_breadth_first(


@goal_rule
async def paths(
console: Console, paths_subsystem: PathsSubsystem, global_options: GlobalOptions
) -> PathsGoal:
async def paths(console: Console, paths_subsystem: PathsSubsystem) -> PathsGoal:

path_from = paths_subsystem.from_
path_to = paths_subsystem.to
Expand All @@ -95,15 +92,13 @@ async def paths(

specs_parser = SpecsParser()

convert_dir_literals = global_options.use_deprecated_directory_cli_args_semantics
from_tgts, to_tgts = await MultiGet(
Get(
Targets,
Specs,
specs_parser.parse_specs(
[path_from],
description_of_origin="the option `--paths-from`",
convert_dir_literal_to_address_literal=convert_dir_literals,
),
),
Get(
Expand All @@ -112,7 +107,6 @@ async def paths(
specs_parser.parse_specs(
[path_to],
description_of_origin="the option `--paths-to`",
convert_dir_literal_to_address_literal=convert_dir_literals,
),
),
)
Expand Down
11 changes: 2 additions & 9 deletions src/python/pants/base/specs.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,6 @@ class DirLiteralSpec(Spec):
def __str__(self) -> str:
return self.directory

def to_address_literal(self) -> AddressLiteralSpec:
"""For now, `dir` can also be shorthand for `dir:dir`."""
return AddressLiteralSpec(path_component=self.directory)

def matches_target_residence_dir(self, residence_dir: str) -> bool:
return residence_dir == self.directory

Expand Down Expand Up @@ -253,7 +249,7 @@ def create(
specs: Iterable[Spec],
*,
description_of_origin: str,
convert_dir_literal_to_address_literal: bool,
convert_dir_literal_to_address_literal: bool = False,
unmatched_glob_behavior: GlobMatchErrorBehavior = GlobMatchErrorBehavior.error,
filter_by_global_options: bool = False,
from_change_detection: bool = False,
Expand All @@ -278,10 +274,7 @@ def create(
elif isinstance(spec, FileGlobSpec):
file_globs.append(spec)
elif isinstance(spec, DirLiteralSpec):
if convert_dir_literal_to_address_literal:
address_literals.append(spec.to_address_literal())
else:
dir_literals.append(spec)
dir_literals.append(spec)
elif isinstance(spec, DirGlobSpec):
dir_globs.append(spec)
elif isinstance(spec, RecursiveGlobSpec):
Expand Down
4 changes: 1 addition & 3 deletions src/python/pants/base/specs_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ def parse_specs(
specs: Iterable[str],
*,
description_of_origin: str,
convert_dir_literal_to_address_literal: bool,
convert_dir_literal_to_address_literal: bool = False,
unmatched_glob_behavior: GlobMatchErrorBehavior = GlobMatchErrorBehavior.error,
) -> Specs:
include_specs = []
Expand All @@ -131,14 +131,12 @@ def parse_specs(
includes = RawSpecs.create(
include_specs,
description_of_origin=description_of_origin,
convert_dir_literal_to_address_literal=convert_dir_literal_to_address_literal,
unmatched_glob_behavior=unmatched_glob_behavior,
filter_by_global_options=True,
)
ignores = RawSpecs.create(
ignore_specs,
description_of_origin=description_of_origin,
convert_dir_literal_to_address_literal=convert_dir_literal_to_address_literal,
unmatched_glob_behavior=unmatched_glob_behavior,
# By setting the below to False, we will end up matching some targets
# that cannot have been resolved by the include specs. For example, if the user runs
Expand Down
4 changes: 1 addition & 3 deletions src/python/pants/bsp/util_rules/targets.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,7 @@ class _ParseOneBSPMappingRequest:
async def parse_one_bsp_mapping(request: _ParseOneBSPMappingRequest) -> BSPBuildTargetInternal:
specs_parser = SpecsParser()
specs = specs_parser.parse_specs(
request.definition.addresses,
description_of_origin=f"the BSP mapping {request.name}",
convert_dir_literal_to_address_literal=False,
request.definition.addresses, description_of_origin=f"the BSP mapping {request.name}"
).includes
return BSPBuildTargetInternal(request.name, specs, request.definition)

Expand Down
86 changes: 6 additions & 80 deletions src/python/pants/core/goals/tailor.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from dataclasses import dataclass
from typing import Iterable, Iterator, Mapping, cast

from pants.base.specs import AncestorGlobSpec, RawSpecs, Spec, Specs
from pants.base.specs import AncestorGlobSpec, RawSpecs, Specs
from pants.build_graph.address import Address
from pants.engine.collection import DeduplicatedCollection
from pants.engine.console import Console
Expand Down Expand Up @@ -41,7 +41,6 @@
UnexpandedTargets,
)
from pants.engine.unions import UnionMembership, union
from pants.option.global_options import GlobalOptions
from pants.option.option_types import BoolOption, DictOption, StrListOption, StrOption
from pants.source.filespec import FilespecMatcher
from pants.util.docutil import bin_name, doc_url
Expand All @@ -58,33 +57,17 @@
@dataclass(frozen=True)
class PutativeTargetsRequest(metaclass=ABCMeta):
dirs: tuple[str, ...]
deprecated_recursive_dirs: tuple[str, ...] = ()

def path_globs(self, *filename_globs: str) -> PathGlobs:
return PathGlobs(
globs=(
*(os.path.join(d, glob) for d in self.dirs for glob in filename_globs),
*(
os.path.join(d, "**", glob)
for d in self.deprecated_recursive_dirs
for glob in filename_globs
),
)
)
return PathGlobs(os.path.join(d, glob) for d in self.dirs for glob in filename_globs)


@dataclass(frozen=True)
class PutativeTargetsSearchPaths:
dirs: tuple[str, ...]
deprecated_recursive_dirs: tuple[str, ...] = ()

def path_globs(self, filename_glob: str) -> PathGlobs:
return PathGlobs(
globs=(
*(os.path.join(d, filename_glob) for d in self.dirs),
*(os.path.join(d, "**", filename_glob) for d in self.deprecated_recursive_dirs),
)
)
return PathGlobs(globs=(os.path.join(d, filename_glob) for d in self.dirs))


@memoized
Expand Down Expand Up @@ -571,53 +554,6 @@ def make_content(bf_path: str, pts: Iterable[PutativeTarget]) -> FileContent:
return EditedBuildFiles(new_digest, tuple(sorted(created)), tuple(sorted(updated)))


def specs_to_dirs(specs: RawSpecs) -> tuple[str, ...]:
"""Extract cmd-line specs that look like directories.
Error on all other specs.
This is a hack that allows us to emulate "directory specs" while we deprecate the shorthand of
`dir` being `dir:dir`.
"""
dir_specs = [dir_spec.directory for dir_spec in specs.dir_literals]
other_specs: list[Spec] = [
*specs.file_literals,
*specs.file_globs,
*specs.dir_globs,
*specs.recursive_globs,
*specs.ancestor_globs,
]
for spec in specs.address_literals:
if spec.is_directory_shorthand:
dir_specs.append(spec.path_component)
else:
other_specs.append(spec)
if other_specs:
raise ValueError(
softwrap(
f"""
The global option `use_deprecated_cli_args_semantics` is set to `true`, so the
tailor goal is using deprecated semantics for CLI arguments. In this mode, the
tailor goal only accepts literal directories as arguments, which it will run
recursively on. You specified {', '.join(str(spec) for spec in other_specs)}
To fix, either use the default value of `use_deprecated_cli_args_semantics` of
false, or rerun with
specifying only literal directories, e.g. `{bin_name()} tailor dir1 dir2`. If
changing `use_deprecated_cli_args_semantics` to false, you should specify which
directories to run on when using `tailor`:
* `{bin_name()} tailor ::` to run on everything
* `{bin_name()} tailor dir::` to run on `dir` and subdirs
* `{bin_name()} tailor dir` to run on `dir`
* `{bin_name()} --changed-since=HEAD tailor` to only run on changed and new files
"""
)
)
# No specs at all means search the entire repo.
return tuple(dir_specs) or ("",)


@goal_rule
async def tailor(
tailor_subsystem: TailorSubsystem,
Expand All @@ -626,7 +562,6 @@ async def tailor(
union_membership: UnionMembership,
specs: Specs,
build_file_options: BuildFileOptions,
global_options: GlobalOptions,
) -> TailorGoal:
tailor_subsystem.validate_build_file_name(build_file_options.patterns)
if not specs:
Expand All @@ -647,20 +582,11 @@ async def tailor(
)
return TailorGoal(exit_code=0)

dir_search_paths: tuple[str, ...] = ()
recursive_search_paths: tuple[str, ...] = ()
if global_options.use_deprecated_directory_cli_args_semantics:
recursive_search_paths = specs_to_dirs(specs.includes)
else:
specs_paths = await Get(SpecsPaths, Specs, specs)
dir_search_paths = tuple(sorted({os.path.dirname(f) for f in specs_paths.files}))
specs_paths = await Get(SpecsPaths, Specs, specs)
dir_search_paths = tuple(sorted({os.path.dirname(f) for f in specs_paths.files}))

putative_targets_results = await MultiGet(
Get(
PutativeTargets,
PutativeTargetsRequest,
req_type(dir_search_paths, recursive_search_paths),
)
Get(PutativeTargets, PutativeTargetsRequest, req_type(dir_search_paths))
for req_type in union_membership[PutativeTargetsRequest]
)
putative_targets = PutativeTargets.merge(putative_targets_results)
Expand Down
50 changes: 0 additions & 50 deletions src/python/pants/core/goals/tailor_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

import pytest

from pants.base.specs import AddressLiteralSpec, DirLiteralSpec, FileLiteralSpec, RawSpecs
from pants.core.goals import tailor
from pants.core.goals.tailor import (
AllOwnedSources,
Expand All @@ -26,7 +25,6 @@
default_sources_for_target_type,
group_by_dir,
make_content_str,
specs_to_dirs,
)
from pants.core.util_rules import source_files
from pants.engine.fs import DigestContents, FileContent, PathGlobs, Paths
Expand Down Expand Up @@ -431,54 +429,6 @@ def test_group_by_dir() -> None:
} == group_by_dir(paths)


def test_specs_to_dirs() -> None:
assert specs_to_dirs(RawSpecs(description_of_origin="tests")) == ("",)
assert specs_to_dirs(
RawSpecs(
address_literals=(AddressLiteralSpec("src/python/foo"),), description_of_origin="tests"
)
) == ("src/python/foo",)
assert specs_to_dirs(
RawSpecs(dir_literals=(DirLiteralSpec("src/python/foo"),), description_of_origin="tests")
) == ("src/python/foo",)
assert specs_to_dirs(
RawSpecs(
address_literals=(
AddressLiteralSpec("src/python/foo"),
AddressLiteralSpec("src/python/bar"),
),
description_of_origin="tests",
)
) == ("src/python/foo", "src/python/bar")

with pytest.raises(ValueError):
specs_to_dirs(
RawSpecs(
file_literals=(FileLiteralSpec("src/python/foo.py"),), description_of_origin="tests"
)
)

with pytest.raises(ValueError):
specs_to_dirs(
RawSpecs(
address_literals=(AddressLiteralSpec("src/python/bar", "tgt"),),
description_of_origin="tests",
)
)

with pytest.raises(ValueError):
specs_to_dirs(
RawSpecs(
address_literals=(
AddressLiteralSpec(
"src/python/bar", target_component=None, generated_component="gen"
),
),
description_of_origin="tests",
)
)


def test_tailor_rule_write_mode(rule_runner: RuleRunner) -> None:
rule_runner.write_files(
{
Expand Down
4 changes: 1 addition & 3 deletions src/python/pants/engine/internals/engine_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -927,9 +927,7 @@ def test_streaming_workunits_expanded_specs(run_tracker: RunTracker) -> None:
}
)
specs = SpecsParser().parse_specs(
["src/python/somefiles::", "src/python/others/b.py"],
convert_dir_literal_to_address_literal=False,
description_of_origin="tests",
["src/python/somefiles::", "src/python/others/b.py"], description_of_origin="tests"
)

class Callback(WorkunitsCallback):
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/engine/internals/graph_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -903,7 +903,7 @@ def assert_generated(
# TODO: Adjust the `TransitiveTargets` API to expose the complete mapping.
# see https://github.com/pantsbuild/pants/issues/11270
specs = SpecsParser(rule_runner.build_root).parse_specs(
["::"], convert_dir_literal_to_address_literal=False, description_of_origin="tests"
["::"], description_of_origin="tests"
)
addresses = rule_runner.request(Addresses, [specs])
dependency_mapping = rule_runner.request(
Expand Down
Loading

0 comments on commit a75ab14

Please sign in to comment.