Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Don't require hardcoded name in python_distribution.provides #17522

Merged
merged 1 commit into from Nov 10, 2022
Merged

Don't require hardcoded name in python_distribution.provides #17522

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
61 changes: 31 additions & 30 deletions src/python/pants/backend/python/goals/setup_py_test.py
View file
Open in desktop
Expand Up @@ -82,7 +82,7 @@ def create_setup_py_rule_runner(*, rules: Iterable) -> RuleRunner:
ResourcesGeneratorTarget,
FileTarget,
],
objects={"setup_py": PythonArtifact},
objects={"python_artifact": PythonArtifact},
)
rule_runner.set_options([], env_inherit={"PATH", "PYENV_ROOT", "HOME"})
return rule_runner
Expand Down Expand Up @@ -192,7 +192,7 @@ def test_use_existing_setup_script(chroot_rule_runner) -> None:
':setup',
],
generate_setup=False,
provides=setup_py(
provides=python_artifact(
name='foo', version='1.2.3',
)
)
Expand Down Expand Up @@ -272,7 +272,7 @@ def test_use_generate_setup_script_package_provenance_agnostic(chroot_rule_runne
'src/python/foo',
],
generate_setup=True,
provides=setup_py(
provides=python_artifact(
name='foo', version='1.2.3',
)
)
Expand Down Expand Up @@ -355,7 +355,7 @@ def test_generate_chroot(chroot_rule_runner: RuleRunner) -> None:
python_distribution(
name="baz-dist",
dependencies=[':baz'],
provides=setup_py(
provides=python_artifact(
name='baz',
version='1.1.1'
)
Expand Down Expand Up @@ -387,7 +387,7 @@ def test_generate_chroot(chroot_rule_runner: RuleRunner) -> None:
dependencies=[
':foo',
],
provides=setup_py(
provides=python_artifact(
name='foo', version='1.2.3'
),
entry_points={
Expand Down Expand Up @@ -465,8 +465,9 @@ def test_generate_chroot_entry_points(chroot_rule_runner: RuleRunner) -> None:
"qux":"foo.qux",
},
},
provides=setup_py(
name='foo', version='1.2.3',
provides=python_artifact(
name='foo',
version='1.2.3',
entry_points={
"console_scripts":{
"foo_qux":"foo.baz.qux:main",
Expand Down Expand Up @@ -533,7 +534,7 @@ def test_generate_long_description_field_from_file(chroot_rule_runner: RuleRunne
python_distribution(
name='foo-dist',
long_description_path="src/python/foo/readme.md",
provides=setup_py(
provides=python_artifact(
name='foo',
version='1.2.3',
)
Expand Down Expand Up @@ -574,7 +575,7 @@ def test_generate_long_description_field_from_file_already_having_it(
python_distribution(
name='foo-dist',
long_description_path="src/python/foo/readme.md",
provides=setup_py(
provides=python_artifact(
name='foo',
version='1.2.3',
long_description="Some long description.",
Expand Down Expand Up @@ -602,7 +603,7 @@ def test_generate_long_description_field_from_non_existing_file(
python_distribution(
name='foo-dist',
long_description_path="src/python/foo/readme.md",
provides=setup_py(
provides=python_artifact(
name='foo',
version='1.2.3',
)
Expand Down Expand Up @@ -631,7 +632,7 @@ def test_invalid_binary(chroot_rule_runner: RuleRunner) -> None:
pex_binary(name='invalid_entrypoint_unowned2', entry_point='invalid_binary.app2')
python_distribution(
name='invalid_bin1',
provides=setup_py(
provides=python_artifact(
name='invalid_bin1', version='1.1.1'
),
entry_points={
Expand All @@ -642,7 +643,7 @@ def test_invalid_binary(chroot_rule_runner: RuleRunner) -> None:
)
python_distribution(
name='invalid_bin2',
provides=setup_py(
provides=python_artifact(
name='invalid_bin2', version='1.1.1'
),
entry_points={
Expand All @@ -653,7 +654,7 @@ def test_invalid_binary(chroot_rule_runner: RuleRunner) -> None:
)
python_distribution(
name='invalid_bin3',
provides=setup_py(
provides=python_artifact(
name='invalid_bin3', version='1.1.1'
),
entry_points={
Expand Down Expand Up @@ -694,7 +695,7 @@ def test_binary_shorthand(chroot_rule_runner: RuleRunner) -> None:
pex_binary(name='bin', entry_point='app.py:func')
python_distribution(
name='dist',
provides=setup_py(
provides=python_artifact(
name='bin', version='1.1.1'
),
entry_points={
Expand Down Expand Up @@ -768,7 +769,7 @@ def assert_sources(
python_distribution(
name="dist",
dependencies=["{'","'.join(addr.spec for addr in addrs)}"],
provides=setup_py(name="foo", version="3.2.1"),
provides=python_artifact(name="foo", version="3.2.1"),
)
"""
),
Expand Down Expand Up @@ -887,7 +888,7 @@ def test_get_requirements() -> None:
python_distribution(
name='bar-dist',
dependencies=[':bar'],
provides=setup_py(name='bar', version='9.8.7'),
provides=python_artifact(name='bar', version='9.8.7'),
)

python_sources(dependencies=['src/python/foo/bar/baz', 'src/python/foo/bar/qux'])
Expand All @@ -900,7 +901,7 @@ def test_get_requirements() -> None:
name='corge-dist',
# Tests having a 3rdparty requirement directly on a python_distribution.
dependencies=[':corge', '3rdparty:ext3'],
provides=setup_py(name='corge', version='2.2.2'),
provides=python_artifact(name='corge', version='2.2.2'),
)

python_sources(dependencies=['src/python/foo/bar'])
Expand Down Expand Up @@ -965,7 +966,7 @@ def test_get_requirements_with_exclude() -> None:
python_distribution(
name='bar-dist',
dependencies=['!!3rdparty:ext2',':bar'],
provides=setup_py(name='bar', version='9.8.7'),
provides=python_artifact(name='bar', version='9.8.7'),
)

python_sources(dependencies=['src/python/foo/bar/baz', 'src/python/foo/bar/qux'])
Expand Down Expand Up @@ -1023,7 +1024,7 @@ def test_owned_dependencies() -> None:
python_distribution(
name='bar1-dist',
dependencies=[':bar1'],
provides=setup_py(name='bar1', version='1.1.1'),
provides=python_artifact(name='bar1', version='1.1.1'),
)

python_sources(
Expand All @@ -1046,7 +1047,7 @@ def test_owned_dependencies() -> None:
python_distribution(
name='foo-dist',
dependencies=[':foo'],
provides=setup_py(name='foo', version='3.4.5'),
provides=python_artifact(name='foo', version='3.4.5'),
)

python_sources(
Expand Down Expand Up @@ -1146,7 +1147,7 @@ def test_get_owner_simple(exporting_owner_rule_runner: RuleRunner) -> None:
python_distribution(
name='bar1',
dependencies=['src/python/foo/bar/baz:baz1'],
provides=setup_py(name='bar1', version='1.1.1'),
provides=python_artifact(name='bar1', version='1.1.1'),
)
python_sources(
name='bar2',
Expand All @@ -1161,13 +1162,13 @@ def test_get_owner_simple(exporting_owner_rule_runner: RuleRunner) -> None:
python_distribution(
name='foo1',
dependencies=['src/python/foo/bar/baz:baz2'],
provides=setup_py(name='foo1', version='0.1.2'),
provides=python_artifact(name='foo1', version='0.1.2'),
)
python_sources(name='foo2')
python_distribution(
name='foo3',
dependencies=['src/python/foo/bar:bar2'],
provides=setup_py(name='foo3', version='3.4.5'),
provides=python_artifact(name='foo3', version='3.4.5'),
)
"""
),
Expand Down Expand Up @@ -1222,7 +1223,7 @@ def test_get_owner_siblings(exporting_owner_rule_runner: RuleRunner) -> None:
python_distribution(
name='sibling2',
dependencies=['src/python/siblings:sibling1'],
provides=setup_py(name='siblings', version='2.2.2'),
provides=python_artifact(name='siblings', version='2.2.2'),
)
"""
),
Expand Down Expand Up @@ -1254,7 +1255,7 @@ def test_get_owner_not_an_ancestor(exporting_owner_rule_runner: RuleRunner) -> N
python_distribution(
name='bbb',
dependencies=['src/python/notanancestor/aaa'],
provides=setup_py(name='bbb', version='11.22.33'),
provides=python_artifact(name='bbb', version='11.22.33'),
)
"""
),
Expand Down Expand Up @@ -1282,7 +1283,7 @@ def test_get_owner_multiple_ancestor_generations(exporting_owner_rule_runner: Ru
python_distribution(
name='bbb',
dependencies=['src/python/aaa/bbb/ccc'],
provides=setup_py(name='bbb', version='1.1.1'),
provides=python_artifact(name='bbb', version='1.1.1'),
)
"""
),
Expand All @@ -1291,7 +1292,7 @@ def test_get_owner_multiple_ancestor_generations(exporting_owner_rule_runner: Ru
python_distribution(
name='aaa',
dependencies=['src/python/aaa/bbb/ccc'],
provides=setup_py(name='aaa', version='2.2.2'),
provides=python_artifact(name='aaa', version='2.2.2'),
)
"""
),
Expand Down Expand Up @@ -1367,15 +1368,15 @@ def test_no_dist_type_selected() -> None:
QueryRule(BuiltPackage, (PythonDistributionFieldSet,)),
],
target_types=[PythonDistribution],
objects={"setup_py": PythonArtifact},
objects={"python_artifact": PythonArtifact},
)
rule_runner.write_files(
{
"src/python/aaa/BUILD": textwrap.dedent(
"""
python_distribution(
name='aaa',
provides=setup_py(name='aaa', version='2.2.2'),
provides=python_artifact(name='aaa', version='2.2.2'),
wheel=False,
sdist=False
)
Expand Down Expand Up @@ -1412,7 +1413,7 @@ def test_too_many_interpreter_constraints(chroot_rule_runner: RuleRunner) -> Non
"""
python_distribution(
name='foo-dist',
provides=setup_py(
provides=python_artifact(
name='foo',
version='1.2.3',
)
Expand Down
25 changes: 2 additions & 23 deletions src/python/pants/backend/python/macros/python_artifact.py
View file
Open in desktop
Expand Up @@ -17,7 +17,7 @@ def _normalize_entry_points(
raise ValueError(
softwrap(
f"""
The `entry_points` in `setup_py()` must be a dictionary,
The `entry_points` in `python_artifact()` must be a dictionary,
but was {all_entry_points!r} with type {type(all_entry_points).__name__}.
"""
)
Expand All @@ -42,7 +42,7 @@ def _values_to_entry_points(values):
raise ValueError(
softwrap(
f"""
The values of the `entry_points` dictionary in `setup_py()` must be
The values of the `entry_points` dictionary in `python_artifact()` must be
a list of strings or a dictionary of string to string,
but got {values!r} of type {type(values).__name__}.
"""
Expand All @@ -62,19 +62,6 @@ def __init__(self, **kwargs):
:param kwargs: Passed to `setuptools.setup
<https://pythonhosted.org/setuptools/setuptools.html>`_.
"""
if "name" not in kwargs:
raise ValueError("`setup_py()` requires `name` to be specified.")
name = kwargs["name"]
if not isinstance(name, str):
raise ValueError(
softwrap(
f"""
The `name` in `setup_py()` must be a string, but was {repr(name)} with type
{type(name)}.
"""
)
)

if "entry_points" in kwargs:
# coerce entry points from Dict[str, List[str]] to Dict[str, Dict[str, str]]
kwargs["entry_points"] = _normalize_entry_points(kwargs["entry_points"])
Expand All @@ -87,11 +74,6 @@ def __init__(self, **kwargs):
# Instead we stringify and precompute a hash to use in our own __hash__, since we know
# that this object is immutable.
self._hash: int = hash(json.dumps(kwargs, sort_keys=True))
self._name: str = name

@property
def name(self) -> str:
Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran all the tests, and manually built some dists, to verify that this property isn't used.

return self._name

@property
def kwargs(self) -> Dict[str, Any]:
Expand All @@ -104,6 +86,3 @@ def __eq__(self, other: Any) -> bool:

def __hash__(self) -> int:
return self._hash

def __str__(self) -> str:
Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran all the tests, and manually built some dists, with this __str__ method modified to raise an exception, thus verifying that it's not actually called, let alone does it matter what this stringifies to.

return self.name
4 changes: 2 additions & 2 deletions src/python/pants/backend/python/macros/python_artifact_test.py
View file
Open in desktop
Expand Up @@ -41,7 +41,7 @@
pytest.raises(
ValueError,
match=re.escape(
r"The `entry_points` in `setup_py()` must be a dictionary, but was ['not=ok'] with type list."
r"The `entry_points` in `python_artifact()` must be a dictionary, but was ['not=ok'] with type list."
),
),
),
Expand All @@ -61,7 +61,7 @@
pytest.raises(
ValueError,
match=re.escape(
r"The values of the `entry_points` dictionary in `setup_py()` must be a list of strings "
r"The values of the `entry_points` dictionary in `python_artifact()` must be a list of strings "
r"or a dictionary of string to string, but got 'whops = this.is.a:mistake' of type str."
),
),
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/python/target_types.py
View file
Open in desktop
Expand Up @@ -1325,7 +1325,7 @@ class PythonDistributionDependenciesField(Dependencies):
class PythonProvidesField(ScalarField, AsyncFieldMixin):
alias = "provides"
expected_type = PythonArtifact
expected_type_help = "setup_py(name='my-dist', **kwargs)"
expected_type_help = "python_artifact(name='my-dist', **kwargs)"
value: PythonArtifact
required = True
help = softwrap(
Expand Down