Adding support for building manylinux2010 wheels

[YannickJadoul]

That was actually easier than expected (if it now also works, of course). See also #65.

A few comments/remarks/things to consider:

• I like the tag approach, but do we build everything, by default? Do we let cibuildwheel generate both manylinux1 and manylinux2010 wheels, by default?
• manylinux2010 is almost there, but still considered experimental. If this works, do we release a new version that builds manylinux2010 by default, or do we turn it of by default?
• manylinux2010_i686 is not available yet. This page seems to indicate it will still be created at some point.
• Soon, we might not need to set the --plat argument to auditwheel repair anymore: https://github.com/matthew-brett/multibuild/issues/238#issuecomment-484684645
• When wheels are both manylinux1 and manylinux2010 compatible, auditwheel repair will produce both (see Tracking issue for manylinux2010 rollout manylinux#179 (comment) and ENH: add multilinux2010 python-manylinux-demo#19). So delocated_wheel=(/tmp/delocated_wheel/*.whl) and "$delocated_wheel" to select the first won't work anymore? (I'm testing this in a minute to confirm this.)
• A few minor things in the code, but I'll review myself and comment on it.

[YannickJadoul]

• When wheels are both manylinux1 and manylinux2010 compatible, auditwheel repair will produce both (see Tracking issue for manylinux2010 rollout manylinux#179 (comment) and ENH: add multilinux2010 python-manylinux-demo#19). So delocated_wheel=(/tmp/delocated_wheel/*.whl) and "$delocated_wheel" to select the first won't work anymore? (I'm testing this in a minute to confirm this.)

This is actually happening in the tests. I'm not entirely sure why, but I'll see if we can test that the right wheels are copied.

See here, for example: https://travis-ci.org/joerick/cibuildwheel/jobs/522327295#L976

[YannickJadoul]

Should these default images be here, or should they actually be in main.py?

[YannickJadoul]

@joerick This is about the code style, rather than platform/ABI-related. Any preferences, here?

[YannickJadoul]

@joerick Thus linor code organizational detail is still open, as well.

[mayeut]

I'm not sure we'll be seeing manylinux2010_i686 anytime soon. There's an issue with toolchain availability and auditwheel:

• Tracking issue for manylinux2010 rollout manylinux#179 (comment)
• Update manylinux2010 policy with i686 symbol versions auditwheel#141

I'm not sure the --plat argument should be dropped in cibuildwheel to take advantage of image detection (if it does come in auditwheel, preliminary support through environment variables already done in all manylinux images). Having it present and dependent on cibuildwheel CIBW_BUILD and CIBW_SKIP as the major advantage that I could just say "do not build manylinux2010, build only manylinux1" and then override the docker image to get the manylinux2010 image instead while enforcing a manylinux1 wheel is produced (taking advantage of a more recent toolchain, on x86_64 only, and maybe for "basic" C extensions only).

[mayeut]

When wheels are both manylinux1 and manylinux2010 compatible, auditwheel repair will produce both

I'm not a fan of this feature in fact. I don't know if you are but I'll probably open an issue and/or PR over at auditwheel to at least having an option to enforce the --plat argument with only one wheel produced if it's not the default behavior.

Would (/tmp/delocated_wheel/*_{platform_tag}.whl) work ?

[YannickJadoul]

I'm not a fan of this feature in fact.

It's a bit weird indeed; but then again, it's nice to signal to the user that it's even more compatible than expected/asked for? In principle, there's no good reason for uploading manylinux2010 wheels when you have manylinux1 wheels? Or is there?

Would (/tmp/delocated_wheel/*_{platform_tag}.whl) work ?

I thought of that as well, but it doens't work with the pure Python case:

if [[ "$built_wheel" == *none-any.whl ]]; then
    # pure python wheel - just copy
    mv "$built_wheel" /tmp/delocated_wheel

[YannickJadoul]

Thanks for the quick replies, @mayeut!

I hope this commit has a workaround for multiple repaired images, if we want to only return one, instead of all, to the cibuildwheel user. I do feel that there'd be something transparent about just copying all the wheels generated by auditwheel.

On a separate note, @joerick: we should rework the tests, I feel. There's no way of specifying what output wheels are expected by each test, so I don't see an obvious and clean way of checking the manylinux2010 wheels are copied to the output folder.

[mayeut]

it's nice to signal to the user that it's even more compatible than expected/asked for?

Agreed for signaling.

In principle, there's no good reason for uploading manylinux2010 wheels when you have manylinux1 wheels? Or is there?

Once again, agreed.

Now that I'm reading it again, I might change my point of view. The thing I disliked was that the wheels produced might change from manylinux1 to manylinux2010 silently (and I want to know this, not discover it, but maybe I'm too paranoid). This can be overcome by forcing manylinux1 with the --plat argument (i.e. with your PR in its current state, just continue building for manylinux1, but pulling the manylinux2010 image).

Still remains the fact that they're are 2 wheels to deal with which, as you saw, is something that needs work when integrating. Maybe only one shall be produced, i.e. the manylinux1 rather than the manylinux2010 when possible.

If you want a test that only produces manylinux2010 wheel, you can add that to one of the tests (I did add it on test/01_basic/spam.c when testing the manylinux2010 image):

@ -1,14 +1,26 @@
 #include <Python.h>
+#if defined(__linux__)
+#include <malloc.h>
+#endif

 static PyObject *
 spam_system(PyObject *self, PyObject *args)
 {
     const char *command;
-    int sts;
+    int sts = 0;

     if (!PyArg_ParseTuple(args, "s", &command))
         return NULL;
-    sts = system(command);

+#if defined(__GLIBC_PREREQ) && __GLIBC_PREREQ(2, 10)
+    {
+        sts = malloc_info(0, stdout);
+    }
+#endif
+
+    if (sts == 0) {
+        sts = system(command);
+    }
     return PyLong_FromLong(sts);
 }

[joerick]

@YannickJadoul yes the tests are much overdue some improvements. I'm thinking each test folder gets some kind of script that can set env, run cibuildwheel and then assert what ever it likes. I can maybe look at this early next week

On the multiple wheel thing - (I'm very happy to delegate decisions to you both since you're way more knowledgeable than me!) I'm a little confused about the difference between the ABI and the toolchain. i.e. what it means to have a manylinux1 wheel produced with the manylinux2010 toolchain... is that better or worse than a manylinux1 wheel from the manylinux1 toolchain?

[YannickJadoul]

The thing I disliked was that the wheels produced might change from manylinux1 to manylinux2010 silently (and I want to know this, not discover it, but maybe I'm too paranoid).

This is a good point, actually. I hadn't considered that.

Still remains the fact that they're are 2 wheels to deal with which, as you saw, is something that needs work when integrating.

But indeed, I think this is the most confusing/hard-to-deal-with part. But I like your suggestion, there. If you consider it like this, the --plat argument is just the "minimal requirement" of the wheel to be checked.

To relate it to my previous comment:

In principle, there's no good reason for uploading manylinux2010 wheels when you have manylinux1 wheels? Or is there?

There are probably efficiency advantages of building on the manylinux2010 platform and allowing for non-manylinux1 symbols (as new compilers and standard library versions probably improve performance?). But íf this still results in manylinux1-compatible wheels, you have the best of both worlds, and manylinux1 wheels produced on the manylinux2010 are probably still better than or as good as manylinux1 wheels built on the older platform?

I'll wait for further discussion in pypa/manylinux2010#179 before deciding whether to revert that (/tmp/delocated_wheel/*-{platform_tag}.whl) commit?

Thanks for the test example, by the way! That seems very useful. I'll quickly add it in a new test :-)

[YannickJadoul]

@joerick

yes the tests are much overdue some improvements. I'm thinking each test folder gets some kind of script that can set env, run cibuildwheel and then assert what ever it likes. I can maybe look at this early next week

I like that idea! Though maybe we'd only move the asserts to this script, and have the main test-script still call cibuildwheel? I'll have a look at it; seems easier than I first thought :-)

since you're way more knowledgeable than me!

@mayeut definitely is. I've just been forced into this thing, when I wanted to use C++14 for manylinux1 wheels, but I still feel my knowledge is very fragmented. So that's why I'm very grateful for @mayeut's feedback and sanity checks!

I'm a little confused about the difference between the ABI and the toolchain. i.e. what it means to have a manylinux1 wheel produced with the manylinux2010 toolchain... is that better or worse than a manylinux1 wheel from the manylinux1 toolchain?

As far as I understand (please correct me if I'm wrong), the newer toolchains are allowed to use the newer ABI, but if they happen to not (as in our very basic test, apparently), the produced wheel is still manylinux1 compatible. And is probably equally good or slightly better, because a newer toolchain probably has an improved optimizer?

[YannickJadoul]

I'll quickly add it in a new test :-)

@mayeut Am I doing something wrong, there? I made a mistake when specifying the CIBW_SKIP, resulting in the manylinux1 tags not actually being skipped. But somehow that code also perfectly compiles on manylinux1 and results in manylinux1-compatible wheels?
See https://travis-ci.org/joerick/cibuildwheel/jobs/522504054#L4790

[mayeut]

As far as I understand (please correct me if I'm wrong), the newer toolchains are allowed to use the newer ABI, but if they happen to not (as in our very basic test, apparently), the produced wheel is still manylinux1 compatible. And is probably equally good or slightly better, because a newer toolchain probably has an improved optimizer?

Exactly my thought. We could benefit from a newer toolchain with probably an improved optimizer (or support for extended instruction set like AVX512) and still be manylinux1 compatible if only using the allowed subset of symbols of manylinux1.

I'm quite surprised the manylinux2010 specific test is building on manylinux1.
According to http://man7.org/linux/man-pages/man3/malloc_info.3.html, it was introduced in glibc 2.10 so, not available in manylinux1 image. If i'm not mistaken, no tests are actually ran with built wheel. I guess one thing that could be happening is that when building, we get a warning for an implicit-function-declaration but the build just goes on and the symbol for that function doesn't get versioned obviously. It should probably fail at runtime. I wonder if using -Werror=implicit-function-declaration extra flags would actually fail the build as expected

[YannickJadoul]

If i'm not mistaken, no tests are actually ran with built wheel. I guess one thing that could be happening is that when building, we get a warning for an implicit-function-declaration but the build just goes on and the symbol for that function doesn't get versioned obviously. It should probably fail at runtime. I wonder if using -Werror=implicit-function-declaration extra flags would actually fail the build as expected

Good point; it does indeed seem as if no tests are run. And given that the file it's spam.c and that the C compiler tends to be more forgiving for these kinds of things, you might indeed be right. I'll get a closer look tomorrow.

[YannickJadoul]

@mayeut Yesss, you were right: spam.c:16:5: error: implicit declaration of function ‘malloc_info’ [-Werror=implicit-function-declaration]

[joerick]

Exactly my thought. We could benefit from a newer toolchain with probably an improved optimizer (or support for extended instruction set like AVX512) and still be manylinux1 compatible if only using the allowed subset of symbols of manylinux1.

Just following this logic... can we, then, drop the manylinux1 image and build both manylinux1 and manylinux2010 wheels on the manylinux2010 image? Then, by default, our users get an updated toolchain, even if they're still building manylinux1-compatible wheels?

EDIT- I guess i686 would have to be the old manylinux1 image, though.

[YannickJadoul]

Just following this logic... can we, then, drop the manylinux1 image and build both manylinux1 and manylinux2010 wheels on the manylinux2010 image?

Hmmm, I don't think so, because things built inside the manylinux2010 image míght still be (probably are?) manylinux1-incompatible. See for example the manylinux2010_only test I've added.

[YannickJadoul]

Right, rebased on #137, and tests are working again.

So now the last things before this can be merged are:

• What should the cibuildwheel defaults be/do? Should both manylinux1 and manylinux2010 be run? And should that already be the case in a new release, or do we wait until manylinux2010 is completely stable (i.e., the issue closed)?
• What happens with the different wheels? Has a decision been taken by the PyPA developers (as far as I can see in the linked discussion, not yet, but please correct me, @mayeut)? Do we wait for a decision, or do we update if auditwheel changes?

Anything else missing? I don't think so, but I might be forgetting something important, ofc.

[mayeut]

My best guess would be not to change the defaults for now in cibuildwheel. That is, continue building for manylinux1 as a default, manylinux2010 shall be opt-in. This will prevent any side effects for users of cibuildwheel when upgrading for other reasons than getting this feature.

As for what to do with the 2 wheels, they're still being produced and I don't think it'll change in the short term. However, it was also confirmed by former auditwheel maintainer that only the manylinux1 should be uploaded.

[YannickJadoul]

My best guess would be not to change the defaults for now in cibuildwheel. That is, continue building for manylinux1 as a default, manylinux2010 shall be opt-in. This will prevent any side effects for users of cibuildwheel when upgrading for other reasons than getting this feature.

I think I agree, but technically this is not entirely straightforward, since the current solution just offers more 'build tags', rather than a switch between manylinux1 and manylinux2010.
One way of setting this default is setting a different default CIBW_BUILD or CIBW_SKIP, but this might also break if people change this? But I guess we could have an opt-in, like CIBW_BUILD_MANYLINUX2010 = 1 (or CIBW_MANYLINUX2010_ENABLE), that enables CIBW_BUILD=* to match manylinux2010 as well?

Alternatively, we could approach this in a different way, and rather than adding build tags, add something like CIBW_MANYLINUX_VERSION? But then I do quite like @joerick's initial intuition of just adding more tags, as I have done now.

As for what to do with the 2 wheels, they're still being produced and I don't think it'll change in the short term. However, it was also confirmed by former auditwheel maintainer that only the manylinux1 should be uploaded.

OK, good to know, thanks!
Does that mean we need to change the current implementation? We are currently always returning the manylinux2010 one, even if a manylinux1 was built. So if we want to encourage people to upload manylinux1 wheels when possible, we should probably not do that?

[joerick]

Hello! Looks like this is waiting on a decision on UI/options? Maybe I can try a recap of this, let me know if I've got this right:

• Manylinux2010 wheels are only produced on the new manylinux2010 docker image
• when using the manylinux2010 image, if that extension only uses symbols from manylinux1, that wheel is also produced.
• a project that previously produced a manylinux1 wheel on the manylinux1 docker image will produce a manylinux2010 and a manylinux1 wheel on the new manylinux2010 docker image.

Have I got that right 😬 ?

[YannickJadoul]

Yes, except for a few minor decisions, this should be ready.

• Manylinux2010 wheels are only produced on the new manylinux2010 docker image

Kind of. manylinux2010 is a newer/looser standard, so in principle you could auditwheel a wheel built on the manylinux1 docker image as manylinux2010, but there wouldn't really be a point.

• when using the manylinux2010 image, if that extension only uses symbols from manylinux1, that wheel is also produced.

Yes.

• a project that previously produced a manylinux1 wheel on the manylinux1 docker image will produce a manylinux2010 and a manylinux1 wheel on the new manylinux2010 docker image.

No. cibuildwheel currently only copies the wheel matching the docker image (i.e. manylinux2010) to the output directory. But yes, auditwheel will produce two wheels.

EDIT:
The problematic part with copying back the two wheels produced by auditwheel is that the filename conflicts/overwrites the manylinux1 wheel that was created on the manylinux1 image. Which is not a problem by itself, because the manylinux1 wheel produced on the manylinux2010 image can only be better (since built with a newer compiler), I believe, but it's still a bit arbitrary and depends on the order of the builds.

[YannickJadoul]

@mayeut I saw your PR pypa/manylinux#293 adding AUDITWHEEL_PLAT was merged. Do you suggest removing the --plat flag (being simpler), or do you advise keeping it here?

[jbarlow83]

Is there anything that prevents this from being merged?

Based on the recent discussion at pypa/manylinux#179, it would be ideal to make the manylinux2010 image the default builder (because it's CentOS 6), even when manylinux1 policy is the target. For security. pypa no longer links to the manylinux1 Docker image because it's a few years beyond EOL.

[joerick]

@jbarlow83 I've been following that thread and Elana's post today is exactly what I was waiting for in terms of how we support manylinux2010 in cibuildwheel. Which is, I think- we should build both manylinux1 and manylinux2010 wheels using the manylinux2010 image, and discontinue the manylinux1 image going forward. For cibuildwheel's API, building using manylinux1 image should be discontinued (or at best, available via an option), and the Linux builds should take place on the manylinux2010 images, by default.

Thoughts @YannickJadoul @mayeut? I'm wondering- Does this put us in a tricky place wrt how the CIBW_BUILD and CIBW_SKIP options work? Now auditwheel decides whether to output a manylinux1 wheel or not, rather than the options provided to cibuildwheel. Perhaps we should now consider manylinux1 wheels a bonus side effect, but not request them explicitly in the CIBW_BUILD param. (We could add another option to fail the build if they're not produced)

[YannickJadoul]

@jbarlow83 @joerick Sorry for the delay; I'm currently on holiday and I've been putting off thinking about and replying to this messy matter 😉

My 5 cents:

We should build both manylinux1 and manylinux2010 wheels using the manylinux2010 image

If I understand correctly, the problem with that is that you're not guaranteed to build manylinux1 wheels on a manylinux2010 image. As far as I understand, for example glibc adds symbols in subsequential versions, and manylinux2010 has a later version.

This means that if you are lucky, and your library/compiler uses only symbols that are already present in the manylinux1 version of glibc (GLIBC_2.5, according to PEP 513), you produce a wheel compatible with both manylinux1 and manyiinux2010. For example, ehashman's example used only puts, present in both the manylinux2010 as well as the manylinux1 images' glibc.

However, because a later version of glibc is present (GLIBC_2.12, according to PEP571), your library (and maybe even the standard library or compiler?) will be able to use later symbols and produce a manylinux2010 wheel that's not manylinux1-compatible. (glibc is only an example, btw: there are more specifications about what it means to be a manylinux1- or manylinux2010-compatible wheel in those two referenced PEPs.)

So sometimes, a wheel built on a platform that isn't built on the manylinux1 image does not use anything more than available on the manylinux1 image and will be a manylinux1-compatible wheel. But you have no guarantees that it will always be; hence the whole setup with manylinux1 Docker images. I must say I have no idea how likely you are to "accidentally" build a manylinux1 wheel on manylinux2010.

it would be ideal to make the manylinux2010 image the default builder (because it's CentOS 6)

In the current version of the PR, this wouldn't be the default. The default would be to produce both. Which might be a bit of overkill?

I'm wondering- Does this put us in a tricky place wrt how the CIBW_BUILD and CIBW_SKIP options work

In this PR, I just added manylinux2010 as another platform/build tag (rather than ... I don't know, choosing the manylinux version with a separate option or so). So it's completely independent from the manylinux1 build (and vice versa), and both wheels would be produced, unless CIBW_BUILD or CIBW_SKIP specify differently.

pypa no longer links to the manylinux1 Docker image because it's a few years beyond EOL.

I've just checked, and NumPy seems to still produce manylinux1 wheels. I might be wrong, but I tend to look towards NumPy for guidance for best practices for these types of things.

[jbarlow83]

It's not overkill to make the manylinux2010 image the default builder. It's preferable, because that image can still produce manylinux1 wheels and will do a better job of that thanks to newer compilers.

numpy is a good source for best practices, but not on this topic. The best practice for numpy is to use the system version rather than a wheel, because the version from a wheel is currently built with the CentOS 5 version of gcc so it will be way behind in terms of CPU instructions and optimizations. At the same time, it's not the sort of library that would be actively looking for the latest syscalls and so require a newer glibc. numpy has its own wheel-builder project, numpy-wheels, which uses multibuild, which still uses the manylinux1 Docker images. multibuild also doesn't support manylinux2010 yet. I think just about everyone is using manylinux1, given the slow pace of rollout and lack of tooling support from cibuildwheel and multibuild.

As for how auditwheel does its thing, it determines what symbols are missing just by reading the relevant sections of the ELF and comparing to them a built-in policy. cibuildwheel could either call auditwheel show or import auditwheel figure out whether manylinux1 or manylinux2010 or both are possible targets. auditwheel uses manylinux1 where possible and only picks manylinux2010 wheels when needed to match symbols. Potentially we could compare that to environment variables that specify the user's requirements.

[YannickJadoul]

It's not overkill to make the manylinux2010 image the default builder.

OK, yes, I agree with that :-) I thought the suggestion was to abolish manylinux1 completely, in cibuildwheel.

It's preferable, because that image can still produce manylinux1 wheels

Yeah, but it won't reliably do that, will it? There's no way to force building a manylinux1 wheel on a manylinux2010 image, is there?
My point is that "potentially (by accident) producing a manylinux1 wheel" is not the same as "being a dependable way of producing manylinux1 wheels", so we cibuildwheel can't promise manylinux1 wheel on the manylinux2010 image.

numpy is a good source for best practices, but not on this topic.

OK, fair enough. Thanks for the explanation!

Either way, maybe the current PR is not the best way to do this, then, given that manylinux1 and manylinux2010 are just two separate tags, without version. Maybe something with CIBW_MANYLINUX_VERSION would be better? I'll think a bit more, and I'll propose a new PR soon, so we can compare.

[joerick]

Closed in favour of #155