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

cli.py is marked executable but is missing a shebang #1590

Closed
gotlougit opened this issue Mar 7, 2022 · 6 comments · Fixed by #1610
Closed

cli.py is marked executable but is missing a shebang #1590

gotlougit opened this issue Mar 7, 2022 · 6 comments · Fixed by #1610

Comments

@gotlougit
Copy link
Contributor

I was encountering issue #1323, so I executed the program by directly calling ./cli.py, however I would get an error on line 13 due to the big comment block there. Upon further inspection I found that there was no shebang in the file, and upon adding one (#!/usr/bin/env python3.9), it worked just fine. In both situations, python3.9 cli.py works.

I decided to create an issue to gather some feedback on what should be done:

  1. Should the file not be marked executable?
  2. Should a shebang be added?
@XDRAGON2002
Copy link
Contributor

XDRAGON2002 commented Mar 7, 2022

Can you elaborate on what you mean by both situations? Does is resolve the entry point error?
Can you also share what error did you face on line 13?

@terriko
Copy link
Contributor

terriko commented Mar 8, 2022

We haven't got one because we don't recommend people run cli.py directly (it can cause weird behaviour with the checkers where sometimes they won't load). We prefer that people install it for the most consistent user experience (and recommend virutalenvs for folk who don't want to system-wide install it). The issue's described a bit in #677. We have a workaround that re-generated the egg for installed copies, but it currently doesn't work if you run cli.py directly.

If we want to do add the line anyhow, I'd suggest we go with something more system-neutral #!python (or possibly #!python3) if we can. I don't know if those work on windows but I think they work on many linux systems. I'd prefer it if we found a good consistent solution for #677 first.

(Incidentally, if you want to run something without a shebang you can always use python <filename> so not having the shebang has never stopped anyone determined to run it directly, it's a speed hump at best. Maybe a comment in the file warning people about #677 would be more useful?)

@gotlougit
Copy link
Contributor Author

A comment would probably be more useful, or rather some sort of warning about running ./cli.py directly. I was just confused seeing it be marked executable and was then promptly confused when it didn't run right, so created the issue to get some more clarification.

@terriko
Copy link
Contributor

terriko commented Mar 10, 2022

Looking at the docs, I don't think we've been unclear about how to run the tool, but in the interest of making this easier let's do the following:

  • mark cli.py as non-executable (test that this doesn't affect installed versions but it shouldn't)
  • put a comment or docstring near the top of the file pointing people to the docs and indicating that it should be installed so the checkers will load correctly

@gotlougit
Copy link
Contributor Author

This seems like a good course of action

terriko pushed a commit that referenced this issue Mar 23, 2022
* mark cli.py as non-executable
* add comments indicating the file should not be run directly
@gotlougit
Copy link
Contributor Author

gotlougit commented Oct 11, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants