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

Non-executable shell script with shebang line should be detected as "shell" #350

Closed
l0b0 opened this issue Dec 6, 2022 · 6 comments
Closed

Comments

@l0b0
Copy link

l0b0 commented Dec 6, 2022

Actual behaviour:

$ lorri init
[…]
$ identify-cli .envrc 
["file", "non-executable", "text"]
$ chmod u+x .envrc
$ identify-cli .envrc 
["bash", "executable", "file", "shell", "text"]

Expected behaviour: the results should be identical for the executable and non-executable versions of the file, because the first line in the file is a valid shell script shebang line: #!/usr/bin/env bash.

@asottile
Copy link
Member

asottile commented Dec 6, 2022

@asottile asottile closed this as completed Dec 6, 2022
@l0b0 l0b0 changed the title Non-executable shell script should be detected as "shell" Non-executable shell script with shebang line should be detected as "shell" Dec 6, 2022
@l0b0
Copy link
Author

l0b0 commented Dec 6, 2022

@asottile I'm not sure I understand. The relevant example uses an executable file:

>>> identify.tags_from_path('/path/to/file-with-shebang')
{'file', 'text', 'shell', 'bash', 'executable'}

So this is an issue with the integration with pre-commit?

@asottile
Copy link
Member

asottile commented Dec 6, 2022

nope, this is a "you stopped reading before you got to the relevant part" problem:

If executable, the shebang is read and the interpreter interpreted

@l0b0
Copy link
Author

l0b0 commented Dec 6, 2022

OK, so it's not working as expected. Why does it not read the shebang line of non-executables?

@l0b0
Copy link
Author

l0b0 commented Dec 6, 2022

Tools like IDEs and ShellCheck do read the shebang line of non-executables, after all. So does the file command:

$ file .envrc 
.envrc: Bourne-Again shell script, ASCII text executable

@asottile
Copy link
Member

asottile commented Dec 6, 2022

the point of a shebang is to indicate to the OS how to execute it -- it is only relevant when it is executable

parsing shebangs of non-executables would also mean everything has to read every file always which is slow

this is not up for debate so please do not waste your time and my time

@pre-commit pre-commit locked as resolved and limited conversation to collaborators Dec 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

2 participants