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

Bring back x86_64-sun-solaris target to rustup #85252

Merged
merged 2 commits into from
May 26, 2021

Conversation

kulikjak
Copy link
Contributor

Change #82216 removed now deprecated target x86_64-sun-solaris from CI, thus making it no longer possible to use $ rustup target add x86_64-sun-solaris to install given target (see #85098 for details). Since there should be a period of time between the deprecation and removal, this PR brings it back (while keeping the new one as well).

Please, correct me if I am wrong; my assumption that these Docker scripts are being used to build artifacts later used by rustup might be incorrect.

Closes #85098.

@rust-highfive
Copy link
Collaborator

r? @kennytm

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 13, 2021
@kulikjak kulikjak changed the title Fix solaris ci Bring back x86_64-sun-solaris to rustup May 13, 2021
@kulikjak kulikjak changed the title Bring back x86_64-sun-solaris to rustup Bring back target x86_64-sun-solaris to rustup May 13, 2021
@kulikjak kulikjak changed the title Bring back target x86_64-sun-solaris to rustup Bring back x86_64-sun-solaris target to rustup May 13, 2021
@kennytm
Copy link
Member

kennytm commented May 13, 2021

r? @nagisa

@rust-highfive rust-highfive assigned nagisa and unassigned kennytm May 13, 2021
@nagisa
Copy link
Member

nagisa commented May 14, 2021

r? @Mark-Simulacrum

@Mark-Simulacrum Mark-Simulacrum added the beta-nominated Nominated for backporting to the compiler in the beta channel. label May 14, 2021
@Mark-Simulacrum
Copy link
Member

Preemptively nominating for beta, but will need to review later.

@jhpratt
Copy link
Member

jhpratt commented May 14, 2021

If this is just an issue with the binary being available, this technically wouldn't need to be tied to a release, correct? I think having a build for Rust 1.52 would be useful, as otherwise there would be a seemingly arbitrary gap in the availability of the target.

@Mark-Simulacrum
Copy link
Member

I don't think it's likely that we'll produce another point release this cycle, so there probably won't be 1.52 artifacts published. cc @rust-lang/release for thoughts on that.

@Mark-Simulacrum
Copy link
Member

This PR looks OK to me otherwise, though I note it doesn't attempt to alter the build toolchain for the illumos target back to sun from pc, which changed in https://github.com/rust-lang/rust/pull/82216/files#diff-f7fe7525804df85adb4a03efa70e96ee893009d6260066cde665abb8088b8cbeR21.

I admit to not really knowing if the build toolchain makes any difference - I don't think that PR changed anything else. Certainly we don't ship -pc-illumos and -sun-illumos or anything like that...

cc @steveklabnik @jculow @pmooney

@jonas-schievink jonas-schievink added the T-release Relevant to the release subteam, which will review and decide on the PR/issue. label May 14, 2021
@jhpratt
Copy link
Member

jhpratt commented May 15, 2021

Perhaps I should be a bit clearer on what I was saying. Why does this need a point release at all? All this does is generate the binary that rustup then downloads, correct? If so, what's to prevent the creation of a binary for Rust 1.52.0/1.52.1? Checksums for something?

@Mark-Simulacrum
Copy link
Member

We don't have the ability right now to partially edit an existing release, not sure whether it's completely impossible due to checksums, but not something we can do right now on an implementation level.

@Mark-Simulacrum Mark-Simulacrum added the beta-accepted Accepted for backporting to the compiler in the beta channel. label May 26, 2021
@Mark-Simulacrum
Copy link
Member

@bors r+ rollup=never

Alright, let's get this merged in. I think it's an improvement -- not quite back to where we where before, particularly on the illumos target, but having not heard back there I'm inclined to let that stay as-is. It's not clear how much the toolchain that builds the compiler matters for those purposes, and regardless, further work is always possible in future PRs -- for now this should get the breakage in 1.52 due to a missing tier-2 target fixed.

Approving for beta backport so we only have one 'missing' release -- 1.52.

@bors
Copy link
Contributor

bors commented May 26, 2021

📌 Commit b13185d 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 May 26, 2021
@bors
Copy link
Contributor

bors commented May 26, 2021

⌛ Testing commit b13185d with merge 54bdfa1...

@bors
Copy link
Contributor

bors commented May 26, 2021

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 54bdfa1 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 26, 2021
@bors bors merged commit 54bdfa1 into rust-lang:master May 26, 2021
@rustbot rustbot added this to the 1.54.0 milestone May 26, 2021
@kulikjak
Copy link
Contributor Author

This PR looks OK to me otherwise, though I note it doesn't attempt to alter the build toolchain for the illumos target back to sun from pc, which changed in https://github.com/rust-lang/rust/pull/82216/files#diff-f7fe7525804df85adb4a03efa70e96ee893009d6260066cde665abb8088b8cbeR21.

I admit to not really knowing if the build toolchain makes any difference - I don't think that PR changed anything else. Certainly we don't ship -pc-illumos and -sun-illumos or anything like that...

Yes, those were my thoughts as well. There is no target like -pc-illumos or -sun-illumos and considering that the build toolchain was different from the target before (and presumably it worked), I assume that changing that should not affect anything. But I am not completely sure - @jclulow might know more considering that he added that illumos target.

@Mark-Simulacrum Mark-Simulacrum modified the milestones: 1.54.0, 1.53.0 Jun 11, 2021
@Mark-Simulacrum Mark-Simulacrum removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jun 11, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 11, 2021
…ulacrum

[beta] backports

* Disable the machine outliner by default rust-lang#86020
* Fix incorrect gating of nonterminals in key-value attributes rust-lang#85445
* Build crtbegin.o/crtend.o from source code rust-lang#85395
* Bring back x86_64-sun-solaris target to rustup rust-lang#85252
* Preserve SyntaxContext for invalid/dummy spans in crate metadata rust-lang#85211
* [beta] backport: Remove unsound TrustedRandomAccess implementations rust-lang#86222

r? `@Mark-Simulacrum`
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-release Relevant to the release subteam, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

x86_64-sun-solaris target was removed without warning in Rust 1.52
9 participants