Add a tidy rule forbidding modules named build by Zalathar · Pull Request #133404 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation8 Commits5 Checks6 Files changed
Conversation
This file contains hidden or 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
[ Show hidden characters]({{ revealButtonHref }})
The module directory compiler/rustc_mir_build/src/build/ causes a number of contributor papercuts:
- GitHub's go-to-file feature doesn't work for any files in this directory.
- We need a negated rule in .gitignore to override the blanket ignore rule for
build/. - Some git-adjacent tools automatically parse .gitignore but don't understand negative rules, so they inappropriately ignore the directory.
- Some tidy checks are accidentally not applied within this directory, probably for similar reasons.
Any one of these might be fixable or ignorable, but all of them together suggest that allowing modules named build is just not worth the hassle, especially when it isn't even necessary.
This PR therefore adds a tidy check forbidding mod build; and similar, and renames the existing modules that would be forbidden by that check. There are no functional changes in the affected modules.
(The check also has the side-effect of forbidding build.rs files that aren't build scripts, which seems net-positive.)
This was not detected by x test tidy, presumably because it's inside a
directory named "build". Fix it now, because later changes in this PR will
cause it to be detected.
r? @pnkfelix
rustbot has assigned @pnkfelix.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r? to explicitly pick a reviewer
Area: The testsuite used to check the correctness of rustc
Status: Awaiting review from the assignee but also interested parties.
Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Relevant to the compiler team, which will review and decide on the PR/issue.
The Rustc Trait System Refactor Initiative (-Znext-solver)
labels
Because we have a .gitignore rule for build/, having a module directory named
"build" causes various problems for tools that try to skip ignored files.
Forbidding this in tidy is trickier than it looks, because tidy itself tries to skip ignored files. So the rule instead looks in non-ignored files for module declarations that would imply the existence of potentially-ignored module directories.
This also has the side-effect of prohibiting build.rs files that aren't build
scripts.
Replaced manual LazyLock with static_regex! (diff).
Ignoring non-root build directories was done deliberately in #106440.
Hi @Zalathar, thanks for the PR. I'm going to close this in favor of #133592, as I think not having the special .gitignore hacks is overall a better experience and it avoids having to add more special cases to tidy.
I would be in favor, however, of renaming rustc_mir_build/src/build/ to something... more descriptive if those more familiar with MIR has better names...
Renaming rustc_mir_build::build extracted to #134365.
jhpratt added a commit to jhpratt/rust that referenced this pull request
Rename rustc_mir_build::build to builder
GitHub's “Go to file” feature silently ignores all files in this module, presumably because they are in a directory named build, which is mistaken for a build-output directory. That makes it meaningfully harder to view those files and their history via GitHub.
This PR sidesteps that issue by renaming build to builder, which in context has basically the same meaning, but can't be mistaken for a build-output directory.
Extracted from rust-lang#133404, after rust-lang#133592 changed the .gitignore rule from build/ to /build. The problem of GitHub ignoring these files still exists even after that change, which suggests that GitHub's behaviour is a hard-coded heuristic that isn't influenced by the repository's git settings.
Currently this PR doesn't include the tidy rule forbidding build as a module name, but that could be added if people think it's a good idea.
jhpratt added a commit to jhpratt/rust that referenced this pull request
Rename rustc_mir_build::build to builder
GitHub's “Go to file” feature silently ignores all files in this module, presumably because they are in a directory named build, which is mistaken for a build-output directory. That makes it meaningfully harder to view those files and their history via GitHub.
This PR sidesteps that issue by renaming build to builder, which in context has basically the same meaning, but can't be mistaken for a build-output directory.
Extracted from rust-lang#133404, after rust-lang#133592 changed the .gitignore rule from build/ to /build. The problem of GitHub ignoring these files still exists even after that change, which suggests that GitHub's behaviour is a hard-coded heuristic that isn't influenced by the repository's git settings.
Currently this PR doesn't include the tidy rule forbidding build as a module name, but that could be added if people think it's a good idea.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
Rollup merge of rust-lang#134365 - Zalathar:builder, r=nnethercote
Rename rustc_mir_build::build to builder
GitHub's “Go to file” feature silently ignores all files in this module, presumably because they are in a directory named build, which is mistaken for a build-output directory. That makes it meaningfully harder to view those files and their history via GitHub.
This PR sidesteps that issue by renaming build to builder, which in context has basically the same meaning, but can't be mistaken for a build-output directory.
Extracted from rust-lang#133404, after rust-lang#133592 changed the .gitignore rule from build/ to /build. The problem of GitHub ignoring these files still exists even after that change, which suggests that GitHub's behaviour is a hard-coded heuristic that isn't influenced by the repository's git settings.
Currently this PR doesn't include the tidy rule forbidding build as a module name, but that could be added if people think it's a good idea.
Labels
Area: The testsuite used to check the correctness of rustc
Status: Awaiting decision from the relevant subteam (see the T- label).
Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Relevant to the compiler team, which will review and decide on the PR/issue.
The Rustc Trait System Refactor Initiative (-Znext-solver)