Bug 174875 – download-built-product should allow for a proxy option

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Lucas Forschler]

the script currently uses: 
return subprocess.call(["curl", "--fail", "--output", archivePath, url])


We should allow an option to pass in a proxy server.

(This will be necessary for us to move to using S3 for our archive storage)

[Radar WebKit Bug Importer]

<rdar://problem/33551694>

[Lucas Forschler]

We need to ensure the proxy is only used for Mac clients, so we'll have to teach the tool to either accept it as an optional argument from buildbot, or to have logic in the script to do the right thing.

[Lucas Forschler]

Created attachment 316996 [details]
Add proxy config for apple webkit

[Lucas Forschler]

Created attachment 316997 [details]
v2

[Lucas Forschler]

Created attachment 317010 [details]
Add ChangeLog to patch

[Lucas Forschler]

Created attachment 317025 [details]
Updated patch based on feedback.

[Dean Johnson]

Comment on attachment 317025 [details]
Updated patch based on feedback.

View in context: https://bugs.webkit.org/attachment.cgi?id=317025&action=review

LGTM overall. Unofficial r+.

> Tools/ChangeLog:9
> +        (DownloadBuiltProduct.start): teach buildbot to pass along proxy for Apple bots

Nit: teach => Teach.

[Alexey Proskuryakov]

Comment on attachment 317025 [details]
Updated patch based on feedback.

View in context: https://bugs.webkit.org/attachment.cgi?id=317025&action=review

> Tools/BuildSlaveSupport/download-built-product:55
> +        command += ["-x", options.proxy]

Can we set a curl environment variable from outside, and not modify the script at all?

       http_proxy [protocol://]<host>[:port]
              Sets the proxy server to use for HTTP.

       HTTPS_PROXY [protocol://]<host>[:port]
              Sets the proxy server to use for HTTPS.

[Lucas Forschler]

(In reply to Alexey Proskuryakov from comment #8)
> Comment on attachment 317025 [details]
> Updated patch based on feedback.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=317025&action=review
> 
> > Tools/BuildSlaveSupport/download-built-product:55
> > +        command += ["-x", options.proxy]
> 
> Can we set a curl environment variable from outside, and not modify the
> script at all?
> 
>        http_proxy [protocol://]<host>[:port]
>               Sets the proxy server to use for HTTP.
> 
>        HTTPS_PROXY [protocol://]<host>[:port]
>               Sets the proxy server to use for HTTPS.

This does work:
HTTPS_PROXY=http://54.190.50.182:873 python ./Tools/BuildSlaveSupport/download-built-product --platform=mac --debug https://s3-us-west-2.amazonaws.com/minified-archives.webkit.org/mac-elcapitan-x86_64-debug/219249.zip

[Lucas Forschler]

Created attachment 317042 [details]
patch addressing feedback

[Lucas Forschler]

Created attachment 317043 [details]
updated patch based on feedback.

[Lucas Forschler]

Created attachment 317045 [details]
updated patch based on feedback.

[Lucas Forschler]

Created attachment 317056 [details]
fixed return call

[Lucas Forschler]

Committed revision 220166.