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

Remove Windows 32-bit from the Python 3.10 build. #1237

Merged
merged 5 commits into from
Jan 20, 2022

Conversation

jpivarski
Copy link
Member

No description provided.

@jpivarski jpivarski mentioned this pull request Jan 19, 2022
@codecov
Copy link

codecov bot commented Jan 19, 2022

Codecov Report

Merging #1237 (3b7372f) into main (0a0e9be) will decrease coverage by 0.86%.
The diff coverage is 83.05%.

Impacted Files Coverage Δ
src/awkward/_v2/contents/bitmaskedarray.py 64.45% <ø> (-0.79%) ⬇️
src/awkward/_v2/contents/emptyarray.py 69.54% <ø> (ø)
src/awkward/_v2/_typetracer.py 68.94% <33.33%> (-0.25%) ⬇️
src/awkward/_v2/contents/content.py 81.57% <33.33%> (-1.62%) ⬇️
src/awkward/_v2/highlevel.py 65.02% <60.00%> (+0.03%) ⬆️
...c/awkward/_v2/operations/reducers/ak_linear_fit.py 85.71% <66.66%> (-1.52%) ⬇️
src/awkward/_v2/operations/reducers/ak_all.py 91.66% <75.00%> (-8.34%) ⬇️
src/awkward/_v2/operations/reducers/ak_any.py 91.66% <75.00%> (-8.34%) ⬇️
src/awkward/_v2/operations/reducers/ak_count.py 91.66% <75.00%> (-8.34%) ⬇️
...wkward/_v2/operations/reducers/ak_count_nonzero.py 91.66% <75.00%> (-8.34%) ⬇️
... and 42 more

@jpivarski
Copy link
Member Author

Testing the deployment here, by the way: https://github.com/scikit-hep/awkward-1.0/actions/runs/1719986605

@henryiii
Copy link
Member

You could skip it instead (or also) in pyproject.toml; the benefit of the skip is that it works locally too. Technically, we don't really support local Windows runs yet... But the next cibuildwheel release should. Quite minor, this is fine.

@jpivarski
Copy link
Member Author

Now the MacOS build gets this far before failing:

2022-01-19T21:11:38.4243380Z   build/lib.macosx-10.9-x86_64-3.9/awkward/libawkward-cpu-kernels.dylib
2022-01-19T21:11:38.4243790Z   build/lib.macosx-10.9-x86_64-3.9/_ext.cpython-39-darwin.so
2022-01-19T21:11:38.4244150Z   build/lib.macosx-10.9-x86_64-3.9/libawkward.dylib
2022-01-19T21:11:38.4244530Z   build/lib.macosx-10.9-x86_64-3.9/libawkward-cpu-kernels.dylib
2022-01-19T21:11:38.4244930Z   --- copying includes ------------------------------------------
2022-01-19T21:11:38.4245350Z   error: [Errno 17] File exists: 'build/lib.macosx-10.9-x86_64-3.8/awkward/include'
2022-01-19T21:11:38.4245790Z   Building wheel for awkward (pyproject.toml): finished with status 'error'
2022-01-19T21:11:38.4246070Z   ERROR: Failed building wheel for awkward
2022-01-19T21:11:38.4246280Z Failed to build awkward
2022-01-19T21:11:38.4246500Z ERROR: Failed to build one or more wheels

I suppose we don't really need any includes, now that we're not supporting downstream dependencies of the C++ part.

They'd be needed in the SDist, but not in wheels.

I'm going to try this:

--- a/setup.py
+++ b/setup.py
@@ -193,11 +193,6 @@ class Install(setuptools.command.install.install):
         print("--- build directory -------------------------------------------")
         tree("build")
 
-        print("--- copying includes ------------------------------------------")
-        shutil.copytree(
-            os.path.join("include"), os.path.join(outerdir, "awkward", "include")
-        )
-
         print("--- outerdir after copy ---------------------------------------")
         tree(outerdir)

By the same logic, we also don't need to bundle any static libraries, just the dynamic ones. I'll start with this, though.

@jpivarski
Copy link
Member Author

@henryiii
Copy link
Member

henryiii commented Jan 19, 2022

I think the problem is setuptools doesn't understand that PyPy and CPython are different. If you split the CPython and PyPy jobs, I think it will be fine. Not sure why PyPy 3.7 doesn't show that problem, and 3.8 does, though - but it might be "better" (more CPython-like) handling of build paths for PyPy 3.8.

See pypa/setuptools#2912

@jpivarski
Copy link
Member Author

I keep doing other things and then coming back to check up on this. You have something specific in mind—could you set it up the way you're thinking?

@henryiii
Copy link
Member

I've reverted the include file removal to verify my suspicion was correct, but if it's not useful, the static files should be removed, as they make the wheels larger (libs much more than include).

@jpivarski
Copy link
Member Author

Great, thank you very much! I see that it has been tested here: https://github.com/scikit-hep/awkward-1.0/actions/runs/1721784145 (the commit hex matches), so we know that it will work in a real release. I'll get started on that release right now.

@jpivarski
Copy link
Member Author

if it's not useful, the static files should be removed, as they make the wheels larger (libs much more than include).

The static libraries are not useful, but the next release will be a prerelease. The frequency of non-prereleases has gone down considerably in the past year, so that alleviates some of the disk space pressure. Nevertheless, before 1.8.0, I'll try removing the static libraries.

@jpivarski jpivarski merged commit 834f674 into main Jan 20, 2022
@jpivarski jpivarski deleted the jpivarski/remove-win32-from-py310-build branch January 20, 2022 16:27
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