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

Use rust's docker images for mips*, powerpc*, and s390x #1724

Merged
merged 2 commits into from
Mar 29, 2019

Conversation

cuviper
Copy link
Member

@cuviper cuviper commented Mar 23, 2019

When building for linux-gnu targets, we need to be careful about which
versioned GLIBC symbols we'll link, as this sets the minimum supportable
system version in ABI. The images in rust-lang/rust/src/ci/docker/ have
already taken this into account, and since they're uploaded to S3, we
can just use those exact images to get the same ABI compatibility.

The builds for mips*, powerpc*, and s390x were identified in #1681 as
having newer glibc symbols than rustc, so those are now changed to use
rust's docker images. In the future, we might want to do this for other
architectures too, so all toolchains are in sync.

When building for `linux-gnu` targets, we need to be careful about which
versioned GLIBC symbols we'll link, as this sets the minimum supportable
system version in ABI. The images in rust-lang/rust/src/ci/docker/ have
already taken this into account, and since they're uploaded to S3, we
can just use those exact images to get the same ABI compatibility.

The builds for mips*, powerpc*, and s390x were identified in rust-lang#1681 as
having newer glibc symbols than rustc, so those are now changed to use
rust's docker images. In the future, we might want to do this for other
architectures too, so all toolchains are in sync.
@kinnison
Copy link
Contributor

This looks plausible to me but I'm still very new to docker and travis when it comes down to it. @alexcrichton can you confirm if this looks sane?

@kinnison kinnison requested a review from alexcrichton March 23, 2019 09:29
}

# Use images from rustc 1.35.0-nightly (94fd04589 2019-03-21)
# https://travis-ci.com/rust-lang/rust/builds/105351531
Copy link
Contributor

Choose a reason for hiding this comment

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

For how long images like that are retained?

@alexcrichton
Copy link
Member

I think using the exact same images as rust-lang/rust is a great idea! I think though this may need to be tweaked slightly becaues the hashes are all stored on s3 which is deleted every 90 days or so. That means we'll have to manually update the hashes here :(

I wonder if we could tweak our CI to maybe upload a file describing each target and the container it was built in? For example /rustc-builds/$target would always contain the sha512 and sha256 to do what this script does (or something like that) and while still not guaranteed to work it's "much more likely to work"

@cuviper
Copy link
Member Author

cuviper commented Mar 25, 2019

I think though this may need to be tweaked slightly because the hashes are all stored on s3 which is deleted every 90 days or so.

I didn't realize that -- does this mean rust CI is forced to rebuild and reupload the images periodically? I haven't witnessed this, but I certainly don't watch every PR go by. :)

I wonder if we could tweak our CI to maybe upload a file describing each target and the container it was built in? For example /rustc-builds/$target would always contain the sha512 and sha256 to do what this script does (or something like that) and while still not guaranteed to work it's "much more likely to work"

OK, I can try adding something like that.

I think we'd only want to update that when building for nightly / the master branch, or else include the branch in that path name. I'm not sure how to tell though -- src/ci/run.sh sets the channel, but src/ci/docker/run.sh doesn't know that. I guess I could grep it or something...

@alexcrichton
Copy link
Member

In practice we tend to modify each image at least once every 90 days so the rebuilds happen on that PR vs others, or we just have tons of timeouts randomly as everything is rebuilt and we don't really see it much.

One alternative could which may avoid grepping around is that we upload a well-known filename into the s3 bucket /rustc-builds/${commit_sha}-${image_name} which contains the sha512 and sha256 here in this PR. That way this PR would look up the current commit on the master branch and then use that plus a shared name of the cross builders to look-up the sha512 and such perhaps?

@cuviper
Copy link
Member Author

cuviper commented Mar 25, 2019

Ah, deployed artifacts already use s3://rust-lang-ci2/rustc-builds/$TRAVIS_COMMIT/, so I guess we can drop image shas in there too, and on the rustup side use git ls-remote to find master.

@alexcrichton
Copy link
Member

Sounds reasonable to me!

@cuviper
Copy link
Member Author

cuviper commented Mar 25, 2019

OK, I've added image info in rust-lang/rust#59420, and updated this PR to use it -- though of course that won't work until we've merged and deployed the new info. It seems to work locally, at least.

@kinnison
Copy link
Contributor

kinnison commented Mar 26, 2019

Thanks for the review @alexcrichton -- We'll look to merge this later tonight unless one of the other wg-rustup members has concerns.

@tesuji
Copy link
Contributor

tesuji commented Mar 26, 2019

I think this PR is blocked on rust-lang/rust#59420.

@cuviper
Copy link
Member Author

cuviper commented Mar 26, 2019

Right, it won't work without the Rust PR. It's not clear to me when the extra CI jobs actually run for this repo though -- is that only for the stable branch? Maybe I should temporarily toggle that branch filter in .travis.yml when we're ready to try this?

@kinnison
Copy link
Contributor

Oh right, I was under the impression it was approved because things were ready, we'll hold off then 👍

@kinnison
Copy link
Contributor

And yes, the non-default builds only happen on the stable branch in order to reduce the load on the appveyor and other travis workers.

cuviper added a commit to cuviper/rust that referenced this pull request Mar 28, 2019
[CI] record docker image info for reuse

This writes an extra `dist/image-$image.txt` which contains the S3 URL
of the cached image and the `sha256` digest of the docker entry point.
This will be uploaded with the rest of the deployed artifacts in the
Travis `after_success` script.

cc rust-lang/rustup#1724
r? @alexcrichton
@cuviper
Copy link
Member Author

cuviper commented Mar 28, 2019

OK, the Rust PR has merged and deployed, and it works for me to read through that and fetch images. Should be ready to go!

@kinnison
Copy link
Contributor

Let's give it a spin and see how badly things go :D Thank for your efforts.

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.

5 participants