allow choosing the test docker image #314

mattip · 2020-02-28T05:56:52Z

I opened a new github org multi-build to hold recipes for new docker images for testing, and pushed a few to the dockerhub organization "multibuild". This PR adds an option to use them, and modifies 2 CI runs.

If this looks good, I can push more images to docker hub: xenial32 and bionic64, maybe reproduce the trusty ones too?

mattip · 2020-02-28T07:39:25Z

Hmm, I don't actually see any use of install_run in tests :(. Am I missing something?

matthew-brett · 2020-02-28T08:25:55Z

How about allowing an arbitrary test image with something like : https://github.com/matthew-brett/multibuild/pull/103 ?

See: https://github.com/matthew-brett/multibuild/issues/311#issuecomment-589696498

mattip · 2020-02-28T08:55:14Z

I think #103 is what I did with this PR: by specifying MB_TEST_VER you use a different docker test image when testing. What did I miss? My comment about install_run was referring to this repo's CI actually testing that install_run works. I did test this PR in the numpy-wheels repo and it does what I wanted, but I was hoping it would get tested here.

matthew-brett · 2020-02-28T10:48:29Z

Oops - sorry - forgive my rush - you're right. If there was something coherent in my reply (there may not have been), it was wondering whether we should add the implicit constraint that the images have to have bitness tags associated with them. For example, do we want to constrain any testing images to have 32 or 64 tags? Or, should we allow arbitrary image names?

Is MB_TEST_VER the right variable? Maybe DOCKER_TEST_IMAGE, by comparison to the earlier PR?

mattip · 2020-02-28T12:24:35Z

fixed up:

changed to DOCKER_TEST_IMAGE
image name must be complete (defaults to previous behaviour)
update README
actually run install_run in the tests to make sure the image works

mattip · 2020-02-28T13:33:51Z

running install_run pointed out some problems:

my xenial64 image was wrong
missing test images for aarch64, s390x, ppc64le
and some macOS problem.

matthew-brett · 2020-02-28T14:23:16Z

Now I think about it - I wonder whether we can make this be (optionally) a format string, as in the default would be something like matthewbrett/trusty:${plat}, and we do substitution. Now I'm trying to deal with the annoying repetion of test images for each grid entry - so a compromise with your earlier solution.

Of course this wouldn't match what we do with DOCKER_IMAGE, but perhaps we could deprecate that, copy it over to DOCKER_BUILD_IMAGE, if present, and set that to a default ofquay.io/pypa/manylinux{$MB_ML_VER}_${plat}. What do you think?

mattip · 2020-02-28T15:02:10Z

For: you only need to give the base image name, the rest should have been set up by the image builders
Against: harder to figure out what is actually going on, you need to be a bash expert. How do we actually do the escaping of matthewbrett/trusty:${plat} in the travis.tml?

As an aside: Is there an "image naming for dummies" guide? I don't understand when the : is a qualifier and when it is a marker of a version of a specific image

matthew-brett · 2020-02-28T18:05:17Z

We could make a format-stringey thing like this:

DOCKER_TEST_IMAGE="mattip/xenial:{bitness}"

docker_test_image="${DOCKER_TEST_IMAGE/\{bitness\}/$bitness}"

The complexity would only be apparent for those who wanted to use it. So:

DOCKER_TEST_IMAGE="mattip/xenial_something"

docker_test_image="${DOCKER_TEST_IMAGE/\{bitness\}/$bitness}"

gives the expected result of the unchanged "mattip/xenial_something".

The stuff after the colon is up the the image author - I don't know what the traditions are, of course it could be a version.

mattip · 2020-02-29T20:18:03Z

The test images do not like using MB_PYTHON_VERSION=pypy3.6-7.3 since the choose_python.sh (in the docker images) script's py_nodot assumes MB_PYTHON_VERSON is a string like 3.8 or 2.7. Fixing, and adding some PyPy versions to the image. If the desired version is not found, choose_python will download, unpack, and put it on the path.

mattip · 2020-02-29T20:30:42Z

I think the content after the : is meant to be a tag like latest or a hash. See for instance the pypa images for manylinux2014 images. Clicking on the far right download icon gives a doownload like docker pull quay.io/pypa/manylinux2014_ppc64le:2020-02-29-9dfc632. So we should be calling the trusty images trusty_x86_64 and trusty_i686 to be consistent with the docker images used to build the wheels. Then we can use a tag system for reproducible builds

mattip · 2020-02-29T21:40:44Z

If we go with more standard names for the images, we could do

PLAT=x64_86
DOCKER_TEST_IMAGE="multibuild/xenial_{PLAT}:latest"
local docker_test_image="${DOCKER_TEST_IMAGE/\{PLAT\}/$PLAT}"

matthew-brett · 2020-02-29T23:02:58Z

Fine with me.

matthew-brett · 2020-03-02T12:33:39Z

Yes, that's definitely cleaner, thanks. What do you think about refactoring DOCKER_IMAGE / DOCKER_BUILD_IMAGE ?

matthew-brett · 2020-03-02T16:26:36Z

