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

[WIP] wheel.mk: Migrate to using status cookie #6389

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

th0ma7
Copy link
Contributor

@th0ma7 th0ma7 commented Jan 13, 2025

Description

Migrate to using status cookie

Follow-up to #6282, in particular #6282 (comment)

Checklist

  • Build rule all-supported completed successfully
  • New installation of package completed successfully
  • Package upgrade completed successfully (Manually install the package again)
  • Package functionality was tested
  • Any needed documentation is updated/created

Type of change

  • Bug fix
  • New Package
  • Package update
  • Includes small framework changes
  • This change requires a documentation update (e.g. Wiki)

TODO

  • Allow using WHEELS="<name>-<version>" make wheel-<arch>-<tcversion>
  • Manage end-of-line comments in requirements files
  • Allow downloading to distrib/pip all types (abi3, crossenv, pure)
  • Allow downloading wheels part of the github-action preparation phase
  • Update python wheel wiki documentation

@th0ma7 th0ma7 changed the title wheel.mk: Migrate to using status cookie [WIP] wheel.mk: Migrate to using status cookie Jan 13, 2025
@th0ma7 th0ma7 self-assigned this Jan 13, 2025
@th0ma7
Copy link
Contributor Author

th0ma7 commented Jan 13, 2025

Functionality for wheel_download now migrated to status cookie, next to be wheel_compile then wheel_install which shouldn't be too long by the look of things.

@hgy59
Copy link
Contributor

hgy59 commented Jan 14, 2025

@th0ma7 it would be convenient having a wheelclean-% target to rebuilt a specific wheel like:

make wheelclean-numpy-1.26.4

just my 5 cents 🪙

@th0ma7
Copy link
Contributor Author

th0ma7 commented Jan 15, 2025

@hgy59 make wheelclean-<wheel>-<version> functionality added. Also the code is now fully migrated to using status cookies for all wheel builds which should have a significant impact on build time (if not from a github-action it will from a debugging and testing perspective).

If you have a moment to test things up would be great. Similarly to crossenv I've now added a make wheel-<arch>-<tcversion> call. It can be extended to:

make WHEELS="<name>-<version>" wheel-<arch>-<tcversion>

This will add that "wheel" to the list of wheels to process. Noting that Makefile first WHEELS entry, if using = instead of += will discard argument passed in parameter. Although for the sake of testing, commenting out WHEELS entries from your makefile will allow you to test-build a wheel you are struggling with. So the following should help a bit for testing and debugging purpose:

make wheelclean-<name>-<version>
make wheelcleancache
make WHEELS="<name>-<version>" wheel-<arch>-<tcversion>

I'm sure there will be a few rough-edges and missing is the ability to download only for using with github-action (next on my TODO).

@th0ma7 th0ma7 requested a review from hgy59 January 15, 2025 23:41
# post_wheel_compile_target (override with POST_WHEEL_COMPILE_TARGET)
# Variables:
# REQUIREMENT Requirement formatted wheel information
# WHEEL_NAME Name of wheel to process
Copy link
Contributor

Choose a reason for hiding this comment

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

how to document, that WHEEL_NAME must not contain -?
Since name and version are separated by - the name must not contain -.

can we add a check here
or fail if name has - instead of _
or implement an auto renaming?

or can we still use = as version separator? (can't remeber which make file is affected).

Comment on lines +55 to +60
# $(MSG) requirement: [$(REQUIREMENT)] ; \
# $(MSG) requirement-grep-egg: [$$(grep -s egg <<< $(REQUIREMENT))] ; \
# $(MSG) name: [$(WHEEL_NAME)] ; \
# $(MSG) type: [$(WHEEL_TYPE)] ; \
# $(MSG) version: [$(WHEEL_VERSION)] ; \
# $(MSG) type: [$(WHEEL_TYPE)] ; \
Copy link
Contributor

Choose a reason for hiding this comment

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

If this does not work, it might all be commented out (\ wraps all on a single line, and all following the first # is comment)

just a guess

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.

2 participants