Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --print-build-identifiers to GitHub Action #872

Closed
wants to merge 1 commit into from

Conversation

abitrolly
Copy link
Contributor

For troubleshooting

@henryiii
Copy link
Contributor

henryiii commented Oct 14, 2021

Print the build identifiers matched by the current invocation
and exit.

This is going to be a bit problematic. ;)

Anyway, we already print out identifiers as we go.

@abitrolly
Copy link
Contributor Author

But how to troubleshoot these build then?

I specified to use manylinux_2_24 and in the end here is Starting Docker image quay.io/pypa/manylinux2010_x86_64:2021-10-06-94da8f1... and bam.

Error: Command ['sh', '-c', 'apt-get update && apt-get -y install unixodbc-dev'] failed with code 127. 

Error: Process completed with exit code 1.

The full log.

Run pipx run --spec '/home/runner/work/_actions/pypa/cibuildwheel/v2.1.3' cibuildwheel . --output-dir wheelhouse 2>&1
  pipx run --spec '/home/runner/work/_actions/pypa/cibuildwheel/v2.1.3' cibuildwheel . --output-dir wheelhouse 2>&1
  shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
  env:
    CIBW_MANYLINUX_X86_64_IMAGE: manylinux_2_24
    CIBW_BEFORE_ALL_LINUX: apt-get update && apt-get -y install unixodbc-dev
    CIBW_SKIP: *-win32 *-manylinux_i686

     _ _       _ _   _       _           _
 ___|_| |_ _ _|_| |_| |_ _ _| |_ ___ ___| |
|  _| | . | | | | | . | | | |   | -_| -_| |
|___|_|___|___|_|_|___|_____|_|_|___|___|_|

cibuildwheel version 2.1.3

Build options:
  platform: 'linux'
  architectures: {<Architecture.i686: 'i686'>, <Architecture.x86_64: 'x86_64'>}
  before_all: 'apt-get update && apt-get -y install unixodbc-dev'
  before_build: ''
  before_test: ''
  build_frontend: 'pip'
  build_selector: BuildSelector(build_config='*', skip_config='*-win32 *-manylinux_i686')
  build_verbosity: 0
  dependency_constraints: DependencyConstraintsPosixPath('/opt/pipx/.cache/927dea022bdb306/lib/python3.8/site-packages/cibuildwheel/resources/constraints.txt'))
  environment: ParsedEnvironment([])
  manylinux_images: {'x86_64': 'quay.io/pypa/manylinux_2_24_x86_64:2021-10-06-94da8f1', 'i686': 'quay.io/pypa/manylinux2010_i686:2021-10-06-94da8f1', 'pypy_x86_64': 'quay.io/pypa/manylinux2010_x86_64:2021-10-06-94da8f1', 'aarch64': 'quay.io/pypa/manylinux2014_aarch64:2021-10-06-94da8f1', 'ppc64le': 'quay.io/pypa/manylinux2014_ppc64le:2021-10-06-94da8f1', 's390x': 'quay.io/pypa/manylinux2014_s390x:2021-10-06-94da8f1', 'pypy_aarch64': 'quay.io/pypa/manylinux2014_aarch64:2021-10-06-94da8f1', 'pypy_i686': 'quay.io/pypa/manylinux2010_i686:2021-10-06-94da8f1'}
  output_dir: PosixPath('wheelhouse')
  package_dir: PosixPath('.')
  repair_command: 'auditwheel repair -w {dest_dir} {wheel}'
  test_command: ''
  test_extras: ''
  test_requires: []
  test_selector: TestSelector(build_config='*')

Here we go!

Starting Docker image quay.io/pypa/manylinux_2_24_x86_64:2021-10-06-94da8f1...
                                                             ✓ 19.98s
Copying project into Docker...
                                                              ✓ 0.22s
Running before_all...
                                                              ✓ 5.11s

Building cp36-manylinux_x86_64 wheel
CPython 3.6 manylinux x86_64

Setting up build environment...
                                                              ✓ 0.04s
Building wheel...
                                                              ✓ 5.79s
Repairing wheel...
                                                              ✓ 0.88s

✓ cp36-manylinux_x86_64 finished in 6.81s

Building cp37-manylinux_x86_64 wheel
CPython 3.7 manylinux x86_64

Setting up build environment...
                                                              ✓ 0.03s
Building wheel...
                                                              ✓ 5.67s
Repairing wheel...
                                                              ✓ 0.38s

✓ cp37-manylinux_x86_64 finished in 6.19s

Building cp38-manylinux_x86_64 wheel
CPython 3.8 manylinux x86_64

Setting up build environment...
                                                              ✓ 0.03s
Building wheel...
                                                              ✓ 5.72s
Repairing wheel...
                                                              ✓ 0.38s

✓ cp38-manylinux_x86_64 finished in 6.22s

Building cp39-manylinux_x86_64 wheel
CPython 3.9 manylinux x86_64

Setting up build environment...
                                                              ✓ 0.03s
Building wheel...
                                                              ✓ 5.66s
Repairing wheel...
                                                              ✓ 0.38s

✓ cp39-manylinux_x86_64 finished in 6.16s

Building cp310-manylinux_x86_64 wheel
CPython 3.10 manylinux x86_64

Setting up build environment...
                                                              ✓ 0.03s
Building wheel...
                                                              ✓ 6.08s
Repairing wheel...
                                                              ✓ 0.38s

✓ cp310-manylinux_x86_64 finished in 6.59s
Copying wheels back to host...
                                                              ✓ 0.12s
Starting Docker image quay.io/pypa/manylinux2010_x86_64:2021-10-06-94da8f1...
                                                             ✓ 21.85s
Copying project into Docker...
                                                              ✓ 0.19s
Running before_all...
                                                              ✕ 0.35s
Error: Command ['sh', '-c', 'apt-get update && apt-get -y install unixodbc-dev'] failed with code 127. 

Error: Process completed with exit code 1.

https://github.com/mkleehammer/pyodbc/runs/3890149950?check_suite_focus=true

@abitrolly
Copy link
Contributor Author

Ideally the info that helps is a build matrix as a table, which shows both builds scheduled and filtered out, with a reason why they are filtered.

@henryiii
Copy link
Contributor

That's PyPy, you didn't change the image for pypy_x86_64. What we could do is print all the "planned" selectors when starting a docker launch - that will be more relevant once #854 hits, too, since you can split up docker launches.
This dies in BEFORE_ALL, where there's not a specific wheel to be built, but what it's trying to do is build the PyPy wheels.

@abitrolly
Copy link
Contributor Author

That's PyPy, you didn't change the image for pypy_x86_64.

Well, manulinux is the platform for Linux. I changed the image for the platform, and I didn't expect to also change it for each supported Python interpreter.

What we could do is print all the "planned" selectors when starting a docker launch.

Yep, that would help.

@abitrolly
Copy link
Contributor Author

It will even help if before starting Docker image it was explained what it is started for.

-Starting Docker image quay.io/pypa/manylinux_2_24_x86_64:2021-10-06-94da8f1...
+Starting wheel builder image for CPython [3.7, 3.8, 3.9] platform [manylinux_2_24]
+  docker://quay.io/pypa/manylinux_2_24_x86_64:2021-10-06-94da8f1...

@henryiii
Copy link
Contributor

henryiii commented Oct 14, 2021

That's basically what I meant; I was thinking:

Starting wheel builder image for cp37-manylinux_x86_64,
                                 cp38-manylinux_x86_64,
                                 cp39-manylinux_x86_64:
  docker://quay.io/pypa/manylinux_2_24_x86_64:2021-10-06-94da8f1

The "manylinux_2_24" is lost at that point, and isn't required, you could target arbitrary images. But that's fine, you see that from the docker image started.

