From 9cbbea673821a9387196a51000b9cadcac66551f Mon Sep 17 00:00:00 2001 From: Martina Ferrari Date: Thu, 11 Nov 2021 17:54:20 +0000 Subject: [PATCH 1/2] Fixes to test framework and GitHub workflow After many tries, I think I finally manage to find the right combination of tweaks to make pytest, tox, and GitHub actions to work. This also adds caching of the librocksdb compilation, which is a very slow step, and restricts building of wheels and other artifacts to run only when pushing to main. --- .github/workflows/build.yml | 152 ++++++++++++++---------------------- .github/workflows/dist.yml | 102 ++++++++++++++++++++++++ setup.cfg | 2 + tox.ini | 15 +++- 4 files changed, 176 insertions(+), 95 deletions(-) create mode 100644 .github/workflows/dist.yml diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 054b5f5..7389cd3 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -4,31 +4,33 @@ name: 'Build' on: ['push', 'pull_request'] jobs: - test: - name: 'Test' + # Build the RocksDB C library and cache the result. + librocksdb: + name: 'Build librocksdb' runs-on: ${{ matrix.os }} strategy: matrix: os: [ubuntu-latest] - #py_ver: ['3.7', '3.8', '3.9'] - #rocksdb_ver: ['v6.14.6', 'v6.11.4', 'v5.17.2'] - py_ver: ['3.9'] - rocksdb_ver: ['v6.14.6'] + rocksdb_ver: ['v6.14.6', 'v6.11.4'] steps: - - uses: actions/checkout@v2 - name: 'Checkout source repository' - - - uses: actions/setup-python@v2 - name: 'Set up Python ${{ matrix.py_ver }}' + - uses: actions/cache@v2 + id: cache-librocksdb with: - python-version: ${{ matrix.py_ver }} + key: ${{ matrix.os }}-librocksdb-${{ matrix.rocksdb_ver }} + path: /opt/rocksdb + restore-keys: | + librocksdb-${{ matrix.os }}- + librocksdb- - name: 'Install dependencies' - run: | - sudo apt install -y libsnappy-dev libbz2-dev liblz4-dev libz-dev libgflags-dev libzstd-dev + if: steps.cache-librocksdb.outputs.cache-hit != 'true' + run: > + sudo apt install -y libsnappy-dev libbz2-dev liblz4-dev libz-dev + libgflags-dev libzstd-dev - - name: 'Install RocksDB ${{ matrix.rocksdb_ver }}' + - name: 'Clone & build RocksDB ${{ matrix.rocksdb_ver }}' + if: steps.cache-librocksdb.outputs.cache-hit != 'true' run: | pushd /opt && git clone https://github.com/facebook/rocksdb && @@ -36,96 +38,62 @@ jobs: git reset --hard ${{ matrix.rocksdb_ver }} && CXXFLAGS='-flto -Os -s' PORTABLE=1 make shared_lib -j 4 && make shared_lib && - sudo make install-shared && popd - - name: Install python-rocksdb - run: | - sudo python3 setup.py install - - - name: Run tests - run: | - python3 setup.py test - - build_wheels: - name: 'Build wheels' - runs-on: ubuntu-latest + test: + name: 'Build and test python-rocksdb' + needs: librocksdb + runs-on: ${{ matrix.os }} + strategy: + matrix: + os: [ubuntu-latest] + py_ver: ['3.7', '3.8', '3.9'] + rocksdb_ver: ['v6.14.6', 'v6.11.4'] steps: - uses: actions/checkout@v2 name: 'Checkout source repository' - uses: actions/setup-python@v2 - name: 'Set up Python 3.9' - with: - python-version: '3.9' - - - name: 'Install cibuildwheel' - run: | - python3 -m pip install cibuildwheel==1.7.1 - - - name: 'Build wheels' - run: | - python3 -m cibuildwheel --output-dir dist - env: - ROCKSDB_VERSION: 'v6.14.6' - CIBW_MANYLINUX_X86_64_IMAGE: 'manylinux2014' - CIBW_BUILD: 'cp3*' - CIBW_SKIP: '*-manylinux_i686' - CIBW_TEST_REQUIRES: '.[test] pytest' - CIBW_TEST_COMMAND: 'python3 -m pytest {project}/rocksdb/tests' - CIBW_BEFORE_BUILD: | - yum install -y bzip2-devel lz4-devel snappy-devel zlib-devel && - pushd /opt && - git clone https://github.com/facebook/rocksdb && - cd rocksdb && - git reset --hard $ROCKSDB_VERSION && - CXXFLAGS='-flto -Os -s' PORTABLE=1 make shared_lib -j 4 && - make install-shared && - popd - - - uses: actions/upload-artifact@v2 - name: 'Upload build artifacts' + name: 'Set up Python ${{ matrix.py_ver }}' with: - path: 'dist/*.whl' - + python-version: ${{ matrix.py_ver }} - build_sdist: - name: 'Build source distribution' - runs-on: 'ubuntu-latest' - steps: - - uses: actions/checkout@v2 - name: 'Checkout source repository' + - name: 'Install C libraries' + # XXX(Tina): The non-development versions are sufficient, but package + # names are difficult to predict. + run: > + sudo apt install -y libsnappy-dev libbz2-dev liblz4-dev libz-dev + libgflags-dev libzstd-dev - - uses: actions/setup-python@v2 - name: 'Set up Python 3.9' + # Recover the pre-built C library. + - uses: actions/cache@v2 + id: cache-librocksdb with: - python-version: '3.9' + key: ${{ matrix.os }}-librocksdb-${{ matrix.rocksdb_ver }} + path: /opt/rocksdb - - name: 'Build sdist' + - name: 'Install RocksDB ${{ matrix.rocksdb_ver }}' + if: steps.cache-librocksdb.outputs.cache-hit == 'true' + # DO NOT FORGET to call `ldconfig`! run: | - python3 setup.py sdist - - - uses: actions/upload-artifact@v2 - name: 'Upload build artifacts' - with: - path: 'dist/*.tar.gz' + pushd /opt/rocksdb && + sudo make install-shared && + sudo ldconfig && + popd + - name: Build and install python-rocksdb + # Use `pip install` instead of `setup.py` so build-dependencies from + # `pyproject.toml` are installed, in particular `Cython`, without which + # the build fails in confusing ways. + run: | + python3 -m pip install '.[test]' -# upload_pypi: -# name: 'Upload packages' -# needs: ['build_wheels', 'build_sdist'] -# runs-on: 'ubuntu-latest' -# if: github.event_name == 'push' && startsWith(github.event.ref, 'refs/tags/v') -# steps: -# - uses: actions/download-artifact@v2 -# name: 'Download artifacts' -# with: -# name: 'artifact' -# path: 'dist' -# -# - uses: pypa/gh-action-pypi-publish@master -# name: 'Publish built packages' -# with: -# user: '__token__' -# password: '${{ secrets.PYPI_API_TOKEN }}' + - name: Run tests + # Use `--pyargs` to interpret parameter as module to import, not as a + # path, and do not use `python3 -m pytest`. This way we prevent + # importing the module from the current directory instead of the + # installed package, and failing when it cannot find the shared + # library. + run: | + pytest --pyargs rocksdb diff --git a/.github/workflows/dist.yml b/.github/workflows/dist.yml new file mode 100644 index 0000000..7ad9d8e --- /dev/null +++ b/.github/workflows/dist.yml @@ -0,0 +1,102 @@ +# vim:ts=2:sw=2:et:ai:sts=2 +name: 'Build distribution' + +on: + # Only run when pushing to main/merging PRs. + push: + branches: + - main + +jobs: + build_wheels: + name: 'Build wheels' + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v2 + name: 'Checkout source repository' + + - uses: actions/setup-python@v2 + name: 'Set up Python 3.9' + with: + python-version: '3.9' + + - name: 'Install cibuildwheel' + run: | + python3 -m pip install cibuildwheel==1.7.1 + + - name: 'Build wheels' + run: | + python3 -m cibuildwheel --output-dir dist + env: + ROCKSDB_VERSION: 'v6.14.6' + CIBW_MANYLINUX_X86_64_IMAGE: 'manylinux2014' + CIBW_BUILD: 'cp3*' + CIBW_SKIP: '*-manylinux_i686' + # Install python package and test-deps. + CIBW_TEST_REQUIRES: '.[test] pytest' + # Use `--pyargs` to interpret parameter as module to import, not as a + # path, and do not use `python3 -m pytest`. This way we prevent + # importing the module from the current directory instead of the + # installed package, and failing when it cannot find the shared + # library. + CIBW_TEST_COMMAND: 'pytest --pyargs rocksdb' + # Avoid re-building the C library in every iteration by testing for + # the build directory. + CIBW_BEFORE_BUILD: > + yum install -y bzip2-devel lz4-devel snappy-devel zlib-devel + python3-Cython && + pushd /opt && + test -d rocksdb || ( + git clone https://github.com/facebook/rocksdb && + cd rocksdb && + git reset --hard $ROCKSDB_VERSION && + CXXFLAGS='-flto -Os -s' PORTABLE=1 make shared_lib -j 4 && + make install-shared + ) && + popd + + - uses: actions/upload-artifact@v2 + name: 'Upload build artifacts' + with: + path: 'dist/*.whl' + + build_sdist: + name: 'Build source distribution' + runs-on: 'ubuntu-latest' + steps: + - uses: actions/checkout@v2 + name: 'Checkout source repository' + + - uses: actions/setup-python@v2 + name: 'Set up Python 3.9' + with: + python-version: '3.9' + + - name: 'Build sdist' + run: | + python3 setup.py sdist + + - uses: actions/upload-artifact@v2 + name: 'Upload build artifacts' + with: + path: 'dist/*.tar.gz' + + +# upload_pypi: +# name: 'Upload packages' +# needs: ['build_wheels', 'build_sdist'] +# runs-on: 'ubuntu-latest' +# if: github.event_name == 'push' && startsWith(github.event.ref, 'refs/tags/v') +# steps: +# - uses: actions/download-artifact@v2 +# name: 'Download artifacts' +# with: +# name: 'artifact' +# path: 'dist' +# +# - uses: pypa/gh-action-pypi-publish@master +# name: 'Publish built packages' +# with: +# user: '__token__' +# password: '${{ secrets.PYPI_API_TOKEN }}' diff --git a/setup.cfg b/setup.cfg index 154eaa1..3d5afa7 100644 --- a/setup.cfg +++ b/setup.cfg @@ -41,6 +41,8 @@ where = . doc = sphinx sphinx_rtd_theme +test = + pytest [build_sphinx] source-dir = docs diff --git a/tox.ini b/tox.ini index a25ec96..dacd8bf 100644 --- a/tox.ini +++ b/tox.ini @@ -1,15 +1,24 @@ [tox] envlist = py35,py36,py37,py38,py39 minversion = 2.0 +isolated_build = True +skipsdist = True [testenv] -deps = pytest -commands = pytest {posargs:rocksdb/tests} +# Install the module in `.` and its `test` extra dependencies from +# setup.cfg. +deps = + .[test] +changedir = / +# Point directly to the installed package, and do not use `python3 -m pytest`. +# This way we prevent importing the module from the current directory instead +# of the installed package, and failing when it cannot find the shared library. +commands = pytest {envsitepackagesdir}/rocksdb [testenv:docs] deps = .[doc] commands = python3 setup.py build_sphinx -W [pytest] -addopts = --verbose +addopts = --verbose --pyargs norecursedirs = .tox From 2f942de6bca79ab11318e6e3f68ec9ddeb3795cc Mon Sep 17 00:00:00 2001 From: Martina Ferrari Date: Fri, 12 Nov 2021 16:58:34 +0000 Subject: [PATCH 2/2] Rework caching and building librocksdb. Following suggestions from code review, this commit avoid re-using the working directory from different versions of the library. --- .github/workflows/build.yml | 26 +++++++++++++------------- .github/workflows/dist.yml | 24 +++++++++++++++--------- 2 files changed, 28 insertions(+), 22 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 7389cd3..f637027 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -1,13 +1,16 @@ # vim:ts=2:sw=2:et:ai:sts=2 name: 'Build' -on: ['push', 'pull_request'] +on: + push: jobs: # Build the RocksDB C library and cache the result. librocksdb: name: 'Build librocksdb' runs-on: ${{ matrix.os }} + env: + LIBROCKSDB_PATH: /opt/rocksdb-${{ matrix.rocksdb_ver }} strategy: matrix: os: [ubuntu-latest] @@ -18,10 +21,7 @@ jobs: id: cache-librocksdb with: key: ${{ matrix.os }}-librocksdb-${{ matrix.rocksdb_ver }} - path: /opt/rocksdb - restore-keys: | - librocksdb-${{ matrix.os }}- - librocksdb- + path: ${{ env.LIBROCKSDB_PATH }} - name: 'Install dependencies' if: steps.cache-librocksdb.outputs.cache-hit != 'true' @@ -31,19 +31,19 @@ jobs: - name: 'Clone & build RocksDB ${{ matrix.rocksdb_ver }}' if: steps.cache-librocksdb.outputs.cache-hit != 'true' - run: | - pushd /opt && - git clone https://github.com/facebook/rocksdb && - cd rocksdb && - git reset --hard ${{ matrix.rocksdb_ver }} && + run: > + git clone https://github.com/facebook/rocksdb --depth 1 + --branch ${{ matrix.rocksdb_ver }} ${{ env.LIBROCKSDB_PATH }} && + pushd ${{ env.LIBROCKSDB_PATH }} && CXXFLAGS='-flto -Os -s' PORTABLE=1 make shared_lib -j 4 && - make shared_lib && popd test: name: 'Build and test python-rocksdb' needs: librocksdb runs-on: ${{ matrix.os }} + env: + LIBROCKSDB_PATH: /opt/rocksdb-${{ matrix.rocksdb_ver }} strategy: matrix: os: [ubuntu-latest] @@ -71,13 +71,13 @@ jobs: id: cache-librocksdb with: key: ${{ matrix.os }}-librocksdb-${{ matrix.rocksdb_ver }} - path: /opt/rocksdb + path: ${{ env.LIBROCKSDB_PATH }} - name: 'Install RocksDB ${{ matrix.rocksdb_ver }}' if: steps.cache-librocksdb.outputs.cache-hit == 'true' # DO NOT FORGET to call `ldconfig`! run: | - pushd /opt/rocksdb && + pushd ${{ env.LIBROCKSDB_PATH }} && sudo make install-shared && sudo ldconfig && popd diff --git a/.github/workflows/dist.yml b/.github/workflows/dist.yml index 7ad9d8e..0a3cb3f 100644 --- a/.github/workflows/dist.yml +++ b/.github/workflows/dist.yml @@ -10,7 +10,13 @@ on: jobs: build_wheels: name: 'Build wheels' - runs-on: ubuntu-latest + runs-on: ${{ matrix.os }} + env: + LIBROCKSDB_PATH: /opt/rocksdb-${{ matrix.rocksdb_ver }} + strategy: + matrix: + os: [ubuntu-latest] + rocksdb_ver: ['v6.14.6'] steps: - uses: actions/checkout@v2 @@ -29,7 +35,6 @@ jobs: run: | python3 -m cibuildwheel --output-dir dist env: - ROCKSDB_VERSION: 'v6.14.6' CIBW_MANYLINUX_X86_64_IMAGE: 'manylinux2014' CIBW_BUILD: 'cp3*' CIBW_SKIP: '*-manylinux_i686' @@ -46,14 +51,15 @@ jobs: CIBW_BEFORE_BUILD: > yum install -y bzip2-devel lz4-devel snappy-devel zlib-devel python3-Cython && - pushd /opt && - test -d rocksdb || ( - git clone https://github.com/facebook/rocksdb && - cd rocksdb && - git reset --hard $ROCKSDB_VERSION && - CXXFLAGS='-flto -Os -s' PORTABLE=1 make shared_lib -j 4 && - make install-shared + test -d ${{ env.LIBROCKSDB_PATH }} || ( + git clone https://github.com/facebook/rocksdb --depth 1 + --branch ${{ matrix.rocksdb_ver }} ${{ env.LIBROCKSDB_PATH }} && + cd ${{ env.LIBROCKSDB_PATH }} && + CXXFLAGS='-flto -Os -s' PORTABLE=1 make shared_lib -j 4 ) && + pushd ${{ env.LIBROCKSDB_PATH }} && + make install-shared && + ldconfig && popd - uses: actions/upload-artifact@v2