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

Add musl support for MIPS64 & bump to 0.2.63 #1449

Merged
merged 4 commits into from
Aug 17, 2019

Conversation

xen0n
Copy link
Contributor

@xen0n xen0n commented Jul 31, 2019

Tested with patched stage2; both static and dynamic binaries confirmed working.

Initial CI support in the form of no-core targets are added.

@rust-highfive
Copy link

r? @gnzlbg

(rust_highfive has picked a reviewer for you, use r? to override)

@xen0n
Copy link
Contributor Author

xen0n commented Jul 31, 2019

failure is the same as #1450

@xen0n xen0n force-pushed the mips64-musl-targets branch 4 times, most recently from d7b3a2e to c6a8efd Compare August 2, 2019 07:59
@xen0n xen0n changed the title [WIP] add musl support for MIPS64 Add musl support for MIPS64 Aug 2, 2019
@xen0n xen0n marked this pull request as ready for review August 2, 2019 09:27
@xen0n
Copy link
Contributor Author

xen0n commented Aug 2, 2019

The i686 mingw failure seems unrelated.

@bors
Copy link
Contributor

bors commented Aug 8, 2019

☔ The latest upstream changes (presumably #1376) made this pull request unmergeable. Please resolve the merge conflicts.

ci/azure.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@gnzlbg gnzlbg left a comment

Choose a reason for hiding this comment

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

So this all looks good to me.

If there is librustc_target support upstream for these, I would prefer if you would add these targets to ci/build.sh (to the LINUX_NIGHTLY_NOCORE env variable), so that we at least test that these build properly.

Once you get libstd support up and running in rust-lang/rust, then it might be worth it to consider adding the run-time tests for these, but that should happen in a future PR, and not here.

@xen0n
Copy link
Contributor Author

xen0n commented Aug 9, 2019

For the record, I for some unknown reason included CI changes in both of the PRs, so neither would build... 🤦‍♂

@gnzlbg
Copy link
Contributor

gnzlbg commented Aug 9, 2019

You can remove the changes to azure.yml here. If you don't want to wait for the target to be available, you can add a .json file here if you want, but since there is a PR upstream already, I'll just wait on this till that lands.

Centril added a commit to Centril/rust that referenced this pull request Aug 9, 2019
…ichton

Add builtin targets for mips64(el)-unknown-linux-musl

This is prerequisite for rust-lang/libc#1449.

Tested locally to produce working static and dynamic binaries, ~~but CI config is untested for now~~ CI is to be added in a follow-up PR.

*edit: dynamic binaries also confirmed working!*
@gnzlbg
Copy link
Contributor

gnzlbg commented Aug 12, 2019

This needs to be updated (e.g. remove the comments in the azure.yml file, or enable those jobs), also, these targets need to be added to the ci/build.sh file (probably to the NIGHTLY_NOCORE list until we start shipping core and/or libstd for these targets).

Centril added a commit to Centril/rust that referenced this pull request Aug 12, 2019
…ichton

Add builtin targets for mips64(el)-unknown-linux-muslabi64

This is prerequisite for rust-lang/libc#1449.

Tested locally to produce working static and dynamic binaries, ~~but CI config is untested for now~~ CI is to be added in a follow-up PR.

*edit: dynamic binaries also confirmed working!*

*edit 2: changed triples to include ABI, and removed stray `crt_static_default = false` declarations to be consistent with other musl targets`
@xen0n
Copy link
Contributor Author

xen0n commented Aug 12, 2019

Updated. The librustc_target enablement should land today; this should go in afterwards. (And with a version bump.)

@gnzlbg
Copy link
Contributor

gnzlbg commented Aug 14, 2019

@bors: r+

@bors
Copy link
Contributor

bors commented Aug 14, 2019

📌 Commit da1aca0 has been approved by gnzlbg

@bors
Copy link
Contributor

bors commented Aug 14, 2019

⌛ Testing commit da1aca0 with merge 832549a...

bors added a commit that referenced this pull request Aug 14, 2019
Add musl support for MIPS64

Tested with patched stage2; both static and dynamic binaries confirmed working.

Initial CI support in the form of no-core targets are added.
@xen0n
Copy link
Contributor Author

xen0n commented Aug 14, 2019

Ah. The rustc change didn't make it in, in fact it's still waiting.

Once rust-lang/rust#63165 lands this can be retried; merging of this should be avoided before that.

@bors
Copy link
Contributor

bors commented Aug 14, 2019

💥 Test timed out

Centril added a commit to Centril/rust that referenced this pull request Aug 15, 2019
…ichton

Add builtin targets for mips64(el)-unknown-linux-muslabi64

This is prerequisite for rust-lang/libc#1449.

Tested locally to produce working static and dynamic binaries, ~~but CI config is untested for now~~ CI is to be added in a follow-up PR.

*edit: dynamic binaries also confirmed working!*

*edit 2: changed triples to include ABI, and removed stray `crt_static_default = false` declarations to be consistent with other musl targets*
@gnzlbg
Copy link
Contributor

gnzlbg commented Aug 15, 2019

@xen0n while the PR to rust-lang/rust landed, new nightly toolchains are only created approx. once per day, so usually you have to wait until tomorrow. Don't despair ;)

@gnzlbg
Copy link
Contributor

gnzlbg commented Aug 16, 2019

@bors: r+

@bors
Copy link
Contributor

bors commented Aug 16, 2019

📌 Commit 42139b8 has been approved by gnzlbg

@bors
Copy link
Contributor

bors commented Aug 16, 2019

⌛ Testing commit 42139b8 with merge dd8f908...

bors added a commit that referenced this pull request Aug 16, 2019
Add musl support for MIPS64

Tested with patched stage2; both static and dynamic binaries confirmed working.

Initial CI support in the form of no-core targets are added.
@bors
Copy link
Contributor

bors commented Aug 16, 2019

💔 Test failed - status-azure

@xen0n
Copy link
Contributor Author

xen0n commented Aug 17, 2019

Turns out I misunderstood the new CI pipeline...

- script: |
set -ex
if [ -n "${TARGET}" ]; then
rustup target add $TARGET
fi
condition: ne( variables['Agent.OS'], 'Windows_NT' )
displayName: Install target (unix)

It seems any target in the build matrix needs to have std already built. Hence I'm taking the new targets out of the build matrix just like other NO_CORE targets.

@gnzlbg
Copy link
Contributor

gnzlbg commented Aug 17, 2019

It seems any target in the build matrix needs to have std already built.

Yes, you only can do that once you have enabled std support in rust-lang/rust, which needs to happen after the initial changes here (where we only check that the library builds correctly).

@gnzlbg
Copy link
Contributor

gnzlbg commented Aug 17, 2019

@bors: r+

@bors
Copy link
Contributor

bors commented Aug 17, 2019

📌 Commit 435cdee has been approved by gnzlbg

@bors
Copy link
Contributor

bors commented Aug 17, 2019

⌛ Testing commit 435cdee with merge 7decbdd...

bors added a commit that referenced this pull request Aug 17, 2019
Add musl support for MIPS64

Tested with patched stage2; both static and dynamic binaries confirmed working.

Initial CI support in the form of no-core targets are added.
@xen0n
Copy link
Contributor Author

xen0n commented Aug 17, 2019

We're blocked by #1473. Should we just cherry-pick the build-fixing commit to at least unblock things?

@bors
Copy link
Contributor

bors commented Aug 17, 2019

💔 Test failed - status-azure

@gnzlbg
Copy link
Contributor

gnzlbg commented Aug 17, 2019

@bors: retry

@xen0n xen0n changed the title Add musl support for MIPS64 Add musl support for MIPS64 & bump to 0.2.63 Aug 17, 2019
bors added a commit that referenced this pull request Aug 17, 2019
Add musl support for MIPS64 & bump to 0.2.63

Tested with patched stage2; both static and dynamic binaries confirmed working.

Initial CI support in the form of no-core targets are added.
@bors
Copy link
Contributor

bors commented Aug 17, 2019

⌛ Testing commit 435cdee with merge 4b610e6...

@bors
Copy link
Contributor

bors commented Aug 17, 2019

☀️ Test successful - checks-cirrus-freebsd-11, checks-cirrus-freebsd-12, status-azure
Approved by: gnzlbg
Pushing 4b610e6 to master...

@bors bors merged commit 435cdee into rust-lang:master Aug 17, 2019
@xen0n xen0n deleted the mips64-musl-targets branch August 19, 2019 05:11
@xen0n
Copy link
Contributor Author

xen0n commented Aug 19, 2019

Could this version bump be pushed to crates.io, so I can continue work on upstreaming the corresponding std support?

@gnzlbg
Copy link
Contributor

gnzlbg commented Aug 19, 2019

Sure

@gnzlbg
Copy link
Contributor

gnzlbg commented Aug 20, 2019

Released, sorry for the delay, we had too many time outs the last couple of days.

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.

4 participants