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

Revert "popen: Do not suppress stderr" #3173

Merged
merged 1 commit into from Sep 20, 2017

Conversation

Projects
None yet
3 participants
@ilovezfs
Contributor

ilovezfs commented Sep 19, 2017

Reverts #3099

@ilovezfs

This comment has been minimized.

Show comment
Hide comment
@ilovezfs

ilovezfs Sep 19, 2017

Contributor

CC @sjackman @MikeMcQuaid #3099 is causing messages like

xcrun: error: unable to find utility "gcc-4.0", not a developer tool or in PATH
xcrun: error: unable to find utility "gcc-4.2", not a developer tool or in PATH

to get printed on every build failure.

Contributor

ilovezfs commented Sep 19, 2017

CC @sjackman @MikeMcQuaid #3099 is causing messages like

xcrun: error: unable to find utility "gcc-4.0", not a developer tool or in PATH
xcrun: error: unable to find utility "gcc-4.2", not a developer tool or in PATH

to get printed on every build failure.

@sjackman

This comment has been minimized.

Show comment
Hide comment
@sjackman

sjackman Sep 19, 2017

Contributor

brew config -v replicates this issue as well. I've opened PR #3174 to address it.

Contributor

sjackman commented Sep 19, 2017

brew config -v replicates this issue as well. I've opened PR #3174 to address it.

@ilovezfs

This comment has been minimized.

Show comment
Hide comment
@ilovezfs

ilovezfs Sep 19, 2017

Contributor

@sjackman OK, thanks. I'll close this one out.

Contributor

ilovezfs commented Sep 19, 2017

@sjackman OK, thanks. I'll close this one out.

@ilovezfs ilovezfs closed this Sep 19, 2017

@ilovezfs ilovezfs reopened this Sep 20, 2017

@ilovezfs

This comment has been minimized.

Show comment
Hide comment
@ilovezfs

ilovezfs Sep 20, 2017

Contributor

@sjackman @MikeMcQuaid going to revert because the xcrun: error: unable to find utility "gcc-4.0" isn't the only issue.

@chdiza reports in #3181

Also, brew config is showing the rather unhelpful and probably unintentional:
Unable to find any JVMs matching version "(null)".

Contributor

ilovezfs commented Sep 20, 2017

@sjackman @MikeMcQuaid going to revert because the xcrun: error: unable to find utility "gcc-4.0" isn't the only issue.

@chdiza reports in #3181

Also, brew config is showing the rather unhelpful and probably unintentional:
Unable to find any JVMs matching version "(null)".

@ilovezfs ilovezfs merged commit 5d888c0 into master Sep 20, 2017

0 of 3 checks passed

codecov/patch 0% of diff hit (target 67.11%)
Details
codecov/project 67.09% (-0.02%) compared to 81d9f71
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@sjackman

This comment has been minimized.

Show comment
Hide comment
@sjackman

sjackman Sep 20, 2017

Contributor

I'll address #3181 in PR #3174.

Contributor

sjackman commented Sep 20, 2017

I'll address #3181 in PR #3174.

@MikeMcQuaid MikeMcQuaid deleted the revert-3099-popen_read_stderr branch Sep 20, 2017

@MikeMcQuaid

This comment has been minimized.

Show comment
Hide comment
@MikeMcQuaid

MikeMcQuaid Sep 20, 2017

Member

@sjackman I don't really want to play whack-a-mole with this, I'm afraid.

Member

MikeMcQuaid commented Sep 20, 2017

@sjackman I don't really want to play whack-a-mole with this, I'm afraid.

@MikeMcQuaid

This comment has been minimized.

Show comment
Hide comment
@MikeMcQuaid

MikeMcQuaid Sep 20, 2017

Member

An undocumented environment variable for this may be the best call if you really need this functionality.

Member

MikeMcQuaid commented Sep 20, 2017

An undocumented environment variable for this may be the best call if you really need this functionality.

@sjackman

This comment has been minimized.

Show comment
Hide comment
@sjackman

sjackman Sep 20, 2017

Contributor

Note that these messages are only displayed when either -d or -v is enabled. I suggest changing it so that these messages are only displayed when both HOMEBREW_DEVELOPER=1 and -v are enabled. Changing…

$stderr.reopen("/dev/null", "w") if !ARGV.debug? && !ARGV.verbose?

to

$stderr.reopen("/dev/null", "w") unless ARGV.homebrew_developer? && ARGV.verbose?
Contributor

sjackman commented Sep 20, 2017

Note that these messages are only displayed when either -d or -v is enabled. I suggest changing it so that these messages are only displayed when both HOMEBREW_DEVELOPER=1 and -v are enabled. Changing…

$stderr.reopen("/dev/null", "w") if !ARGV.debug? && !ARGV.verbose?

to

$stderr.reopen("/dev/null", "w") unless ARGV.homebrew_developer? && ARGV.verbose?
@MikeMcQuaid

This comment has been minimized.

Show comment
Hide comment
@MikeMcQuaid

MikeMcQuaid Sep 20, 2017

Member

@sjackman That's still going to hit Homebrew developers, non-tag users and CI.

Member

MikeMcQuaid commented Sep 20, 2017

@sjackman That's still going to hit Homebrew developers, non-tag users and CI.

@sjackman

This comment has been minimized.

Show comment
Hide comment
@sjackman

sjackman Sep 20, 2017

Contributor

How about…

$stderr.reopen("/dev/null", "w") unless ARGV.homebrew_developer? && ARGV.debug? && ARGV.verbose?

Does CI have ARGV.debug? enabled?

Contributor

sjackman commented Sep 20, 2017

How about…

$stderr.reopen("/dev/null", "w") unless ARGV.homebrew_developer? && ARGV.debug? && ARGV.verbose?

Does CI have ARGV.debug? enabled?

@MikeMcQuaid

This comment has been minimized.

Show comment
Hide comment
@MikeMcQuaid

MikeMcQuaid Sep 20, 2017

Member

An undocumented environment variable is the only thing that's not going to annoy existing maintainers/users.

Member

MikeMcQuaid commented Sep 20, 2017

An undocumented environment variable is the only thing that's not going to annoy existing maintainers/users.

@sjackman

This comment has been minimized.

Show comment
Hide comment
@sjackman

sjackman Sep 20, 2017

Contributor

I've updated PR #3174 to use the environment variable HOMEBREW_STDERR.

Contributor

sjackman commented Sep 20, 2017

I've updated PR #3174 to use the environment variable HOMEBREW_STDERR.

@ilovezfs ilovezfs referenced this pull request Sep 21, 2017

Closed

High Sierra sandbox logging throws compatibility messages #3175

4 of 4 tasks complete

@Homebrew Homebrew locked and limited conversation to collaborators May 4, 2018

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