| msg326478 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2018-09-26 16:07 |
| The support.args_from_interpreter_flags() function recreates Python command line arguments from sys.flags, but it omits -I (sys.flags.isolated). Because of that, "./python -I -m test ..." behaves differently than "./python -I -m test -j0 ...": https://bugs.python.org/issue28655#msg326477 |
|
|
| msg326669 - (view) |
Author: Karthikeyan Singaravelan (xtreak) * |
Date: 2018-09-29 07:09 |
| Thanks Victor for the details. Can this be classified as an easy issue? I guess the fix will be as below : 1. Add an entry for '-I' at https://github.com/python/cpython/blob/4b430e5f6954ef4b248e95bfb4087635dcdefc6d/Lib/subprocess.py#L260 2. Add a test for '-I' at https://github.com/python/cpython/blob/4b430e5f6954ef4b248e95bfb4087635dcdefc6d/Lib/test/test_support.py#L472. The only thing here is that '-I' returns '-s -E -I' unlike other options where args can be used for comparison logic in check_options. check_options should be changed so that this can take given args and the expected args for comparison to accommodate -I. Maybe there is a better way? Off topic : I don't know why '-I' is not documented as sys.flags.isolated at https://docs.python.org/3.7/library/sys.html#sys.flags . Maybe I will open up a separate issue for this? |
|
|
| msg326787 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2018-10-01 10:21 |
| In the C code, sys.flags.isolated clearly documented as linked to the -I option: static PyStructSequence_Field flags_fields[] = { {"debug", "-d"}, {"inspect", "-i"}, {"interactive", "-i"}, {"optimize", "-O or -OO"}, {"dont_write_bytecode", "-B"}, {"no_user_site", "-s"}, {"no_site", "-S"}, {"ignore_environment", "-E"}, {"verbose", "-v"}, /* {"unbuffered", "-u"}, */ /* {"skip_first", "-x"}, */ {"bytes_warning", "-b"}, {"quiet", "-q"}, {"hash_randomization", "-R"}, {"isolated", "-I"}, {"dev_mode", "-X dev"}, {"utf8_mode", "-X utf8"}, {0} }; > The only thing here is that '-I' returns '-s -E -I' unlike other options where args can be used for comparison logic in check_options. I expect to get: $ python3 -I -c 'import subprocess; print(subprocess._args_from_interpreter_flags())' ['-I'] instead of: ['-s', '-E'] -I is different from -s -E: it also avoids to add the script directory or an empty string to sys.path. |
|
|
| msg326790 - (view) |
Author: Karthikeyan Singaravelan (xtreak) * |
Date: 2018-10-01 10:36 |
| Thanks Victor for the details. > In the C code, sys.flags.isolated clearly documented as linked to the -I option: With respect to documentation I was talking about '-I' not being documented in the table at https://docs.python.org/3.7/library/sys.html#sys.flags though it's present in the C code and in sys.flags.isolated. > -I is different from -s -E: it also avoids to add the script directory or an empty string to sys.path. '-I' also implies '-s -E' and hence adding isolated to args_from_interpreter_flags will also return ['-s', '-E', '-I'] as output and hence I suggested modifying the comparison logic. # Since '-I' implies '-s' and '-E' those flags are also set returning '-s -E -I' ./python.exe --help | rg '\-I' -I : isolate Python from the user's environment (implies -E and -s) ./python.exe -I -c 'import sys; print(sys.flags)' sys.flags(debug=0, inspect=0, interactive=0, optimize=0, dont_write_bytecode=0, no_user_site=1, no_site=0, ignore_environment=1, verbose=0, bytes_warning=0, quiet=0, hash_randomization=1, isolated=1, dev_mode=False, utf8_mode=0) # patching args_from_interpreter_flags to support '-I' would return below ./python.exe -I -c 'import subprocess; print(subprocess._args_from_interpreter_flags())' ['-s', '-E', '-I'] Thanks |
|
|
| msg326792 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2018-10-01 11:10 |
| > ./python.exe -I -c 'import subprocess; print(subprocess._args_from_interpreter_flags())' > ['-s', '-E', '-I'] This looks wrong, I would prefer to only get ['-I']. |
|
|
| msg326875 - (view) |
Author: Danish Prakash (danishprakash) * |
Date: 2018-10-02 12:16 |
| > With respect to documentation I was talking about '-I' not being documented in the table at https://docs.python.org/3.7/library/sys.html#sys.flags though it's present in the C code and in sys.flags.isolated. Thanks for bringing this up Karthikeyan, however, could there be another reason why -I would be left out. Also, have you filed an issue for this? Also, Victor and Karthikeyan, since this issue has been categorized as an easy issue, I would like to fix this if none of you have started working on this. |
|
|
| msg326876 - (view) |
Author: Karthikeyan Singaravelan (xtreak) * |
Date: 2018-10-02 12:25 |
| > Thanks for bringing this up Karthikeyan, however, could there be another reason why -I would be left out. Also, have you filed an issue for this? I couldn't see any related issue for this though the table was changed in 3.7.0 > Also, Victor and Karthikeyan, since this issue has been categorized as an easy issue, I would like to fix this if none of you have started working on this. I am not working on this. Feel free to pick it up. |
|
|
| msg326879 - (view) |
Author: Danish Prakash (danishprakash) * |
Date: 2018-10-02 12:46 |
| Thank you Karthikeyan, I'm going to take care of both of these issues. |
|
|
| msg327099 - (view) |
Author: Danish Prakash (danishprakash) * |
Date: 2018-10-05 04:28 |
| Linking this[1] here in case someone else stumbles upon this thread. I've created an issue and a PR for the documentation issue regarding the absence of -I flag from the sys.flags table which came into picture from the discussions in this thread. [1]: https://bugs.python.org/issue34901 |
|
|
| msg327280 - (view) |
Author: Danish Prakash (danishprakash) * |
Date: 2018-10-07 13:40 |
| > I expect to get: ['-I'] instead of ['-s', '-E', '-I'] From what I understand, this can be done in one of two ways. First, we could edit https://github.com/python/cpython/blob/ad73a9cf97770023665a1bb1c6390a3c99478139/Modules/main.c#L430 and not incrementing -s and -E. But I believe this would have consequences that I'm unable to think of right now, so I'd like some inputs on this. Secondly, we could handle this condition in `_args_from_interpreted_flags()` itself but that might be looked upon as bad practice. |
|
|
| msg327289 - (view) |
Author: Karthikeyan Singaravelan (xtreak) * |
Date: 2018-10-07 16:47 |
| > From what I understand, this can be done in one of two ways. First, we could edit https://github.com/python/cpython/blob/ad73a9cf97770023665a1bb1c6390a3c99478139/Modules/main.c#L430 and not incrementing -s and -E. But I believe this would have consequences that I'm unable to think of right now, so I'd like some inputs on this. As in the docs for -I it implies -s and -E so removing the increment is not a good solution in my opinion and will break code. I don't know how this can be handled since -I sets -s and -E implicitly and _args_from_interpreted_flags just looks for the set flag. This could also get a little complex if we remove -s and -E based on -I since one might pass -I and -s. Maybe we can do an intersection of the command line arguments passes and the set bits in _args_from_interpreted_flags so that only -I remains? Victor prefers -I only and maybe has an approach to solve this? |
|
|
| msg327318 - (view) |
Author: Danish Prakash (danishprakash) * |
Date: 2018-10-08 03:37 |
| You're right Karthikeyan, although I personally think that returning ['-s', '-E', '-I'] should be a plausible solution here since it has been stated explicitly that it implies '-s' and '-E' but I'm still waiting for what Victor has to say on this. > The only thing here is that '-I' returns '-s -E -I' unlike other options where args can be used for comparison logic in check_options. Karthikeyan, do you happen to have a use case where this might come into action? |
|
|
| msg327321 - (view) |
Author: Karthikeyan Singaravelan (xtreak) * |
Date: 2018-10-08 04:08 |
| >> The only thing here is that '-I' returns '-s -E -I' unlike other options where args can be used for comparison logic in check_options. > Karthikeyan, do you happen to have a use case where this might come into action? I don't have a use case in mind. The comment was that returning '-s -E -I' would need the helper function used in the test to be changed. Thanks |
|
|
| msg330295 - (view) |
Author: Danish Prakash (danishprakash) * |
Date: 2018-11-23 03:02 |
| Sorry for bumping this thread but Victor, could you please share your inputs on this if you have the time for it, thanks. |
|
|
| msg330325 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2018-11-23 14:03 |
| I tried to explain how to fix the bug, but nobody came up with a working change 2 months, so I wrote the PR myself. It's an important security issue, since the function is used by multiprocessing and distutils modules to spawn new child processes. |
|
|
| msg330340 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2018-11-23 16:54 |
| New changeset 9de363271519e0616f4a7b59427057c4810d3acc by Victor Stinner in branch 'master': bpo-34812: subprocess._args_from_interpreter_flags(): add isolated (GH-10675) https://github.com/python/cpython/commit/9de363271519e0616f4a7b59427057c4810d3acc |
|
|
| msg330344 - (view) |
Author: miss-islington (miss-islington) |
Date: 2018-11-23 17:13 |
| New changeset 01e579949ab546cd4cdd0d6d18e3ef41ce94f46e by Miss Islington (bot) in branch '3.7': bpo-34812: subprocess._args_from_interpreter_flags(): add isolated (GH-10675) https://github.com/python/cpython/commit/01e579949ab546cd4cdd0d6d18e3ef41ce94f46e |
|
|
| msg330351 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2018-11-23 18:02 |
| New changeset cc0e0a2214d6515cf6ba4c7b164902a87e321b45 by Victor Stinner in branch '3.6': bpo-34812: subprocess._args_from_interpreter_flags(): add isolated (GH-10675) (GH-10688) https://github.com/python/cpython/commit/cc0e0a2214d6515cf6ba4c7b164902a87e321b45 |
|
|
| msg330352 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2018-11-23 18:03 |
| Ok, the bug is now fixed in Python 3.6, 3.7 and master branches ;-) |
|
|