Support projects without setup.py (pyproject.toml and setup.cfg are also valid)

[heitorlessa]

Issue no: #359

Description of change:

• Allow projects that only contain pyproject.toml e.g. poetry
• Integ test for a dummy poetry package with no setup.py

[heitorlessa]

I'm also testing my dummy package with Github actions here: https://github.com/heitorlessa/test-actions/blob/a0b6668027fb8189da62c05a24b4d763a9d93e7f/.github/workflows/python-package.yml#L50

[heitorlessa]

@YannickJadoul do you want me to create a unit test to confirm pyproject.toml alone works? or integ test is enough?

I'm also happy to bring over the test from #358 if you want me

[YannickJadoul]

@YannickJadoul do you want me to create a unit test to confirm pyproject.toml alone works? or integ test is enough?

As far as I'm concerned, we already need to mock stuff in the unit tests in fake_package_dir (where we insist os.path.join(MOCK_PACKAGE_DIR, 'setup.py') does exist). So I don't think it makes a lot of sense to do this for pyproject.toml as well. If you want to and can come up with something that's not just tautologically checking the same as the actual code, feel free to add something, but I don't think it's necessary.

I'm also happy to bring over the test from #358 if you want me

Yeah, I'm wondering if we can somehow combine these. It would make our test suite faster than having two separate tests. #358 still has setup.py as well, though. @henryiii, could these tests be combined from your perspective?
If this is possible, it might be nice to rebase onto #358's branch? Or we just wait for @joerick to merge #358, and then merge the two things ourselves. That's also fine.

[joerick]

As far as I'm concerned, we already need to mock stuff in the unit tests in fake_package_dir (where we insist os.path.join(MOCK_PACKAGE_DIR, 'setup.py') does exist). So I don't think it makes a lot of sense to do this for pyproject.toml as well. If you want to and can come up with something that's not just tautologically checking the same as the actual code, feel free to add something, but I don't think it's necessary.

👍

I'm also happy to bring over the test from #358 if you want me

Yeah, I'm wondering if we can somehow combine these. It would make our test suite faster than having two separate tests. #358 still has setup.py as well, though. @henryiii, could these tests be combined from your perspective?
If this is possible, it might be nice to rebase onto #358's branch? Or we just wait for @joerick to merge #358, and then merge the two things ourselves. That's also fine.

They're not testing the same thing, are they? This one is a no-setup.py test, using a different build backend. #358 is still setuptools, but uses some pyproject.toml features. I think we could include both.

[heitorlessa]

As far as I'm concerned, we already need to mock stuff in the unit tests in fake_package_dir (where we insist os.path.join(MOCK_PACKAGE_DIR, 'setup.py') does exist). So I don't think it makes a lot of sense to do this for pyproject.toml as well. If you want to and can come up with something that's not just tautologically checking the same as the actual code, feel free to add something, but I don't think it's necessary.

👍

I'm also happy to bring over the test from #358 if you want me

Yeah, I'm wondering if we can somehow combine these. It would make our test suite faster than having two separate tests. #358 still has setup.py as well, though. @henryiii, could these tests be combined from your perspective?
If this is possible, it might be nice to rebase onto #358's branch? Or we just wait for @joerick to merge #358, and then merge the two things ourselves. That's also fine.

They're not testing the same thing, are they? This one is a no-setup.py test, using a different build backend. #358 is still setuptools, but uses some pyproject.toml features. I think we could include both.

Yes. As you pointed out, this tests a poetry package project (no setup.py) as per #359.

[heitorlessa]

It's failing CI tests because Python 2.7 manylinux image has an outdated pip - Is there a way to upgrade pip within the docker image it builds the wheels?

I think I also saw this error on Windows x86 when testing: https://github.com/heitorlessa/test-actions/runs/725413768?check_suite_focus=true

I'm gonna skip 2.7 temporarily, check the results tomorrow, and see if it also fails on Windows x86 like it did to me.

---

Error - Full log, error at line 650

    gcc -pthread -fno-strict-aliasing -g -O2 -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -fPIC -I/opt/_internal/cpython-2.7.18-ucs2/include/python2.7 -c build/temp.linux-i686-2.7/_openssl.c -o build/temp.linux-i686-2.7/build/temp.linux-i686-2.7/_openssl.o -Wconversion -Wno-error=sign-conversion
    build/temp.linux-i686-2.7/_openssl.c:546:10: fatal error: openssl/opensslv.h: No such file or directory
     #include <openssl/opensslv.h>
              ^~~~~~~~~~~~~~~~~~~~
    compilation terminated.
    error: command 'gcc' failed with exit status 1
    ----------------------------------------
    ERROR: Failed building wheel for cryptography
  Successfully built functools32 subprocess32 glob2 pyrsistent secretstorage msgpack scandir
  Failed to build cryptography
  ERROR: Could not build wheels for cryptography which use PEP 517 and cannot be installed directly

[Czaki]

It's failing CI tests because Python 2.7 manylinux image has an outdated pip - Is there a way to upgrade pip within the docker image it builds the wheels?

In cibuildwheel/resources/pinned_docker_images.cfg bump images to latest image. It should have pip in version 20.1.1

[heitorlessa]

It's failing CI tests because Python 2.7 manylinux image has an outdated pip - Is there a way to upgrade pip within the docker image it builds the wheels?

In cibuildwheel/resources/pinned_docker_images.cfg bump images to latest image. It should have pip in version 20.1.1

Thanks for the pointer - I'll check on those tomorrow (11:45pm here)

[henryiii]

They're not testing the same thing, are they?

This tests PEP 517, selectable build backend (technically, the same infrastructure gets run to "pick" setuptools if you have a pyproject.toml, which is why Pip prints out a comment about PEP 517, but since setuptools is the default anyway, it's valid to test something that really relies on PEP 517 to work).

#358 is a test of PEP 518 (ignore my muddling the two PEPs in the title and branch name), setting up an environment to run the setup. It mostly makes a difference if the tool uses a python configuration file, such as setuptools and setup.py.

So they are mostly orthogonal. Interestingly, I think the PEP 518 test passed on 2.7.

[henryiii]

This isn't related, but looks like it needs fixing:

##[warning]cannot collect test class 'TestProject' because it has a __init__ constructor (from: test/test_cpp_standards.py)
##[warning]cannot collect test class 'TestProject' because it has a __init__ constructor (from: test/test_subdir_package.py)

Dumb question though; poetry does not support binary packages last I checked, so how is cibuildwheel helping with it? I am not aware of any current PEP 517 build tool that supports compiling code. Scikit-build would if it had a PEP 517 mode, but it doesn't.

That's why the rest of the builds are failing, by the way - it is producing universal wheels (as it should, I think). I think it's a good change to make, but I don't think we can test it as an integration test until there's a PEP 517 tool that can compile. (Correct me if I'm wrong, I'd love one, but compiling is hard, making pure Python packages is easy)

[heitorlessa]

This isn't related, but looks like it needs fixing:

##[warning]cannot collect test class 'TestProject' because it has a __init__ constructor (from: test/test_cpp_standards.py)
##[warning]cannot collect test class 'TestProject' because it has a __init__ constructor (from: test/test_subdir_package.py)

Dumb question though; poetry does not support binary packages last I checked, so how is cibuildwheel helping with it? I am not aware of any current PEP 517 build tool that supports compiling code. Scikit-build would if it had a PEP 517 mode, but it doesn't.

That's why the rest of the builds are failing, by the way - it is producing universal wheels (as it should, I think). I think it's a good change to make, but I don't think we can test it as an integration test until there's a PEP 517 tool that can compile. (Correct me if I'm wrong, I'd love one, but compiling is hard, making pure Python packages is easy)

Not a dumb question at all. I'm not well versed in this topic yet so I'm learning as I contribute a tiny thing.

For me, I'd like to use cibuildwheel to create universal wheel for this project, where the only piece that needs compiling is a peer dependency.

I'm not 100% on the problem CI is going through yet, so I've created the same dummy package, installed cibuildwheel from the fork it, and I skipped the ones I couldn't get to work, and it works. I'd love to understand the issue itself, as it initially seemed that pip wasn't installing cryptography wheel (aib3) - I can test that hypothesis by upgrading the pinned docker images to the latest.

[heitorlessa]

They're not testing the same thing, are they?

This tests PEP 517, selectable build backend (technically, the same infrastructure gets run to "pick" setuptools if you have a pyproject.toml, which is why Pip prints out a comment about PEP 517, but since setuptools is the default anyway, it's valid to test something that really relies on PEP 517 to work).

#358 is a test of PEP 518 (ignore my muddling the two PEPs in the title and branch name), setting up an environment to run the setup. It mostly makes a difference if the tool uses a python configuration file, such as setuptools and setup.py.

So they are mostly orthogonal. Interestingly, I think the PEP 518 test passed on 2.7.

I believe it passed because the build system doesn't rely on poetry. I was able to replicate that by using a build.py as a replacement for needing poetry (distutils).

My understanding is that the requires line instructs pip wheel to install that dep before it attempts to generate the wheel

[build-system]
requires = ["poetry>=0.12"]
build-backend = "poetry.masonry.api"

[heitorlessa]

Okay, that worked - Tests are failing due to the assertion now as actual wheel set vs expected by utility tests differ -- What's the best way to assert for the expected wheel?

I'm not entirely sure as to whether it'll create a single universal wheel, or it'll create a wheel for each distro at the end of each test hence why utils.expected_wheels with no params as I saw in Henrii's test.

Meantime, just hard coded the assertion to expect an universal wheel, so I can see how the tests progresses.

Tests asserting wheel creation using test utils

    # WHEN we attempt to build wheels for multiple platforms
    # for dummy_package package
    actual_wheels = utils.cibuildwheel_run(project_dir, add_env=skip_outdated_pip_images_env)

    # THEN we should have a dummy_package wheel with version 0.1.0
    expected_wheels = utils.expected_wheels("dummy_package", "0.1.0")
    assert set(actual_wheels) == set(expected_wheels)

Cleaned up sets (no newline)

actual_wheels = {'dummy_package-0.1.0-py2.py3-none-any.whl'} 

expected_wheels = {'dummy_package-0.1.0-cp27-cp27m-manylinux1_i686.whl',
                    'dummy_package-0.1.0-cp27-cp27m-manylinux1_x86_64.whl',
                    'dummy_package-0.1.0-cp27-cp27m-manylinux2010_i686.whl',
                    'dummy_package-0.1.0-cp27-cp27m-manylinux2010_x86_64.whl',
                    'dummy_package-0.1.0-cp27-cp27mu-manylinux1_i686.whl',
                    'dummy_package-0.1.0-cp27-cp27mu-manylinux1_x86_64.whl',
                    'dummy_package-0.1.0-cp27-cp27mu-manylinux2010_i686.whl',
                    'dummy_package-0.1.0-cp27-cp27mu-manylinux2010_x86_64.whl',
                    'dummy_package-0.1.0-cp35-cp35m-manylinux1_i686.whl',
                    'dummy_package-0.1.0-cp35-cp35m-manylinux1_x86_64.whl',
                    'dummy_package-0.1.0-cp35-cp35m-manylinux2010_i686.whl',
                    'dummy_package-0.1.0-cp35-cp35m-manylinux2010_x86_64.whl',
                    'dummy_package-0.1.0-cp36-cp36m-manylinux1_i686.whl',
                    'dummy_package-0.1.0-cp36-cp36m-manylinux1_x86_64.whl',
                    'dummy_package-0.1.0-cp36-cp36m-manylinux2010_i686.whl',
                    'dummy_package-0.1.0-cp36-cp36m-manylinux2010_x86_64.whl',
                    'dummy_package-0.1.0-cp37-cp37m-manylinux1_i686.whl',
                    'dummy_package-0.1.0-cp37-cp37m-manylinux1_x86_64.whl',
                    'dummy_package-0.1.0-cp37-cp37m-manylinux2010_i686.whl',
                    'dummy_package-0.1.0-cp37-cp37m-manylinux2010_x86_64.whl',
                    'dummy_package-0.1.0-cp38-cp38-manylinux1_i686.whl',
                    'dummy_package-0.1.0-cp38-cp38-manylinux1_x86_64.whl',
                    'dummy_package-0.1.0-cp38-cp38-manylinux2010_i686.whl',
                    'dummy_package-0.1.0-cp38-cp38-manylinux2010_x86_64.whl',
                    'dummy_package-0.1.0-pp27-pypy_73-manylinux1_x86_64.whl',
                    'dummy_package-0.1.0-pp27-pypy_73-manylinux2010_x86_64.whl',
                    'dummy_package-0.1.0-pp36-pypy36_pp73-manylinux1_x86_64.whl',
                    'dummy_package-0.1.0-pp36-pypy36_pp73-manylinux2010_x86_64.whl'}

Full Assertion error

E       AssertionError: assert {'dummy_package-0.1.0-py2.py3-none-any.whl'} == {'dummy_package-0.1.0-cp27-cp27m-manylinux1_i686.whl',\n 'dummy_package-0.1.0-cp27-cp27m-manylinux1_x86_64.whl',\n 'dummy_package-0.1.0-cp27-cp27m-manylinux2010_i686.whl',\n 'dummy_package-0.1.0-cp27-cp27m-manylinux2010_x86_64.whl',\n 'dummy_package-0.1.0-cp27-cp27mu-manylinux1_i686.whl',\n 'dummy_package-0.1.0-cp27-cp27mu-manylinux1_x86_64.whl',\n 'dummy_package-0.1.0-cp27-cp27mu-manylinux2010_i686.whl',\n 'dummy_package-0.1.0-cp27-cp27mu-manylinux2010_x86_64.whl',\n 'dummy_package-0.1.0-cp35-cp35m-manylinux1_i686.whl',\n 'dummy_package-0.1.0-cp35-cp35m-manylinux1_x86_64.whl',\n 'dummy_package-0.1.0-cp35-cp35m-manylinux2010_i686.whl',\n 'dummy_package-0.1.0-cp35-cp35m-manylinux2010_x86_64.whl',\n 'dummy_package-0.1.0-cp36-cp36m-manylinux1_i686.whl',\n 'dummy_package-0.1.0-cp36-cp36m-manylinux1_x86_64.whl',\n 'dummy_package-0.1.0-cp36-cp36m-manylinux2010_i686.whl',\n 'dummy_package-0.1.0-cp36-cp36m-manylinux2010_x86_64.whl',\n 'dummy_package-0.1.0-cp37-cp37m-manylinux1_i686.whl',\n 'dummy_package-0.1.0-cp37-cp37m-manylinux1_x86_64.whl',\n 'dummy_package-0.1.0-cp37-cp37m-manylinux2010_i686.whl',\n 'dummy_package-0.1.0-cp37-cp37m-manylinux2010_x86_64.whl',\n 'dummy_package-0.1.0-cp38-cp38-manylinux1_i686.whl',\n 'dummy_package-0.1.0-cp38-cp38-manylinux1_x86_64.whl',\n 'dummy_package-0.1.0-cp38-cp38-manylinux2010_i686.whl',\n 'dummy_package-0.1.0-cp38-cp38-manylinux2010_x86_64.whl',\n 'dummy_package-0.1.0-pp27-pypy_73-manylinux1_x86_64.whl',\n 'dummy_package-0.1.0-pp27-pypy_73-manylinux2010_x86_64.whl',\n 'dummy_package-0.1.0-pp36-pypy36_pp73-manylinux1_x86_64.whl',\n 'dummy_package-0.1.0-pp36-pypy36_pp73-manylinux2010_x86_64.whl'}

[joerick]

Ah, poetry doesn't build binary wheels yet? Building pure/universal wheels isn't something cibuildwheel really supports - it should be simple to generate one of those using pip wheel --no-deps ., on any platform. We have actually discussed in #255 how cibuildwheel should raise an error if such a wheel is produced.

So @henryiii is right - to integration test this change, we'd require a build-backend that can build binary 'platform' wheels. It looks like setuptools can't be configured using pyproject.toml alone. So maybe we're not ready for an integration test here, yet. Unless anyone knows of an alternative build-backend that can build platform wheels?

[heitorlessa]

Ah! Now I understand it why the actual wheels and expected wheels are different - I had a wrong understanding of the different types of wheels, as my first experience with building one was with Poetry. My incorrect understanding was that audit repair was bundling all platform specific into a single wheel, but that's not the case.

Feel free to close this PR, and I'll look into how to use a custom build to still leverage this project for building platform specific wheels.

Thanks for all the learnings

[joerick]

Gotcha! I'd be happy to keep the modifications to __main__.py, if you're able to remove the other changes?

[heitorlessa]

Sure thing, I’ll remove them after I get back from lunch.

…

On Mon, 1 Jun 2020 at 11:32, Joe Rickerby ***@***.***> wrote: Gotcha! I'd be happy to keep the modifications to __main__.py, if you're able to remove the other changes? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#360 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZPQBCXVGBQ6BPICZAG6YDRUN7SLANCNFSM4NPHLBLQ> .

[YannickJadoul]

@heitorlessa Sorry, I also hadn't realized you were trying to build universal wheels with cibuildwheel, or I would have stopped you in time :-/

@joerick, regarding #255 and this, would it still make sense to use cibuildwheel to test pure Python wheels on multiple platforms? It's something we could consider in the context of #317 as well.

[joerick]

@joerick, regarding #255 and this, would it still make sense to use cibuildwheel to test pure Python wheels on multiple platforms? It's something we could consider in the context of #317 as well.

I feel like tox has this use case covered pretty well...?

[YannickJadoul]

I feel like tox has this use case covered pretty well...?

Ah, yes, good point. And testing different platforms is even less commonly needed, I suppose.