Skip to content

Commit

Permalink
Remove support for referencing 2nd owner via file. (pantsbuild#19400)
Browse files Browse the repository at this point in the history
  • Loading branch information
jsirois authored Jun 30, 2023
1 parent 4023643 commit 0468cce
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 23 deletions.
2 changes: 1 addition & 1 deletion src/python/pants/base/exiter_integration_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def test_unicode_containing_exception(use_pantsd: bool) -> None:
[
"--backend-packages=pants.backend.python",
"run",
os.path.join(tmpdir, "exiter_integration_test_harness", "main.py"),
os.path.join(tmpdir, "exiter_integration_test_harness:bin"),
],
use_pantsd=use_pantsd,
)
Expand Down
24 changes: 9 additions & 15 deletions src/python/pants/engine/internals/specs_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from typing import Iterable

from pants.backend.project_info.filter_targets import FilterSubsystem
from pants.base.deprecated import warn_or_error
from pants.base.deprecated import CodeRemovedError
from pants.base.specs import (
AddressLiteralSpec,
AncestorGlobSpec,
Expand Down Expand Up @@ -488,7 +488,7 @@ def __init__(


# NB: Remove when SecondaryOwnerMixin is removed
def _maybe_warn_deprecated_secondary_owner_semantics(
def _maybe_error_secondary_owner_semantics(
addresses_from_nonfile_specs: Addresses,
owners_from_filespecs: Owners,
matched_addresses: collections.abc.Set[Address],
Expand All @@ -511,22 +511,16 @@ def _maybe_warn_deprecated_secondary_owner_semantics(
}

if problematic_target_specs:
warn_or_error(
# TODO(Joshua Cannon): removal at 2.18.0.dev3
removal_version="2.18.0.dev3",
entity=softwrap(
"""
indirectly referring to a target by using a corresponding file argument, when the
target owning the file isn't applicable
"""
),
hint=softwrap(
raise CodeRemovedError(
softwrap(
f"""
Refer to the following targets by their addresses:
Support for indirectly referring to a target by using a corresponding file argument,
when the target owning the file isn't applicable was removed in version 2.18.0.dev3.
Refer to the following targets by their addresses:
{bullet_list(sorted(problematic_target_specs))}
"""
),
)
)


Expand Down Expand Up @@ -580,7 +574,7 @@ async def find_valid_field_sets_for_target_roots(

# NB: Remove when SecondaryOwnerMixin is removed
if targets_to_applicable_field_sets:
_maybe_warn_deprecated_secondary_owner_semantics(
_maybe_error_secondary_owner_semantics(
# NB: All of these should be memoized, so it's not inappropriate to request simply for warning sake.
*(
await MultiGet(
Expand Down
16 changes: 9 additions & 7 deletions src/python/pants/engine/internals/specs_rules_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1124,14 +1124,16 @@ def run_rule(specs: Iterable[Spec]):
assert "Refer to the following targets" not in caplog.text
assert not result.mapping

run_rule([FileLiteralSpec("a.ft")])
assert len(caplog.records) == 1
assert "Refer to the following targets by their addresses:\n\n * //:secondary1" in caplog.text
with pytest.raises(ExecutionError) as exc:
run_rule([FileLiteralSpec("a.ft")])
assert len(caplog.records) == 0
assert "Refer to the following targets by their addresses:\n * //:secondary1" in str(exc.value)

run_rule([FileLiteralSpec("a.ft"), AddressLiteralSpec("", "secondary1")])
assert len(caplog.records) == 1
assert "secondary1" not in caplog.text
assert "secondary2" in caplog.text
with pytest.raises(ExecutionError) as exc:
run_rule([FileLiteralSpec("a.ft"), AddressLiteralSpec("", "secondary1")])
assert len(caplog.records) == 0
assert "secondary1" not in str(exc.value)
assert "secondary2" in str(exc.value)

result = run_rule([RecursiveGlobSpec("")])
assert len(caplog.records) == 0
Expand Down

0 comments on commit 0468cce

Please sign in to comment.