Add two scripts to generate universal2 artifacts for macOS#795
Add two scripts to generate universal2 artifacts for macOS#795komaldesai13 wants to merge 14 commits intoansible:develfrom
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as resolved.
This comment was marked as resolved.
webknjaz
changed the title
|
@komaldesai13 could you see if it'd be possible to replace Line 246 in 3498943 cibuildwheel locally? Perhaps, brew uninstall libssh before doing that. Pay attention at whether your scripts leave Git in a dirty state or create untracked files in the working directory. We should strive to unify compiling these components in the same manner so the version numbers could be updated once in the repository and influence both macOS and GNU/Linux build scripts.
|
|
@KB-perByte let's not create merge commits in this branch — we'll have to remember to clean them up, which may complicate the process depending on @komaldesai13's experience with using Git. |
komaldesai13
force-pushed
the
macoc_universal2_buils
branch
2 times, most recently
from
b97c58b to
ba132d6
Compare
Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>
Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>
webknjaz
force-pushed
the
macoc_universal2_buils
branch
from
6ab4ccf to
913bd33
Compare
|
@komaldesai13 best avoid adding merge commits into this branch. |
komaldesai13
force-pushed
the
macoc_universal2_buils
branch
from
660f4ea to
7616ccf
Compare
komaldesai13
force-pushed
the
macoc_universal2_buils
branch
from
c2746cf to
780281a
Compare
There was a problem hiding this comment.
We don't need yet another config with duplicate data in it — it's better to use the single source of configuration.
- Shared OpenSSL installer script - Explicit library list in merge script - Removed macOS logic from build backend (security fix) - Pre-built artifacts with download-first approach - Fixed duplicate build-frontend to use "build"
komaldesai13
force-pushed
the
macoc_universal2_buils
branch
from
c02b818 to
58f1632
Compare
| ] | ||
|
|
||
| [tool.cibuildwheel.macos.environment] | ||
| # delocate (bundled with cibuildwheel v3) enforces that the wheel's |
| # make | ||
| # make install/strip | ||
| # working_directory: libssh/build | ||
| "bash {project}/build-scripts/macos/cibuildwheel-before-build.sh", |
There was a problem hiding this comment.
Not sure how I feel about building the deps as a part of the wheel build process. The whole point of the pre-cached artifacts is that they are built separately and don't waste resources every time the project is being build.
| sdist/ | ||
| var/ | ||
| wheels/ | ||
| wheelhouse/ |
There was a problem hiding this comment.
This part is coming from a centralized template, I wouldn't put changes in the middle of it unless the upstream got the same changes.
| .installed.cfg | ||
|
|
||
| # macOS build artifacts | ||
| .macos-static-deps-path* |
| *.manifest | ||
| *.spec | ||
| # Exception: Allow RPM spec files | ||
| !packaging/rpm/*.spec |
There was a problem hiding this comment.
This is definitely unrelated. Plus, it's unnecessary since the file is already being tracked.
There was a problem hiding this comment.
I guess it may make sense to have this but not globally — scope it to the respective folder. Though, this must be a standalone change as it has absolutely nothing to do with the goal of this PR and only distracts from whatever that goal is.
| /src/pylibsshext/_scm_version.py | ||
|
|
||
| # macOS | ||
| .DS_Store |
There was a problem hiding this comment.
This mustn't exist in the project. The contributors must add this to their global gitignore as this is something that is coming from the contrib system and is unrelated to the project.
|
|
||
| echo "Installing OpenSSL to ${INSTALL_PREFIX}..." | ||
| # Install only software and SSL directories (skip docs) | ||
| make install_sw install_ssldirs |
There was a problem hiding this comment.
That was needed in the context of containers. With macOS, we probably need to think of an in-tree location for the built lib cache. I'd reuse PyCA's ideas, of course, instead of reinventing the wheel.
There was a problem hiding this comment.
Or maybe out of the tree.. Anyway, there's separation between building the deps and using them. For using them (while building the wheels), I'd expect us to pre-download the deps like so: https://github.com/pyca/cryptography/blob/43897cbe22d304a93d6e8736fd386516baa9781d/.github/workflows/wheel-builder.yml#L212-L220. I originally expected that this PR would only contain a minimal patch for building the deps and storing them as GHA artifacts but not embed that into the wheel building process just yet.
It seems like we may want to think of a mechanism for pointing to the place where the pre-built deps live. The manylinux config just sets the respective env vars and the macOS one should too. But I was hoping to avoid growing the scope indefinitely.
| gh release upload libssh-v0.12.0-openssl-3.5.0 \ | ||
| build-scripts/macos/build/arm64/libssh-*.tar.gz \ | ||
| build-scripts/macos/build/arm64/libssh-*.tar.gz.sha256 \ | ||
| build-scripts/macos/build/x86_64/libssh-*.tar.gz \ | ||
| build-scripts/macos/build/x86_64/libssh-*.tar.gz.sha256 |
There was a problem hiding this comment.
Just saw this. We should definitely not publish releases with other projects in here.
2 participants
SUMMARY
This patch adds two shell scripts for building OpenSSL and libssh for use in the Python packaging process.
build-libssh-macos.sh: which takes openssl version, libssh version and macos arch as input and generate artifacts.merge-universal2.sh: merges x86 and arm64 artifacts and generates a universal2 libssh build for macOS.ISSUE TYPE
ADDITIONAL INFORMATION