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

EdkRepo: Specify the Python version in installed Python launcher #47

Merged
merged 1 commit into from
Nov 16, 2021

Conversation

ashedesimone
Copy link
Contributor

The Python launchers installed by EdkRepo should explicitly point
to the supported version where possible allowing and git hooks to
'import edkrepo' if needed.

Signed-off-by: Ashley E Desimone [email protected]

Copy link
Member

@nate-desimone nate-desimone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no guarantee that Python 3.8 will be the version of Python that EdkRepo gets installed to.

@@ -86,7 +86,7 @@ public static string EdkrepoPython3LauncherScript
{
get
{
return "python3";
return "python -3.8";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the command to run the Python interpreter. It is the file name for the shell script that launches the Python interpreter. This file name must continue to be "python3". Otherwise, "#!/usr/bin/env python3" will no longer work.

The Python launchers installed by EdkRepo should explicitly point
to the supported version where possible allowing and git hooks to
'import edkrepo' if needed.

Signed-off-by: Ashley E Desimone <[email protected]>
Reviewed-by: Nate DeSimone <[email protected]>
@ashedesimone ashedesimone merged commit 8bd8bde into tianocore:main Nov 16, 2021
@ashedesimone ashedesimone deleted the py_launcher branch November 16, 2021 21:17
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