Prefetch source tarballs for OpenSSL and curl outside the docker build context in Travis by natefoo · Pull Request #174 · pypa/manylinux

natefoo · 2018-03-29T02:40:20Z

Also, upgrade OpenSSL to 1.0.2o.

This should avoid problems with using the old tools (e.g. wget) in EL5 to fetch these sources. Since people may do Docker Hub, quay.io, etc. automated builds of their own forks from the Dockerfile/context only, prefetching is not required and if sources are not prefetched we fall back to in-container fetching.

There's a lot more work that can be done to reduce duplication in the build scripts, especially with bash indirect references.

Incorporates/supercedes #172, xref #170, thanks @jcfr

…tainer using https Co-authored-by: Nate Coraor <[email protected]>

jcfr · 2018-03-29T02:47:20Z

docker/build_scripts/build_utils.sh

    check_var ${git_sha256}
    check_var ${GIT_DOWNLOAD_URL}
-    curl -sSLO ${GIT_DOWNLOAD_URL}/v${git_fname}.tar.gz
+    fetch_source v${git_fname}.tar.gz ${GIT_DOWNLOAD_URL}


look like curl need to be specified

There are 2 functions, this calls fetch_source, which just calls _fetch_source curl "$@". The idea is that fetching with wget is not something that should be done unless explicitly requested, the default is curl.

I just realized, there is _fetch_source and fetch_source

Gotcha. If CI is green that works for me.

jcfr · 2018-03-29T02:47:28Z

docker/build_scripts/build_utils.sh

    check_var ${autoconf_sha256}
    check_var ${AUTOCONF_DOWNLOAD_URL}
-    curl -sSLO ${AUTOCONF_DOWNLOAD_URL}/${autoconf_fname}.tar.gz
+    fetch_source ${autoconf_fname}.tar.gz ${AUTOCONF_DOWNLOAD_URL}


~~look like curl need to be specified~~

jcfr · 2018-03-29T02:47:32Z

docker/build_scripts/build_utils.sh

    check_var ${automake_sha256}
    check_var ${AUTOMAKE_DOWNLOAD_URL}
-    curl -sSLO ${AUTOMAKE_DOWNLOAD_URL}/${automake_fname}.tar.gz
+    fetch_source ${automake_fname}.tar.gz ${AUTOMAKE_DOWNLOAD_URL}


~~look like curl need to be specified~~

jcfr · 2018-03-29T02:48:46Z

docker/build_scripts/prefetch.sh

+    ext=${name}_EXTENSION
+    url=${name}_DOWNLOAD_URL
+    file=${!root}${!ext:-.tar.gz}
+    fetch_source $file ${!url} $dir


~~Same thing here, look like the with argument is not specified~~

jcfr · 2018-03-29T02:52:58Z

docker/build_scripts/build_utils.sh

 }


+function _fetch_source {


May be worth documenting the method. Also what about the following signature:

_fetch_source <url> <dest> [--method (curl|get)]

The file would be obtained from the url using file=$(basename ${url})

with default method being curl

Yeah, I might make another pass at this to pretty things up and reduce duplication after we get the immediate issue fixed.

jcfr · 2018-03-29T03:16:28Z

@njsmith Would be awesome if you could review and integrate 👍

jcfr · 2018-03-29T03:19:15Z

Nitpick: @natefoo May you could squash Fix curl filename with its parent commit ?

fetching for automated builds).

njsmith · 2018-03-29T07:20:14Z

I don't understand why we have this dance with both wget and curl -- isn't the point of prefetching that we can do it on the host system, which presumably has a working curl? What problem do we solve by using wget sometimes?

natefoo · 2018-03-29T13:51:53Z

We aren't using wget if openssl and curl are prefetched. I left the build context fetch with wget functionality in there because forks won't necessarily build with Travis.

Case in point, the whole reason I discovered the OpenSSL version change yesterday was because I was setting up Hub/quay automated/push builds of a fork I need that will retain libpython*.a. If we got rid of the wget stuff, there'd be no way for those types of builds to work. Plus, right now, you can build the images locally entirely with docker build, which also wouldn't be possible if we required prefetching (people would have to be aware of and run the prefetch step).

njsmith · 2018-03-29T14:24:50Z

That doesn't answer the question though. We're not doing this prefetching dance because we want to; we're doing it because the vanilla centos5 image cannot connect to the hosts that distribute the openssl and curl sources.

Or put another way, if the wget thing actually works, then we shouldn't do prefetching either.

jcfr · 2018-03-29T15:34:46Z

I discovered the OpenSSL version change yesterday was because I was setting up Hub/quay automated/push builds of a fork

Makes sense. I also discovered the problem because I used derived image.

if the wget thing actually works, then we shouldn't do prefetching either.

It works without problem on CircleCI. May be we could switch to that ?

jcfr · 2018-03-29T15:37:12Z

In the short term, I think it makes sense to address the issue. Then, we could discuss the best strategy to re-factor the prefetch system.

Would it be a good compromise to get a branch based of 2e37903 (Prefetch source tarballs for OpenSSL and curl outside the docker build context in Travis ) integrated now ?

jcfr · 2018-03-29T15:38:33Z

Would it be a good compromise to get a branch based of 2e37903 (Prefetch source tarballs for OpenSSL and curl outside the docker build context in Travis ) integrated now ?

For convenience, I re-open the associated PR. #172

Thanks @njsmith

njsmith · 2018-03-29T15:54:15Z

It works without problem on CircleCI. May be we could switch to that ?

Using circleci might work around travis's broken network and allow the ftp://... hack to work, but the only reason we're using ftp is as a gross/fragile/incomplete hack. (In particular, we need both openssl and curl to bootstrap, and the curl project doesn't maintain an ftp server.)

natefoo · 2018-03-29T15:56:51Z

we're doing it because the vanilla centos5 image cannot connect to the hosts that distribute the openssl and curl sources.

Only on Travis, right now, due to the travis-ci/travis-ci#9391. It works fine locally and on other build platforms.

jcfr · 2018-03-29T16:01:01Z

In particular, we need both openssl and curl to bootstrap, and the curl project doesn't maintain an ftp server.

Providing a script that prefetch (like what @natefoo started) would be a good compromise then:

We get ride of wget as you suggested,
then we a standalone prefetch script that need to be called before running docker
and we simplify the system expecting that the prefetch script is called beforehand

njsmith · 2018-03-29T16:09:46Z

It works fine locally and on other build platforms.

No. The problem is that the openssl in centos5 doesn't support SNI, but more and more servers are requiring that. If you look at the history of pull requests, we've been scrambling for months to switch between different mirrors for openssl and curl, trying to find ones that still work with centos5. This is the only reason we're using ftp in the first place.

So from my point of view, the whole point of switching to prefetching is that we no longer have play this game – we can use regular download sources like regular people, and stop having the image be broken constantly like it is now. However, centos5's wget can't connect to regular download sources, no matter which build service you're using, so I don't think that fallback is going to work. Sorry.

njsmith · 2018-03-29T16:11:36Z

(edited my last comment to make it clear what I was responding to)

jcfr · 2018-03-29T16:16:01Z

@natefoo Do you think you could you improve your last patch removing the support for wget and making the use of the prefetch a requirement ? Let me know if I can help.

natefoo · 2018-03-29T16:17:10Z

Well, it can connect to the official OpenSSL FTP source, but yeah, for curl it's using the Debian archive of the upstream source, so that one's not regular. I'm not disagreeing that it's a goose chase, and it will indeed continue to break going foward. I guess I was just hoping to retain the functionality for as long as it could be made to work. But, if you feel strongly that it should go, I'll rip out the remaining wget fetching and just switch to prefetches being required for OpenSSL and curl. Sound good?

document prefetching.

jcfr · 2018-03-29T17:07:52Z

Nice.

njsmith · 2018-04-02T05:37:11Z

Looks good. Thanks a lot for taking care of this; it's not fun but it helps a lot.

jcfr · 2018-04-02T13:18:30Z

t's not fun but it helps a lot.

Different type of fun I would say.

Thanks for spending time reviewing and integrating 👍

jcfr and others added 2 commits March 28, 2018 16:08

docker/build_scripts: Update openssl to latest version: 1.0.2n -> 1.0.2o

93c7423

Workaround travis/travis#9391 downloading openssl archive outside con…

2e37903

…tainer using https Co-authored-by: Nate Coraor <[email protected]>

jcfr reviewed Mar 29, 2018

View reviewed changes

This was referenced Mar 29, 2018

Update OpenSSL from 1.0.2n to 1.0.2o (squashed) #172

Merged

Download package required to bootstrap https support outside of the container #173

Closed

Prefetch openssl and curl via Travis (but still support in-build

1b4a554

fetching for automated builds).

natefoo force-pushed the prefetch-source branch from 5a0af36 to 1b4a554 Compare March 29, 2018 03:22

Require prefetching OpenSSL and curl, remove wget functionality,

8ab9d36

document prefetching.

njsmith merged commit ab23930 into pypa:master Apr 2, 2018

njsmith mentioned this pull request Apr 2, 2018

Build scripts cleanup #161

Merged

		}


		function _fetch_source {

Conversation

natefoo commented Mar 29, 2018

Uh oh!

jcfr Mar 29, 2018

Choose a reason for hiding this comment

Uh oh!

natefoo Mar 29, 2018

Choose a reason for hiding this comment

Uh oh!

jcfr Mar 29, 2018

Choose a reason for hiding this comment

Uh oh!

jcfr Mar 29, 2018

Choose a reason for hiding this comment

Uh oh!

jcfr Mar 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcfr Mar 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcfr Mar 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcfr Mar 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

natefoo Mar 29, 2018

Choose a reason for hiding this comment

Uh oh!

jcfr commented Mar 29, 2018

Uh oh!

jcfr commented Mar 29, 2018

Uh oh!

njsmith commented Mar 29, 2018

Uh oh!

natefoo commented Mar 29, 2018

Uh oh!

njsmith commented Mar 29, 2018

Uh oh!

jcfr commented Mar 29, 2018

Uh oh!

jcfr commented Mar 29, 2018

Uh oh!

jcfr commented Mar 29, 2018

Uh oh!

njsmith commented Mar 29, 2018

Uh oh!

natefoo commented Mar 29, 2018

Uh oh!

jcfr commented Mar 29, 2018

Uh oh!

njsmith commented Mar 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

njsmith commented Mar 29, 2018

Uh oh!

jcfr commented Mar 29, 2018

Uh oh!

natefoo commented Mar 29, 2018

Uh oh!

jcfr commented Mar 29, 2018

Uh oh!

njsmith commented Apr 2, 2018

Uh oh!

jcfr commented Apr 2, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jcfr Mar 29, 2018 •

edited

Loading

jcfr Mar 29, 2018 •

edited

Loading

jcfr Mar 29, 2018 •

edited

Loading

jcfr Mar 29, 2018 •

edited

Loading

njsmith commented Mar 29, 2018 •

edited

Loading