Sorry - I was thinking of something like:

if [ -n '$DOCKER_IMAGE" ]; then
    echo "DOCKER_IMAGE deprecated, please use DOCKER_BUILD_IMAGE"
    if [ -n "$DOCKER_BUILD_IMAGE" ]; then
        echo "Cannot use DOCKER_BUILD_IMAGE and DOCKER_IMAGE at the same time."
        echo "We recommend you unset DOCKER_IMAGE"
        exit 1
    fi
    DOCKER_BUILD_IMAGE=$DOCKER_IMAGE
fi

and then allowing DOCKER_BUILD_IMAGE to use the format var stuff, so the default might be:

DOCKER_BUILD_IMAGE=quay.io/pypa/manylinux{MB_ML_VER}_{PLAT}

mattip · 2020-03-02T16:42:33Z

I will back out the changes I made. Those deep changes would need tests and a set of docker images close-to-but-not-exactly manylinux in order to be able to prove it works.

mattip · 2020-03-03T09:21:14Z

Note the problems started when I added install_run $PLAT to the tests, so I hope they show up in gh-315

matthew-brett · 2020-03-03T12:39:20Z

Maybe try merging gh-315 into this branch. There are failures on xcode 8.0 and 7.3 that look unrelated, with errors in updating homebrew.

mattip · 2020-03-03T17:18:46Z

Merging gh-315 made even the most basic linx64 CI run fail since no-one defines PYTHON_EXE.

matthew-brett · 2020-03-03T18:28:51Z

Interesting - I wonder why I didn't see that in my tests?

I think the test is incorrect though - it's reasonable to require PYTHON_EXE there - it only makes sense for the particular Python you're installing into etc, not any generic Python. Just inserting PYTHON_EXE=${PYTHON_EXE:-python} above the test line should be fine.

mattip · 2020-03-03T18:42:07Z

I wonder why I didn't see that in my tests?

I think all the travis tests are failing on gh-315. Until that is ready I tried using python -mpip instead, which some people consider best practice anyway. It is slowly churning through the mac builds now ...

matthew-brett · 2020-03-03T18:44:49Z

Yes - I saw that - but it should still be $PYTHON_EXE -mpip etc

mattip · 2020-03-03T19:19:47Z

It seems you are right, the test failed because it is using python2 instead of python3.7

mattip · 2020-03-03T19:24:23Z

Just inserting PYTHON_EXE=${PYTHON_EXE:-python} above the test line should be fine

Which test line?

matthew-brett · 2020-03-03T19:49:58Z

Just above the failing test in test_common_utils ...

mattip · 2020-03-03T21:08:01Z

If I do that tests might pass, but then when a real project uses install_run, where will PYTHON_EXE be set? I see where it happens in docker_build_wrap.sh and in osx_utils.sh, but where does it get done for docker_test_wrap.sh ?

The readme says the travis run should have a before_install step that calls travis_steps.sh, I don't see that in this repo's .travis.yml. Maybe that is the problem?

matthew-brett · 2020-03-03T22:16:27Z

Ah yes - good point. That's deficiency of the design - that I didn't separate the uses for these two cases. A workaround would be to do something like python_exe=${PYTHON_EXE:-python} in the routine itself, and label the routine as usable for test and build.

mattip · 2020-03-04T16:09:13Z

The macOS xcode 7.3 and 8.0 builds failed brew update with remote: Invalid username or password. These were released in May and Sept 2016 respectively. Are they still supported? In any case, I think this PR is ready for final review, those failures are not connected to it.

matthew-brett

A couple of small things.

matthew-brett · 2020-03-05T14:33:44Z

common_utils.sh

    if [ -z "$supported_wheels" ]; then
        echo "ERROR: no supported wheels found"
+        ls $wheelhouse/*.whl


Maybe an echo here to say this is the listing of the wheelhouse directory?

I will revert the addition, it didn't really help me debug the problem.

matthew-brett · 2020-03-05T14:34:57Z

manylinux_utils.sh

@@ -4,43 +4,11 @@
 MULTIBUILD_DIR=$(dirname "${BASH_SOURCE[0]}")
 source $MULTIBUILD_DIR/common_utils.sh

-# UNICODE_WIDTH selects "32"=wide (UCS4) or "16"=narrow (UCS2/UTF16) builds


Doesn't this need to go with cpython_path?

fixing and adding a big comment

mattip · 2020-03-05T22:49:22Z

the same two outdated xcode 7.3 and 8.0 builds are failing.

matthew-brett · 2020-03-06T11:11:05Z

travis_linux_steps.sh

+        local docker_image="matthewbrett/trusty:$bitness"
+    else
+        # aarch64 is called arm64v8 in Ubuntu
+        local plat_subst=$([ "$plat" == aarch64 ] && echo arm64v8 || echo $plat)


Does this constrain the test image to be Ubuntu, at least for the PLAT substitution case?

I don't know. Is arm64v8 an Ubuntu invention? I was using the base-docker image tags here, which are up to date vs the aarch64 ones which seem stale. google trends doesn't see any hits for arm64v8.

It would be easy enough to change this if needed. I think the chances of anyone but us building test images is quite small: it took me a few iterations to get the choose_python script to work properly.

Just one more note for people who come back to this: travis calls it arm64

matthew-brett · 2020-03-06T12:22:41Z

Ok - then - I propose merging - OK with you?

mattip · 2020-03-06T12:26:47Z

Yes, I think it is ready for merge.

matthew-brett · 2020-03-06T12:28:22Z

Thanks Matti.

allow choosing the test docker image

86862e0

fixes from comments: MB_TEST_VER -> DOCKER_TEST_IMAGE; run tests

54cd2f3

fix for macOS

fd3565f

mattip mentioned this pull request Mar 1, 2020

Support s390x, ppc64le, arm64v8 deadsnakes/issues#110

Closed

mattip added 5 commits March 2, 2020 06:36

move cpython_path to common_utils for pypy discovery during test phase

6658646

rework DOCKER_TEST_IMAGE parsing from review

f16e61f

handle missing value for u_width

45d84c6

fix pattern substitution typo

dfd3405

typo

bb46ec2

mattip force-pushed the test-images branch 2 times, most recently from be87db9 to 3d9327e Compare March 2, 2020 17:48

fix image for aarch64, ppc64le, s390x

fa2c0c6

mattip force-pushed the test-images branch from 3d9327e to fa2c0c6 Compare March 2, 2020 18:43

move pypy -> multibuild/xenial docker, remove some osx builds

47f4b24

use 'python -mpip' to work around macOS problems

9133448

mattip force-pushed the test-images branch from ea3011c to 9133448 Compare March 3, 2020 17:23

mattip force-pushed the test-images branch from b659543 to 1377727 Compare March 4, 2020 11:32

set and use PYTHON_EXE

be1744e

mattip force-pushed the test-images branch from 1377727 to be1744e Compare March 4, 2020 13:06

mattip mentioned this pull request Mar 5, 2020

MAINT: bump to OpenBLAS v0.3.9 MacPython/openblas-libs#24

Merged

odidev mentioned this pull request Mar 5, 2020

Issue facing in running tests using multibuild in aarch64 platform #311

Closed

matthew-brett reviewed Mar 5, 2020

View reviewed changes

fix where UNICODE_WIDTH is defined, remove debug cruft

3ffa2d1

matthew-brett reviewed Mar 6, 2020

View reviewed changes

matthew-brett merged commit 3bd75ee into multi-build:devel Mar 6, 2020

mattip mentioned this pull request Apr 15, 2020

MRG: force use of PYTHON_EXE, PIP_CMD #315

Closed

madig mentioned this pull request Aug 2, 2021

How to select pypy3? #420

Closed

radarhere mentioned this pull request Mar 29, 2022

Fixed tkinter in Alpine multi-build/docker-images#32

Merged

allow choosing the test docker image #314

allow choosing the test docker image #314

Conversation

mattip commented Feb 28, 2020

mattip commented Feb 28, 2020

matthew-brett commented Feb 28, 2020

mattip commented Feb 28, 2020

matthew-brett commented Feb 28, 2020

mattip commented Feb 28, 2020

mattip commented Feb 28, 2020

matthew-brett commented Feb 28, 2020

mattip commented Feb 28, 2020

matthew-brett commented Feb 28, 2020

mattip commented Feb 29, 2020

mattip commented Feb 29, 2020 • edited Loading

mattip commented Feb 29, 2020

matthew-brett commented Feb 29, 2020

matthew-brett commented Mar 2, 2020

matthew-brett commented Mar 2, 2020 • edited Loading

mattip commented Mar 2, 2020

mattip commented Mar 3, 2020

matthew-brett commented Mar 3, 2020

mattip commented Mar 3, 2020

matthew-brett commented Mar 3, 2020

mattip commented Mar 3, 2020

matthew-brett commented Mar 3, 2020 via email

mattip commented Mar 3, 2020

mattip commented Mar 3, 2020

matthew-brett commented Mar 3, 2020 via email

mattip commented Mar 3, 2020

matthew-brett commented Mar 3, 2020 via email

mattip commented Mar 4, 2020

matthew-brett left a comment

Choose a reason for hiding this comment

matthew-brett Mar 5, 2020

Choose a reason for hiding this comment

mattip Mar 5, 2020

Choose a reason for hiding this comment

matthew-brett Mar 5, 2020

Choose a reason for hiding this comment

mattip Mar 5, 2020

Choose a reason for hiding this comment

mattip commented Mar 5, 2020

matthew-brett Mar 6, 2020

Choose a reason for hiding this comment

mattip Mar 6, 2020

Choose a reason for hiding this comment

mattip Mar 6, 2020

Choose a reason for hiding this comment

matthew-brett commented Mar 6, 2020

mattip commented Mar 6, 2020

matthew-brett commented Mar 6, 2020

mattip commented Feb 29, 2020 •

edited

Loading

matthew-brett commented Mar 2, 2020 •

edited

Loading