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

[beta] Revert "Auto merge of #62948 - matklad:failable-file-loading, r=petro… #65273

Closed
wants to merge 1 commit into from

Conversation

matklad
Copy link
Member

@matklad matklad commented Oct 10, 2019

This reverts commit ef1ecbe, reversing
changes made to fc8765d.

That changed unfortunately broke rustfix on windows (rust-lang/rustfix#176).

The proper fix for this is is
#65029, but, as this is
currently in beta, we prefer to backport a reversal commit

r? @pnkfelix

@rust-highfive
Copy link
Collaborator

⚠️ Warning ⚠️

  • Pull requests are usually filed against the master branch for this repo, but this one is against beta. Please double check that you specified the right target!

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 10, 2019
…, r=petrochenkov"

This reverts commit ef1ecbe, reversing
changes made to fc8765d.

That changed unfortunately broke rustfix on windows:

rust-lang/rustfix#176

Specifically, what ef1ecbe did was to
enforce normalization of \r\n to \n at file loading time, similarly to
how we deal with Byte Order Mark. Normalization changes raw offsets in
files, which are exposed via `--error-format=json`, and used by rusfix.

The proper solution here (which also handles the latent case with BOM) is

rust-lang#65074

However, since it's somewhat involved, and we are time sensitive, we
prefer to revert the original change on beta.
@Mark-Simulacrum Mark-Simulacrum changed the title Revert "Auto merge of #62948 - matklad:failable-file-loading, r=petro… [beta] Revert "Auto merge of #62948 - matklad:failable-file-loading, r=petro… Oct 17, 2019
@Mark-Simulacrum Mark-Simulacrum added beta-accepted Accepted for backporting to the compiler in the beta channel. beta-nominated Nominated for backporting to the compiler in the beta channel. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed beta-accepted Accepted for backporting to the compiler in the beta channel. labels Oct 17, 2019
@Mark-Simulacrum
Copy link
Member

Was going to mark this as beta accepted but compiler team should actually approve the revert here, though I expect that to be mostly non-controversial.

@pnkfelix
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Oct 21, 2019

📌 Commit 5a21f76 has been approved by pnkfelix

@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 Oct 21, 2019
@pnkfelix
Copy link
Member

I'm going to go ahead and beta-accept this.

The compiler team discussed this matter two weeks ago and collectively agreed that a revert of PR #62948 was the appropriate fix.

Since this PR literally is just that revert, with no other changes coupled to it, I think we can treat the previous T-compiler discussion as an approval for landing this on the beta-branch.

@pnkfelix pnkfelix added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Oct 21, 2019
@pnkfelix
Copy link
Member

(but thank to you @Mark-Simulacrum for exercising caution here!)

@Centril
Copy link
Contributor

Centril commented Oct 25, 2019

@bors p=-1 (so that it gets rolled up)

@Mark-Simulacrum Mark-Simulacrum removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 25, 2019
@Mark-Simulacrum
Copy link
Member

Including this in #65708 so closing.

bors added a commit that referenced this pull request Oct 26, 2019
[beta] backport rollup

This includes a bunch of PRs:
 *  Fix redundant semicolon lint interaction with proc macro attributes #64387
 *  Upgrade async/await to "used" keywords. #64875
 *  syntax: fix dropping of attribute on first param of non-method assocated fn #64894
 *  async/await: improve not-send errors #64895
 *  Silence unreachable code lint from await desugaring #64930
 *  Always mark rust and rust-call abi's as unwind #65020
 *  Account for macro invocation in `let mut $pat` diagnostic. #65123
 *  Ensure that associated `async fn`s have unique fresh param names #65142
 *  Add troubleshooting section to PGO chapter in rustc book. #65402
 *  Upgrade GCC to 8.3.0, glibc to 1.17.0 and crosstool-ng to 1.24.0 for dist-armv7-linux #65302
 *  Optimize `try_expand_impl_trait_type` #65293
 *  use precalculated dominators in explain_borrow #65172
 *  Fix ICE #64964 #64989
 *  [beta] Revert "Auto merge of #62948 - matklad:failable-file-loading, r=petro… #65273
 *  save-analysis: Don't ICE when resolving qualified type paths in struct members #65353
 *  save-analysis: Nest tables when processing impl block definitions #65511
 *  Avoid ICE when checking `Destination` of `break` inside a closure #65518
 *  Avoid ICE when adjusting bad self ty #65755
 *  workaround msys2 bug #65762
@matklad matklad deleted the revert-crnl-normalization branch July 7, 2020 16:34
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. 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.

6 participants