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

ship tools with sysroot #110365

Merged
merged 2 commits into from
Apr 21, 2023
Merged

Conversation

onur-ozkan
Copy link
Member

@onur-ozkan onur-ozkan commented Apr 15, 2023

Provides tool binaries under the sysroot which can be used/tested with cargo +custom-toolchain $tool

Clippy and fmt works without any problem.

But can't say the same for miri:

~/devspace/.other/chunk-list  stable $ cargo +stage2 miri setup
Running `"rustup" "component" "add" "rust-src"` to install the `rust-src` component for the selected toolchain.
error: stage2 is a custom toolchain
fatal error: failed to install the `rust-src` component for the selected toolchain

it's looking for $sysroot/lib/rustlib/src/rust/library and that simply doesn't exists for x build.

cc @jyn514 (I thought you might be interested on this, since you did few review iterations on previous PRs of adding tools to sysroot)

--

Update

Now we are able to use miri as well.

After running x b miri cargo-miri --stage 2, I am able to run cargo +stage2 miri setup which works as expected.

Resolves #110625
Resolves #97762
Resolves #81431

@rustbot
Copy link
Collaborator

rustbot commented Apr 15, 2023

r? @Mark-Simulacrum

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Apr 15, 2023
@onur-ozkan onur-ozkan force-pushed the ship-tools-with-sysroot branch 2 times, most recently from 1ed4a64 to 8c313e2 Compare April 15, 2023 16:14
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@onur-ozkan onur-ozkan added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 15, 2023
@onur-ozkan onur-ozkan changed the title [wip] ship clippy and rustfmt with sysroot [wip] ship tools with sysroot Apr 15, 2023
@onur-ozkan onur-ozkan marked this pull request as ready for review April 20, 2023 14:07
@onur-ozkan onur-ozkan added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 20, 2023
@onur-ozkan onur-ozkan changed the title [wip] ship tools with sysroot ship tools with sysroot Apr 20, 2023
@onur-ozkan
Copy link
Member Author

I decided to write tests for this @rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 20, 2023
@jyn514
Copy link
Member

jyn514 commented Apr 20, 2023

it's looking for $sysroot/lib/rustlib/src/rust/library and that simply doesn't exists for x build.

it should be possible to add this, right? We can make a symlink from rust to the root of the repository. I thought we already did that today actually but maybe I'm thinking of something else.

@onur-ozkan
Copy link
Member Author

onur-ozkan commented Apr 20, 2023

it should be possible to add this, right? We can make a symlink from rust to the root of the repository.

I wasn't sure if we should do it or not, will do it then.

I don't know how this error didn't raise on the previous PR.

@jyn514 jyn514 mentioned this pull request Apr 20, 2023
3 tasks
@onur-ozkan
Copy link
Member Author

onur-ozkan commented Apr 20, 2023

I decided to write tests for this

This turns out not to be worth doing it. It will require quite some time for just checking if the binaries are in the correct place after x b $tool command which requires compilation. I think it's worth adding this one into #102563 which I will start start working on that soon.

@onur-ozkan
Copy link
Member Author

cargo +stage2 miri test gives the following in simple cargo project:

  ~/devspace/.other/chunk-list  stable $ cargo +stage2 miri test
Preparing a sysroot for Miri (target: x86_64-unknown-linux-gnu)... done
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: CargoMetadata { stderr: "error: the `-Z` flag is only accepted on the nightly channel of Cargo, but this is the `stable` channel\nSee https://doc.rust-lang.org/book/appendix-07-nightly-rust.html for more information about Rust release channels.\n" }', src/tools/miri/cargo-miri/src/util.rs:224:80
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Which seems like an expected result if I am not mistaken?

@jyn514
Copy link
Member

jyn514 commented Apr 20, 2023

Try installing nightly cargo so it gets further (rustup install nightly), or set RUSTC_BOOTSTRAP=1 so the stable cargo works.

@onur-ozkan
Copy link
Member Author

Try installing nightly cargo so it gets further (rustup install nightly), or set RUSTC_BOOTSTRAP=1 so the stable cargo works.

OOps. Why I didn't do that lol.

here we go:

~/devspace/.other/chunk-list  stable $ cargo +stage2 miri test
Preparing a sysroot for Miri (target: x86_64-unknown-linux-gnu)... done
   Compiling chunk-list v0.1.0 (/home/nimda/devspace/.other/chunk-list)
    Finished test [unoptimized + debuginfo] target(s) in 0.06s
     Running unittests src/main.rs (target/miri/x86_64-unknown-linux-gnu/debug/deps/chunk_list-b3b731111ba842c5)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

@jyn514
Copy link
Member

jyn514 commented Apr 20, 2023

this looks amazing! r=me after a rebase :)

Signed-off-by: ozkanonur <work@onurozkan.dev>
…inking

Signed-off-by: ozkanonur <work@onurozkan.dev>
@onur-ozkan
Copy link
Member Author

@bors r=jyn514 rollup

@bors
Copy link
Contributor

bors commented Apr 20, 2023

📌 Commit 68fc568 has been approved by jyn514

It is now in the queue for this repository.

@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 Apr 20, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 21, 2023
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#110365 (ship tools with sysroot)
 - rust-lang#110555 (Substitute missing trait items suggestion correctly)
 - rust-lang#110578 (fix(error): normalize whitespace during msg_to_buffer)
 - rust-lang#110597 (remove unused ftl messages)
 - rust-lang#110611 (Add regression test for rust-lang#46506)
 - rust-lang#110618 (Track if EvalCtxt has been tainted, make sure it can't be used to make query responses after)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f2321ec into rust-lang:master Apr 21, 2023
@rustbot rustbot added this to the 1.71.0 milestone Apr 21, 2023
@onur-ozkan onur-ozkan deleted the ship-tools-with-sysroot branch April 21, 2023 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
6 participants