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

Improve support of using jj to manage rust-lang/rust as a collocated git repo #128708

Closed
yaahc opened this issue Aug 5, 2024 · 3 comments · Fixed by #128755
Closed

Improve support of using jj to manage rust-lang/rust as a collocated git repo #128708

yaahc opened this issue Aug 5, 2024 · 3 comments · Fixed by #128755
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself C-enhancement Category: An issue proposing an enhancement or a PR with one. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@yaahc
Copy link
Member

yaahc commented Aug 5, 2024

Right now using jj to manage the rust-lang/rust repo locally causes x tidy to fail: #128706

This is a result of jj not yet having support for .gitattributes which is responsible for adding the crlf character to the failing test.

While I would love to get that support added to jj upstream I'm assuming it is non-trivial and I'd like a better solution in the short term so I can continue to use jj while contributing to the compiler.

@jyn514 suggested I use --exclude tests/rustdoc-ui/intra-doc/warning-crlf.rs in the meantime which I'm fine with but I'd like a way to set this in my config.toml or some equivalent mechanism doesn't require manually adding this set of arguments to every invocation of x test I make.

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 5, 2024
@yaahc yaahc changed the title Improve support using jj to manage rust-lang/rust as a collocated git repo Improve support of using jj to manage rust-lang/rust as a collocated git repo Aug 5, 2024
@jyn514
Copy link
Member

jyn514 commented Aug 5, 2024

note this is essentially the same as #35678

i actually would like an even more general solution, where the flags and configs are completely interchangable and you can specify either in either place, but a solution just for exclude seems ok too.

@rustbot label T-bootstrap A-contributor-roadblock

@rustbot rustbot added A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Aug 5, 2024
@yaahc
Copy link
Member Author

yaahc commented Aug 5, 2024

So it turns out that x test --exclude tests/rustdoc-ui/intra-doc/warning-crlf.rs won't work as a short term fix even because --exclude does not work with tidy atm.

For now I'm going with git rm tests/rustdoc-ui/intra-doc/warning-crlf.rs && git checkout HEAD !$ but I expect this to break frequently and be a source of annoyance so I'll be looking towards fixing this soon if it does in fact end up being a problem.

@sunshowers
Copy link
Contributor

re crlf, as a source control expert I'd recommend never using any kind of autoconversion -- it is a bit of a nightmare to work with in too many cases.

Rather, I'd consider checking in the file with CRLFs, and disabling autoconversion so files are always checked out as-is.

(Another option is to effectively treat the file as binary content, e.g. use some other means of encoding the file, such as base64 or putting it in a tarball. But I don't know how practical that is)

@jieyouxu jieyouxu added C-enhancement Category: An issue proposing an enhancement or a PR with one. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Aug 6, 2024
@bors bors closed this as completed in e5a3c32 Aug 7, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Aug 7, 2024
Rollup merge of rust-lang#128755 - yaahc:jj-crlf, r=estebank

Integrate crlf directly into related test file instead via of .gitattributes

resolves rust-lang#128708

This PR seeks to resolve a contributor papercut when using jj to manage the git repo locally which does not support .gitattributes. It does so by integrating the crlf characters directly into the related test and disabling Git's end of line normalization logic across platforms for that specific file, instead of configuring git to always check out the files with alternative eol characters.

related documentation: https://git-scm.com/docs/gitattributes#Documentation/gitattributes.txt-Unset-1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself C-enhancement Category: An issue proposing an enhancement or a PR with one. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants