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

build-style/python3-pep517: use a generic glob for wheels #44071

Closed
wants to merge 4 commits into from

Conversation

tornaria
Copy link
Contributor

We replace the current glob of "dist/${wheelbase//-/_}-${version}-*-*-*.whl" for a much simpler dist/*.whl. The former is inconvenient since wheelbase="${pkgname#python3-}" is most of the time correct but often not, and fixing that in a different way seems more complicated than this solution.

Since we only run python -m build once, we should have only one wheel, so trying to be more specific doesn't seem useful. Nevertheless, this can still be overriden via make_install_target as before but hopefully it won't ever be necessary.

Note that several packages that currently need to set make_install_target for this purposes will now work out of the box.

If this is accepted I can have a look at those pkgs and clean up the override once it's no longer necessary.

Testing the changes

  • I tested the changes in this PR: YES

I built all 150 packages that use pep517 as obtained by:

$ git grep -l style=.*pep517 srcpkgs/ | cut -d/ -f2 | wc -l
150

All of them succeed except for two that fail for unrelated reasons (python3-xcffib and synapse).

I also included a minor unrelated change: do not compile bytecode in do_install() since it will be removed in post_install().

CC: @ahesford @icp1994

tornaria and others added 3 commits May 24, 2023 19:51
Also: do not compile bytecode in `do_install()` since
it will be removed in `post_install()`.
build-style/python3-pep517: use a generic glob for wheels, refactor

Co-authored-by: Gonzalo Tornaría <tornaria@cmat.edu.uy>
Co-authored-by: Andrew J. Hesford <ajh@sideband.org>
Now that the wheel glob in do_check and do_install is more generic,
there is no longer a need to override the default behavior.
@ahesford
Copy link
Member

I have made a couple of changes:

  1. A squash commit refactors do_check: a) to short-circuit checks when pytest is not installed, avoiding the need to nest the bulk of the function inside a conditional; b) to avoid setting make_install_target in do_check when it is undefined in the template, deferring the default assignment to do_install.
  2. Remove the now-superfluous make_install_target in all PEP517 templates.
  3. Fix the build of synapse and (by updating) pythone3-xcffib.

Please confirm that you are OK with these changes, and we can squash down my alterations before merge.

@icp1994
Copy link
Contributor

icp1994 commented May 25, 2023

For synapse, there is a update PR #43815 which includes the raised setuptools_rust bound from upstream

@ahesford
Copy link
Member

Thanks; I dropped the synapse fix.

@tornaria
Copy link
Contributor Author

LGTM, thanks! I made a minor suggestion so the style in do_install() matches the style in do_check(). I prefer the other way (setting default values at the top of the function) since that acts as documentation and makes it much more clear what are default values, etc. and it's a style used throughout.

@ahesford ahesford closed this in 1267988 May 25, 2023
@tornaria tornaria deleted the pep517 branch May 25, 2023 17:43
sirkhancision pushed a commit to sirkhancision/void-packages that referenced this pull request Jun 10, 2023
Also: do not compile bytecode in `do_install()` since it will be removed
in `post_install()`.

Co-authored-by: Gonzalo Tornaría <tornaria@cmat.edu.uy>
Co-authored-by: Andrew J. Hesford <ajh@sideband.org>

Closes: void-linux#44071.
sirkhancision pushed a commit to sirkhancision/void-packages that referenced this pull request Jun 12, 2023
Also: do not compile bytecode in `do_install()` since it will be removed
in `post_install()`.

Co-authored-by: Gonzalo Tornaría <tornaria@cmat.edu.uy>
Co-authored-by: Andrew J. Hesford <ajh@sideband.org>

Closes: void-linux#44071.
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.

None yet

3 participants