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

fix: move python header comments below shebang in some backends #1321

Merged
merged 2 commits into from
Nov 23, 2023

Conversation

B4ckslash
Copy link
Contributor

@B4ckslash B4ckslash commented Nov 23, 2023

Description

This PR fixes no specific issue, I just found the bug and fixed it right away.

The changed comments prevent direct execution of the backend Python scripts unless the platform is configured in some way to always execute .py files with Python. For platforms that rely on the shebang (#!) to tell them how to run the script, these comments led to an "exec format error" message because the shebang has to be on the very first line to be properly read by exec(see man 2 execve).
This should be a straightforward merge.

I also adjusted the comments themselves a bit.

Changelist:

  • Move header comments below shebang line where necessary (only in backends/python/)
  • Make header comment for barktts.py a tad more specific
  • Fix header comment in transformers.py referring to the wrong module/library class

Notes for Reviewers

These comments somehow got introduced with #1144.

When a Python script is to be executed directly via exec(3), either the platform knows how to execute
the file itself (i.e. special configuration is necessary) or the first line
contains a shebang (#!) specifying the interpreter to run it (similar to
shell scripts).

The shebang MUST be on the first line for the script to work on all platforms,
so any header comments need to be in the lines following it. Otherwise
executing these scripts as extra backends will yield an "exec format
error" message.

Changes:
* Move introductory comments below the shebang line
* Change header comment in transformers.py to refer to the correct
  python module

Signed-off-by: Marcus Köhler <khler.marcus@gmail.com>
Signed-off-by: Marcus Köhler <khler.marcus@gmail.com>
Copy link

netlify bot commented Nov 23, 2023

Deploy Preview for localai canceled.

Name Link
🔨 Latest commit e1ca8f0
🔍 Latest deploy log https://app.netlify.com/sites/localai/deploys/655f5867873d9800080d3260

@B4ckslash B4ckslash changed the title Move python header comments below shebang in some backends fix: move python header comments below shebang in some backends Nov 23, 2023
@mudler
Copy link
Owner

mudler commented Nov 23, 2023

indeed, thanks!

@mudler mudler merged commit 9dddd11 into mudler:master Nov 23, 2023
18 checks passed
@B4ckslash B4ckslash deleted the python_comments_fix branch November 24, 2023 12:41
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 this pull request may close these issues.

2 participants