build: Fix ELECTRON_BUILDER_ARCH to not throw error#25676
Merged
Conversation
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.
r0mant
approved these changes
May 5, 2023
mdwn
approved these changes
May 5, 2023
public-teleport-github-review-bot
Bot
removed request for
gabrielcorado,
smallinsky,
tcsc and
wadells
camscale
added a commit
that referenced
this pull request
May 5, 2023
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
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>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Do not cause a
make$(error ...)if the arch is not a known electronbuilder architecture. A previous commit should have added
but it was forgotten. Instead of adding that, just add mappings where
needed and let
$(ARCH)map directly to an electron builder arch flagwhen there is no mapping. If this is invalid for an architecture we
want to build for, let EB complain then.
Issue: https://github.com/gravitational/teleport.e/issues/872
Issue: #4226