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.

Sign up for GitHub

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

Jump to bottom

Update Rust version #588

Merged
merged 8 commits into from
Feb 11, 2020
Merged

Update Rust version #588

merged 8 commits into from
Feb 11, 2020

Conversation

blaxill
Copy link
Contributor

@blaxill blaxill commented Feb 10, 2020 •
edited
Loading

  • Updates rust in Dockerfile
  • updates packages as necessary
  • fixes new clippy errors

Similar to #443 but slightly newer nightly (with full component set) and fixes to pass tests

edit: due to this rustc ICE rustfmt example has to be disabled temporarily (grep -v rustfmt in run_examples)

Checklist

  • Pull request affects core Oak functionality (e.g. runtime, SDK, ABI)
    • I have written tests that cover the code changes.
    • I have checked that these tests are run by Cloudbuild
    • I have updated documentation accordingly.
    • I have raised an issue to
      cover any TODOs and/or unfinished work.
  • Pull request includes prototype/experimental work that is under
    construction.

@tiziano88 tiziano88 mentioned this pull request Feb 10, 2020
Closed
@blaxill
Copy link
Contributor Author

blaxill commented Feb 10, 2020 •
edited
Loading

Unfortunately this is running into this open rust error rust-lang/rust#64450

tl;dr: Updating our toolchain requires us to use an updated version of rustfmt (as its dependency rustc-ap-syntax_pos required an update to compile), which now imports rustc-ap-syntax_pos using an alias (edit: after a little investigation this might not be the exact root cause), which when targeting WebAssembly causes a rustc ICE.

@blaxill blaxill requested review from tiziano88 and daviddrysdale and removed request for tiziano88 February 11, 2020 13:48
daviddrysdale
daviddrysdale approved these changes Feb 11, 2020
View reviewed changes
scripts/run_examples
@@ -11,7 +11,7 @@ source "$SCRIPTS_DIR/common"
# Note that the chat example cannot be run because the client is interactive.
# TODO(#462): Re-enable chat example, even if it may be just loading the server-side code and not
# interact with it.
readonly RUST_EXAMPLES="$(find examples -mindepth 2 -maxdepth 4 -type d -regex '.*/module.*/rust$' | cut -d'/' -f2 | uniq | grep -v chat)"
readonly RUST_EXAMPLES="$(find examples -mindepth 2 -maxdepth 4 -type d -regex '.*/module.*/rust$' | cut -d'/' -f2 | uniq | grep -v chat | grep -v rustfmt)"
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we get a TODO(#NNN) to re-instate the rustfmt example?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@blaxill blaxill mentioned this pull request Feb 11, 2020
Closed
tiziano88
tiziano88 approved these changes Feb 11, 2020
View reviewed changes
WORKSPACE
@@ -255,7 +255,7 @@ rust_repository_set(
name = "rust_linux_x86_64",
exec_triple = "x86_64-unknown-linux-gnu",
extra_target_triples = [],
iso_date = "2019-11-06",
iso_date = "2020-02-06",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you add a comment here and in the Dockerfile to make sure these values are kept in sync?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@blaxill blaxill merged commit d362ebd into project-oak:master Feb 11, 2020
@tiziano88
Copy link
Collaborator

Note that the build time now has increased because the cached Docker image is no longer useful (see https://pantheon.corp.google.com/cloud-build/builds/34e7619c-fb81-4323-a0ab-38509de54151;step=1?project=oak-ci ).

@blaxill could you run ./scripts/docker_push to make sure that the new version is uploaded?

@blaxill
Copy link
Contributor Author

blaxill commented Feb 12, 2020

@blaxill could you run ./scripts/docker_push to make sure that the new version is uploaded?

Done, I think it should be updated now.

@daviddrysdale daviddrysdale mentioned this pull request Feb 12, 2020
Merged
daviddrysdale added a commit that referenced this pull request Feb 12, 2020
@daviddrysdale
examples: drop specific-commit deps for dirs/term (#597)
d8e0de3 
Commit d362ebd ("Update Rust version (#588)") upgraded the
'dirs' (2.0.1->2.0.2) and 'term' (0.5.2->0.6.1) dependencies so
the pins to particular commits are no longer needed.
@blaxill blaxill mentioned this pull request Feb 28, 2020
Merged
6 tasks
This was referenced May 27, 2020
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daviddrysdale daviddrysdale daviddrysdale approved these changes

@tiziano88 tiziano88 tiziano88 approved these changes

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@blaxill @tiziano88 @daviddrysdale @googlebot