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 python3 shebang paths and add python option. #86

Merged
merged 1 commit into from
Jul 5, 2024

Conversation

jameshilliard
Copy link
Contributor

@jameshilliard jameshilliard commented Jul 2, 2024

Using absolute paths can result in the wrong python3 binary being used, such as when cross compiling using a non-system python3.

Use the normal python3 env shebang instead.

Also add a meson option to allow overriding the python path.

@hughsie
Copy link
Member

hughsie commented Jul 2, 2024

I thought that was bad? https://jmmv.dev/2016/09/env-considered-harmful.html

@jameshilliard
Copy link
Contributor Author

I thought that was bad? https://jmmv.dev/2016/09/env-considered-harmful.html

This seems to be referring to installed scripts that need a specific python interpreter with a known python path rather than build scripts like this, they are recommending one generate shebang lines for script installation and do not appear to be recommending using hard coded paths for build scripts. In general the behavior of using the python3 binary found in PATH is correct for build scripts AFAIU, hard coding like this does not work as it assumes /usr/bin/python3 is the correct python3 binary to use(which will not be the case in various circumstances).

@hughsie
Copy link
Member

hughsie commented Jul 3, 2024

In fwupd (the main package) we use the python3 picked up by meson -- e.g. https://github.com/fwupd/fwupd/blob/main/generate-build/meson.build#L6C17-L6C24 and https://github.com/fwupd/fwupd/blob/main/meson.build#L306 -- would that work here?

@jameshilliard
Copy link
Contributor Author

would that work here?

I think so, since directly executing the script with a specific python executable should ensure the incorrect shebang is ignored, although probably a good idea to fix the shebang either way.

Using absolute paths can result in the wrong python3 binary being
used, such as when cross compiling using a non-system python3.

Use the normal python3 env shebang instead.

Also add a meson option to allow overriding the python path.

Signed-off-by: James Hilliard <[email protected]>
@jameshilliard jameshilliard changed the title Fix python3 shebang paths Fix python3 shebang paths and add python option. Jul 5, 2024
@jameshilliard
Copy link
Contributor Author

In fwupd (the main package) we use the python3 picked up by meson

Ported this option as well from fwupd.

@superm1 superm1 merged commit 1cd1ff0 into fwupd:main Jul 5, 2024
5 checks passed
@jameshilliard jameshilliard deleted the fix-python-paths branch July 5, 2024 01:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants