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

Misc: better instructions for envrc, ignore /build instead of build/ #133592

Merged
merged 3 commits into from
Nov 29, 2024

Conversation

WaffleLapkin
Copy link
Member

See commits for more information.

r? @jieyouxu

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 28, 2024
@rust-log-analyzer

This comment has been minimized.

Previous setup instructions did not work without. (i.e. the envrc would
not do anything, `nix flake show..` would provide unhelpful error)
this does two things:
1. allows making `build` a symlink (which is not considered a directory
   by git, thus removal of trailing `/`).
2. removes the need to special case `rustc_mir_build/src/build`
   (leading `/` makes git only ignore the `build` in the root)
this is funny though! apparently tidy parsed `.gitignore`, but did not
recognize unignore lines (`!...`), so tidy was ignoring `rustc_mir_build`
this whole time (at least for some lints?).
@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Nov 28, 2024
@WaffleLapkin
Copy link
Member Author

this is funny though! apparently tidy parsed .gitignore, but did not recognize unignore lines (!...), so tidy was ignoring rustc_mir_build this whole time (at least for some lints?).

Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jieyouxu
Copy link
Member

jieyouxu commented Nov 29, 2024

FYI @pnkfelix since you expressed previously that you wanted to run ./x from within subdirectories, and this will modify the gitignore behavior such that it might regress such workflow. However, having gitignore need to ignore directories named build/ but then also using a negated rule to skip the mir build module (not properly supported by many tools incl. github ui and tidy) makes this really weird.

If you still really need to preserve that workflow, you can explicitly specify the build directory location via config.toml.

Thus, I'm inclined to only ignore /build directory from checkout source root and not any further subdirectories.

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Nov 29, 2024

📌 Commit 8e77349 has been approved by jieyouxu

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 Nov 29, 2024
Zalathar added a commit to Zalathar/rust that referenced this pull request Nov 29, 2024
Misc: better instructions for envrc, ignore `/build` instead of `build/`

See commits for more information.

r? `@jieyouxu`
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 29, 2024
Rollup of 7 pull requests

Successful merges:

 - rust-lang#131323 (Support `clobber_abi` in AVR inline assembly)
 - rust-lang#133092 (Always set the deployment target when building std)
 - rust-lang#133134 (Don't use a SyntheticProvider for literally every type)
 - rust-lang#133538 (Better diagnostic for fn items in variadic functions)
 - rust-lang#133590 (Rename `-Zparse-only`)
 - rust-lang#133592 (Misc: better instructions for envrc, ignore `/build` instead of `build/`)
 - rust-lang#133608 (Revert rust-lang#133418 (Store coverage source regions as `Span`) due to regression rust-lang#133606)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 29, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#131323 (Support `clobber_abi` in AVR inline assembly)
 - rust-lang#131718 ([rustdoc] Change impl items indent)
 - rust-lang#133565 (chore: fix 404 status URL)
 - rust-lang#133575 (Fix typo in RELEASES.md)
 - rust-lang#133577 (Document s390x machine access via community cloud)
 - rust-lang#133584 (Update more 2024 tests to remove -Zunstable-options)
 - rust-lang#133592 (Misc: better instructions for envrc, ignore `/build` instead of `build/`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit eabe6db into rust-lang:master Nov 29, 2024
6 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Nov 29, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 29, 2024
Rollup merge of rust-lang#133592 - WaffleLapkin:misc-meowing, r=jieyouxu

Misc: better instructions for envrc, ignore `/build` instead of `build/`

See commits for more information.

r? ``@jieyouxu``
@WaffleLapkin WaffleLapkin deleted the misc-meowing branch November 30, 2024 02:28
jhpratt added a commit to jhpratt/rust that referenced this pull request Dec 17, 2024
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 Dec 17, 2024
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 Dec 17, 2024
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants