Skip to content
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.

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

Specify the encoding and enable warnings in tests when unspecified. #12071

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jaraco
Copy link
Member

@jaraco jaraco commented Jun 2, 2023

@jaraco
Copy link
Member Author

jaraco commented Jun 2, 2023

I'm unsure if the test is correct here. When I ran nox on the project with the test commit, it didn't emit any warnings. It was unclear to me if that was because I wasn't using nox correctly or if maybe the broken behavior isn't triggered as part of the run or if I've implemented the environment variable incorrectly. I'd appreciate if someone would take over, validate the test (and amend it to work), and add the required changelog history. Thanks.

@jaraco jaraco requested a review from sbidoul June 2, 2023 15:49
@pfmoore
Copy link
Member

pfmoore commented Jun 2, 2023

What's the justification for using UTF-8? Can we be sure that any tools that we call via the call_subprocess function will output UTF-8?

I know, for example, that we pass the setuptools output through that interface. And I also know that (at least in the past, it may be fixed now) MSVC didn't use UTF-8 for its output. Does setuptools now force tool output to be UTF-8, or do we get the raw output bytes from build tools?

If the logic is "even if UTF-8 is wrong, we will only get mojibake, things won't fail" then we should be explicit that this is what we're doing, and confirm we're OK with it. Personally, I can be persuaded it's the best choice, but I expect we'll get user complaints about "pip garbling the output of my compiler", and I'm a bit fed up of pip being blamed for things like that...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EncodingWarnings in utils.subprocess and cache modules
2 participants