msg322487 - (view) |
Author: Ronald Oussoren (ronaldoussoren) * |
Date: 2018-07-27 12:07 |
Attached is a very basic launcher for python scripts. This script works as expected in Python 3.6, but not in 3.7.0.
In particular, the launcher sets the environment variable PYTHONOPTIMIZE before calling Py_Initialize and that setting is ignored.
I haven't tried testing with the tip of the tree yet.
|
msg322519 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2018-07-28 03:25 |
This feels like a duplicate of https://bugs.python.org/issue31845 (see https://github.com/python/cpython/commit/d7ac06126db86f76ba92cbca4cb702852a321f78 )
Is this definitely with the 3.7.0 release, or is it potentially with one of the earlier alphas before that fix was applied?
|
msg322520 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2018-07-28 03:44 |
Ah, wait - I see a relevant difference: the test case for issue31845 uses Py_Main, *not* Py_Initialize (since it's a command line test, not an embedding test).
That means it's currently possible for the sequence of operations in https://github.com/python/cpython/blob/7cbde0e09daba4259565738e48f141851287fe29/Python/pylifecycle.c#L913 to be wrong (or missing steps) without causing the test suite to fail.
|
msg322521 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2018-07-28 03:56 |
Comparing to Python 3.6, we can see that the code to update the C global flags from the environment variables used to live directly in _PyInitializeEx_Private: https://github.com/python/cpython/blob/9d85856044a2c7d681ea38b5ff99af375b228a0f/Python/pylifecycle.c#L305
In Python 3.7 that section (https://github.com/python/cpython/blob/3.7/Python/pylifecycle.c#L913 ) currently calls _PyCoreConfig_Read instead, which *only* updates the config struct - it doesn't update the C level global variables.
By contrast, Py_Main eventually calls https://github.com/python/cpython/blob/7cbde0e09daba4259565738e48f141851287fe29/Modules/main.c#L1394 which handles writing the state from the config struct back to the global variables.
|
msg322522 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2018-07-28 04:00 |
Next steps:
- add a test case that runs the same sys.flags checks as in https://github.com/python/cpython/commit/d7ac06126db86f76ba92cbca4cb702852a321f78, but as a test_embed embedding test using Py_Initialize rather than Py_Main
- create a shared internal API that both Py_Main and Py_Initialize can use to write the environment variable settings back to the global variables
|
msg322523 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2018-07-28 04:04 |
Ned, I think we should consider this a release blocker for 3.7.1
|
msg322524 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2018-07-28 05:09 |
This seems closely related to the work Victor is already pursuing to resolve bpo-34170 for Python 3.8 (e.g. https://github.com/python/cpython/commit/56b29b6d6fa3eb32bb1533ee3f21b1e7135648a0 ).
The bpo-34170 changes are more invasive than we'd like for a maintenance release, but it would be preferable to keep at least any new Python 3.7 test cases somewhat aligned with their Python 3.8 counterparts.
|
msg322529 - (view) |
Author: Ronald Oussoren (ronaldoussoren) * |
Date: 2018-07-28 09:13 |
I'm at the EuroPython sprints and currently working on a testcase for this. Should be a nice way to get back into actively contributing to CPython :-)
|
msg322532 - (view) |
Author: Ronald Oussoren (ronaldoussoren) * |
Date: 2018-07-28 09:24 |
FYI: I've filed issue34255 while working on this, test_embed assumes that you're building in the source directory (which I usually don't do).
|
msg322533 - (view) |
Author: Ronald Oussoren (ronaldoussoren) * |
Date: 2018-07-28 09:52 |
Interesting... pylifecycle.c uses code in main.c, I hadn't expected that.
If I've read the code correctly Py_Initialize won't look at PYTHONOPTIMZE at all (and other environment variables that have a command-line equivalent). Those are read in main.c:cmdline_get_env_flags, which is only called (indirectly) from Py_Main and _Py_UnixMain.
I'm note sure what's the correct solution for this.
1. Reading PYTHONOPTIMZE (and related variables) could be done
in _PyCoreConfig_Read, but that would then need to grow a flag to ensure
that this won't replace values calculated earlier in the implementation
of Py_Main
2. Writing back the global variables is probably best done using a new
function (lets _PyCoreConfig_SetGlobals), mirroring
pymain_set_global_config but using a _PyCoreConfig config structure
instead of the command-line structure used by the latter function.
|
msg322550 - (view) |
Author: Ronald Oussoren (ronaldoussoren) * |
Date: 2018-07-28 12:39 |
I've created a pull request that seems to work properly, although I am not entirely sure that this is the right way to fix this.
As I mentioned in the pull request I created it against the 3.7 branch because of bpo-34170, but would happily rebase it for master.
|
msg322562 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2018-07-28 14:22 |
I believe Victor is at the EuroPython sprints as well, so if I'm right about that, I think the best option would be for the two of you to work out a resolution for 3.7.1 in person that at least keeps the test suites reasonably consistent, even if the implementations have diverged (the new tests already added for bpo-34170 were the main reason I stopped working on my initial patch for this).
And agreed on fixing the current dependency inversion between pylifecycle and the py_main code - the functions needed by both Py_Main and Py_Initialize should be migrated down the stack into pylifecycle as part of the bpo-34170 work.
|
msg323068 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2018-08-03 20:27 |
I first proposed to backport all code from master:
https://mail.python.org/pipermail/python-dev/2018-July/154882.html
But I'm not sure that it's a good idea, since I made *many* changes in the master branch since 3.7... I even added more files, and the _PyCoreConfig API is going to change anyway...
So I wrote a simpler PR 8659 which backports unit tests, adapt them for Python 3.7, and fix this issue: PYTHONOPTIMIZE and other environment variables and now read by Py_Initialize(), not only by Py_Main().
|
msg323070 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2018-08-03 20:35 |
I tested manually attached t.c with PR 8659: I get optimize=2 as expected. Full output:
sys.flags(debug=0, inspect=0, interactive=0, optimize=2, dont_write_bytecode=0, no_user_site=0, no_site=0, ignore_environment=0, verbose=0, bytes_warning=0, quiet=0, hash_randomization=1, isolated=0, dev_mode=False, utf8_mode=1)
|
msg323135 - (view) |
Author: Ronald Oussoren (ronaldoussoren) * |
Date: 2018-08-05 07:40 |
The PR works for me as well, and looks ok to me.
|
msg323144 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2018-08-05 10:32 |
New changeset 0c90d6f75931da4fec84d06c2efe9dd94bb96b77 by Victor Stinner in branch '3.7':
[3.7] bpo-34247: Fix Python 3.7 initialization (#8659)
https://github.com/python/cpython/commit/0c90d6f75931da4fec84d06c2efe9dd94bb96b77
|
msg323145 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2018-08-05 10:33 |
On my PR, Nick Coghlan wrote:
"We may want to tweak the porting note in What's New, as I believe this may read more environment variables at init time than the interpreter did previously, so embedding applications that want more complete control may now need to set Py_IgnoreEnvironmentFlag when they could previously get by without it."
https://github.com/python/cpython/pull/8659#pullrequestreview-143397378
I'm not sure about this, since it's really a bug of Python 3.7.0 and it's now fixed.
I let you decide if the issue should be closed or not, I leave it open.
|
msg323156 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2018-08-05 16:07 |
My comment wasn't about the 3.7.0 -> 3.7.1 fix, but rather about the fact that I suspect that 3.7.1 is now going to respect some environment variables in this case that 3.6.x ignored. I don't know for sure though, since we didn't have a test case for this until you wrote one.
So I was thinking we could include a porting note along the lines of:
===============
- starting in 3.7.1, `Py_Initialize` now consistently reads and respects all of the same environment settings as `Py_Main` (in earlier Python versions, it respected an ill-defined subset of those environment variables, while in Python 3.7.0 it didn't read any of them due to :issue:`34247`). If this behaviour is unwanted, set `Py_IgnoreEnvironmentFlag = 1` before calling `Py_Initialize`.
===============
That note then serves two purposes:
- it lets embedders know that `Py_IgnoreEnvironmentFlag = 0` just plain doesn't work properly in 3.7.0 (without their needing to find this bug for themselves)
- it lets embedders know that they may want to set `Py_IgnoreEnvironmentFlag = 1` if their embedded interpreter is now being affected by environmental settings that it ignored in previous versions
|
msg325172 - (view) |
Author: Ned Deily (ned.deily) * |
Date: 2018-09-12 19:26 |
PR 9223 adds Nick's proposed wording to the 3.7 What's News.
|
msg325281 - (view) |
Author: Ned Deily (ned.deily) * |
Date: 2018-09-13 18:49 |
New changeset 66755cbb1e529f54c9066639ebbbac81add0affd by Ned Deily in branch 'master':
bpo-34247: add porting note to 3.7 What's New (GH-9223)
https://github.com/python/cpython/commit/66755cbb1e529f54c9066639ebbbac81add0affd
|
msg325283 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2018-09-13 18:51 |
> (in earlier Python versions, it respected an ill-defined subset of those environment variables, ...)
I'm not aware of the Python 3.6 issue ("ill-defined"): which env vars were not properly handled?
|
msg325288 - (view) |
Author: miss-islington (miss-islington) |
Date: 2018-09-13 19:14 |
New changeset 305056494d7e1debec3df268b8925725b0110293 by Miss Islington (bot) in branch '3.7':
bpo-34247: add porting note to 3.7 What's New (GH-9223)
https://github.com/python/cpython/commit/305056494d7e1debec3df268b8925725b0110293
|
msg325289 - (view) |
Author: Ned Deily (ned.deily) * |
Date: 2018-09-13 19:19 |
I've merged Nick's suggested porting note so I'm going to remove the "release blocker" status. Can we also close this now?
|
msg325862 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2018-09-20 12:32 |
The "ill-defined" in Python 3.6 relates to the fact that we never actually defined or tested which environment variables were read by Py_Main and which ones were read by Py_Initialize, since the majority of our tests only covered Py_Main (by launching a full Python subprocess).
Thus the only way to find out which environment variables fell into which category would be to read the Python 3.6 source code.
That's technically still the case for Python 3.7, but Victor's done some excellent refactoring work so it's much clearer in the code which vars may be updated if Py_Initialize is run a second time.
|
msg325874 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2018-09-20 13:42 |
> The "ill-defined" in Python 3.6 relates to the fact that we never actually defined or tested which environment variables were read by Py_Main and which ones were read by Py_Initialize, since the majority of our tests only covered Py_Main (by launching a full Python subprocess).
>
> Thus the only way to find out which environment variables fell into which category would be to read the Python 3.6 source code.
Oh ok, I see. Thanks for the explanation.
> That's technically still the case for Python 3.7, but Victor's done some excellent refactoring work so it's much clearer in the code which vars may be updated if Py_Initialize is run a second time.
Well, for me the most important part is not the code, but tests. test_embed currently tests the following environment variables:
static void test_init_env_putenvs(void)
{
putenv("PYTHONHASHSEED=42");
putenv("PYTHONMALLOC=malloc_debug");
putenv("PYTHONTRACEMALLOC=2");
putenv("PYTHONPROFILEIMPORTTIME=1");
putenv("PYTHONMALLOCSTATS=1");
putenv("PYTHONUTF8=1");
putenv("PYTHONVERBOSE=1");
putenv("PYTHONINSPECT=1");
putenv("PYTHONOPTIMIZE=2");
putenv("PYTHONDONTWRITEBYTECODE=1");
putenv("PYTHONUNBUFFERED=1");
putenv("PYTHONNOUSERSITE=1");
putenv("PYTHONFAULTHANDLER=1");
putenv("PYTHONDEVMODE=1");
/* FIXME: test PYTHONWARNINGS */
/* FIXME: test PYTHONEXECUTABLE */
/* FIXME: test PYTHONHOME */
/* FIXME: test PYTHONDEBUG */
/* FIXME: test PYTHONDUMPREFS */
/* FIXME: test PYTHONCOERCECLOCALE */
/* FIXME: test PYTHONPATH */
}
As you can see, the test suite is not complete yet. But since the tests are there, it shouldn't be hard to extend the test suite.
test_embed has InitConfigTests in 3.7 and master branches. I'm not interested to backport these tests to 3.6. I modified Python initialization deeply in 3.7, and the status of the code in 3.6 is "undefined". I don't think that it's worth it to backport these tests to 2.7 or 3.6. I success to focus on the master branch where we want to finish the implementation of the PEP 432 which should provide a better *public* API for that.
|
msg325877 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2018-09-20 13:50 |
Yep, exactly - things are much improved already, thanks primarily to your work, and it seems likely they'll be even further improved by the time 3.8.0 comes around :)
|
msg337294 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2019-03-06 11:51 |
New changeset 25d13f37aa6743282d0b8b4df687ff89999964b2 by Victor Stinner in branch 'master':
bpo-36142: PYTHONMALLOC overrides PYTHONDEV (GH-12191)
https://github.com/python/cpython/commit/25d13f37aa6743282d0b8b4df687ff89999964b2
|
|
Date |
User |
Action |
Args |
2022-04-11 14:59:03 | admin | set | github: 78428 |
2019-03-06 11:51:55 | vstinner | set | messages:
+ msg337294 |
2019-03-06 11:30:57 | vstinner | set | pull_requests:
+ pull_request12187 |
2018-09-20 13:50:47 | ncoghlan | set | messages:
+ msg325877 |
2018-09-20 13:42:19 | vstinner | set | messages:
+ msg325874 |
2018-09-20 12:32:35 | ncoghlan | set | status: open -> closed resolution: fixed messages:
+ msg325862
stage: patch review -> resolved |
2018-09-13 19:19:33 | ned.deily | set | priority: release blocker ->
messages:
+ msg325289 |
2018-09-13 19:14:49 | miss-islington | set | nosy:
+ miss-islington messages:
+ msg325288
|
2018-09-13 18:51:52 | vstinner | set | messages:
+ msg325283 |
2018-09-13 18:50:01 | miss-islington | set | pull_requests:
+ pull_request8707 |
2018-09-13 18:49:50 | ned.deily | set | messages:
+ msg325281 |
2018-09-12 19:26:49 | ned.deily | set | messages:
+ msg325172 |
2018-09-12 19:23:47 | ned.deily | set | stage: commit review -> patch review pull_requests:
+ pull_request8654 |
2018-08-05 16:07:17 | ncoghlan | set | messages:
+ msg323156 stage: patch review -> commit review |
2018-08-05 11:45:42 | serhiy.storchaka | set | title: PYTHONOPTIMZE ignored in 3.7.0 when using custom launcher -> PYTHONOPTIMIZE ignored in 3.7.0 when using custom launcher |
2018-08-05 10:33:50 | vstinner | set | messages:
+ msg323145 |
2018-08-05 10:32:04 | vstinner | set | messages:
+ msg323144 |
2018-08-05 07:40:09 | ronaldoussoren | set | messages:
+ msg323135 |
2018-08-03 20:35:30 | vstinner | set | messages:
+ msg323070 |
2018-08-03 20:27:52 | vstinner | set | messages:
+ msg323068 |
2018-08-03 20:25:08 | vstinner | set | pull_requests:
+ pull_request8153 |
2018-07-28 14:22:57 | ncoghlan | set | messages:
+ msg322562 |
2018-07-28 12:39:34 | ronaldoussoren | set | messages:
+ msg322550 |
2018-07-28 12:36:12 | ronaldoussoren | set | keywords:
+ patch stage: patch review pull_requests:
+ pull_request8037 |
2018-07-28 09:52:51 | ronaldoussoren | set | messages:
+ msg322533 |
2018-07-28 09:24:48 | ronaldoussoren | set | messages:
+ msg322532 |
2018-07-28 09:13:43 | ronaldoussoren | set | messages:
+ msg322529 |
2018-07-28 05:09:18 | ncoghlan | set | assignee: vstinner messages:
+ msg322524 |
2018-07-28 04:04:44 | ncoghlan | set | priority: normal -> release blocker nosy:
+ ned.deily messages:
+ msg322523
|
2018-07-28 04:00:08 | ncoghlan | set | messages:
+ msg322522 |
2018-07-28 03:56:39 | ncoghlan | set | messages:
+ msg322521 |
2018-07-28 03:44:06 | ncoghlan | set | messages:
+ msg322520 |
2018-07-28 03:25:39 | ncoghlan | set | nosy:
+ vstinner messages:
+ msg322519
|
2018-07-27 17:30:45 | serhiy.storchaka | set | nosy:
+ brett.cannon, ncoghlan, eric.snow
|
2018-07-27 12:07:53 | ronaldoussoren | set | files:
+ foo.py |
2018-07-27 12:07:40 | ronaldoussoren | set | files:
+ t.c |
2018-07-27 12:07:30 | ronaldoussoren | create | |