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

Drop using setup_requires. #98

Merged
merged 2 commits into from
Apr 20, 2023
Merged

Drop using setup_requires. #98

merged 2 commits into from
Apr 20, 2023

Conversation

icemac
Copy link
Member

@icemac icemac commented Apr 18, 2023

This is due to constant problems on GHA for MacOS leading to the error of a not empty directory during build if there is no wheel.

@icemac icemac self-assigned this Apr 18, 2023
@icemac icemac marked this pull request as ready for review April 19, 2023 06:21
@icemac
Copy link
Member Author

icemac commented Apr 19, 2023

I am planning to release theses changes as a dev release as Python 3.12 support is currently not working. But it should unblock the building error on AccessControl.

Copy link
Member

@dataflake dataflake left a comment

Choose a reason for hiding this comment

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

I'll keep my fingers crossed.

@dataflake
Copy link
Member

To get the most realistic test you should only release a .tar.gz source release and not prebuild eggs. The dependent package must be forced to build its own wheel from that.

Copy link
Member

@mgedmin mgedmin left a comment

Choose a reason for hiding this comment

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

lgtm

@d-maurer
Copy link
Contributor

d-maurer commented Apr 19, 2023 via email

Copy link
Contributor

@d-maurer d-maurer left a comment

Choose a reason for hiding this comment

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

I think we still use a prebuilt wheel (dist/zope.security-6.2.dev0-cp312-cp312-linux_x86_64.whl) rather than the git checkout. I believe that this explains the different behavior between the GHA test and a local built.

I suggest to prevent the use of the wheel (even though I do not know how; maybe delete it on PyPI) and I hope that we will then get identical behavior between the GHA test and a local test (and maybe the ubuntu problem disappears).

@dataflake
Copy link
Member

dataflake commented Apr 19, 2023

Dieter we are talking about two different things. I do not care about the Ubuntu error. I am only talking about wheels failing to build on macOS with the "directory not empty" error.

Example: https://github.com/zopefoundation/AccessControl/actions/runs/4730992430/jobs/8395395407

@icemac
Copy link
Member Author

icemac commented Apr 20, 2023

@d-maurer zope.security-6.2.dev0-cp312-cp312-linux_x86_64.whl is not on PyPI. The whole idea behind the GHA workflow to build the packages containing C code is to build the wheel in a first step and later on use that wheel to install it for the test run and to publish it to PyPI in case of a release commit.

@icemac
Copy link
Member Author

icemac commented Apr 20, 2023

@dataflake wrote:

To get the most realistic test you should only release a .tar.gz source release and not prebuild eggs. The dependent package must be forced to build its own wheel from that.

As the problem only occurs on Python 3.12, we are lucky with our current toolchain: it never releases a wheel for a future Python version as the ABI might not yet be stable and using a wheel created for an older ABI version will lead to a crash.

@icemac icemac merged commit 869deee into master Apr 20, 2023
@icemac icemac deleted the drop-setup-requires branch April 20, 2023 06:17
@icemac
Copy link
Member Author

icemac commented Apr 20, 2023

Thank you for reviewing this PR. 😃

@icemac
Copy link
Member Author

icemac commented Apr 20, 2023

@dataflake
Copy link
Member

Looks like copying the header files solves the macOS build issue. This whole song and dance trying to pre-install packages to get at the header files was just too fancy and error-prone.

@d-maurer
Copy link
Contributor

d-maurer commented Apr 20, 2023 via email

@dataflake
Copy link
Member

I have not seen a trace of the wheel building in the GHA output.

Does it help if I confirm that this wheel does not exist on PyPI? It is built as part of the GHA action. I don't think it's important if you see specific output from this wheel build process. Rest assured, it is being built right then and there. It's not coming from PyPI.

@d-maurer
Copy link
Contributor

d-maurer commented Apr 20, 2023 via email

@dataflake
Copy link
Member

Let me repeat myself: We are talking about two entirely different problems here. You keep bringing up the Ubuntu test failure. That's not what I am concerned with right now at all. And it has nothing to do with the macOS wheel build issue. You keep confusing these two separate issues. The macOS wheel build issue that I am interested in seems to be fixed by the changes in this PR. The Ubuntu issue can be dealt with in a separate issue and a separate PR. So please let's stop this needless discussion.

@dataflake
Copy link
Member

I have created a separate issue for the test failures at #99, all further discussions should happen there.

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

Successfully merging this pull request may close these issues.

4 participants