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

Revert #71372 ("Fix #! (shebang) stripping account space issue"). #71634

Merged
merged 1 commit into from
Apr 28, 2020

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Apr 28, 2020

While #71372 fixed some of the problems #!-stripping had, it introduced others:

  • inefficient implementation (.chars().filter(...).collect() on the entire input file)
  • it ignores whitespace anywhere, stripping # ! ... which isn't a valid shebang
    • the definition of "whitespace" it uses includes newlines, which means even \n#\n!\n... is stripped as a shebang (and anything matching the regex \s*#\s*!\s*, and not followed by [, really)
  • it's backward-incompatible but didn't go through Crater

Now, #71487 is already open and will solve all of these issues. But for running Crater, and just in case #71487 takes a bit longer, I decided it's safer to just revert #71372.

This will also make #71372's diff clearer, as it will start again from the original whitespace-unaware version.

r? @petrochenkov

…ipping, r=estebank"

This reverts commit 46a8dce, reversing
changes made to f28e387.
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 28, 2020
@petrochenkov
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Apr 28, 2020

📌 Commit 4d67c8d has been approved by petrochenkov

@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 Apr 28, 2020
@petrochenkov
Copy link
Contributor

petrochenkov commented Apr 28, 2020

Did #71372 go into beta, btw?
UPD: Looks like it didn't.

bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 28, 2020
Rollup of 5 pull requests

Successful merges:

 - rust-lang#71311 (On `FnDef` type annotation suggestion, use fn-pointer output)
 - rust-lang#71488 (normalize field projection ty to fix broken MIR issue)
 - rust-lang#71489 (Fix off by one in treat err as bug)
 - rust-lang#71585 (remove obsolete comment)
 - rust-lang#71634 (Revert rust-lang#71372 ("Fix #! (shebang) stripping account space issue").)

Failed merges:

r? @ghost
@bors bors merged commit 6cad1e3 into rust-lang:master Apr 28, 2020
@eddyb eddyb deleted the revert-71372 branch April 28, 2020 16:39
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants