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

📦 Move packaging to PEP 517 in-tree backend #893

Merged
merged 1 commit into from
Nov 16, 2023

Conversation

webknjaz
Copy link
Member

@webknjaz webknjaz commented Jun 15, 2023

This essentially allows the cythonization opt-out to be controlled by the --pure-python PEP 517 config setting that can be passed to the corresponding build frontends via their respective CLIs.

Are there changes in behavior for the user?

Yes, the users, and packagers are now supposed to explicitly pass the --pure-python config setting to their PEP 517 build frontends in use, when they need to make pure-python wheels.

$ python -m pip install . --config-settings=--pure-python=
$ python -m pip install .

$ python -m pip install -e . --config-settings=--pure-python=
$ python -m pip install -e .

$ python -m pip wheel . --no-deps --config-settings=--pure-python=
$ python -m pip wheel . --no-deps

$ python -m build --config-settings=--pure-python=
$ python -m build

$ python -m build --wheel --config-settings=--pure-python=
$ python -m build --wheel

Related issue number

N/A

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • Add a new news fragment into the CHANGES folder
    • name it <issue_id>.<type> (e.g. 588.bugfix)
    • if you don't have an issue_id change it to the pr id after creating the PR
    • ensure type is one of the following:
      • .feature: Signifying a new feature.
      • .bugfix: Signifying a bug fix.
      • .doc: Signifying a documentation improvement.
      • .removal: Signifying a deprecation or removal of public API.
      • .misc: A ticket has been closed, but it is not of interest to users.
    • Make sure to use full sentences with correct case and punctuation, for example: Fix issue with non-ascii contents in doctest text files.

pyproject.toml Outdated Show resolved Hide resolved
packaging/pep517_backend/hooks.py Dismissed Show dismissed Hide dismissed
packaging/pep517_backend/_backend.py Dismissed Show dismissed Hide dismissed
Copy link
Member Author

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

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

NOTE: for some reason, pure-python wheels get an .so file too. Needs investigation.

@codecov
Copy link

codecov bot commented Jun 15, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b81a868) 99.74% compared to head (70b26b6) 99.74%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #893   +/-   ##
=======================================
  Coverage   99.74%   99.74%           
=======================================
  Files           4        4           
  Lines         772      772           
  Branches      219      219           
=======================================
  Hits          770      770           
  Misses          2        2           
Flag Coverage Δ
unit 99.61% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@webknjaz
Copy link
Member Author

@mjpieters this still needs work, but you're welcome to poke around. Also, I'm not going to fix linting errors right now since the formatters are too invasive and make incorrect changes sometimes.

@mjpieters
Copy link
Contributor

Also, I'm not going to fix linting errors right now since the formatters are too invasive and make incorrect changes sometimes.

That's fine for now. I'd be interested in examples of incorrect changes, if you have any?

@webknjaz webknjaz self-assigned this Sep 20, 2023
@webknjaz webknjaz force-pushed the packaging/pep517-in-tree-backend branch from d20e7a4 to c610139 Compare November 15, 2023 01:28
@webknjaz
Copy link
Member Author

Also, I'm not going to fix linting errors right now since the formatters are too invasive and make incorrect changes sometimes.

That's fine for now. I'd be interested in examples of incorrect changes, if you have any?

@mjpieters like moving noqa who-knows-where, so they don't work:

-from ._backend import (  # noqa: WPS436  # Re-exporting PEP 517 hooks
+from ._backend import (  # Re-exporting PEP 517 hooks

Comment on lines 15 to 19
from ._backend import ( # noqa: WPS436 # Re-exporting PEP 660 hooks
build_editable,
get_requires_for_build_editable,
prepare_metadata_for_build_editable,
)

Check notice

Code scanning / CodeQL

Unused import Note

Import of 'build_editable' is not used.
Import of 'get_requires_for_build_editable' is not used.
Import of 'prepare_metadata_for_build_editable' is not used.
@webknjaz
Copy link
Member Author

Status update: I made the packaging work. Will have to polish stuff before undrafting the PR. But in general, I'm pretty happy with the interface here.

@webknjaz
Copy link
Member Author

So while working on this, I realized that introducing a --build-c-extensions inverses the default behavior of building C-extensions, making it an opt-in rather than an opt-out.
With that in mind, I think I'll replace it by --pure-python and preserve the opt-out way.

webknjaz added a commit to webknjaz/yarl that referenced this pull request Nov 16, 2023
@webknjaz webknjaz force-pushed the packaging/pep517-in-tree-backend branch from 21e2abe to 7d1836b Compare November 16, 2023 02:09
@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Nov 16, 2023
@webknjaz webknjaz force-pushed the packaging/pep517-in-tree-backend branch 2 times, most recently from 08362f4 to 93a5c4d Compare November 16, 2023 03:07
@webknjaz webknjaz marked this pull request as ready for review November 16, 2023 03:07
@webknjaz webknjaz enabled auto-merge (squash) November 16, 2023 03:08
@webknjaz webknjaz force-pushed the packaging/pep517-in-tree-backend branch 2 times, most recently from 92f59a8 to 18d49b2 Compare November 16, 2023 03:19
This essentially allows the cythonization opt-out be controlled by the
`--pure-python` PEP 517 config setting that can be passed to
the corresponding build frontends via their respective CLIs.
@webknjaz webknjaz force-pushed the packaging/pep517-in-tree-backend branch from 18d49b2 to 70b26b6 Compare November 16, 2023 15:31
@webknjaz webknjaz merged commit 98eac52 into aio-libs:master Nov 16, 2023
35 checks passed
webknjaz added a commit that referenced this pull request Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot:chronographer:provided There is a change note present in this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants