Skip to content

bpo-37369: Fix initialization of sys members when launched via an app container #14428

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

Merged
merged 17 commits into from
Jun 29, 2019

Conversation

zooba
Copy link
Member

@zooba zooba commented Jun 27, 2019

@vstinner
Copy link
Member

@vstinner I expected to be able to just override prefix and exec_prefix, but they don't seem to flow through properly?

Hum, I tried the following change:

diff --git a/Modules/main.c b/Modules/main.c
index 853afedd7b..471dd0803c 100644
--- a/Modules/main.c
+++ b/Modules/main.c
@@ -65,6 +65,11 @@ pymain_init(const _PyArgv *args)
         return status;
     }
 
+    status = PyConfig_SetString(&config, &config.exec_prefix, L"/victor");
+    if (_PyStatus_EXCEPTION(status)) {
+        return status;
+    }
+
     /* pass NULL as the config: config is read from command line arguments,
        environment variables, configuration files */
     if (args->use_bytes_argv) {

It works for me:

$ ./python -c 'import sys; print(sys.exec_prefix)'
/victor

@zooba
Copy link
Member Author

zooba commented Jun 27, 2019

I think I was expecting that updating the prefix would be enough to locate the files under the prefix, but that doesn't work - it still comes up with the wrong sys.path default. So it's probably best to set home and let it infer prefix and exec_prefix.

Thanks for your other comments!

@vstinner
Copy link
Member

I think I was expecting that updating the prefix would be enough to locate the files under the prefix, but that doesn't work - it still comes up with the wrong sys.path default. So it's probably best to set home and let it infer prefix and exec_prefix.

Python Path Configuration is a black box. Nobody knows how it works! :-D

@zooba
Copy link
Member Author

zooba commented Jun 29, 2019

I'm going to give this one more review in the morning with fresh eyes (and coffee), but it should basically match the fix that's gone in for 3.7 already. This one defines more new API rather than just using a hack though (but I still want it in for the next 3.8.0 beta).

@zooba zooba merged commit 9048c49 into python:master Jun 29, 2019
@miss-islington
Copy link
Contributor

Thanks @zooba for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@zooba zooba deleted the bpo-37369 branch June 29, 2019 17:34
@miss-islington
Copy link
Contributor

Sorry, @zooba, I could not cleanly backport this to 3.8 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 9048c49322a5229ff99610aba35913ffa295ebb7 3.8

zooba added a commit to zooba/cpython that referenced this pull request Jun 29, 2019
… container (pythonGH-14428)

sys._base_executable is now always defined on all platforms, and can be overridden through configuration.
Also adds test.support.PythonSymlink to encapsulate platform-specific logic for symlinking sys.executable
@bedevere-bot
Copy link

GH-14467 is a backport of this pull request to the 3.8 branch.

lisroach pushed a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
… container (pythonGH-14428)

sys._base_executable is now always defined on all platforms, and can be overridden through configuration.
Also adds test.support.PythonSymlink to encapsulate platform-specific logic for symlinking sys.executable
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
… container (pythonGH-14428)

sys._base_executable is now always defined on all platforms, and can be overridden through configuration.
Also adds test.support.PythonSymlink to encapsulate platform-specific logic for symlinking sys.executable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants