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

respect quiet flag in cargo metadata #256

Merged
merged 2 commits into from
Jun 28, 2022
Merged

Conversation

davidhewitt
Copy link
Member

Follow up to #254 to make the formatting reusable & tested, and respect quiet flag for stderr.

@davidhewitt davidhewitt mentioned this pull request Jun 28, 2022
Comment on lines +7 to +15
>>> format_called_process_error(subprocess.CalledProcessError(
... 1, ['cargo', 'foo bar'], 'message', None
... ))
"`cargo 'foo bar'` failed with code 1\\n-- Output captured from stdout:\\nmessage"
>>> format_called_process_error(subprocess.CalledProcessError(
... -1, ['cargo'], 'stdout', 'stderr'
... ))
'`cargo` failed with code -1\\n-- Output captured from stdout:\\nstdout\\n-- Output captured from stderr:\\nstderr'
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Great to see this tested! 👍

@davidhewitt davidhewitt merged commit 55211d9 into PyO3:main Jun 28, 2022
@davidhewitt davidhewitt deleted the error-quiet branch June 28, 2022 21:12
@@ -210,21 +212,12 @@ def build_extension(

# Execute cargo
try:
stderr = subprocess.PIPE if quiet else None
Copy link
Contributor

Choose a reason for hiding this comment

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

@davidhewitt
Looking over this again after 1.4.0 didn't help makes me wonder if this condition is not the wrong way around?

Shouldn't we not be listening for stderr output if quiet=True (or at least, be listening for it if quiet=False)?

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.

2 participants