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

build: integrate new cross toolchains w/ Bazel #77960

Merged
merged 1 commit into from
Apr 7, 2022

Conversation

rickystewart
Copy link
Collaborator

@rickystewart rickystewart commented Mar 16, 2022

  1. Add new toolchains that run on ARM hosts.
  2. Add new toolchains that target M1 Mac's.

Release note: None

Jira issue: CRDB-14852

@rickystewart rickystewart requested a review from a team as a code owner March 16, 2022 18:33
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@rickystewart rickystewart force-pushed the crossdarwin branch 10 times, most recently from 518bd8c to beb9d80 Compare March 17, 2022 23:03
@rickystewart rickystewart requested a review from a team as a code owner March 17, 2022 23:03
@rickystewart rickystewart changed the title [WIP] toolchains: update osxcross version build: integrate new cross toolchains w/ Bazel Apr 5, 2022
@rickystewart rickystewart requested review from mari-crl and removed request for a team April 5, 2022 14:53
1. Add new toolchains that run on ARM hosts.
2. Add new toolchains that target M1 Mac's.

Release note: None
Copy link
Contributor

@mari-crl mari-crl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! :D Woohoo, ARM builds!! :lgtm:

Reviewed 12 of 12 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @rickystewart)


build/toolchains/crosstool-ng/toolchain.bzl, line 4 at r1 (raw file):

    if rctx.attr.host == "x86_64":
        url = "https://storage.googleapis.com/public-bazel-artifacts/toolchains/crosstool-ng/{}/20220317-191459/{}.tar.gz".format(rctx.attr.host, rctx.attr.target)
    elif rctx.attr.host == "aarch64":

Just for my curiosity's sake, why the different URL for aarch64? Not supported in the most recent release yet?


c-deps/BUILD.bazel, line 17 at r1 (raw file):

        "@io_bazel_rules_go//go/platform:darwin_arm64": [
            "--host=aarch64-apple-darwin21.2",
            "--with-lg-page=16",

Hmmm, why do we need this for arm and not for anything else?


c-deps/BUILD.bazel, line 186 at r1 (raw file):

        "//build/toolchains:cross": {
            "AUTOM4TE": "$(execpath :autom4te)",
            "krb5_cv_attr_constructor_destructor": "yes",

Why the changes here?

Copy link
Contributor

@mari-crl mari-crl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oop, didn't hit approve. :lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @rickystewart)

@rickystewart
Copy link
Collaborator Author

Just for my curiosity's sake, why the different URL for aarch64? Not supported in the most recent release yet?

To build these toolchains I ran the CI script twice, once on a normal x86_64 agent and once on an ARM agent. The script is written to use the value of date +%Y%m%d-%H%M%S for the URL the toolchains are uploaded to, and so they'll write to different URL's as a matter of course (they'll never initiate this write at the exact same second). See https://teamcity.cockroachdb.com/buildConfiguration/Internal_Release_BuildAndPublishCrossToolchains?mode=builds#all-projects vs. https://teamcity.cockroachdb.com/buildConfiguration/Internal_Release_BuildAndPublishCrossToolchainsArmHost?mode=builds#all-projects

Hmmm, why do we need this for arm and not for anything else?

Our jemalloc has this issue when cross-compiling for macOS ARM for some reason. If I remove this line, I get this error:

cockroach$ ./artifacts/cockroach-short
<jemalloc>: Error allocating TSD for 
zsh: abort      ./artifacts/cockroach-short

Adding the line back gets rid of it.

Why the changes here?

This helps configure figure out a few checks that it otherwise can't perform when we're cross-compiling. Here's another example.

@rickystewart
Copy link
Collaborator Author

bors r=mari-crl

@craig
Copy link
Contributor

craig bot commented Apr 7, 2022

Build succeeded:

@craig craig bot merged commit e54a36c into cockroachdb:master Apr 7, 2022
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.

3 participants