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

manisandro · 2020-06-03T11:00:34Z

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

the-knights-who-say-ni · 2020-06-03T11:00:38Z

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!

vstinner

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

Python/sysmodule.c

bedevere-bot · 2020-06-03T14:09:25Z

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.

manisandro · 2020-06-04T00:19:07Z

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

hroncok · 2020-06-04T16:42:24Z

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

manisandro · 2020-06-04T16:43:57Z

I have made the requested changes; please review again

bedevere-bot · 2020-06-04T16:44:00Z

Thanks for making the requested changes!

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

vstinner

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.

Doc/using/cmdline.rst

Programs/_testembed.c

Python/initconfig.c

Misc/NEWS.d/next/Core and Builtins/2020-06-03-13-53-24.bpo-40854.O6vfQU.rst

bedevere-bot · 2020-06-05T08:54:11Z

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.

manisandro · 2020-06-05T10:18:23Z

I have made the requested changes; please review again

bedevere-bot · 2020-06-05T10:18:26Z

Thanks for making the requested changes!

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

vstinner · 2020-06-05T17:38:26Z

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");
 }

manisandro · 2020-06-06T01:23:38Z

Thanks, added to PR.

vstinner

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

vstinner · 2020-06-07T21:35:49Z

Doc/c-api/init_config.rst

@@ -436,6 +436,14 @@ PyConfig

      :data:`sys.base_prefix`.

+   .. c:member:: wchar_t* platlibdir
+
+      :data:`sys.platlibdir`: platform library dir name, set at configure time


Suggested change

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

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

vstinner · 2020-06-07T21:36:10Z

Misc/python.man

+Override the python platform libdir name from the value set by
+--with-platlibdir when the Python build was configured.


Override sys.platlibdir.

vstinner · 2020-06-07T21:37:20Z

Programs/_testembed.c

+        PyConfig_Clear(&config);
+        Py_ExitStatusException(status);
+    }
+


nitpick: one empty line is enough :-)

…able

manisandro · 2020-06-08T08:34:20Z

Thanks, I've updated the PR accordingly.

vstinner

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.

vstinner · 2020-06-08T12:48:32Z

Python/initconfig.c

@@ -1731,6 +1743,14 @@ config_read(PyConfig *config)
        }
    }

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


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

manisandro · 2020-06-08T14:38:01Z

No problem, pushed as new commit.

vstinner · 2020-06-08T15:28:20Z

Thanks, I merged your PR.

manisandro · 2020-06-08T15:36:49Z

Thank you for merging and for the review!

…ar (GH-20605) (GH-20725) (cherry picked from commit 8f023a2) Co-authored-by: Sandro Mani <[email protected]>

…ar (pythonGH-20605)

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

Python/sysmodule.c Outdated Show resolved Hide resolved

Python/sysmodule.c Outdated Show resolved Hide resolved

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 <[email protected]>

arun-mani-j pushed a commit to arun-mani-j/cpython that referenced this pull request Jul 21, 2020

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

2624a18

…ar (pythonGH-20605)

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

		Override the python platform libdir name from the value set by
		--with-platlibdir when the Python build was configured.

Uh oh!

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

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

Uh oh!

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!

Uh oh!

manisandro commented Jun 3, 2020 •

edited by bedevere-bot

Loading