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.

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

Initial support for building dists using mypyc. #15380

Merged
merged 6 commits into from
May 10, 2022
Merged

Conversation

benjyw
Copy link
Sponsor Contributor

@benjyw benjyw commented May 10, 2022

Handles setting up mypy and any necessary config, requirements
and type stubs, so that a preexisting setup.py can call
mypycify().

Takes advantage of our existing robust support for mypy.
Lays groundwork for supporting other special build-time
requirements, such as cython.

Does not handle supporting mypyc in generated setup.py files.
That will be provided in a future change.

Also fixes a bug where we did not set the sys.path correctly
when running a preexisting setup.py that imported code from
a different source root.

Also switches a test to use provides=python_artifact(), which
is what we document, instead of the older alias setup_py().

[ci skip-rust]

[ci skip-build-wheels]

Handles setting up mypy and any necessary config, requirements
and type stubs, so that a preexisting setup.py can call
mypycify().

Does not handle supporting mypyc in generated setup.py files.
That will be provided in a future change.

Also fixes a bug where we did not set the sys.path correctly
when running a preexisting setup.py that imported code from
a different source root.

Also switches a test to use provides=python_artifact(), which
is what we document, instead of the older alias setup_py().

[ci skip-rust]

[ci skip-build-wheels]
@benjyw
Copy link
Sponsor Contributor Author

benjyw commented May 10, 2022

Related: #14752, which links to related and complementary work from @sureshjoshi, to whom thanks for ideas and inspiration for this.

# Get any extra build-time environment (e.g., native extension requirements).
build_env_requests = []
build_env_request_types = union_membership[DistBuildEnvironmentRequest]
for build_env_request_type in build_env_request_types:
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.

Note that, hypothetically at least, we support multiple build environments. E.g., a single setup.py could use mypyc and (in the future) cython...

@benjyw
Copy link
Sponsor Contributor Author

benjyw commented May 10, 2022

Still need to add a test, but sending this out for an early look. Now there is a test.

benjyw added 4 commits May 9, 2022 20:51
# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
[ci skip-rust]

[ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
@Eric-Arellano
Copy link
Contributor

Also fixes a bug where we did not set the sys.path correctly
when running a preexisting setup.py that imported code from
a different source root.

Should this be cherry-picked to 2.11 or 2.12?

Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

That's pretty sweet!

@@ -401,25 +425,66 @@ async def package_python_dist(
),
)

# Find the source roots for the build-time 1stparty deps (e.g., deps of setup.py).
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a pretty big rule, consider factoring out a @rule_helper. (Not sure that's the right call)

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.

This whole file is huge and needs refactoring, but I will hold off on that for now.

class UsesMyPycField(BoolField):
alias = "uses_mypyc"
default = False
help = "If true, this distribution is built using mypyc."
Copy link
Contributor

Choose a reason for hiding this comment

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

What else would be relevant to a user? For example

  • do we expect a hand-written setup.py / pyproject.toml? If so, with what content?
  • clarifying it uses the version of mypy from [mypy].version

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.

Added more detail.

# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
@benjyw benjyw merged commit 56fb3b2 into pantsbuild:main May 10, 2022
@benjyw benjyw deleted the mypyc branch May 10, 2022 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants