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

Update rust-toolchain #1776

Closed
wants to merge 6 commits into from
Closed

Update rust-toolchain #1776

wants to merge 6 commits into from

Conversation

gendx
Copy link
Contributor

@gendx gendx commented Apr 21, 2020

Pull Request Overview

This pull request updates the Rust toolchain to today's nightly (nightly-2020-04-21).

  • Fixed the sed commands in tools/update_rust_version.sh to work on Linux.
  • Fixed some new warnings about unnecessary parentheses.
  • Migrated from the now-deprecated asm! macro to its llvm_asm! replacement.

I've seen that the Tock 1.5 version is in progress (#1685), but almost no test have been done yet. It would be a nice opportunity to upgrade to a nightly as recent as possible for this release.

Testing Strategy

This pull request was tested by Travis.

TODO or Help Wanted

Documentation Updated

  • Updated the relevant files in /docs, or no updates are required.

Formatting

  • Ran make formatall.

error: use of deprecated item 'asm': the syntax of asm! will change soon, use llvm_asm! to avoid breakage
ppannuto
ppannuto previously approved these changes Apr 21, 2020
Copy link
Member

@ppannuto ppannuto left a comment

Choose a reason for hiding this comment

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

@tock/core-wg I've reviewed all the changes and they're purely cosmetic. I think this is something we can/should pull in for 1.5 before doing testing.

@gendx
Copy link
Contributor Author

gendx commented Apr 21, 2020

Hum, this caused the following compiler crash on Travis.

thread 'rustc' panicked at 'failed to lookup `SourceFile` in new context', src/librustc_middle/ty/query/on_disk_cache.rs:456:17
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
error: internal compiler error: unexpected panic
note: the compiler unexpectedly panicked. this is a bug.
note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports
note: rustc 1.44.0-nightly (20fc02f83 2020-04-20) running on x86_64-unknown-linux-gnu
note: compiler flags: -C opt-level=z -C debuginfo=2 -C debug-assertions=on -C incremental --crate-type lib
note: some of the compiler flags provided by cargo are hidden

I observed a similar issue at some point on my machine (rust-lang/rust#71380).

So it looks like this particular nightly version is a bad one. Maybe we should keep the previous pinned version for the release.

@bradjc
Copy link
Contributor

bradjc commented Apr 21, 2020

We normally update every two-ish months unless there is a problem, plus with this not passing travis, I say we continue with 1.5-rc1 to try to make this a timely release.

@ppannuto
Copy link
Member

Upon second thinking, you're right, a last-minute nightly update doesn't really make sense if the goal is a stable release, but I do think this can merge right after the 1.5 freeze.

Re the build issue: That's a known cacheing problem, rust-lang/rust#70924 -- clearing the travis cache fixed the build here, running clean locally should fix as well. Not sure whether that will be fixed or something people will just need to do as a one-off.

@gendx
Copy link
Contributor Author

gendx commented Apr 21, 2020

@ppannuto did someone manually re-run the Travis build? I wanted to reference it but now it unfortunately shows as "passed"...

@ppannuto
Copy link
Member

I did, sorry

alistair23
alistair23 previously approved these changes Apr 22, 2020
Copy link
Contributor

@alistair23 alistair23 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 for after 1.5

@bradjc bradjc dismissed stale reviews from alistair23 and ppannuto via 5882230 April 30, 2020 22:22
Copy link
Contributor

@bradjc bradjc left a comment

Choose a reason for hiding this comment

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

Might as well go even newer!

But can the script be changed back or something? I now get a bunch of new files:

On branch pr/1776
Untracked files:
  (use "git add <file>..." to include in what will be committed)

	.travis.yml.bak
	.vscode/settings.json.bak
	doc/Getting_Started.md.bak
	rust-toolchain.bak
	shell.nix.bak
	tools/netlify-build.sh.bak

@bradjc bradjc marked this pull request as ready for review April 30, 2020 22:24
@ppannuto
Copy link
Member

ppannuto commented May 1, 2020

I updated the script to fix the .bak file problem

@ppannuto ppannuto force-pushed the nightly-2020-04 branch 2 times, most recently from e7af2e3 to 08194db Compare May 1, 2020 17:01
@bradjc
Copy link
Contributor

bradjc commented May 1, 2020

Can we redo the build?

@gendx gendx marked this pull request as draft May 4, 2020 12:35
@gendx
Copy link
Contributor Author

gendx commented May 4, 2020

Marking as draft until nightly is usable again: rust-lang/rust#70924

@hudson-ayers
Copy link
Contributor

Separately from the issue that requires cargo clean, make format fails on this branch, throwing the error "failed to find targets"

@bradjc
Copy link
Contributor

bradjc commented May 15, 2020

The blocking issue is only relevant to travis, correct? I don't think we can wait indefinitely for Rust to fix this. But hopefully this is patched soon.

I also just tried make format on today's nightly and it ran without issue.

@hudson-ayers
Copy link
Contributor

Yeah, the make format issue was actually from an out of tree file getting picked up by the formatting script, sorry forgot to update here.

@gendx
Copy link
Contributor Author

gendx commented May 18, 2020

The blocking issue is only relevant to travis, correct? I don't think we can wait indefinitely for Rust to fix this. But hopefully this is patched soon.

As discussed in #1654, this is unfortunately the trade-off of choosing Rust nightly over Rust stable. When some things break on nightly there is no prioritization to fix them within a day. And the bigger the codebase + the more unstable features we rely on, the likelier it is that at least one thing breaks in a nightly release (as we've already seen with nightly versions not supporting Rustfmt or Clippy).

In this particular case, we can get away by asking developers to explicitly cargo clean before each call to cargo (at the expense of longer compilation times) or whenever they see the compiler error.

On the other hand, after a month and a half lingering around, this bug is now "high priority", so i guess there is a chance it gets fixed in the next few weeks: rust-lang/rust#70924 (comment).

@bradjc bradjc mentioned this pull request May 28, 2020
3 tasks
bors bot added a commit that referenced this pull request May 28, 2020
1884: Remove unused `asm` feature flags r=ppannuto a=bradjc

### Pull Request Overview

I noticed in #1776 that not all `asm` features are actually required. This removes many of the `asm` feature.

Part of #1654, technically.


### Testing Strategy

This pull request was tested by travis. (Can I still say that?)


### TODO or Help Wanted

n/a


### Documentation Updated

- [x] Updated the relevant files in `/docs`, or no updates are required.

### Formatting

- [x] Ran `make format`.
- [x] Fixed errors surfaced by `make clippy`.


Co-authored-by: Brad Campbell <[email protected]>
@hudson-ayers
Copy link
Contributor

rust-lang/rust#70924 has been fixed! So if we update this to latest nightly we are probably good to go.

@gendx
Copy link
Contributor Author

gendx commented Jun 2, 2020

rust-lang/rust#70924 has been fixed! So if we update this to latest nightly we are probably good to go.

It's probably easier to start from scratch to avoid merge conflicts from the past couple months. And the branch name is not valid anymore ;)

@gendx gendx closed this Jun 2, 2020
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