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

Upgrade Rust (1.76->1.81) + fix Makefile and lints #492

Merged
merged 11 commits into from
Nov 14, 2024

Conversation

r-n-o
Copy link
Contributor

@r-n-o r-n-o commented Nov 12, 2024

Summary & Motivation (Problem vs. Solution)

This branch contains a bunch of chore-style QOL improvements:

  • Currently we use Rust 1.81 locally because of rust-toolchain.yml, but our StageX images (used in CI) are pointed to Rust 1.76. I'm aligning on 1.81.
  • Replaced Containerfile imports with recent version-tagged stagex images wherever possible. Keep in mind this containerfile doesn't actually impact mono builds because we build all artifacts from there. I've aligned versions as much as possible (with our common mono Containerfile).
  • Made the main Makefile call into src/Makefile targets instead of defining its own recipe. The drift is causing major inconsistencies.
  • Removed calls to +nightly toolchain. Yes the extra lints are nice, but they're not worth inconsistencies between stagex and local linting jobs. Going forward we're only using one version of Rust for everything: 1.81 stable ✅ (as the nightly lint rules move into stable rustfmt, we can re-enable them if we still like them)
  • Fix make shell to work correctly on MacOS (where env's -C option doesn't exist!)
  • Remove src/integration/src/bin/gen_att_doc.rs which was failing lints and was not compiling (using non-existent APIs). It's been "dead" code for a long time because of the mock feature. It's not defined in qos_integration, hence the code is never compiled.

How I Tested These Changes

CI

@r-n-o r-n-o requested a review from a team as a code owner November 12, 2024 17:19
@r-n-o r-n-o force-pushed the rno/fix-makefile-and-rust-version branch from b09f3c6 to dd54694 Compare November 12, 2024 20:29
Copy link
Contributor

@jack-kearney jack-kearney left a comment

Choose a reason for hiding this comment

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

Looks good to me -- I think we should pair this up with a PR to mono that bumps the equivalent StageX packages

@r-n-o
Copy link
Contributor Author

r-n-o commented Nov 13, 2024

@jack-kearney this is already done actually: I've aligned versions with what's in our mono containerfile.

(another way to think about this PR is we're catching up to mono for Rust and other stagex packages)

@jack-kearney
Copy link
Contributor

Calling out a couple differences here:

  • ca-certificates in mono is sx2024.09.0 but bumps to sx2024.11.0 here
  • eif_build is at the same version in both mono & here (0.2.2) but in mono is 291653f1ca528af48fd05858749c443300f6b24d2ffefa7f5a3a06c27c774566 and here is 9d086a2743f9df4eddf934c7b68c9dee4a7fb131b6465a24237a67f6c359dfb0
  • file is at the same version in both mono & here (5.45) but in mono is b43a7f0bd50419a39d91d77a316bb888ed87c94aeb6f9eb11f12efd275ca4ab8 and here is f1053114ea2ef35dc04bd1d1f1572c3f1b86e3d57dffda99faac9e191bd7ab5d
  • linux-nitro is set to 5.19.6 here but is sx2024.03.0 in mono

Don't have a ton of context on where these hashes came from but would like to chat through the differences

@r-n-o
Copy link
Contributor Author

r-n-o commented Nov 14, 2024

Writing down what we uncovered looking into this:

  • Mono's common Containerfile doesn't have all the versions in it. We also pull in stagex packages in e.g. mono qos_enclave Containerfile (that where we pull e.g. linux-nitro)
  • docker tags don't matter, sha values do! When they disagree, sha wins regardless of tag, and there are no errors thrown

The latest diff aligns with mono on all shas, including the ones I missed. This is out of an abundance of caution: as noted before, the version of these dependencies aren't used in actual enclave builds: mono is authoritative anyway! This alignment is good to avoid drift and catch build-time problems early.

@jack-kearney jack-kearney merged commit 37da3ac into main Nov 14, 2024
5 checks passed
@jack-kearney jack-kearney deleted the rno/fix-makefile-and-rust-version branch November 14, 2024 15:26
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.

2 participants