-
-
Notifications
You must be signed in to change notification settings - Fork 36
ci(macos): add workflow to build universal2 OpenSSL + libssh from source #783
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
base: devel
Are you sure you want to change the base?
Conversation
|
Congratulations! One of the builds has completed. 🍾 You can install the built RPMs by following these steps:
Please note that the RPMs should be used only in a testing environment. |
|
@Ruchip16 did we agree on a new workflow for macos? |
The intent was about the build logic, not the workflow boundary. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll probably need to integrate this back into the same workflow that builds the container images and reuse some vers. Though, we'll talk about it later.
Seeing the amount of copy-paste, I'm pretty sure the build job definitions can be squashed into one too.
We'll also need to see if we can/need to move some portions into scripts.
|
@Ruchip16 in case you haven't tried it, running |
| name: 🧪 Test${{ '' }} # nest jobs under the same sidebar category | ||
| needs: | ||
| - build-bin-macos | ||
| - macos-merge-universal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This breaks the CI. We can't and shouldn't depend on things that aren't in the same workflow run. It's a separate process that runs independently on schedule.
| - macos-merge-universal |
| macos-universal-artifact-name: | ||
| description: Name of artifact with universal2 OpenSSL+libssh (macOS) | ||
| required: false | ||
| type: string | ||
| openssl-lib-name: | ||
| description: Directory name for OpenSSL inside the artifact | ||
| required: false | ||
| type: string | ||
| default: openssl | ||
| libssh-lib-name: | ||
| description: Directory name for libssh inside the artifact | ||
| required: false | ||
| type: string | ||
| default: libssh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could probably just hardcode these for now. The artifacts aren't coming from the same workflow run anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if we should be changing the testing workflow, though. These artifacts are supposed to be used under cibuildwheel. That's the main target, at least.
| LIBSSH_DIR="$(pwd)/.deps/u/${{ inputs.libssh-lib-name }}" | ||
| { | ||
| echo "OPENSSL_DIR=$OPENSSL_DIR" | ||
| echo "LIBSSH_DIR=$LIBSSH_DIR" | ||
| echo "PKG_CONFIG_PATH=$OPENSSL_DIR/lib/pkgconfig:$LIBSSH_DIR/lib/pkgconfig:${PKG_CONFIG_PATH:-}" | ||
| echo "LDFLAGS=-L$OPENSSL_DIR/lib -L$LIBSSH_DIR/lib ${LDFLAGS:-}" | ||
| echo "CPPFLAGS=-I$OPENSSL_DIR/include -I$LIBSSH_DIR/include ${CPPFLAGS:-}" | ||
| } >> "$GITHUB_ENV" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be possible to set these vars directly in the env: option of the Pre-populate tox env step down below. It's best to move it here.
| path: .deps/ | ||
| - name: (macOS) Unpack deps and export build env | ||
| if: >- | ||
| runner.os == 'macOS' && inputs.macos-universal-artifact-name != '' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This conditional should probably be
| runner.os == 'macOS' && inputs.macos-universal-artifact-name != '' | |
| runner.os == 'macOS' && inputs.dist-type == 'source' |
instead.
| if: >- | ||
| runner.os == 'macOS' | ||
| run: brew install libssh | ||
| runner.os == 'macOS' && inputs.macos-universal-artifact-name != '' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| runner.os == 'macOS' && inputs.macos-universal-artifact-name != '' | |
| runner.os == 'macOS' && inputs.dist-type == 'source' |
| runner.os == 'macOS' | ||
| run: brew install libssh | ||
| runner.os == 'macOS' && inputs.macos-universal-artifact-name != '' | ||
| uses: actions/download-artifact@v4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This action can only download artifacts from within the same workflow run. To get things from other workflows (and workflow runs), we need something beyond the standard actions.
For example, PyCA uses this: https://github.com/pyca/cryptography/blob/84d85b4b2a21418a3110a06078f281d30e18a2bd/.github/workflows/ci.yml#L317-L325 — in their case, it gets artifacts from a companion repo, not the same. This is something to reproduce here and in the cibuildwheel workflow.
webknjaz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to note that it's best to only implement producing the artifacts within this PR. And not downloading/integrating them.
This is because the new workflow will have to actually run and create those artifacts for consumption, which is only possible post-merge.
So we'll merge a PR emitting said artifacts, run the workflow and only then will we start integrating them. This may end up requiring us scripting something around GH CLI and adding it into the cibuildwheel config for macos only. But that's a separate part of the effort. We won't be able to start implementing it until the previous part is merged.
So my recommendation is to revert things other than producing the artifacts and polish that+merge. Just do note the feedback on the reverted bits and take it into account in future PRs.
|
@Ruchip16 this PR will likely need a change note of type |
SUMMARY
This PR introduces a new GitHub Actions workflow,
macOS-universal.yaml— that builds and packages OpenSSL and libssh from source on macOS, producing universal2 artifacts (for both arm64 and x86_64 architectures).The goal is to replace the current
brew install libsshdependency in CI and enable fully reproducible macOS builds of libssh and its dependencies.ci(macos): It builds OpenSSL + libssh per arch, uploads artifacts, then merges with lipo into universal2.
Refs: #699 #207 #73
Issues: ACA-3946
ISSUE TYPE