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

Ensure that checkout is with \n line endings #62564

Merged
merged 1 commit into from
Jul 10, 2019

Conversation

Mark-Simulacrum
Copy link
Member

@Mark-Simulacrum Mark-Simulacrum commented Jul 10, 2019

During installation of mingw, at least, the git directories change, so
we need to reset the core.autocrlf config to false.

Once we finish checking out submodules, check that the line endings are
\n and not \r\n.

Artifacts were built via the last try on #62545; I've manually confirmed that install.sh appears to no longer have \r\n line endings.

Fixes #62276.

During installation of mingw, at least, the git directories change, so
we need to reset the core.autocrlf config to false.

Once we finish checking out submodules, check that the line endings are
\n and not \r\n.
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 10, 2019
@pietroalbini
Copy link
Member

@bors r+ p=100

@bors
Copy link
Contributor

bors commented Jul 10, 2019

📌 Commit df725c2 has been approved by pietroalbini

@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 Jul 10, 2019
@bors
Copy link
Contributor

bors commented Jul 10, 2019

⌛ Testing commit df725c2 with merge cd2cd4c...

bors added a commit that referenced this pull request Jul 10, 2019
…bini

Ensure that checkout is with \n line endings

During installation of mingw, at least, the git directories change, so
we need to reset the core.autocrlf config to false.

Once we finish checking out submodules, check that the line endings are
\n and not \r\n.

Artifacts were built via the last try on #62545; I've manually confirmed that `install.sh` appears to no longer have `\r\n` line endings.

Fixes #62276.
@bors
Copy link
Contributor

bors commented Jul 10, 2019

☀️ Test successful - checks-azure, checks-travis, status-appveyor
Approved by: pietroalbini
Pushing cd2cd4c to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 10, 2019
@bors bors merged commit df725c2 into rust-lang:master Jul 10, 2019
@pietroalbini
Copy link
Member

@rustbot modify labels: beta-nominated T-infra

Nominating for beta backport.

@rustbot rustbot added beta-nominated Nominated for backporting to the compiler in the beta channel. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Jul 11, 2019
@alexcrichton
Copy link
Member

If we can control this in each repository would that perhaps be a better solution? It seems odd to just sort of reconfigure git a bunch of time seemingly randomly in the azure configuration...

@Mark-Simulacrum
Copy link
Member Author

The problem is likely to come in e.g. tests, etc. as well -- IMO, this is a bug in pipelines that they set CRLF line endings to enabled for us, we just need to work around that.

@alexcrichton
Copy link
Member

But it's not problematic in the sense of all our tests pass? W/e azure does by default seems to run for our test suite since it's all passing? This just seemed like an overly large change to our configuration on Azure relative to the change here

@Mark-Simulacrum
Copy link
Member Author

I believe compiletest intentionally normalizes line endings and such so that's probably why our tests are passing. I do think it's plausible we can get a more targeted fix in -- this was mostly intended to "stop the bleeding" and we can revert and apply selective patches if that route would be preferable to you.

I mostly wanted it done in rust-lang/rust itself so that we can avoid having to patch multiple repositories to disable autocrlf, but maybe that was a false fear. We'll need to update rust-lang/rust's .gitattributes anyway because they're insufficient to remove CRLF from all files, e.g., the rust-src component is shipping with CRLF line endings in Cargo.toml, README, etc. prior to this change I believe which seems not great.

I also think I can relatively easily reduce the amount of calls to ~2 -- I think one of them in this PR is probably not necessary, though I can't be certain.

@mati865
Copy link
Contributor

mati865 commented Jul 12, 2019

FWIW Miri tests were failing until .gitattributes were added.
Clippy in the past had issues with CRLF and tests so it added .gitattributes back in December. Otherwise it would also fail.

@pietroalbini pietroalbini added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Jul 22, 2019
@pietroalbini pietroalbini removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jul 22, 2019
bors added a commit that referenced this pull request Jul 22, 2019
[beta] Rollup backports

Cherry picked:

* rustc_target: avoid negative register counts in the SysV x86_64 ABI. #62380
* Fix ICEs when `Self` is used in type aliases #62417
* Raise the default recursion limit to 128 #62450
* Handle errors during error recovery gracefully #62604
* Correctly break out of recovery loop #62607
* Cancel unemitted diagnostics during error recovery #62666
* ci: pin awscli dependencies #62856
* Ensure that checkout is with \n line endings #62564

Rolled up:

* [beta] Backport #62615 #62793
* [beta] Fix #62660 #62792

r? @ghost
bors added a commit that referenced this pull request Jul 22, 2019
[beta] Rollup backports

Cherry picked:

* rustc_target: avoid negative register counts in the SysV x86_64 ABI. #62380
* Fix ICEs when `Self` is used in type aliases #62417
* Raise the default recursion limit to 128 #62450
* Handle errors during error recovery gracefully #62604
* Correctly break out of recovery loop #62607
* Cancel unemitted diagnostics during error recovery #62666
* ci: pin awscli dependencies #62856
* Ensure that checkout is with \n line endings #62564

Rolled up:

* [beta] Backport #62615 #62793
* [beta] Fix #62660 #62792

r? @ghost
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DOS Line Endings in install.sh on some rust-std nightlies
7 participants