Use SystemCommand for curl.

[reitermarkus]

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example.
• Have you successfully run brew style with your changes locally?
• Have you successfully run brew tests with your changes locally?

[MikeMcQuaid]

Can you indent this two be further left? It's not clear it's a multiline block as-is

[MikeMcQuaid]

Might be nice if (eventually) capturing output like this is something possible with SystemCommand

[reitermarkus]

Done.

[MikeMcQuaid]

@reitermarkus Nice! I think it would be cool if eventually all the calls to system, Formula#system, Utils.popen* end up wrapping system_command calls.

[commitay]

This seems to have broken ant. #4565

[commitay]

Also getting weird curl output with this commit.

$ brew fetch llvm
==> Downloading https://homebrew.bintray.com/bottles/llvm-6.0.1.high_sierra.bottle.tar.gz
##################################                                        47.7%
###################################                                       49.2%
#####################################                                     52.6%
########################################                                  55.6%
##########################################                                58.8%
##########################################                                58.9%
############################################                              61.6%

[asharpe]

Before workaround

$ brew cask upgrade vagrant --debug
==> Upgrading 1 outdated package, with result:
vagrant 2.1.2
==> Started upgrade process for Cask vagrant
==> Printing caveats
==> Hbc::Installer#fetch
==> Satisfying dependencies
==> Downloading
==> Downloading https://releases.hashicorp.com/vagrant/2.1.2/vagrant_2.1.2_x86_64.dmg
==> env -u SSL_CERT_FILE /usr/bin/curl -q --show-error --user-agent Homebrew/1.7.1-41-g3c3b05d-dirty\ \(Macintosh\;\ Intel\ Mac\ OS\ X\ 10.10.5\)\ curl/7.43.0 --fail --progress-bar --location --remote-time --continue-at - --output /Users/asharpe/Library/Caches/Homebrew/Cask/vagrant--2.1.2.dmg.incomplete https://releases.hashicorp.com/vagrant/2.1.2/vagrant_2.1.2_x86_64.dmg
env: illegal option -- u
usage: env [-i] [name=value ...] [utility [argument ...]]
==> Purging files for version 2.1.2 of Cask vagrant
Error: Download failed on Cask 'vagrant' with message: Download failed: https://releases.hashicorp.com/vagrant/2.1.2/vagrant_2.1.2_x86_64.dmg
/usr/local/Homebrew/Library/Homebrew/cask/lib/hbc/download.rb:42:in `rescue in fetch'
/usr/local/Homebrew/Library/Homebrew/cask/lib/hbc/download.rb:39:in `fetch'
/usr/local/Homebrew/Library/Homebrew/cask/lib/hbc/download.rb:15:in `perform'
/usr/local/Homebrew/Library/Homebrew/cask/lib/hbc/installer.rb:135:in `download'
/usr/local/Homebrew/Library/Homebrew/cask/lib/hbc/installer.rb:56:in `fetch'
/usr/local/Homebrew/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb:60:in `block in run'
/usr/local/Homebrew/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb:33:in `each'
/usr/local/Homebrew/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb:33:in `run'
/usr/local/Homebrew/Library/Homebrew/cask/lib/hbc/cli/abstract_command.rb:33:in `run'
/usr/local/Homebrew/Library/Homebrew/cask/lib/hbc/cli.rb:93:in `run_command'
/usr/local/Homebrew/Library/Homebrew/cask/lib/hbc/cli.rb:159:in `run'
/usr/local/Homebrew/Library/Homebrew/cask/lib/hbc/cli.rb:124:in `run'
/usr/local/Homebrew/Library/Homebrew/cmd/cask.rb:7:in `cask'
/usr/local/Homebrew/Library/Homebrew/brew.rb:87:in `<main>'
/usr/local/Homebrew/Library/Homebrew/cask/lib/hbc/cli.rb:164:in `exit'
/usr/local/Homebrew/Library/Homebrew/cask/lib/hbc/cli.rb:164:in `rescue in run'
/usr/local/Homebrew/Library/Homebrew/cask/lib/hbc/cli.rb:147:in `run'
/usr/local/Homebrew/Library/Homebrew/cask/lib/hbc/cli.rb:124:in `run'
/usr/local/Homebrew/Library/Homebrew/cmd/cask.rb:7:in `cask'
/usr/local/Homebrew/Library/Homebrew/brew.rb:87:in `<main>'

env on Yosemite doesn't support -u

[ilovezfs]

Reverted in #4567 due to the above issues and because it broke brew test-bot --ci-upload. See https://jenkins.brew.sh/job/Homebrew%20Bottles/33195/console

[reitermarkus]

Also getting weird curl output with this commit.

I cannot reproduce that. This is specifically handled here:

https://github.com/Homebrew/brew/pull/4561/files#diff-a9ba9eb033a1940106d96b8be0edcff2R41

env on Yosemite doesn't support -u

Any other way to unset a variable?

[ilovezfs]

I cannot reproduce that.

I can:

iMac-TMP:Homebrew joe$ git reset --hard 3c3b05d45732915f7f1d2bd12fc01e7b4318bb31
HEAD is now at 3c3b05d Merge pull request #4564 from reitermarkus/not-a-cask-file-error
iMac-TMP:Homebrew joe$ brew fetch llvm
==> Downloading https://homebrew.bintray.com/bottles/llvm-6.0.1.el_capitan.bottle.tar.gz
########                                                                  11.1%
############################################                              61.3%
######################################################                    76.0%
###################################################################       93.3%
######################################################################## 100.0%
Downloaded to: /Users/joe/Library/Caches/Homebrew/llvm-6.0.1.el_capitan.bottle.tar.gz
SHA256: cdfb1c08bf5a0862c51edf302b6edba29eff09414bb8ac35093b7d74863a7cfb
iMac-TMP:Homebrew joe$ rm /Users/joe/Library/Caches/Homebrew/llvm-6.0.1.el_capitan.bottle.tar.gz
iMac-TMP:Homebrew joe$ git reset --hard 10a7e998a120336612f59b4e3ae27fb879ae9362
HEAD is now at 10a7e99 Merge pull request #4567 from Homebrew/revert-4561-curl-system-command
iMac-TMP:Homebrew joe$ brew fetch llvm
==> Downloading https://homebrew.bintray.com/bottles/llvm-6.0.1.el_capitan.bottle.tar.gz
######################################################################## 100.0%
Downloaded to: /Users/joe/Library/Caches/Homebrew/llvm-6.0.1.el_capitan.bottle.tar.gz
SHA256: cdfb1c08bf5a0862c51edf302b6edba29eff09414bb8ac35093b7d74863a7cfb
iMac-TMP:Homebrew joe$ 

[reitermarkus]

Can you try changing the whole when :stderr section in system_command.rb part to just $stderr.print line?

[chdiza]

Any other way to unset a variable?

What's wrong with unset?

[asharpe]

I would question why we're modifying the environment!

What if I wanted to have that variable set? Is it now an issue because it's being set automatically by some other software, or is this some people with a particular setup that's getting in the way?

Perhaps just look for it being set and provide a warning instead of making the change (silently)?

[MikeMcQuaid]

@asharpe Homebrew intentionally filters and controls environment variables. This is a feature rather than a bug as it makes building from source more reliable.

[asharpe]

@MikeMcQuaid yep, totally get that while building software. The variable being controlled here is just for downloads, and SSL configuration at that. I'm still skeptical about the need to control this one :)

[reitermarkus]

I'm still skeptical about the need to control this one

This one is already filtered from the user environment, but it could be set by Ruby with require "openssl". We have not had any reports which would require this to be set by the user, otherwise this would be whitelisted.

[MikeMcQuaid]

@asharpe With respect, we wouldn't do so if we didn't need to.

otherwise this would be whitelisted.

@reitermarkus This is not necessarily the case 😉

[reitermarkus]

This is not necessarily the case 😉

If there is a good enough reason, of course, as with any of the other whitelisted variables.