bpo-40854: Allow overriding sys.platlibdir via PYTHONPLATLIBDIR env-var by manisandro · Pull Request #20605 · python/cpython · GitHub

You can currently point the python interpreter to a different install say via

export PYTHONHOME=<Prefix>
export PYTHONPATH=<Prefix>/lib/python3.9
python3

With the newly added platlibdir [1], if python was configured with platlibdir=lib64, this will break because i.e. the site-packages dir as returned by sysconfig.get_paths() will use lib64 and not lib as the other install may be using.

This PR adds the possibility to override the platlibdir via environment variable.

Full rationale: [2].

[1] #8068
[2] https://src.fedoraproject.org/rpms/python3.9/pull-request/10

https://bugs.python.org/issue40854

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

Recognized GitHub username

We couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames:

@manisandro

This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

See https://pythondev.readthedocs.io/pyconfig.html#add-an-environment-variable for the different steps to add a new environment variables:

Update usage_xxx to document the new env var
Doc/using/cmdline.rst: Document the new env var
Misc/python.man: Document the new env var
You may also mention it at https://docs.python.org/dev/library/sys.html#sys.platlibdir

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@vstinner Thanks for your prompt feedback, I've reworked the PR

@manisandro Could you please copy paste and use the phrase as suggested by the bot? It will label this PR accordingly.

I have made the requested changes; please review again

Thanks for making the requested changes!

@vstinner: please review the changes made to this pull request.

Thanks, the update PR is much better! New review.

Please document also the new PyConfig field in https://docs.python.org/dev/c-api/init_config.html#c.PyConfig documentation with a ".. versionadded:: 3.10" markup.

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

I have made the requested changes; please review again

Thanks for making the requested changes!

@vstinner: please review the changes made to this pull request.

It seems possible to test a custom PYTHONPATHLIBDIR in test_embed. At least, the following patch worked for me:

diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py
index 3b551e9015..f1371db866 100644
--- a/Lib/test/test_embed.py
+++ b/Lib/test/test_embed.py
@@ -586,13 +586,14 @@ class InitConfigTests(EmbeddingTestsMixin, unittest.TestCase):
             if value is self.GET_DEFAULT_CONFIG:
                 expected[key] = config[key]
 
-        pythonpath_env = expected['pythonpath_env']
-        if pythonpath_env is not None:
-            paths = pythonpath_env.split(os.path.pathsep)
-            expected['module_search_paths'] = [*paths, *expected['module_search_paths']]
-        if modify_path_cb is not None:
-            expected['module_search_paths'] = expected['module_search_paths'].copy()
-            modify_path_cb(expected['module_search_paths'])
+        if expected['module_search_paths'] is not self.IGNORE_CONFIG:
+            pythonpath_env = expected['pythonpath_env']
+            if pythonpath_env is not None:
+                paths = pythonpath_env.split(os.path.pathsep)
+                expected['module_search_paths'] = [*paths, *expected['module_search_paths']]
+            if modify_path_cb is not None:
+                expected['module_search_paths'] = expected['module_search_paths'].copy()
+                modify_path_cb(expected['module_search_paths'])
 
         for key in self.COPY_PRE_CONFIG:
             if key not in expected_preconfig:
@@ -765,6 +766,8 @@ class InitConfigTests(EmbeddingTestsMixin, unittest.TestCase):
             'buffered_stdio': 0,
             'user_site_directory': 0,
             'faulthandler': 1,
+            'platlibdir': 'my_platlibdir',
+            'module_search_paths': self.IGNORE_CONFIG,
 
             'check_hash_pycs_mode': 'always',
             'pathconfig_warnings': 0,
@@ -796,6 +799,8 @@ class InitConfigTests(EmbeddingTestsMixin, unittest.TestCase):
             'user_site_directory': 0,
             'faulthandler': 1,
             'warnoptions': ['EnvVar'],
+            'platlibdir': 'env_platlibdir',
+            'module_search_paths': self.IGNORE_CONFIG,
             '_use_peg_parser': 0,
         }
         self.check_all_configs("test_init_compat_env", config, preconfig,
@@ -824,6 +829,8 @@ class InitConfigTests(EmbeddingTestsMixin, unittest.TestCase):
             'user_site_directory': 0,
             'faulthandler': 1,
             'warnoptions': ['EnvVar'],
+            'platlibdir': 'env_platlibdir',
+            'module_search_paths': self.IGNORE_CONFIG,
             '_use_peg_parser': 0,
         }
         self.check_all_configs("test_init_python_env", config, preconfig,
diff --git a/Makefile.pre.in b/Makefile.pre.in
index 1d0bd32ffb..a7d5dd2945 100644
--- a/Makefile.pre.in
+++ b/Makefile.pre.in
@@ -782,9 +782,7 @@ Programs/python.o: $(srcdir)/Programs/python.c
 	$(MAINCC) -c $(PY_CORE_CFLAGS) -o $@ $(srcdir)/Programs/python.c
 
 Programs/_testembed.o: $(srcdir)/Programs/_testembed.c
-	$(MAINCC) -c $(PY_CORE_CFLAGS) \
-		-DPLATLIBDIR='"$(PLATLIBDIR)"' \
-		-o $@ $(srcdir)/Programs/_testembed.c
+	$(MAINCC) -c $(PY_CORE_CFLAGS) -o $@ $(srcdir)/Programs/_testembed.c
 
 Modules/_sre.o: $(srcdir)/Modules/_sre.c $(srcdir)/Modules/sre.h $(srcdir)/Modules/sre_constants.h $(srcdir)/Modules/sre_lib.h
 
diff --git a/Programs/_testembed.c b/Programs/_testembed.c
index c6a0d6c9e0..7621c46260 100644
--- a/Programs/_testembed.c
+++ b/Programs/_testembed.c
@@ -549,7 +549,7 @@ static int test_init_from_config(void)
     /* FIXME: test path config: module_search_path .. dll_path */
 
     putenv("PYTHONPLATLIBDIR=env_platlibdir");
-    status = PyConfig_SetBytesString(&config, &config.platlibdir, PLATLIBDIR);
+    status = PyConfig_SetBytesString(&config, &config.platlibdir, "my_platlibdir");
     if (PyStatus_Exception(status)) {
         PyConfig_Clear(&config);
         Py_ExitStatusException(status);
@@ -676,7 +676,7 @@ static void set_most_env_vars(void)
     putenv("PYTHONFAULTHANDLER=1");
     putenv("PYTHONIOENCODING=iso8859-1:replace");
     putenv("PYTHONOLDPARSER=1");
-    putenv("PYTHONPLATLIBDIR="PLATLIBDIR);
+    putenv("PYTHONPLATLIBDIR=env_platlibdir");
 }

Thanks, added to PR.

Ok, the PR now looks complete: all documentations are updated, thanks!

I didn't expect the new sys.platlibdir to land into PyConfig, but I think that I understood the rationale in https://bugs.python.org/issue40854

Overriding PYTHONHOME is uncommon. If you do that, I'm not surprised that you fall into corner cases. So having the ability to override platlibdir sounds reasonable.

Hum. It seems like PyConfig.platlibdir is a parameter which affects the "Path Configuration". So please add it to "Path configuration inputs" at:
https://docs.python.org/dev/c-api/init_config.html#path-configuration

Suggested change

      
                  :data:`sys.platlibdir`: platform library dir name, set at configure time
          
                  :data:`sys.platlibdir`: platform library directory name, set at configure time

Override sys.platlibdir.

nitpick: one empty line is enough :-)

…able

Thanks, I've updated the PR accordingly.

Oh, one last request, sorry!

When you update a PR, please add a new commit rather than always rebase/squash your commits. New commits help to see what has been modified since my latest review.

Oh, I didn't notice the Makefile change to define PLATLIBDIR macro. Please add an explicit check to ensure that it's defined. See Modules/getpath.c. I sugges to add at the top of this file, after includes (on Windows, it's defined by PC/pyconfig.h which is included by Python.h):

#ifndef PLATLIBDIR
#  error "PLATLIBDIR macro must be defined"
#endif

No problem, pushed as new commit.

Thanks, I merged your PR.

Thank you for merging and for the review!

…ar (GH-20605) (GH-20725) (cherry picked from commit 8f023a2) Co-authored-by: Sandro Mani <manisandro@gmail.com>

the-knights-who-say-ni added the CLA not signed label Jun 3, 2020

bedevere-bot added the awaiting review label Jun 3, 2020

vstinner requested changes Jun 3, 2020

View reviewed changes

Comment thread Python/sysmodule.c Outdated

Comment thread Python/sysmodule.c Outdated

bedevere-bot added awaiting changes and removed awaiting review labels Jun 3, 2020

manisandro changed the title bpo-40854: Allow overriding sys.platlibdir via _PYTHON_PLATLIBDIR env-var bpo-40854: Allow overriding sys.platlibdir via PYTHONPLATLIBDIR env-var Jun 3, 2020

the-knights-who-say-ni added CLA signed and removed CLA not signed labels Jun 3, 2020

manisandro requested a review from vstinner June 4, 2020 00:18

bedevere-bot added awaiting change review and removed awaiting changes labels Jun 4, 2020

vstinner requested changes Jun 5, 2020

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting change review labels Jun 5, 2020

bedevere-bot added awaiting change review and removed awaiting changes labels Jun 5, 2020

bedevere-bot requested a review from vstinner June 5, 2020 10:18

vstinner reviewed Jun 7, 2020

View reviewed changes

Allow overriding sys.platlibdir via PYTHONPLATLIBDIR environment vari…

a22a8e3

…able

vstinner reviewed Jun 8, 2020

View reviewed changes

Add check to ensure PLATLIBDIR is defined

203701c

vstinner merged commit 8f023a2 into python:master Jun 8, 2020

bedevere-bot removed the awaiting change review label Jun 8, 2020

vstinner added a commit that referenced this pull request Jun 8, 2020

bpo-40854 : Allow overriding sys.platlibdir via PYTHONPLATLIBDIR env-v…

8175064

…ar (GH-20605) (GH-20725) (cherry picked from commit 8f023a2) Co-authored-by: Sandro Mani <manisandro@gmail.com>

	:data:`sys.platlibdir`: platform library dir name, set at configure time
	:data:`sys.platlibdir`: platform library directory name, set at configure time

+                  if(config->platlibdir == NULL) {
+                      status = CONFIG_SET_BYTES_STR(config, &config->platlibdir, PLATLIBDIR,
+                                                    "PLATLIBDIR macro");

bpo-40854: Allow overriding sys.platlibdir via PYTHONPLATLIBDIR env-var by manisandro · Pull Request #20605 · python/cpython · GitHub

Repository navigation

bpo-40854: Allow overriding sys.platlibdir via PYTHONPLATLIBDIR env-var - #20605

bpo-40854: Allow overriding sys.platlibdir via PYTHONPLATLIBDIR env-var#20605vstinner merged 2 commits intopython:masterpython/cpython:masterfrom manisandro:platlibdir_envCopy head branch name to clipboard

Conversation

manisandro commented Jun 3, 2020 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

the-knights-who-say-ni commented Jun 3, 2020

Recognized GitHub username

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bedevere-bot commented Jun 3, 2020

Uh oh!

manisandro commented Jun 4, 2020

Uh oh!

hroncok commented Jun 4, 2020

Uh oh!

manisandro commented Jun 4, 2020

Uh oh!

bedevere-bot commented Jun 4, 2020

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bedevere-bot commented Jun 5, 2020

Uh oh!

manisandro commented Jun 5, 2020

Uh oh!

bedevere-bot commented Jun 5, 2020

Uh oh!

vstinner commented Jun 5, 2020

Uh oh!

manisandro commented Jun 6, 2020

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

vstinner Jun 7, 2020

Choose a reason for hiding this comment

Uh oh!

vstinner Jun 7, 2020

Choose a reason for hiding this comment

Uh oh!

vstinner Jun 7, 2020

Choose a reason for hiding this comment

Uh oh!

manisandro commented Jun 8, 2020

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

vstinner Jun 8, 2020

Choose a reason for hiding this comment

Uh oh!

manisandro commented Jun 8, 2020

Uh oh!

vstinner commented Jun 8, 2020

Uh oh!

manisandro commented Jun 8, 2020

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

bpo-40854: Allow overriding sys.platlibdir via PYTHONPLATLIBDIR env-var#20605
vstinner merged 2 commits into
python:masterfrom
manisandro:platlibdir_env

manisandro commented Jun 3, 2020 •
edited by bedevere-bot
Loading