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

Bump versions for release, add pinned-toolchain for err checks #67

Merged
merged 1 commit into from
Nov 22, 2022

Conversation

bnaecker
Copy link
Collaborator

No description provided.

@ahl
Copy link
Collaborator

ahl commented Nov 22, 2022

I'm fine with whatever you choose, but I'd suggest we keep the compile-time output checks to keep tabs on the current rustc output.

@bnaecker
Copy link
Collaborator Author

Thanks Adam! Just checking, but do you mean keep the output and the tests themselves, but don't necessarily run them in CI?

@ahl
Copy link
Collaborator

ahl commented Nov 22, 2022

In dropshot we pin the rust toolchain https://github.com/oxidecomputer/dropshot/blob/main/rust-toolchain.toml and use renovate to nudge that forward. I'd advocate that we run the build tests in CI, but the only times we should see these (possibly) spurious changes is when we rev the rust version.

(did you mean to add the rust-toolchain?)

@bnaecker
Copy link
Collaborator Author

Yeah, I had already pinned the toolchain in the Buildomat job description, yet still saw a failure in those tests. Of course I can't find the output now. I'll try it again.

@bnaecker
Copy link
Collaborator Author

Ok, so this is what I was talking about. That is the Buildomat output of a run just checking the compiler error messages. It uses a toolchain pinned to nightly-2021-11-24. Running that on my Helios box completes successfully, but it fails in CI with a single different character AFAICT (a spurious LF in the pest error message). I have no idea why that is, these should be running exactly the same code with exactly the same toolchain.

@bnaecker bnaecker force-pushed the release branch 2 times, most recently from cd594b0 to ad2230c Compare November 22, 2022 18:14
@bnaecker
Copy link
Collaborator Author

Ugh, lockfiles! We need to prevent the lockfile from being updated in CI, this was pulling a newer version of pest, which changed the error outputs.

@bnaecker bnaecker merged commit fdd1ba4 into master Nov 22, 2022
@bnaecker bnaecker deleted the release branch November 22, 2022 18:51
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