-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Allow fmt to run on rmake.rs test files #124613
Conversation
Some changes occurred in run-make tests. cc @jieyouxu |
@@ -13,7 +13,7 @@ ignore = [ | |||
|
|||
# tests for now are not formatted, as they are sometimes pretty-printing constrained | |||
# (and generally rustfmt can move around comments in UI-testing incompatible ways) | |||
"/tests/", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's safe to remove this path since it's not whitelisted, nothing will be done on it apparently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean by whitelisted? Where is that list?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way we build the "walker" with the ignore
crate is a bit strange. From what I understood, if something is not added to the list, it's ignored by default.
In this case for example, I removed the tests
folder and added rmake.rs
files and only the rmake.rs
files have been formatted.
if let Some(ignore) = ignore.strip_prefix('!') { | ||
fmt_override.add(ignore).expect(ignore); | ||
} else { | ||
fmt_override.add(&format!("!{ignore}")).expect(&ignore); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add an explanation comment (about why we remove "!" when it is present and add it when it is not present) for this block?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure!
…support, r=jieyouxu Cleanup: Rid the `rmake` test runners of `extern crate run_make_support;` `run_make_support` is part of the *extern prelude* of `rmake` test runners rendering `extern crate run_make_support` redundant: https://github.com/rust-lang/rust/blob/80451a485b006bd32732c003a54ee7de457d8266/src/tools/compiletest/src/runtest.rs#L3826-L3827 ~~Contains some fmt'ing changes because I've enabled format-on-save in my editor and because we don't run `x fmt` for `rmake` test runners yet (this gets addressed by rust-lang#124613). I can revert those if you'd like me to.~~ (reverted) r? jieyouxu or testing-devex(?) or boostrap(?)
50f2248
to
2134921
Compare
Added a code comment to explain what's going on. :) |
Thanks! @bors r+ |
…support, r=jieyouxu Cleanup: Rid the `rmake` test runners of `extern crate run_make_support;` `run_make_support` is part of the *extern prelude* of `rmake` test runners rendering `extern crate run_make_support` redundant: https://github.com/rust-lang/rust/blob/80451a485b006bd32732c003a54ee7de457d8266/src/tools/compiletest/src/runtest.rs#L3826-L3827 ~~Contains some fmt'ing changes because I've enabled format-on-save in my editor and because we don't run `x fmt` for `rmake` test runners yet (this gets addressed by rust-lang#124613). I can revert those if you'd like me to.~~ (reverted) r? jieyouxu or testing-devex(?) or boostrap(?)
Rollup merge of rust-lang#124622 - fmease:yeet-extern-crate-run_make_support, r=jieyouxu Cleanup: Rid the `rmake` test runners of `extern crate run_make_support;` `run_make_support` is part of the *extern prelude* of `rmake` test runners rendering `extern crate run_make_support` redundant: https://github.com/rust-lang/rust/blob/80451a485b006bd32732c003a54ee7de457d8266/src/tools/compiletest/src/runtest.rs#L3826-L3827 ~~Contains some fmt'ing changes because I've enabled format-on-save in my editor and because we don't run `x fmt` for `rmake` test runners yet (this gets addressed by rust-lang#124613). I can revert those if you'd like me to.~~ (reverted) r? jieyouxu or testing-devex(?) or boostrap(?)
…iaskrgr Rollup of 4 pull requests Successful merges: - rust-lang#124345 (Enable `--check-cfg` by default in UI tests) - rust-lang#124412 (io safety: update Unix explanation to use `Arc`) - rust-lang#124441 (String.truncate comment microfix (greater or equal)) - rust-lang#124604 (library/std: Remove unused `gimli-symbolize` feature) Failed merges: - rust-lang#124607 (`rustc_expand` cleanups) - rust-lang#124613 (Allow fmt to run on rmake.rs test files) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 4 pull requests Successful merges: - rust-lang#124345 (Enable `--check-cfg` by default in UI tests) - rust-lang#124412 (io safety: update Unix explanation to use `Arc`) - rust-lang#124441 (String.truncate comment microfix (greater or equal)) - rust-lang#124604 (library/std: Remove unused `gimli-symbolize` feature) Failed merges: - rust-lang#124607 (`rustc_expand` cleanups) - rust-lang#124613 (Allow fmt to run on rmake.rs test files) r? `@ghost` `@rustbot` modify labels: rollup
needs rebase |
2134921
to
8f47f97
Compare
@bors r=onur-ozkan rollup |
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#124461 (handle the targets that are missing in stage0) - rust-lang#124492 (Generalize `adjust_from_tcx` for `Allocation`) - rust-lang#124588 (Use `ObligationCtxt` in favor of `TraitEngine` in many more places) - rust-lang#124612 (Add support for inputing via stdin with run-make-support) - rust-lang#124613 (Allow fmt to run on rmake.rs test files) - rust-lang#124649 (Fix HorizonOS build broken by rust-lang#124210) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#124613 - GuillaumeGomez:fmt-run-make, r=onur-ozkan Allow fmt to run on rmake.rs test files As discussed with `@jieyouxu,` `rmake.rs` from the `run-make` testsuite would benefit from being formatted as well. Only thing needed to be done for it to work: allow support for `!` in our `rustfmt.toml` file parsing. r? `@onur-ozkan`
I'm seeing errors when trying to run
Re-adding |
I posted #124704 to fix the issue. |
…llaumeGomez Fix ignored tests for formatting This PR fixes the ignored rules in `rustfmt.toml` that were changed in rust-lang#124613 to allow formatting `rmake.rs` but ended up allowing formatting every Rust files in `tests/`. The fix is a bit involved since we need to workaround a [`.gitignore` pattern limitation](https://git-scm.com/docs/gitignore#_pattern_format): > An optional prefix "!" which negates the pattern; any matching file excluded by a previous pattern will become included again. It is not possible to re-include a file if a parent directory of that file is excluded. Git doesn’t list excluded directories for performance reasons, so any patterns on contained files have no effect, no matter where they are defined. Workaround using https://stackoverflow.com/a/5534865 I tested the fix by changing the formatting in an `rmake.rs` and UI test, and verifying that only the `rmake.rs` files were formatted. Fixes rust-lang#124613 (comment) cc `@GuillaumeGomez` r? `@onur-ozkan`
…aumeGomez Fix ignored tests for formatting This PR fixes the ignored rules in `rustfmt.toml` that were changed in rust-lang#124613 to allow formatting `rmake.rs` but ended up allowing formatting every Rust files in `tests/`. The fix is a bit involved since we need to workaround a [`.gitignore` pattern limitation](https://git-scm.com/docs/gitignore#_pattern_format): > An optional prefix "!" which negates the pattern; any matching file excluded by a previous pattern will become included again. It is not possible to re-include a file if a parent directory of that file is excluded. Git doesn’t list excluded directories for performance reasons, so any patterns on contained files have no effect, no matter where they are defined. Workaround using https://stackoverflow.com/a/5534865 I tested the fix by changing the formatting in an `rmake.rs` and UI test, and verifying that only the `rmake.rs` files were formatted. Fixes rust-lang#124613 (comment) cc `@GuillaumeGomez` r? `@onur-ozkan`
…aumeGomez Fix ignored tests for formatting This PR fixes the ignored rules in `rustfmt.toml` that were changed in rust-lang#124613 to allow formatting `rmake.rs` but ended up allowing formatting every Rust files in `tests/`. The fix is a bit involved since we need to workaround a [`.gitignore` pattern limitation](https://git-scm.com/docs/gitignore#_pattern_format): > An optional prefix "!" which negates the pattern; any matching file excluded by a previous pattern will become included again. It is not possible to re-include a file if a parent directory of that file is excluded. Git doesn’t list excluded directories for performance reasons, so any patterns on contained files have no effect, no matter where they are defined. Workaround using https://stackoverflow.com/a/5534865 I tested the fix by changing the formatting in an `rmake.rs` and UI test, and verifying that only the `rmake.rs` files were formatted. Fixes rust-lang#124613 (comment) cc `@GuillaumeGomez` r? `@onur-ozkan`
Fix ignored tests for formatting This PR fixes the ignored rules in `rustfmt.toml` that were changed in rust-lang/rust#124613 to allow formatting `rmake.rs` but ended up allowing formatting every Rust files in `tests/`. The fix is a bit involved since we need to workaround a [`.gitignore` pattern limitation](https://git-scm.com/docs/gitignore#_pattern_format): > An optional prefix "!" which negates the pattern; any matching file excluded by a previous pattern will become included again. It is not possible to re-include a file if a parent directory of that file is excluded. Git doesn’t list excluded directories for performance reasons, so any patterns on contained files have no effect, no matter where they are defined. Workaround using https://stackoverflow.com/a/5534865 I tested the fix by changing the formatting in an `rmake.rs` and UI test, and verifying that only the `rmake.rs` files were formatted. Fixes rust-lang/rust#124613 (comment) cc `@GuillaumeGomez` r? `@onur-ozkan`
Fix ignored tests for formatting This PR fixes the ignored rules in `rustfmt.toml` that were changed in rust-lang/rust#124613 to allow formatting `rmake.rs` but ended up allowing formatting every Rust files in `tests/`. The fix is a bit involved since we need to workaround a [`.gitignore` pattern limitation](https://git-scm.com/docs/gitignore#_pattern_format): > An optional prefix "!" which negates the pattern; any matching file excluded by a previous pattern will become included again. It is not possible to re-include a file if a parent directory of that file is excluded. Git doesn’t list excluded directories for performance reasons, so any patterns on contained files have no effect, no matter where they are defined. Workaround using https://stackoverflow.com/a/5534865 I tested the fix by changing the formatting in an `rmake.rs` and UI test, and verifying that only the `rmake.rs` files were formatted. Fixes rust-lang/rust#124613 (comment) cc `@GuillaumeGomez` r? `@onur-ozkan`
Fix ignored tests for formatting This PR fixes the ignored rules in `rustfmt.toml` that were changed in rust-lang/rust#124613 to allow formatting `rmake.rs` but ended up allowing formatting every Rust files in `tests/`. The fix is a bit involved since we need to workaround a [`.gitignore` pattern limitation](https://git-scm.com/docs/gitignore#_pattern_format): > An optional prefix "!" which negates the pattern; any matching file excluded by a previous pattern will become included again. It is not possible to re-include a file if a parent directory of that file is excluded. Git doesn’t list excluded directories for performance reasons, so any patterns on contained files have no effect, no matter where they are defined. Workaround using https://stackoverflow.com/a/5534865 I tested the fix by changing the formatting in an `rmake.rs` and UI test, and verifying that only the `rmake.rs` files were formatted. Fixes rust-lang/rust#124613 (comment) cc `@GuillaumeGomez` r? `@onur-ozkan`
As discussed with @jieyouxu,
rmake.rs
from therun-make
testsuite would benefit from being formatted as well.Only thing needed to be done for it to work: allow support for
!
in ourrustfmt.toml
file parsing.r? @onur-ozkan