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

Change built-in kernel targets to be os = none throughout #77916

Merged
merged 1 commit into from
Mar 7, 2021

Conversation

Ericson2314
Copy link
Contributor

@Ericson2314 Ericson2314 commented Oct 13, 2020

Whether for Rust's own target_os, LLVM's triples, or GNU config's, the
OS-related have fields have been for code running on that OS, not code
hat is part of the OS.

The difference is huge, as syscall interfaces are nothing like
freestanding interfaces. Kernels are (hypervisors and other more exotic
situations aside) freestanding programs that use the interfaces provided
by the hardware. It's those interfaces, the ones external to the
program being built and its software dependencies, that are the content
of the target.

For the Linux Kernel in particular, target_env: "gnu" is removed for
the same reason: that -gnu refers to glibc or GNU/linux, neither of
which applies to the kernel itself.

Relates to #74247

@rust-highfive
Copy link
Collaborator

r? @lcnr

(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 Oct 13, 2020
@lcnr
Copy link
Contributor

lcnr commented Oct 14, 2020

maybe r? @Mark-Simulacrum

Copy link

@kloenk kloenk left a comment

Choose a reason for hiding this comment

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

Those names sounds much better

@bors
Copy link
Contributor

bors commented Oct 15, 2020

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

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@Mark-Simulacrum
Copy link
Member

Tentatively r? @joshtriplett

@Ericson2314
Copy link
Contributor Author

Per the issue thread (just to make things easier for anyone just reading the PR), While I prefer this to the status quo, my first preference is removing these targets outright. having per-project targets baked into rustc seems like a layer violation and maintenance hazard anyways.

@joshtriplett
Copy link
Member

joshtriplett commented Oct 18, 2020

LGTM.

r=me when ready.

@rust-lang rust-lang deleted a comment from bors Oct 18, 2020
@joshtriplett
Copy link
Member

@Ericson2314 I understand what you mean by a "layering violation", but given that we don't have stable support for custom targets, and given that it seems extremely unlikely we'll stabilize that because it's so dependent on specific details of LLVM internals, I think we need to have at least minimal scaffolding for important specialized targets.

We also don't have full support for building std/core from source yet, let alone for replacing it with a crate from the ecosystem.

Right now, there really isn't any path towards stable support for such targets without at least some infrastructure-level support like this.

I'm not suggesting that we should have targets specific to using Rust as a plugin to arbitrary specific pieces of software, but the Linux kernel is a very popular and desirable target to be able to run Rust in, and it provides enough infrastructure that substantial portions of the standard library (including alloc and a fair bit of std) would work.

@Ericson2314
Copy link
Contributor Author

Ericson2314 commented Oct 20, 2020

@joshtriplett This was the main thing we discussed on the call.

It's my understanding that the plan for the -z build-std work to stabilize without needing to stabilizing any of the language features we need to build standard library crates.

How might this work? The tools merely need to authenticate that the crates being built are in fact the officials stdlib crates, and unlock the nightly features only for that code. Then there's no risk of user code using things it shouldn't.

It was also discussed that given how the timeline LLVM Linux work as progressed, there ought to be be plenty of runway to be on Nightly until such functionality is worked out if Rust is held to the same standard.

@crlf0710 crlf0710 added I-needs-decision Issue: In need of a decision. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 6, 2020
@Dylan-DPC-zz
Copy link

@Ericson2314 any updates? also you have a few conflicts to resolve

@ojeda
Copy link
Contributor

ojeda commented Nov 26, 2020

given that it seems extremely unlikely we'll stabilize that because it's so dependent on specific details of LLVM internals

I don't think users in the freestanding world would mind if the target spec options themselves are documented to not be compatible between Rust releases. Being able to use a stable release of the compiler in such environments is usually way more important.

@Ericson2314
Copy link
Contributor Author

OK the merge conflicts are fixed. I would still advocate for these targets to be removed, but I think merging this PR is still a (less controversial) improvement over the status quo, so happy if we start with that and return to the question of removal after.

@Dylan-DPC-zz
Copy link

@bors r=joshtriplett

@bors
Copy link
Contributor

bors commented Mar 1, 2021

📌 Commit 9d25ccf has been approved by joshtriplett

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 1, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 2, 2021
…, r=joshtriplett

Change built-in kernel targets to be os = none throughout

Whether for Rust's own `target_os`, LLVM's triples, or GNU config's, the
OS-related have fields have been for code running *on* that OS, not code
hat is *part* of the OS.

The difference is huge, as syscall interfaces are nothing like
freestanding interfaces. Kernels are (hypervisors and other more exotic
situations aside) freestanding programs that use the interfaces provided
by the hardware. It's *those* interfaces, the ones external to the
program being built and its software dependencies, that are the content
of the target.

For the Linux Kernel in particular, `target_env: "gnu"` is removed for
the same reason: that `-gnu` refers to glibc or GNU/linux, neither of
which applies to the kernel itself.

Relates to rust-lang#74247
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 3, 2021
…, r=joshtriplett

Change built-in kernel targets to be os = none throughout

Whether for Rust's own `target_os`, LLVM's triples, or GNU config's, the
OS-related have fields have been for code running *on* that OS, not code
hat is *part* of the OS.

The difference is huge, as syscall interfaces are nothing like
freestanding interfaces. Kernels are (hypervisors and other more exotic
situations aside) freestanding programs that use the interfaces provided
by the hardware. It's *those* interfaces, the ones external to the
program being built and its software dependencies, that are the content
of the target.

For the Linux Kernel in particular, `target_env: "gnu"` is removed for
the same reason: that `-gnu` refers to glibc or GNU/linux, neither of
which applies to the kernel itself.

Relates to rust-lang#74247
@bors
Copy link
Contributor

bors commented Mar 3, 2021

⌛ Testing commit 9d25ccf with merge cc8aa2c31407e3a9c742ab7f865e8820cc104379...

@bors
Copy link
Contributor

bors commented Mar 3, 2021

💥 Test timed out

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 3, 2021
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
##[group]Virtual Environment
Environment: ubuntu-20.04
Version: 20210302.0
Included Software: https://github.com/actions/virtual-environments/blob/ubuntu20/20210302.0/images/linux/Ubuntu2004-README.md
Image Release: https://github.com/actions/virtual-environments/releases/tag/ubuntu20%2F
##[group]GITHUB_TOKEN Permissions
Actions: write
Checks: write
Contents: write

@JohnTitor
Copy link
Member

@bors retry

@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 Mar 6, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 6, 2021
…, r=joshtriplett

Change built-in kernel targets to be os = none throughout

Whether for Rust's own `target_os`, LLVM's triples, or GNU config's, the
OS-related have fields have been for code running *on* that OS, not code
hat is *part* of the OS.

The difference is huge, as syscall interfaces are nothing like
freestanding interfaces. Kernels are (hypervisors and other more exotic
situations aside) freestanding programs that use the interfaces provided
by the hardware. It's *those* interfaces, the ones external to the
program being built and its software dependencies, that are the content
of the target.

For the Linux Kernel in particular, `target_env: "gnu"` is removed for
the same reason: that `-gnu` refers to glibc or GNU/linux, neither of
which applies to the kernel itself.

Relates to rust-lang#74247
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 7, 2021
Rollup of 13 pull requests

Successful merges:

 - rust-lang#77916 (Change built-in kernel targets to be os = none throughout)
 - rust-lang#82130 (Make some Option, Result methods unstably const)
 - rust-lang#82292 (Prevent specialized ZipImpl from calling `__iterator_get_unchecked` twice with the same index)
 - rust-lang#82402 (Remove RefCell around `module_trait_cache`)
 - rust-lang#82592 (Improve transmute docs with further clarifications)
 - rust-lang#82651 (Cleanup rustdoc warnings)
 - rust-lang#82720 (Fix diagnostic suggests adding type `[type error]`)
 - rust-lang#82751 (improve offset_from docs)
 - rust-lang#82793 (Move some tests to more suitable subdirs)
 - rust-lang#82803 (rustdoc: Add an unstable option to print all unversioned files)
 - rust-lang#82808 (Sync rustc_codegen_cranelift)
 - rust-lang#82822 (Fix typo)
 - rust-lang#82837 (tweak MaybeUninit docs)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d1dc166 into rust-lang:master Mar 7, 2021
@rustbot rustbot added this to the 1.52.0 milestone Mar 7, 2021
@Ericson2314 Ericson2314 deleted the kernel-code-targets-os-none branch April 16, 2021 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-needs-decision Issue: In need of a decision. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.