Skip to content

Disallow versionless Python shebangs#58275

Closed
samestep wants to merge 2 commits intopytorch:masterfrom
samestep:lint-shebang-python-version
Closed

Disallow versionless Python shebangs#58275
samestep wants to merge 2 commits intopytorch:masterfrom
samestep:lint-shebang-python-version

Conversation

@samestep
Copy link
Contributor

Some machines don't have a versionless python on their PATH, which breaks these existing shebangs.

I'm assuming that all the existing versionless python shebangs are meant to be python3 and not python2; please let me know if my assumption was incorrect for any of these.

Test plan:

CI.

@samestep samestep requested a review from a team May 13, 2021 23:02
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented May 13, 2021

💊 CI failures summary and remediations

As of commit 7c4f7eb (more details on the Dr. CI page):


💚 💚 Looks good so far! There are no failures yet. 💚 💚


This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@facebook-github-bot
Copy link
Contributor

@samestep has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@samestep has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@zhouzhuojie
Copy link
Contributor

thanks! many places have safeguard on python2 usage, I think we are good

@codecov
Copy link

codecov bot commented May 14, 2021

Codecov Report

Merging #58275 (7c4f7eb) into master (7756cb6) will increase coverage by 6.38%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #58275      +/-   ##
==========================================
+ Coverage   70.39%   76.78%   +6.38%     
==========================================
  Files        1964     1987      +23     
  Lines      195938   198685    +2747     
==========================================
+ Hits       137940   152560   +14620     
+ Misses      57998    46125   -11873     

@facebook-github-bot
Copy link
Contributor

@samestep merged this pull request in 2e26976.

facebook-github-bot pushed a commit that referenced this pull request May 17, 2021
Summary:
This is the only line (not in `third_party`) matching the regex `^#!.*python2`, and [it is not the first line of its file](https://github.com/koalaman/shellcheck/wiki/SC1128), so it has no effect. As a followup to #58275, this PR removes that shebang to reduce confusion, so now all Python shebangs in this repo are `python3`.

Pull Request resolved: #58409

Reviewed By: walterddr

Differential Revision: D28478469

Pulled By: samestep

fbshipit-source-id: c17684c8651e45d3fc383cbbc04a31192d10f52f
krshrimali pushed a commit to krshrimali/pytorch that referenced this pull request May 19, 2021
Summary:
Some machines don't have a versionless `python` on their PATH, which breaks these existing shebangs.

I'm assuming that all the existing versionless `python` shebangs are meant to be `python3` and not `python2`; please let me know if my assumption was incorrect for any of these.

Pull Request resolved: pytorch#58275

Test Plan: CI.

Reviewed By: zhouzhuojie

Differential Revision: D28428143

Pulled By: samestep

fbshipit-source-id: 6562be3d12924db72a92a0207b060ef740f61ebf
krshrimali pushed a commit to krshrimali/pytorch that referenced this pull request May 19, 2021
Summary:
This is the only line (not in `third_party`) matching the regex `^#!.*python2`, and [it is not the first line of its file](https://github.com/koalaman/shellcheck/wiki/SC1128), so it has no effect. As a followup to pytorch#58275, this PR removes that shebang to reduce confusion, so now all Python shebangs in this repo are `python3`.

Pull Request resolved: pytorch#58409

Reviewed By: walterddr

Differential Revision: D28478469

Pulled By: samestep

fbshipit-source-id: c17684c8651e45d3fc383cbbc04a31192d10f52f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants