-
Notifications
You must be signed in to change notification settings - Fork 531
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
Removing shebang in python files that are not meant to be executed #2345
Conversation
Hi @jpodivin could you provide me some references about the claims "it's best practice to set permission for scripts with shebang"? I'm a bit chilly to change those permissions to be honest. The modified files are never executed on there own. Scrips in I would be fine merging a PR that removes the shebang from the files you've referenced though. I don't think they have any purpose to be used on their own. |
The general approach, mostly for ease of use, is to set execution bits on files that are meant to be executed, and provide shebang in scripts so that the right interpreter can be chosen for them. When the bit is missing, it usually indicates that the permissions were not committed properly (that happens way too often), or that filesystem without permissions was used when the files were added (slightly less often). Either way there is little reason to believe it's on purpose and not a mistake, because shebang in non-executable is pretty much useless (you can only run the file if you provide interpreter manually). I don't really know when it got started, but it's embedded all the way to pre-commit If they are never executed directly they don't need shebang or The permission bit does have a small, secondary, benefit, in that it simplifies finding executable files. If these were never meant to be directly executable, and from the looks if it they were not. Removing shebangs is preferrable alternative to make sure it's clear. |
Thanks for the more detailed explanation @jpodivin, that makes it much clearer to me. I can confirm that those 4 files are never meant to be executed on their own so the correct fix would be to remove the shebang on those. Would you like to update the PR in that sense? Thanks! |
Signed-off-by: Jiri Podivin <jpodivin@redhat.com>
Done. The PR is now just removing shebang. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for iterating @jpodivin! PR looks better this way as it's solving the root discrepancy here :) Will merge soon
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
The best practice calls for executable scripts, such as those with shebang, to have their execution bits set.
Among other things, this helps to immediately highlight them as scripts in the file system.