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

Rollup of 3 pull requests #126821

Closed
wants to merge 16 commits into from

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

Kobzol and others added 16 commits June 20, 2024 11:33
This function both handles error printing and early/late failures, but it also always returns the actual output of the command
This trait allows cloning DSTs, but is unsafe to implement and use
because it writes to possibly-uninitialized memory which must be of the
correct size, and must initialize that memory.

It is only implemented for `T: Clone` and `[T] where T: Clone`, but
additional implementations could be provided for specific `dyn Trait`
or custom-DST types.
This requires introducing a new internal type `RcUninit` (and
`ArcUninit`), which can own an `RcBox<T>` without requiring it to be
initialized, sized, or a slice. This is similar to `UniqueRc`, but
`UniqueRc` doesn't support the allocator parameter, and there is no
`UniqueArc`.
 Generalize `{Rc,Arc}::make_mut()` to unsized types.

* `{Rc,Arc}::make_mut()` now accept any type implementing the new unstable trait `core::clone::CloneToUninit`.
* `CloneToUninit` is implemented for `T: Clone` and for `[T] where T: Clone`.
* `CloneToUninit` is a generalization of the existing internal trait `alloc::alloc::WriteCloneIntoRaw`.
* New feature gate: `clone_to_uninit`

This allows performing `make_mut()` on `Rc<[T]>` and `Arc<[T]>`, which was not previously possible.

---

Previous PR description, now obsolete:

>  Add `{Rc, Arc}::make_mut_slice()`
>
> These functions behave identically to `make_mut()`, but operate on `Arc<[T]>` instead of `Arc<T>`.
>
> This allows performing the operation on slices, which was not previously possible because `make_mut()` requires `T: Clone` (and slices, being `!Sized`, do not and currently cannot implement `Clone`).
>
> Feature gate: `make_mut_slice`

try-job: test-various
…=compiler-errors

Add `#[rustc_dump_{predicates,item_bounds}]`

Conflicts with rust-lang#126668.

As discussed
r? compiler-errors CC `@fee1-dead`
…nur-ozkan

Bootstrap command refactoring: refactor `BootstrapCommand` (step 1)

This PR is a first step towards https://rust-lang.zulipchat.com/#narrow/stream/326414-t-infra.2Fbootstrap.

It refactors `BoostrapCommand` to get it closer to a state where it is an actual command wrapper that can be routed through a central place of command execution, and also to make the distinction between printing output vs handling output programatically clearer (since now it's a mess).

The existing usages of `BootstrapCommand` are complicated primarily because of different ways of handling output. There are commands that:
1) Want to eagerly print stdout/stderr of the executed command, plus print an error message if the command fails (output mode `PrintAll`). Note that this error message attempts to print stdout/stderr of the command when `-v` is enabled, but that will always be empty, since this mode uses `.status()` and not `.output()`.
2) Want to eagerly print stdout/stderr of the executed command, but do not print any additional error message if it fails (output mode `PrintOutput`)
3) Want to capture stdout/stderr of the executed command, but print an error message if it fails (output mode `PrintFailure`). This means that the user wants to either ignore the output or handle it programatically, but that's not obvious from the name.

The difference between 1) and 2) (unless explicitly specified) is determined dynamically based on the bootstrap verbosity level.

It is very difficult for me to wrap my head around all these modes. I think that in a future PR, we should split these axes into e.g. this:
1) Do I want to handle the output programmatically or print it to the terminal? This should be a separate axis, true/false. (Note that "hiding the output" essentially just means saying that I handle it programmatically, and then I ignore the output).
2) Do I want to print a message if the command fails? Yes/No/Based on verbosity (which would be the default).

Then there is also the failure mode, but that is relatively simple to handle, the command execution will just shutdown bootstrap (either eagerly or late) when the command fails.

Note that this is just a first refactoring steps, there are a lot of other things to be done, so some things might not look "final" yet. The next steps are (not necessarily in this order):
- Remove `run` and `run_cmd` and implement everything in terms of `run_tracked` and rename `run_tracked` to `run`
- Implement the refactoring specified above (change how output modes work)
- Modify `BootstrapCmd` so that it stores `Command` instead of `&mut Command` and remove all the annoying `BootstrapCmd::from` by changing `Command::new` to `BootstrapCmd::new`
- Refactor the rest of command executions not currently using `BootstrapCmd` that can access Builder to use the correct output and failure modes. This will include passing Builder to additional places.
- Handle the most complex cases, such as output streaming. That will probably need to be handled separately.
- Refactor the rest of commands that cannot access builder (e.g. `Config::parse`) by introducing a new command context that will be passed to these places, and then stored in `Builder`. Move certain fields (such as `fail_fast`) from `Builder` to the command context.
- Handle the co-operation of `Builder`, `Build`, `Config` and command context. There are some fields and logic used during command execution that are distributed amongst `Builder/Build/Config`, so it will require some refactoring to make it work if the execution will happen on a separate place (in the command context).
- Refactor logging of commands, so that it is either logged to a file or printed in a nice hierarchical way that cooperates with the `Step` debug hierarchical output.
- Implement profiling of commands (add command durations to the command log, print a log of slowest commands and their execution counts at the end of bootstrap execution, perhaps store command executions to `metrics.json`).
- Implement caching of commands.
- Implement testing of commands through snapshot tests/mocking.

Best reviewed commit by commit.

r? `@onur-ozkan`
@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) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Jun 22, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=3

@bors
Copy link
Contributor

bors commented Jun 22, 2024

📌 Commit ec876dd has been approved by matthiaskrgr

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-review Status: Awaiting review from the assignee but also interested parties. labels Jun 22, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 22, 2024
…iaskrgr

Rollup of 3 pull requests

Successful merges:

 - rust-lang#116113 ( Generalize `{Rc,Arc}::make_mut()` to unsized types.)
 - rust-lang#126686 (Add `#[rustc_dump_{predicates,item_bounds}]`)
 - rust-lang#126731 (Bootstrap command refactoring: refactor `BootstrapCommand` (step 1))

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Contributor

bors commented Jun 22, 2024

⌛ Testing commit ec876dd with merge aabb52e...

@rust-log-analyzer
Copy link
Collaborator

The job test-various failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@bors
Copy link
Contributor

bors commented Jun 22, 2024

💔 Test failed - checks-actions

@bors bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jun 22, 2024
@bors bors added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 22, 2024
@matthiaskrgr matthiaskrgr deleted the rollup-cg3le1f branch September 1, 2024 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rollup A PR which is a rollup 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) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants