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

Support building mypycified mypy with PEP517 interface #13445

Merged
merged 10 commits into from
Sep 17, 2022
Merged

Conversation

emmatyping
Copy link
Collaborator

@emmatyping emmatyping commented Aug 18, 2022

Description

This is part one of 2 PRs which enable building wheels for WebAssembly, specifically using pyodide build. That tool just wraps build which is a PEP 517 package builder.

This allows you to use build or pip and pass --use-mypyc so the wheel is built with compiled mypy.

Test Plan

Examples of how to test this:

  1. warning: very slow, see the second version for a faster way to test this
$ pip wheel --config-settings="--build-option=--use-mypyc" .

(The way to parse the above, for the curious, --config-settings is a pip argument, --build-option goes to setuptools...)

  1. Alternatively and a lot faster to build the wheel:
$ pip install build
$ python -m build --config-settings="--build-option=--use-mypyc"

The downside of this PR is that we have to move things from build-requirements.txt to pyproject.toml, but I don't think that is an issue? A few extra packages (that are all small) shouldn't be a big problem.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

pyproject.toml Outdated Show resolved Hide resolved
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@github-actions

This comment has been minimized.

@emmatyping
Copy link
Collaborator Author

Ah, we kinda require type requirements to be installed in the environment at test time for linting ourselves, so they will need to be included outside of build-system.requires, so really this PR will be duplicating build-requirements.txt and mypy-requirements.txt into build-system.requires, which is kinda unfortunate, but I can't think of a better way.

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

hauntsaninja added a commit that referenced this pull request Sep 16, 2022
### Description

This PR tweaks two things about how mypyc generates and builds C code to
better support WebAssembly. First, we search `sysconfig` for the size of
`size_t`, which works much better for cross-compiling. Second, newer
versions of clang have `-Wno-unused-but-set-variable` and so it is added
to the default list of arguments (this should probably land regardless
the decision on merging this PR).

## Test Plan

This PR depends on #13445. To test
this PR, you can do the following:

*assuming mypy checkout with both PRs applied, must be on Python
3.10(!)*
```
$ pip install pyodide-build
$ pyodide build --exports pyinit backend-args --global-option=--use-mypyc
```
Note: you will get a warning about using `--global-option`, you can
ignore it for now. I'm trying to find out why `--build-option` isn't
working...

Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

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

Could you add the "keep in sync" comment to build-requirements.txt as well?

@henryiii
Copy link
Contributor

Have you tried adding cibw 2.10’s new config setting support?

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Sep 17, 2022

Thanks for the tip! When we use cibuildwheel, we get cibuildwheel to set the MYPY_USE_MYPYC env var.

See:

@henryiii
Copy link
Contributor

I kind of want to make sure the new feature works ;). Could it be tested in a PR? just env["CIBW_CONFIG_SETTINS"] = "--build-option=--use-mypyc" would work, I think.

(Also, pyproject.toml or other toml file static configuration support was added after build_wheel.py was written, and I think it complishes the same goal, avoiding extensive configuration in vendor specific ci files and supporting local builds.)

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

emmatyping added a commit to emmatyping/mypy_mypyc-wheels that referenced this pull request Sep 17, 2022
This will not work until python/mypy#13445 is merged.
@hauntsaninja hauntsaninja merged commit d619c78 into master Sep 17, 2022
@hauntsaninja hauntsaninja deleted the pep517 branch September 17, 2022 06:37
@hauntsaninja
Copy link
Collaborator

Yeah, would be nice to have https://github.com/mypyc/mypy_mypyc-wheels use cibuildwheel's new configuration stuff. If you want to make a PR to that repo, I'll review it. Otherwise it might take me a little bit of time to get to this :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants