Skip to content

Commit

Permalink
Support building dists from preexisting setup.py.
Browse files Browse the repository at this point in the history
[ci skip-rust]

[ci skip-build-wheels]
  • Loading branch information
benjyw committed Jun 27, 2021
1 parent dc59df8 commit ba0761a
Show file tree
Hide file tree
Showing 2 changed files with 122 additions and 18 deletions.
59 changes: 44 additions & 15 deletions src/python/pants/backend/python/goals/setup_py.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,27 +71,26 @@
logger = logging.getLogger(__name__)


class InvalidSetupPyArgs(Exception):
class SetupPyError(Exception):
def __init__(self, msg: str):
super().__init__(f"{msg} See {bracketed_docs_url('python-distributions')}.")


class InvalidSetupPyArgs(SetupPyError):
"""Indicates invalid arguments to setup.py."""


class TargetNotExported(Exception):
class TargetNotExported(SetupPyError):
"""Indicates a target that was expected to be exported is not."""


class InvalidEntryPoint(Exception):
class InvalidEntryPoint(SetupPyError):
"""Indicates that a specified binary entry point was invalid."""


class OwnershipError(Exception):
class OwnershipError(SetupPyError):
"""An error related to target ownership calculation."""

def __init__(self, msg: str):
super().__init__(
f"{msg} See {bracketed_docs_url('python-distributions')} for "
f"how python_library targets are mapped to distributions."
)


class NoOwnerError(OwnershipError):
"""Indicates an exportable target has no owning exported target."""
Expand Down Expand Up @@ -201,8 +200,14 @@ def __init__(
self, kwargs: Mapping[str, Any], *, address: Address, _allow_banned_keys: bool = False
) -> None:
super().__init__()
if "name" not in kwargs:
raise InvalidSetupPyArgs(
f"Missing a `name` kwarg in the `provides` field for {address}."
)
if "version" not in kwargs:
raise ValueError(f"Missing a `version` kwarg in the `provides` field for {address}.")
raise InvalidSetupPyArgs(
f"Missing a `version` kwarg in the `provides` field for {address}."
)

