Crater run for cargo build-dir changes #149852
Conversation
|
Some changes occurred in src/tools/cargo cc @ehuss |
rustbot
added
the
S-waiting-on-review
|
|
Do you happen to know the crater command you want to run? (The command syntax is documented at https://github.com/rust-lang/crater/blob/master/docs/bot-usage.md) |
|
Let's do @bors try |
This comment has been minimized.
This comment has been minimized.
…, r=<try> Crater run for cargo build-dir changes
|
@Kivooeo IIUC, that's not going to be sufficient for this because the new layout is not enabled by default. There may be some special settings like environment variables that will be needed to test this properly. And as for the mode, I suspect we want to build, not check, because we'll want to see if build scripts or proc-macros are doing any shenanigans that are required for linking correctly. (Or maybe |
|
For this, you can either set Note that I guess A full |
|
@craterbot run mode=build-and-test start=master#198328ad7960b1bece0dc48496bff6c62dd5d339 end=try#311784882baa42a9638298cb325fa01a062c9d0f+cargoflags=-Zbuild-dir-new-layout crates=random-1000 p=1 Small run just to make sure it mostly works. |
|
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
craterbot
added
S-waiting-on-crater
|
☔ The latest upstream changes (presumably #149934) made this pull request unmergeable. Please resolve the merge conflicts. |
|
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
🎉 Experiment
Footnotes
|
craterbot
added
S-waiting-on-review
|
Example from https://github.com/ikorason/headr at 7f4ae60bfd5efd605d9a7e2572ba482b3ac02551
Example from chute-kun 0.1.0
|
This also makes artifact deps available for consistency purposes. I originally proposed this for when moving Cargo off of Cargo's own internals but went with a different solution. Re-visiting this because `assert_cmd` has 2300+ dependents and it seems like making `assert_cmd::cargo_bin` work would be a better use of time than migrating all of those users to `assert_cmd::cargo_bin!`. For example, within the top 130 dependents (100,000 downloads or more), I found 66 that used `assert_cmd::cargo_bin` (rust-lang/rust#149852). > The reason to make `CARGO_BIN_EXE` set only at build-time was to > address the concern of manually running the test executable. > It's not > too common for tests to look at any runtime environment variables > (that I know of), so it usually just works (like running > `gdb target/debug/.../test-xxx`). > The concern was that if it were set at > runtime it would move more down the path where directly running the test > executable doesn't "just work". See https://rust-lang.zulipchat.com/#narrow/channel/246057-t-cargo/topic/cargo_bin_exe.20and.20tests/near/513638220 However, - This is user opt-in - Users can run with `-vv` to see the env variables that are set - Users can run `CARGO_TARGET_..._RUNNER=gdb` - We can notify the `cargo nextest`, the main third-party test runner, about this. It should be easy to support as they have their own version of this, see https://nexte.st/docs/configuration/env-vars/?h=nextest_bin_exe#environment-variables-nextest-sets
|
Realized a better route for this might be to move forward with making First step is rust-lang/cargo#16421 |
Unless I'm mistaken it already does that.
Footnotes |
That is a bug and it always falls back to the |
Well, yes, currently it always falls back, as the env var isn't currently set at runtime, but if your suggestion is implemented to also set the env var at runtime then it should use the value of the env var without requiring changes to the |
|
True. That bug was not copied over to |
This also makes artifact deps available for consistency purposes. I originally proposed this for when moving Cargo off of Cargo's own internals but went with a different solution. Re-visiting this because `assert_cmd` has 2300+ dependents and it seems like making `assert_cmd::cargo_bin` work would be a better use of time than migrating all of those users to `assert_cmd::cargo_bin!`. For example, within the top 130 dependents (100,000 downloads or more), I found 66 that used `assert_cmd::cargo_bin` (rust-lang/rust#149852). > The reason to make `CARGO_BIN_EXE` set only at build-time was to > address the concern of manually running the test executable. > It's not > too common for tests to look at any runtime environment variables > (that I know of), so it usually just works (like running > `gdb target/debug/.../test-xxx`). > The concern was that if it were set at > runtime it would move more down the path where directly running the test > executable doesn't "just work". See https://rust-lang.zulipchat.com/#narrow/channel/246057-t-cargo/topic/cargo_bin_exe.20and.20tests/near/513638220 However, - This is user opt-in - Users can run with `-vv` to see the env variables that are set - Users can run `CARGO_TARGET_..._RUNNER=gdb` - We can notify the `cargo nextest`, the main third-party test runner, about this. It should be easy to support as they have their own version of this, see https://nexte.st/docs/configuration/env-vars/?h=nextest_bin_exe#environment-variables-nextest-sets
|
Note that the crater run above was partial -- should we run a full one across all crates? Or does the above results already have enough value that we want to tweak before scheduling a full run? |
|
Waiting on a cargo feature to be merged to reduce known issues. |
Mark-Simulacrum
added
S-waiting-on-author
9 participants
PR for crater run of rust-lang/cargo#16336
Before running, we should probably get a sanity check from @epage that there is nothing I missed.
See Zulip Thread
r? @Mark-Simulacrum