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

[beta] fix failing const validation #71441

Merged
merged 2 commits into from
Apr 25, 2020

Conversation

RalfJung
Copy link
Member

This is the beta branch fix for #71353, by reverting #70566.

r? @oli-obk
Not sure if there is any extra process for the beta part. This is not a backport; we intend to "properly" fix this on master but for beta a revert is faster and less risky.

@rust-highfive
Copy link
Collaborator

⚠️ Warning ⚠️

  • Pull requests are usually filed against the master branch for this repo, but this one is against beta. Please double check that you specified the right target!

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 22, 2020
@RalfJung RalfJung changed the title Beta const validation fix [beta] fix failing const validation Apr 22, 2020
@Mark-Simulacrum
Copy link
Member

I think given the timeline I'm fine with this going into beta without full compiler team approval (since it could've landed in master just days ago).

cc @pnkfelix @rust-lang/wg-prioritization -- does that seem reasonable? We can also treat this as a beta-nominated PR at tomorrow's meeting, I guess.

@RalfJung
Copy link
Member Author

Some git checkout failed on the GHA builder, no idea what's up with that

fatal: Not a valid commit name b2e36e6c2d229126b59e892c9147fbb68115d292
thread 'main' panicked at 'command did not execute successfully: "git" "merge-base" "a7d891e31a85269d0d0164258f2b6b6670129763" "b2e36e6c2d229126b59e892c9147fbb68115d292"
expected success, got: exit code: 128', src/build_helper/lib.rs:131:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/test/mir-opt --pass=build --target=armv5te-unknown-linux-gnueabi

@JohnTitor
Copy link
Member

Some git checkout failed on the GHA builder, no idea what's up with that

I guess because rollup landed during running CI. That points to b2e36e6.

@Mark-Simulacrum
Copy link
Member

I believe that's a spurious failure -- ls-remote returns the current master commit which may not be checked out locally if master was pushed after CI on the PR started. We only started seeing this now because previously (and still) pipelines didn't run on beta PRs and beta auto branch builds always happened in isolation (not in parallel with master merges).

I've retriggered the builds.

@RalfJung
Copy link
Member Author

This time all three failed...

@spastorino spastorino added I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. beta-nominated Nominated for backporting to the compiler in the beta channel. and removed I-nominated labels Apr 22, 2020
@pnkfelix
Copy link
Member

@Mark-Simulacrum (at this point we're just going to treat this as if it were nominated for beta backport, even though I understand it is not a backport)

@RalfJung RalfJung closed this Apr 24, 2020
@RalfJung RalfJung reopened this Apr 24, 2020
@oli-obk
Copy link
Contributor

oli-obk commented Apr 24, 2020

We discussed this at the compiler meeting yesteday and decided that we'd prefer a master PR and backport that to beta in the regular way. If you worry that the backport will be too big, maybe just do the revert on master, backport that and fix the root issue again

@oli-obk oli-obk closed this Apr 24, 2020
@RalfJung
Copy link
Member Author

RalfJung commented Apr 24, 2020

I thought @pnkfelix was going to forward-port this same change to master, and we could still use this PR here for beta?

@RalfJung RalfJung reopened this Apr 24, 2020
@oli-obk
Copy link
Contributor

oli-obk commented Apr 24, 2020

oh, I guess I misremembered, sorry

@Mark-Simulacrum
Copy link
Member

@bors r+ rollup=never

Received approval from @pnkfelix over Discord and generally it seems we're good to go here.

@bors
Copy link
Contributor

bors commented Apr 24, 2020

📌 Commit 622c84a has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 24, 2020
@Mark-Simulacrum Mark-Simulacrum added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Apr 24, 2020
@Dylan-DPC-zz
Copy link

@bors p=1

@bors
Copy link
Contributor

bors commented Apr 25, 2020

⌛ Testing commit 622c84a with merge b1162ed...

@bors
Copy link
Contributor

bors commented Apr 25, 2020

☀️ Test successful - checks-azure
Approved by: Mark-Simulacrum
Pushing b1162ed to beta...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 25, 2020
@bors bors merged commit b1162ed into rust-lang:beta Apr 25, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 25, 2020
…idation-fix, r=Dylan-DPC

Revert PR 70566 for const validation fix

This is a port of PR rust-lang#71441 but ported to the master branch, as discussed in [yesterday's T-compiler meeting](https://zulip-archive.rust-lang.org/131828tcompiler/88751weeklymeeting2020042354818.html#195065903)
@Mark-Simulacrum Mark-Simulacrum removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label May 9, 2020
@RalfJung RalfJung deleted the beta-const-validation-fix branch September 13, 2020 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants