Skip to content

gh-91954: Emit EncodingWarning from locale and subprocess #91977

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Apr 30, 2022
Merged

gh-91954: Emit EncodingWarning from locale and subprocess #91977

merged 8 commits into from
Apr 30, 2022

Conversation

methane
Copy link
Member

@methane methane commented Apr 27, 2022

Fix: #91954

methane added 4 commits April 27, 2022 11:05
@methane
Emit EncodingWarning
02aa9f4 
locale.getpreferredencoding() and subprocess.Popen() emit EncodingWarning
@methane
add comment
3a6324b
@methane
fixup
903029b
@methane
Better EncodingWarning from subprocess
9163723
@methane methane requested a review from gpshead as a code owner April 27, 2022 04:09
@methane methane merged commit 354ace8 into python:main Apr 30, 2022
@methane methane deleted the encodingwarning branch April 30, 2022 06:53
@sweeneyde
Copy link
Member

I think this may have caused the following failure on recent Windows ARM64 buildbots:

======================================================================
FAIL: test_encoding_warning (test.test_subprocess.RunFuncTestCase.test_encoding_warning)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Workspace\buildarea\3.x.linaro-win-arm64.nondebug\build\Lib\test\test_subprocess.py", line 1746, in test_encoding_warning
    self.assertEqual(len(lines), 2)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 13 != 2
----------------------------------------------------------------------

@sweeneyde
Copy link
Member

I opened #92090 and ran buildbots to find out what the output is:

<string>:3: EncodingWarning: 'encoding' argument not specified.
Traceback (most recent call last):
  File "<string>", line 3, in <module>
  File "C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\Lib\subprocess.py", line 533, in run
    with Popen(*popenargs, **kwargs) as process:
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\Lib\subprocess.py", line 998, in __init__       
    self._execute_child(args, executable, preexec_fn, close_fds,
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\Lib\subprocess.py", line 1467, in _execute_child
    hp, ht, pid, tid = _winapi.CreateProcess(executable, args,
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
FileNotFoundError: [WinError 2] The system cannot find the file specified

@sweeneyde
Copy link
Member

https://stackoverflow.com/questions/10933354/python-why-does-calling-echo-with-subprocess-return-windowserror-2

gpshead
gpshead reviewed May 2, 2022
View reviewed changes
Lib/test/test_subprocess.py
def test_encoding_warning(self):
code = textwrap.dedent("""\
from subprocess import *
args = ["echo", "hello"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notice how the majority of the tests in this file use a child process of [sys.executable, "-c", "print('hello')"] to avoid the platform problems. Depending on a shell command working across platforms always causes future problems. See also the above test which simply skips windows because it relies on sleep 3 instead of using the above idiom. ;)

I'm not going to suggest yet another followup PR as if things are stable, lets just let it be. But the modern idiom in test_subprocess is to not launch things other than sys.executable based processes to avoid depending upon the external execution environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gpshead gpshead gpshead left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

PEP 686: Emit EncodingWarning in more places.
4 participants
@methane @sweeneyde @gpshead @bedevere-bot