build: Support arm64 and universal binaries for MacOS #25460
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
camscale
force-pushed
the
camh/mac-multiarch
branch
from
125437b to
96409c3
Compare
camscale
force-pushed
the
camh/mac-multiarch
branch
2 times, most recently
from
08ec57b to
428d06c
Compare
r0mant
approved these changes
May 2, 2023
There was a problem hiding this comment.
lgtm but I would have @gzdunek and @ravicious look over the Teleport Connect bits.
camscale
force-pushed
the
camh/mac-multiarch
branch
3 times, most recently
from
d9e96fa to
0f16b49
Compare
mdwn
approved these changes
May 3, 2023
| } | ||
|
|
||
| const path = `${packed.appOutDir}/Teleport Connect.app/Contents/MacOS/tsh.app/Contents/Info.plist`; | ||
| if (packed.appOutDir.endsWith('mac-universal--x64')) { |
There was a problem hiding this comment.
Is the double dash here intentional?
There was a problem hiding this comment.
Yes. For whatever reason, that is how the directories are named. Here's a log showing the run: https://github.com/gravitational/teleport.e/actions/runs/4870801505/jobs/8687542119#step:22:615
public-teleport-github-review-bot
bot
removed request for
ravicious,
wadells,
tcsc,
ryanclark,
gzdunek,
fheinecke and
kimlisa
gzdunek
approved these changes
May 4, 2023
public-teleport-github-review-bot
bot
removed the request for review
from ravicious
camscale
force-pushed
the
camh/mac-multiarch
branch
from
0f16b49 to
0290620
Compare
Add support for building/cross-building the fido2 libraries (cbor, openssl and fido2), supporting ARM64 builds. This is done by adding the appropriate flags to the library builds in `build-fido2-macos.sh` based on the `C_ARCH` environment variable. If unset then the host architecture is used. The `Makefile` defined `C_ARCH` based on the `ARCH` variable, mapping it to an appropriate value for the C compiler. Building the libraries should now be done through the new `build-fido2` target, and getting the pkg-config path should be done with the `print-fido2-pkg-path`. This is instead of calling the `build-fido2-macos.sh` script directly as the `Makefile` takes care of setting the `C_ARCH` environment variable appropriately.
Add the `rustup-set-target-toolchain` target to the Makefile to ensure the right rust toolchain is installed for the version of Rust we use as well as the target architecture we wish to generate code for, based on the `ARCH` variable. This is intended to be used by CI jobs to ensure they build with the correct toolchain.
Remove the restriction that allows only AMD64 packages to be built on MacOS for the teleport and tsh packages. This is via the existing `-a` flag to `build-package.sh` and a newly added `-a` flag to `build-pkg-tsh.sh`. This adds the architecture to the filename of the package to distinguish the packages for different architectures. Update the comments in the Makefile mentioning that `arch` is ignored. build: add architecture to package names
When packaging Teleport Connect with electron-builder, pass an architecture flag so that we can cross-build Teleport Connect. This will allow us to build MacOS ARM64 binaries on the AMD64 runners. Add the architecture to the `dmg` filename via the electron-builder config, so that the filenames for different architectures don't clash.
Copy the Mac release artifacts to a release artifact directory so that the CI scripts do not have to. This makes it clearer what is and is not a release artifact and puts the logic in the Makefile instead of the CI yaml, so it can more easily be tested locally and to make it easier to migrate to the next CI system. This will also be useful for building universal binaries for Mac as the CI system can put the architecture-specific binaries from a previous workflow job into a common location. We should look at copying all release artifacts for the other builds (Linux tarballs and packages, etc) into this directory too. It may help with unifying the GitHub Actions release workflows.
Add support for ARCH=universal on Darwin to produce universal (fat) binaries from pre-built arm64 and amd64 binaries. Packages (pkg) and disk images (dmg) for containing universal binaries are named without an architecture in the filename, as that is the current naming for the current AMD64-only releases. These universal ones will replace those AMD64-only ones providing a single release artifact working across architectures. Co-authored-by: Grzegorz Zdunek <grzegorz.zdunek@goteleport.com>
Remove the `clean` prerequisite from the `release-darwin-unsigned` target as it is not needed when building on GitHub Actions, as it starts with a fresh slate each run. We do not make releases manually so we don't need to ensure a clean working directory there either. Not doing a clean makes it easier to build a MacOS universal release as it depends on the architecture-specific tarballs from a previous release build. We would need to manually save the tarballs from the first architecture release build as they would get deleted by the `clean` from the second. So just stop cleaning as it is not needed.
camscale
force-pushed
the
camh/mac-multiarch
branch
from
0290620 to
9fcb495
Compare
camscale
added a commit
that referenced
this pull request
May 5, 2023
* build: Support ARM64 (cross)builds of fido2 et al Add support for building/cross-building the fido2 libraries (cbor, openssl and fido2), supporting ARM64 builds. This is done by adding the appropriate flags to the library builds in `build-fido2-macos.sh` based on the `C_ARCH` environment variable. If unset then the host architecture is used. The `Makefile` defined `C_ARCH` based on the `ARCH` variable, mapping it to an appropriate value for the C compiler. Building the libraries should now be done through the new `build-fido2` target, and getting the pkg-config path should be done with the `print-fido2-pkg-path`. This is instead of calling the `build-fido2-macos.sh` script directly as the `Makefile` takes care of setting the `C_ARCH` environment variable appropriately. * build: Add make target to install rust cross toolchain Add the `rustup-set-target-toolchain` target to the Makefile to ensure the right rust toolchain is installed for the version of Rust we use as well as the target architecture we wish to generate code for, based on the `ARCH` variable. This is intended to be used by CI jobs to ensure they build with the correct toolchain. * build: Support building MacOS packages for ARM64 Remove the restriction that allows only AMD64 packages to be built on MacOS for the teleport and tsh packages. This is via the existing `-a` flag to `build-package.sh` and a newly added `-a` flag to `build-pkg-tsh.sh`. This adds the architecture to the filename of the package to distinguish the packages for different architectures. Update the comments in the Makefile mentioning that `arch` is ignored. build: add architecture to package names * build: Build Teleport Connect with target architecture When packaging Teleport Connect with electron-builder, pass an architecture flag so that we can cross-build Teleport Connect. This will allow us to build MacOS ARM64 binaries on the AMD64 runners. Add the architecture to the `dmg` filename via the electron-builder config, so that the filenames for different architectures don't clash. * build: Copy Mac release artifacts to release directory Copy the Mac release artifacts to a release artifact directory so that the CI scripts do not have to. This makes it clearer what is and is not a release artifact and puts the logic in the Makefile instead of the CI yaml, so it can more easily be tested locally and to make it easier to migrate to the next CI system. This will also be useful for building universal binaries for Mac as the CI system can put the architecture-specific binaries from a previous workflow job into a common location. We should look at copying all release artifacts for the other builds (Linux tarballs and packages, etc) into this directory too. It may help with unifying the GitHub Actions release workflows. * build: Add MacOS universal builds Add support for ARCH=universal on Darwin to produce universal (fat) binaries from pre-built arm64 and amd64 binaries. Packages (pkg) and disk images (dmg) for containing universal binaries are named without an architecture in the filename, as that is the current naming for the current AMD64-only releases. These universal ones will replace those AMD64-only ones providing a single release artifact working across architectures. Co-authored-by: Grzegorz Zdunek <grzegorz.zdunek@goteleport.com> * build: Do not clean before release-darwin Remove the `clean` prerequisite from the `release-darwin-unsigned` target as it is not needed when building on GitHub Actions, as it starts with a fresh slate each run. We do not make releases manually so we don't need to ensure a clean working directory there either. Not doing a clean makes it easier to build a MacOS universal release as it depends on the architecture-specific tarballs from a previous release build. We would need to manually save the tarballs from the first architecture release build as they would get deleted by the `clean` from the second. So just stop cleaning as it is not needed. --------- Co-authored-by: Grzegorz Zdunek <grzegorz.zdunek@goteleport.com> Backport: #25460
camscale
added a commit
that referenced
this pull request
May 5, 2023
* [v13] build: Support arm64 and universal binaries for MacOS * build: Support ARM64 (cross)builds of fido2 et al Add support for building/cross-building the fido2 libraries (cbor, openssl and fido2), supporting ARM64 builds. This is done by adding the appropriate flags to the library builds in `build-fido2-macos.sh` based on the `C_ARCH` environment variable. If unset then the host architecture is used. The `Makefile` defined `C_ARCH` based on the `ARCH` variable, mapping it to an appropriate value for the C compiler. Building the libraries should now be done through the new `build-fido2` target, and getting the pkg-config path should be done with the `print-fido2-pkg-path`. This is instead of calling the `build-fido2-macos.sh` script directly as the `Makefile` takes care of setting the `C_ARCH` environment variable appropriately. * build: Add make target to install rust cross toolchain Add the `rustup-set-target-toolchain` target to the Makefile to ensure the right rust toolchain is installed for the version of Rust we use as well as the target architecture we wish to generate code for, based on the `ARCH` variable. This is intended to be used by CI jobs to ensure they build with the correct toolchain. * build: Support building MacOS packages for ARM64 Remove the restriction that allows only AMD64 packages to be built on MacOS for the teleport and tsh packages. This is via the existing `-a` flag to `build-package.sh` and a newly added `-a` flag to `build-pkg-tsh.sh`. This adds the architecture to the filename of the package to distinguish the packages for different architectures. Update the comments in the Makefile mentioning that `arch` is ignored. build: add architecture to package names * build: Build Teleport Connect with target architecture When packaging Teleport Connect with electron-builder, pass an architecture flag so that we can cross-build Teleport Connect. This will allow us to build MacOS ARM64 binaries on the AMD64 runners. Add the architecture to the `dmg` filename via the electron-builder config, so that the filenames for different architectures don't clash. * build: Copy Mac release artifacts to release directory Copy the Mac release artifacts to a release artifact directory so that the CI scripts do not have to. This makes it clearer what is and is not a release artifact and puts the logic in the Makefile instead of the CI yaml, so it can more easily be tested locally and to make it easier to migrate to the next CI system. This will also be useful for building universal binaries for Mac as the CI system can put the architecture-specific binaries from a previous workflow job into a common location. We should look at copying all release artifacts for the other builds (Linux tarballs and packages, etc) into this directory too. It may help with unifying the GitHub Actions release workflows. * build: Add MacOS universal builds Add support for ARCH=universal on Darwin to produce universal (fat) binaries from pre-built arm64 and amd64 binaries. Packages (pkg) and disk images (dmg) for containing universal binaries are named without an architecture in the filename, as that is the current naming for the current AMD64-only releases. These universal ones will replace those AMD64-only ones providing a single release artifact working across architectures. Co-authored-by: Grzegorz Zdunek <grzegorz.zdunek@goteleport.com> * build: Do not clean before release-darwin Remove the `clean` prerequisite from the `release-darwin-unsigned` target as it is not needed when building on GitHub Actions, as it starts with a fresh slate each run. We do not make releases manually so we don't need to ensure a clean working directory there either. Not doing a clean makes it easier to build a MacOS universal release as it depends on the architecture-specific tarballs from a previous release build. We would need to manually save the tarballs from the first architecture release build as they would get deleted by the `clean` from the second. So just stop cleaning as it is not needed. --------- Co-authored-by: Grzegorz Zdunek <grzegorz.zdunek@goteleport.com> Backport: #25460 * [v13] build: Fix ELECTRON_BUILDER_ARCH to not throw error Do not cause a `make` `$(error ...)` if the arch is not a known electron builder architecture. A previous commit should have added unexport ELECTRON_BUILDER_ARCH but it was forgotten. Instead of adding that, just add mappings where needed and let `$(ARCH)` map directly to an electron builder arch flag when there is no mapping. If this is invalid for an architecture we want to build for, let EB complain then. Backport: #25676 * update e ref This brings in the matching changes to e/Makefile for MacOS universal builds. --------- Co-authored-by: Grzegorz Zdunek <grzegorz.zdunek@goteleport.com>
ravicious
reviewed
May 5, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add support for building arm64 and universal binaries for MacOS, via the
ARCH=arm64andARCH=universalvars in theMakefile. This includeschanges for building C code (libfido2 and dependencies), rust
(rdpclient) and the packaging (
.pkgand.dmg). Go supports thiswithout any extra fuss.
We now also copy the release artifacts into a release artifact directory
so it is easier for CI to find the artifacts to release, as previously
they were all scattered over the repository (they are still scattered,
but also copied to the release artifact directory. This will be cleaned
up when we find any dependencies on the old locations).
Issue: #4226
Issue: https://github.com/gravitational/teleport.e/issues/872