if not _allow_banned_keys:
for arg in {
Expand Down Expand Up @@ -274,9 +279,14 @@ def __init__(self, kwargs: Mapping[str, Any], *, address: Address) -> None:

@dataclass(frozen=True)
class SetupPyChroot:
"""A chroot containing a generated setup.py and the sources it operates on."""
"""A chroot containing a setup.py and the sources it operates on."""

digest: Digest
setup_script: str # The path of the setup script within the digest.

# Note that this field is merely informational, used primarily in tests, and
# to construct the chroot name from the name and version kwargs, when setup is
# run with no args.
setup_kwargs: FinalizedSetupKwargs


Expand Down Expand Up @@ -350,7 +360,7 @@ def validate_commands(commands: Tuple[str, ...]):
# TODO: A `publish` rule, that can invoke Twine to do the actual uploading.
# See https://github.com/pantsbuild/pants/issues/8935.
if "upload" in commands or "register" in commands:
raise InvalidSetupPyArgs("Cannot use the `upload` or `register` setup.py commands")
raise InvalidSetupPyArgs("Cannot use the `upload` or `register` setup.py commands.")


@rule
Expand Down Expand Up @@ -422,7 +432,7 @@ async def run_setup_py(req: RunSetupPyRequest, setuptools: Setuptools) -> RunSet
ProcessResult,
VenvPexProcess(
setuptools_pex,
argv=("setup.py", *req.args),
argv=(req.chroot.setup_script, *req.args),
input_digest=req.chroot.digest,
# setuptools commands that create dists write them to the distdir.
# TODO: Could there be other useful files to capture?
Expand Down Expand Up @@ -490,6 +500,23 @@ async def generate_chroot(request: SetupPyChrootRequest) -> SetupPyChroot:
target = exported_target.target
resolved_setup_kwargs = await Get(SetupKwargs, ExportedTarget, exported_target)
setup_kwargs = resolved_setup_kwargs.kwargs.copy()

setup_script = setup_kwargs.pop("setup_script", None)
if setup_script:
# The target points to an existing setup.py script, so use that.
invalid_keys = set(setup_kwargs.keys()) - {"name", "version"}
if invalid_keys:
raise InvalidSetupPyArgs(
f"The `provides` field in {exported_addr} specifies a setup_script kwarg, so it "
f"must only specify the name and version kwargs, but it also specified "
f"{','.join(sorted(invalid_keys))}."
)
return SetupPyChroot(
sources.digest, setup_script, FinalizedSetupKwargs(setup_kwargs, address=target.address)
)

# There is no existing setup.py script, so we generate one.

# NB: We are careful to not overwrite these values, but we also don't expect them to have been
# set. The user must have have gone out of their way to use a `SetupKwargs` plugin, and to have
# specified `SetupKwargs(_allow_banned_keys=True)`.
Expand Down Expand Up @@ -562,7 +589,9 @@ async def generate_chroot(request: SetupPyChrootRequest) -> SetupPyChroot:
)

chroot_digest = await Get(Digest, MergeDigests((src_digest, extra_files_digest)))
return SetupPyChroot(chroot_digest, FinalizedSetupKwargs(setup_kwargs, address=target.address))
return SetupPyChroot(
chroot_digest, "setup.py", FinalizedSetupKwargs(setup_kwargs, address=target.address)
)


@rule
Expand Down
81 changes: 78 additions & 3 deletions src/python/pants/backend/python/goals/setup_py_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,11 @@ def is_applicable(cls, _) -> bool:

@rule
def setup_kwargs_plugin(request: PluginSetupKwargsRequest) -> SetupKwargs:
return SetupKwargs(
{**request.explicit_kwargs, "plugin_demo": "hello world"}, address=request.target.address
)
if "setup_script" in request.explicit_kwargs:
kwargs = request.explicit_kwargs
else:
kwargs = {**request.explicit_kwargs, "plugin_demo": "hello world"}
return SetupKwargs(kwargs, address=request.target.address)


@pytest.fixture
Expand Down Expand Up @@ -131,6 +133,79 @@ def assert_chroot_error(rule_runner: RuleRunner, addr: Address, exc_cls: Type[Ex
assert type(ex.wrapped_exceptions[0]) == exc_cls


def test_use_existing_setup_script(chroot_rule_runner) -> None:
chroot_rule_runner.add_to_build_file("src/python/foo/bar", "python_library()")
chroot_rule_runner.create_file("src/python/foo/bar/__init__.py")
chroot_rule_runner.create_file("src/python/foo/bar/bar.py")
# Add a `.pyi` stub file to ensure we include it in the final result.
chroot_rule_runner.create_file("src/python/foo/bar/bar.pyi")
chroot_rule_runner.add_to_build_file(
"src/python/foo/resources", 'resources(sources=["js/code.js"])'
)
chroot_rule_runner.create_file("src/python/foo/resources/js/code.js")
chroot_rule_runner.add_to_build_file("files", 'files(sources=["README.txt"])')
chroot_rule_runner.create_file("files/README.txt")
chroot_rule_runner.add_to_build_file(
"src/python/foo",
textwrap.dedent(
"""
python_distribution(
name='foo-dist',
dependencies=[
':foo',
],
provides=setup_py(
setup_script='src/python/foo/setup.py',
name='foo', version='1.2.3'
)
)
python_library(
dependencies=[
'src/python/foo/bar',
'src/python/foo/resources',
'files',
]
)
"""
),
)
chroot_rule_runner.create_file("src/python/foo/__init__.py", _namespace_decl)
chroot_rule_runner.create_file("src/python/foo/foo.py")
chroot_rule_runner.create_file(
"src/python/foo/setup.py",
textwrap.dedent(
"""
from setuptools import setup
setup(
name = "foo",
version = "1.2.3",
packages = ["foo"],
)
"""
),
)
assert_chroot(
chroot_rule_runner,
[
"files/README.txt",
"foo/bar/__init__.py",
"foo/bar/bar.py",
"foo/bar/bar.pyi",
"foo/resources/js/code.js",
"foo/__init__.py",
"foo/foo.py",
"foo/setup.py",
],
{
"name": "foo",
"version": "1.2.3",
},
Address("src/python/foo", target_name="foo-dist"),
)


def test_generate_chroot(chroot_rule_runner: RuleRunner) -> None:
chroot_rule_runner.add_to_build_file(
"src/python/foo/bar/baz",
Expand Down

0 comments on commit ba0761a

Please sign in to comment.