plasma-umass / slipcover

Near Zero-Overhead Python Code Coverage
Apache License 2.0
475 stars 17 forks source link

Add manylinux build to test workflow #33

Closed asford closed 1 year ago

asford commented 1 year ago

Add manylinux build config to test workflow to assist in debugging https://github.com/plasma-umass/slipcover/pull/31

asford commented 1 year ago

@jaltmayerpizzorno This pass validates the manylinux build configuration as part of the test workflow, which is currently failing with the same error you're seeing in #31.

alexford@AB-00RQ8221400C:~/asford/slipcover$ git show
commit a3c5debb560feae747c4226ada368d58cc0ea2c0 (HEAD -> add_manylinux_build_test, origin/add_manylinux_build_test, main)
Author: Alex Ford <alex.ford@abcellera.com>
Date:   Tue Nov 8 12:51:50 2022 -0800

    Add manylinux build to test workflow

diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
index 132f60d..3b81cc7 100644
--- a/.github/workflows/tests.yml
+++ b/.github/workflows/tests.yml
@@ -35,3 +35,32 @@ jobs:

     - name: run tests
       run: python3 -m pytest
+
+  # Run a manylinux wheel build to verify build configuration
+  test-build-wheel-manylinux:
+    runs-on: ubuntu-latest
+    container: ${{ matrix.container }}
+    strategy:
+      matrix:
+        python_version: ['3.8', '3.9', '3.10']
+        include:
+          - os: ubuntu-latest
+            container: quay.io/pypa/manylinux_2_24_x86_64  # https://github.com/pypa/manylinux
+
+    steps:
+      - uses: actions/checkout@v2
+      - name: set up python
+        run: |
+          PYV=`echo "${{ matrix.python_version }}" | tr -d "."`; ls -d -1 /opt/python/cp$PYV*/bin | head -n 1 >> $GITHUB_PATH
+          cat $GITHUB_PATH
+      - name: install dependencies
+        run: |
+          python3 -m pip install setuptools wheel twine
+      - name: build wheel
+        run: |
+            python3 setup.py bdist_wheel
+      - name: run auditwheel for manylinux
+        run: |
+          auditwheel repair dist/*.whl
+          rm -f dist/*.whl
+          mv wheelhouse/*.whl dist/
alexford@AB-00RQ8221400C:~/asford/slipcover$ act workflow_dispatch -j test-build-wheel-manylinux
[tests/test-build-wheel-manylinux-1] 🚧  Skipping unsupported platform -- Try running with `-P ubuntu-latest=...`
[tests/test-build-wheel-manylinux-2] 🚧  Skipping unsupported platform -- Try running with `-P ubuntu-latest=...`
[tests/test-build-wheel-manylinux-3] 🚧  Skipping unsupported platform -- Try running with `-P ubuntu-latest=...`
[tests/test-build-wheel-manylinux-4] πŸš€  Start image=quay.io/pypa/manylinux_2_24_x86_64
[tests/test-build-wheel-manylinux-4]   🐳  docker pull image=quay.io/pypa/manylinux_2_24_x86_64 platform= username= forcePull=false
[tests/test-build-wheel-manylinux-4]   🐳  docker create image=quay.io/pypa/manylinux_2_24_x86_64 platform= entrypoint=["/usr/bin/tail" "-f" "/dev/null"] cmd=[]
[tests/test-build-wheel-manylinux-4]   🐳  docker run image=quay.io/pypa/manylinux_2_24_x86_64 platform= entrypoint=["/usr/bin/tail" "-f" "/dev/null"] cmd=[]
[tests/test-build-wheel-manylinux-4] πŸ§ͺ  Matrix: map[container:quay.io/pypa/manylinux_2_24_x86_64 os:ubuntu-latest]
[tests/test-build-wheel-manylinux-4] ⭐ Run Main actions/checkout@v2
[tests/test-build-wheel-manylinux-4]   🐳  docker cp src=/home/alexford/asford/slipcover/. dst=/home/alexford/asford/slipcover
[tests/test-build-wheel-manylinux-4]   βœ…  Success - Main actions/checkout@v2
[tests/test-build-wheel-manylinux-4] ⭐ Run Main set up python
[tests/test-build-wheel-manylinux-4]   🐳  docker exec cmd=[sh -e -c /var/run/act/workflow/1.sh] user= workdir=
| /opt/python/cp310-cp310/bin
[tests/test-build-wheel-manylinux-4]   βœ…  Success - Main set up python
[tests/test-build-wheel-manylinux-4] ⭐ Run Main install dependencies
[tests/test-build-wheel-manylinux-4]   🐳  docker exec cmd=[sh -e -c /var/run/act/workflow/2.sh] user= workdir=
| Requirement already satisfied: setuptools in /opt/_internal/cpython-3.10.8/lib/python3.10/site-packages (65.5.1)
| Requirement already satisfied: wheel in /opt/_internal/cpython-3.10.8/lib/python3.10/site-packages (0.38.2)
| Collecting twine
|   Downloading twine-4.0.1-py3-none-any.whl (36 kB)
| Collecting importlib-metadata>=3.6
|   Downloading importlib_metadata-5.0.0-py3-none-any.whl (21 kB)
| Collecting rich>=12.0.0
|   Downloading rich-12.6.0-py3-none-any.whl (237 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 237.5/237.5 kB 7.4 MB/s eta 0:00:00
| Collecting pkginfo>=1.8.1
|   Downloading pkginfo-1.8.3-py2.py3-none-any.whl (26 kB)
| Collecting requests>=2.20
|   Downloading requests-2.28.1-py3-none-any.whl (62 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 62.8/62.8 kB 10.5 MB/s eta 0:00:00
| Collecting rfc3986>=1.4.0
|   Downloading rfc3986-2.0.0-py2.py3-none-any.whl (31 kB)
| Collecting urllib3>=1.26.0
|   Downloading urllib3-1.26.12-py2.py3-none-any.whl (140 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 140.4/140.4 kB 13.7 MB/s eta 0:00:00
| Collecting requests-toolbelt!=0.9.0,>=0.8.0
|   Downloading requests_toolbelt-0.10.1-py2.py3-none-any.whl (54 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 54.5/54.5 kB 12.6 MB/s eta 0:00:00
| Collecting keyring>=15.1
|   Downloading keyring-23.11.0-py3-none-any.whl (36 kB)
| Collecting readme-renderer>=35.0
|   Downloading readme_renderer-37.3-py3-none-any.whl (14 kB)
| Collecting zipp>=0.5
|   Downloading zipp-3.10.0-py3-none-any.whl (6.2 kB)
| Collecting SecretStorage>=3.2
|   Downloading SecretStorage-3.3.3-py3-none-any.whl (15 kB)
| Collecting jaraco.classes
|   Downloading jaraco.classes-3.2.3-py3-none-any.whl (6.0 kB)
| Collecting jeepney>=0.4.2
|   Downloading jeepney-0.8.0-py3-none-any.whl (48 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 48.4/48.4 kB 10.6 MB/s eta 0:00:00
| Collecting bleach>=2.1.0
|   Downloading bleach-5.0.1-py3-none-any.whl (160 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 160.9/160.9 kB 16.7 MB/s eta 0:00:00
| Collecting Pygments>=2.5.1
|   Downloading Pygments-2.13.0-py3-none-any.whl (1.1 MB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 1.1/1.1 MB 4.7 MB/s eta 0:00:00
| Collecting docutils>=0.13.1
|   Downloading docutils-0.19-py3-none-any.whl (570 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 570.5/570.5 kB 18.0 MB/s eta 0:00:00
| Collecting charset-normalizer<3,>=2
|   Downloading charset_normalizer-2.1.1-py3-none-any.whl (39 kB)
| Collecting certifi>=2017.4.17
|   Downloading certifi-2022.9.24-py3-none-any.whl (161 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 161.1/161.1 kB 22.5 MB/s eta 0:00:00
| Collecting idna<4,>=2.5
|   Downloading idna-3.4-py3-none-any.whl (61 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 61.5/61.5 kB 15.6 MB/s eta 0:00:00
| Collecting commonmark<0.10.0,>=0.9.0
|   Downloading commonmark-0.9.1-py2.py3-none-any.whl (51 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 51.1/51.1 kB 6.1 MB/s eta 0:00:00
| Collecting six>=1.9.0
|   Downloading six-1.16.0-py2.py3-none-any.whl (11 kB)
| Collecting webencodings
|   Downloading webencodings-0.5.1-py2.py3-none-any.whl (11 kB)
| Collecting cryptography>=2.0
|   Downloading cryptography-38.0.3-cp36-abi3-manylinux_2_24_x86_64.whl (4.1 MB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 4.1/4.1 MB 12.5 MB/s eta 0:00:00
| Collecting more-itertools
|   Downloading more_itertools-9.0.0-py3-none-any.whl (52 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 52.8/52.8 kB 11.0 MB/s eta 0:00:00
| Collecting cffi>=1.12
|   Downloading cffi-1.15.1-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (441 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 441.8/441.8 kB 12.0 MB/s eta 0:00:00
| Collecting pycparser
|   Downloading pycparser-2.21-py2.py3-none-any.whl (118 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 118.7/118.7 kB 7.9 MB/s eta 0:00:00
| Installing collected packages: webencodings, commonmark, zipp, urllib3, six, rfc3986, Pygments, pycparser, pkginfo, more-itertools, jeepney, idna, docutils, charset-normalizer, certifi, rich, requests, jaraco.classes, importlib-metadata, cffi, bleach, requests-toolbelt, readme-renderer, cryptography, SecretStorage, keyring, twine
| Successfully installed Pygments-2.13.0 SecretStorage-3.3.3 bleach-5.0.1 certifi-2022.9.24 cffi-1.15.1 charset-normalizer-2.1.1 commonmark-0.9.1 cryptography-38.0.3 docutils-0.19 idna-3.4 importlib-metadata-5.0.0 jaraco.classes-3.2.3 jeepney-0.8.0 keyring-23.11.0 more-itertools-9.0.0 pkginfo-1.8.3 pycparser-2.21 readme-renderer-37.3 requests-2.28.1 requests-toolbelt-0.10.1 rfc3986-2.0.0 rich-12.6.0 six-1.16.0 twine-4.0.1 urllib3-1.26.12 webencodings-0.5.1 zipp-3.10.0
| WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv
[tests/test-build-wheel-manylinux-4]   βœ…  Success - Main install dependencies
[tests/test-build-wheel-manylinux-4] ⭐ Run Main build wheel
[tests/test-build-wheel-manylinux-4]   🐳  docker exec cmd=[sh -e -c /var/run/act/workflow/3.sh] user= workdir=
| running bdist_wheel
| running build
| running build_py
| creating build
| creating build/lib.linux-x86_64-cpython-310
| creating build/lib.linux-x86_64-cpython-310/slipcover
| copying slipcover/__main__.py -> build/lib.linux-x86_64-cpython-310/slipcover
| copying slipcover/bytecode.py -> build/lib.linux-x86_64-cpython-310/slipcover
| copying slipcover/slipcover.py -> build/lib.linux-x86_64-cpython-310/slipcover
| copying slipcover/branch.py -> build/lib.linux-x86_64-cpython-310/slipcover
| copying slipcover/__init__.py -> build/lib.linux-x86_64-cpython-310/slipcover
| running build_ext
| building 'slipcover.probe' extension
| creating build/temp.linux-x86_64-cpython-310
| g++ -pthread -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -I/opt/_internal/cpython-3.10.8/include/python3.10 -c probe.cxx -o build/temp.linux-x86_64-cpython-310/probe.o -std=c++17
| probe.cxx:21:10: error: β€˜byte’ in namespace β€˜std’ does not name a type
|      std::byte* _code;
|           ^~~~
| probe.cxx: In constructor β€˜Probe::Probe(PyObject*, PyObject*, PyObject*, PyObject*)’:
| probe.cxx:29:61: error: class β€˜Probe’ does not have any field named β€˜_code’
|          _d_miss_threshold(PyLong_AsLong(d_miss_threshold)), _code(nullptr) {}
|                                                              ^~~~~
| probe.cxx: In member function β€˜PyObject* Probe::signal()’:
| probe.cxx:45:29: error: β€˜_code’ was not declared in this scope
|          if (!_signalled || (_code == nullptr && _d_miss_threshold < -1)) {
|                              ^~~~~
| probe.cxx:69:17: error: β€˜_code’ was not declared in this scope
|              if (_code) {    // immediate de-instrumentation
|                  ^~~~~
| probe.cxx:70:43: error: β€˜byte’ in namespace β€˜std’ does not name a type
|                  *_code = static_cast<std::byte>(JUMP_FORWARD);
|                                            ^~~~
| probe.cxx: In member function β€˜PyObject* Probe::set_immediate(PyObject*, PyObject*)’:
| probe.cxx:117:9: error: β€˜_code’ was not declared in this scope
|          _code = reinterpret_cast<std::byte*>(PyBytes_AsString(code_bytes));
|          ^~~~~
| probe.cxx:117:39: error: β€˜byte’ in namespace β€˜std’ does not name a type
|          _code = reinterpret_cast<std::byte*>(PyBytes_AsString(code_bytes));
|                                        ^~~~
| probe.cxx:117:43: error: expected β€˜>’ before β€˜*’ token
|          _code = reinterpret_cast<std::byte*>(PyBytes_AsString(code_bytes));
|                                            ^
| probe.cxx:117:43: error: expected β€˜(’ before β€˜*’ token
| probe.cxx:117:44: error: expected primary-expression before β€˜>’ token
|          _code = reinterpret_cast<std::byte*>(PyBytes_AsString(code_bytes));
|                                             ^
| probe.cxx:117:75: error: expected β€˜)’ before β€˜;’ token
|          _code = reinterpret_cast<std::byte*>(PyBytes_AsString(code_bytes));
|                                                                            ^
| error: command '/usr/bin/g++' failed with exit code 1
[tests/test-build-wheel-manylinux-4]   ❌  Failure - Main build wheel
[tests/test-build-wheel-manylinux-4] exitcode '1': failure
[tests/test-build-wheel-manylinux-4] 🏁  Job failed
Error: Job 'test-build-wheel-manylinux' failed
asford commented 1 year ago

There's something wrong with the manylinux build image you're targeting, it has a really old version of gcc which doesn't support c++17.

This doesn't match the upstream manylinix spec and is very like a container bug. https://github.com/pypa/manylinux

Must have broken recently, because your old builds worked?

alexford@AB-00RQ8221400C:~/asford/slipcover$ docker run -it quay.io/pypa/manylinux2014_x86_64 gcc --version
gcc (GCC) 10.2.1 20210130 (Red Hat 10.2.1-11)
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

alexford@AB-00RQ8221400C:~/asford/slipcover$ docker run -it quay.io/pypa/manylinux_2_24_x86_64 gcc --version
gcc (Debian 6.3.0-18+deb9u1) 6.3.0 20170516
Copyright (C) 2016 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
asford commented 1 year ago

Not %100 sure on downstream implications of manylinux2014, as I live in the conda-verse, but switching to manylinux 2014 fixes the issue by reintroducing c++17 support.

I'd recommend looking at cibuildwheel, which defaults to manylinux 2014.

jaltmayerpizzorno commented 1 year ago

Not %100 sure on downstream implications of manylinux2014, as I live in the conda-verse, but switching to manylinux 2014 fixes the issue by reintroducing c++17 support.

C++17 support isn't entirely missing in GCC 6, which is what manylinux_2_24 has, but I don't use a lot of it. Anyway, it's certainly way better in the newer GCCs.

I'd recommend looking at cibuildwheel, which defaults to manylinux 2014.

Yea, I look into that while setting this up, but it seemed overkill/complicated.

It's a good idea to have the build be part of the tests... I didn't do it before because it takes so long to run (our build & upload action, when started manually, uploads to testpypi). I'll probably want to move to manylinux_2_28 eventually, for compatibility, but since auditwheel is downgrading the requirements anyway, this looks good to me.

@asford would you consider contributing a workflow that tests building for conda, so that this isn't missed again?

asford commented 1 year ago

Certainly can, but the β€œfull” conda build is also quite slow to setup and run.

I’m more than happy to continue maintaining the conda-forge recipe for this once it merges, and will push a build test here for that.