@henryiii
Copy link
Contributor

henryiii commented Oct 14, 2021

Well, manulinux is the platform for Linux. I changed the image for the platform, and I didn't expect to also change it for each supported Python interpreter.

This is a historical artifact; we discussed changing it for 2.0, but it remained as is. See #711. TL;DR: We only recently merged PyPy + manylinux images; before that, you had to have different images. And PyPy doesn't support manylinux1, which 70% of our users use last I checked. So we left the setting separate. Overrides (#854) would actually make this really easy to merge now, but it wouldn't be backwards compatible. (3.0 idea?)

One other downside is that currently I think you will always get two docker launches, even if you don't use different images; that might be fixed in the overrides branch though.

@joerick
Copy link
Contributor

joerick commented Oct 14, 2021

+1 to what Henry said.

that might be fixed in the overrides branch though.

It isn't fixed in the current implementation, we still include the platform_tag(?) in the build step computation. but that's a good point, maybe we should just remove that and split it up based on (Docker image, before_all, arch)? Do we even need arch? I suppose it might be handy for x86_64/i686?

@henryiii
Copy link
Contributor

Isn't the arch part of the docker image? I don't think there can be a single image that can build two archs?

@henryiii
Copy link
Contributor

@joerick, would you like to do that? I'm working on adding a bit more testing currently, then I think it will be ready. If you can't get to it soonish, I can add it by or before mid tomorrow.

@henryiii
Copy link
Contributor

Correction as I'm adding tests:

It isn't fixed in the current implementation

Yes, it is. If you set manylinux2014, the runs do get merged with pp37. I would assume if you set pypy to 2010, it would get merged that way too.

@abitrolly
Copy link
Contributor Author

Not sure I am getting the thread. All I want is to be able to see current build matrix before it is executed. Docker images are a little too low level concept for discussing it.

@henryiii
Copy link
Contributor

We are going off on a tangent about #854. Ignore us. :)

Basically, there's a good opportunity here to add this - for each scheduled Docker launch, we can print the identifiers that are going to be built in that image. We are discussing the new details about how cibuildwheel groups identifiers in a launch.

In fact, I've got the change here for the new code:

-            log.step(f"Starting Docker image {build_step.docker_image}...")
+            ids_to_build = [x.identifier for x in build_step.platform_configs]
+            log.step(f"Starting Docker image {build_step.docker_image} for {', '.join(ids_to_build)}...")

@abitrolly
Copy link
Contributor Author

Yes, it looks Python versions are already known here.

python_configurations = get_python_configurations(options.build_selector, options.architectures)

@henryiii henryiii mentioned this pull request Oct 14, 2021
@joerick
Copy link
Contributor

joerick commented Oct 14, 2021

Apologies for the off-topic chats @abitrolly !

Yes, it is. If you set manylinux2014, the runs do get merged with pp37.

Ah, that's nice! I was getting confused as sometimes in that code we have things like pypy_x86_64, but this is not one of those cases :)

In fact, I've got the change here for the new code:

Cool, I see that commit in #854. I think that will help you with this issue @abitrolly , so I'll close this PR. Let me know if you disagree!

@joerick joerick closed this Oct 14, 2021
@abitrolly
Copy link
Contributor Author

@joerick so how does #854 look like?

For pyodbc that I am trying to build, PyPy didn't work because of the missing function https://foss.heptapod.net/pypy/pypy/-/issues/2766 so I need to disable that.

cubuildwheel could use a chapter named "Select Python versions to build (wheels for)" https://cibuildwheel.readthedocs.io/en/stable/options/#build-selection which also explains why prefixes are cp37, pp37, what does they mean and why they are not py37 and pypy37 like in tox.

@joerick
Copy link
Contributor

joerick commented Oct 15, 2021

🤔 that's exactly what the docs on CIBW_BUILD / CIBW_SKIP do...?

@henryiii
Copy link
Contributor

There's always my JS selector idea, where we could have a way to preview the selectors. I still have the code for that.

@abitrolly
Copy link
Contributor Author

thinking that's exactly what the docs on CIBW_BUILD / CIBW_SKIP do...?

No, they don't explain why selectors are no not py3 and pypy3.

@henryiii
Copy link
Contributor

Why do we need to explain that? Our selectors are not the same thing at all - pypy3 != pp37-manylinux_x86_64. We will be dual-supporting pp37 and pp38 soon, and we have had pp36 in the past. We have to be able to select between cp and pp - for example, you might try to build for CPython only, but py* would match everything if we tried to make the python part look like tox (I'm guessing that's what you mean, since tox has no notion of the platform / arch). tox is probably using virtualenv, and virtualenv supports several methods of selecting a pre-installed Python, including those funny little short codes.

Making the docs longer takes emphasis away from what we need to get across and make them less readable; I don't think explaining differences with tox's not-really-the-same concept is likely worth it.

@abitrolly
Copy link
Contributor Author

abitrolly commented Oct 15, 2021

I agree that docs needs less text. One way to do that is to add some tables, with short explanation why such prefixes are used. It is probably described in some PEP.. in fact in https://www.python.org/dev/peps/pep-0425/ which is referenced somewhere inline down below.

The 425 is based on https://www.python.org/dev/peps/pep-0427/#file-name-convention which says wheel names are these.

{distribution}-{version}(-{build tag})?-{python tag}-{abi tag}-{platform tag}.whl

where {python tag}-{abi tag}-{platform tag} is defined by 425, but in 427 it is

  • {python tag}: E.g. 'py27', 'py2', 'py3'.
  • {abi tag}: E.g. 'cp33m', 'abi3', 'none'.
  • {platform tag}: E.g. 'linux_x86_64', 'any'.

So what is m in cp33m? Why there are no abi3 wheels? Why cp33 which looks strangely like {abi tag} is used as {python tag} in cibuildwheel?

If the Build selection docs could start with the explanation, then users would be more aware what are they going to choose from. And a better table. The cibuildwheel build identifiers are invalid PEP selectors in the sense that they strip one of the obligatory parts in {python tag}-{abi tag} of wheel name. And there is no mapping between the wheel name you're going to get and the selector.

@henryiii
Copy link
Contributor

henryiii commented Oct 15, 2021

You can't have a 1:1 map between cibuildwheel selectors and wheel names. The selector defines the platform the wheel is built on. The wheel name defines the platform the wheel is run on. Those are often closely related, but not always. If you have an abi3 or none wheel, then all different CPython versions you build on map to exactly the same final wheel name. You can also have multiple platform_tags - manylinux wheels get both the 1/2010/2014 version of the tag and the 2_* version of the tag. macOS can be multi tagged too - the selector based on the target platform, not the build platform.

The "m" was removed in Python 3.4 or 3.5 (don't remember which), so the last time it was in a build selector was with Python 2, in cibuildwheel 1.x. Yes, you had to build all CPython 2.7 wheels twice. Yes, some wheels (like Pandas) forgot to do this. But yes, this is the build platform's ABI, which is probably where the name came from. Plus it's easy to select on.

Personally, I love the table in https://cibuildwheel.readthedocs.io/en/stable/options/#build-skip and the only thing I'd be tempted to change is to duplicate it onto the main page / readme, as I feel like I have to jump there all the time. Adding explanations to it would extend the docs, and not really add all that much, I think. You can easily see the mapping to Python versions on one axes, and platforms on the other.

@henryiii
Copy link
Contributor

The format is python_tag-platform_tag, with tags similar to those in PEP 425.

I didn't realize that was in the docs; I think that's already a reasonable level of detail, since we also show all the selectors in a table. Technically it's probably closer to abi_tag-platform_tag, though using "python_tag" is probably easier for people to understand/digest here.

@abitrolly abitrolly deleted the patch-1 branch October 15, 2021 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants