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

Remove shebang from nonexecutable script #7959

Merged

Conversation

hrnciar
Copy link
Contributor

@hrnciar hrnciar commented Apr 2, 2020

When packaging pip in Fedora, we have realised
that there is a nonexecutable file with a shebang line.

It seems that the primary purpose of this file is to be imported from Python
code and hence the shebang appears to be unnecessary.

Shebangs are hard to handle when doing downstream packaging because it makes
sense for upstream to use #!/usr/bin/env python while in the RPM package, we
need to avoid that and use a more specific interpreter. Since the shebang was
unused, I propose to remove it to avoid the problems.

We have found more shebangs but in vendored packages. I have also opened PRs there:
ActiveState/appdirs#144
psf/requests#5410
chardet/chardet#192

@hrnciar hrnciar force-pushed the remove-shebang-from-nonexecutable-script branch from a82b0c6 to 4905618 Compare April 2, 2020 07:40
@hrnciar hrnciar force-pushed the remove-shebang-from-nonexecutable-script branch 2 times, most recently from 28fa65d to 54da45c Compare April 9, 2020 08:26
@@ -0,0 +1,17 @@
Removes shebang from nonexecutable script.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think removal is used for a functionality that has been deprecated and removed. A change like this doesn't actually remove / break anything, the shebang had no purpose. As such, I'd consider going with trivial.

https://pip.pypa.io/en/latest/development/contributing/#choosing-the-type-of-news-entry

When packaging pip in Fedora, we have realised
that there is a nonexecutable file with a shebang line.

It seems that the primary purpose of this file is to be imported from Python
code and hence the shebang appears to be unnecessary.

Shebangs are hard to handle when doing downstream packaging because it makes
sense for upstream to use `#!/usr/bin/env python` while in the RPM package, we
need to avoid that and use a more specific interpreter. Since the shebang was
unused, I propose to remove it to avoid the problems.

We have found more shebangs but in vendored packages. I have also opened PRs there:
ActiveState/appdirs#144
psf/requests#5410
chardet/chardet#192

x
@hrnciar hrnciar force-pushed the remove-shebang-from-nonexecutable-script branch from 54da45c to b438d47 Compare April 9, 2020 09:53
@pradyunsg pradyunsg added type: enhancement Improvements to functionality and removed type: enhancement Improvements to functionality labels Apr 9, 2020
@pradyunsg pradyunsg merged commit c5da021 into pypa:master Apr 9, 2020
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label May 20, 2020
@lock lock bot locked as resolved and limited conversation to collaborators May 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants