Rollup of 4 pull requests #134677
Merged
Rollup of 4 pull requests #134677
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
`recursive_remove` is intended to be a wrapper around `std::fs::remove_dir_all`, but which also allows the removal target to be a non-directory entry, i.e. a file or a symlink. It also tries to remove read-only attributes from filesystem entities on Windows for non-dir entries, as `std::fs::remove_dir_all` handles the dir (and its children) read-only cases. Co-authored-by: Chris Denton <chris@chrisdenton.dev>
`aggressive_rm_rf` does not correctly handle distinction between symlink-to-file vs symlink-to-dir on Windows.
This facade is like other `run_make_support::fs` APIs that panic-on-failure but includes the path that the operation was called on in the panic message.
Add platform docs for FreeBSD.
…Denton test-infra: improve compiletest and run-make-support symlink handling I was trying to implement rust-lang#134656 to port `tests/run-make/incr-add-rust-src-component.rs`, but found some blockers related to symlink handling, so in this PR I tried to resolve them by improving symlink handling in compiletest and run-make-support (particularly for native windows msvc environment). Key changes: - I needed to copy symlinks (duplicate a symlink pointing to the same file), so I pulled out the copy symlink logic and re-exposed it as `run_make_support::rfs::copy_symlink`. This helper correctly accounts for the Windows symlink-to-file vs symlink-to-dir distinction (hereafter "Windows symlinks") when copying symlinks. - `recursive_remove`: - I needed a way to remove symlinks themselves (no symlink traversal). `std::fs::remove_dir_all` handles them, but only if the root path is a directory. So I wrapped `std::fs::remove_dir_all` to also handle when the root path is a non-directory entity (e.g. file or symlink). Again, this properly accounts for Windows symlinks. - I wanted to use this for both compiletest and run-make-support, so I put the implementation and accompanying tests in `build_helper`. - In this sense, it's a reland of rust-lang#129302 with proper test coverage. - It's a thin wrapper around `std::fs::remove_dir_all` (`remove_dir_all` correctly handles read-only entries on Windows). The helper has additional permission-setting logic for when the root path is a non-dir entry on Windows to handle read-only non-dir entry. Fixes rust-lang#126334.
…ue-fmt, r=ytmimi Make sure we don't lose default struct value when formatting struct The reason why rust-lang/rustfmt#6424 broke when rust-lang#129514 landed is because the parser now *successfully* parses default struct values. That means that where we used to fail in `rewrite_macro_inner`: https://github.com/rust-lang/rust/blob/e108481f74ff123ad98a63bd107a18d13035b275/src/tools/rustfmt/src/macros.rs#L263-L267 ... we now succeed, and we now proceed to format the inner struct as a macro arg. Since formatting was never implemented for default struct values, this means that we’ll always rewrite the struct to exclude the default value. This PR makes it so that we simply don’t rewrite struct fields if they have a default value. r? `@ytmimi`
…esleywiser Revert stabilization of the `#[coverage(..)]` attribute Due to a process mixup, the PR to stabilize the `#[coverage(..)]` attribute (rust-lang#130766) was merged while there are still outstanding concerns. The default action in that situation is to revert, and the feature is not sufficiently urgent or uncontroversial to justify special treatment, so this PR reverts that stabilization. --- - A key point that came up in offline discussions is that unlike most user-facing features, this one never had a proper RFC, so parts of the normal stabilization process that implicitly rely on an RFC break down in this case. - As the implementor and de-facto owner of the feature in its current form, I would like to think that I made good choices in designing and implementing it, but I don't feel comfortable proceeding to stabilization without further scrutiny. - There hasn't been a clear opportunity for T-compiler to weigh in or express concerns prior to stabilization. - The stabilization PR cites a T-lang FCP that occurred in the tracking issue, but due to the messy design and implementation history (and lack of a clear RFC), it's unclear what that FCP approval actually represents in this case. - At the very least, we should not proceed without a clear statement from T-lang or the relevant members about the team's stance on this feature, especially in light of the other concerns listed here. - The existing user-facing documentation doesn't clearly reflect which parts of the feature are stable commitments, and which parts are subject to change. And there doesn't appear to be a clear consensus anywhere about where that line is actually drawn, or whether the chosen boundary is acceptable to the relevant teams and individuals. - For example, the [stabilization report comment](rust-lang#84605 (comment)) mentions that some aspects are subject to change, but that text isn't consistent with my earlier comments, and there doesn't appear to have been any explicit discussion or approval process. - [The current reference text](https://github.com/rust-lang/reference/blob/4dfaa4f/src/attributes/coverage-instrumentation.md) doesn't mention this distinction at all, and instead simply describes the current implementation behaviour. - When the implementation was changed to its current form, the associated user-facing error messages were not updated, so they still refer to the attribute only being allowed on functions and closures. - On its own, this might have been reasonable to fix-forward in the absence of other concerns, but the fact that it never came up earlier highlights the breakdown in process that has occurred here. --- Apologies to everyone who was excited for this stabilization to land, but unfortunately it simply isn't ready yet.
rustbot
added
A-compiletest
|
@bors r+ rollup=never p=5 |
bors
added
S-waiting-on-bors
|
📣 Toolstate changed by #134677! Tested on commit 85c3989. 💔 reference on windows: test-pass → test-fail (cc @ehuss @matthewjasper @Havvy). |
rust-highfive
added a commit
to rust-lang-nursery/rust-toolstate
that referenced
this pull request
Dec 23, 2024
Tested on commit rust-lang/rust@85c3989. Direct link to PR: <rust-lang/rust#134677> 💔 reference on windows: test-pass → test-fail (cc @ehuss @matthewjasper @Havvy). 💔 reference on linux: test-pass → test-fail (cc @ehuss @matthewjasper @Havvy).
|
☀️ Test successful - checks-actions |
|
📌 Perf builds for each rolled up PR:
previous master: 66bb586952 In the case of a perf regression, run the following command for each PR you suspect might be the cause: |
This was referenced Dec 23, 2024
|
Finished benchmarking commit (85c3989): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (primary -0.5%, secondary 0.0%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (secondary 2.7%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 762.559s -> 763.753s (0.16%) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Successful merges:
#[coverage(..)]attribute #134672 (Revert stabilization of the#[coverage(..)]attribute)r? @ghost
@rustbot modify labels: rollup
Create a similar rollup