Skip to content

Commit

Permalink
refactor(py): remove Cython header and bindings using it
Browse files Browse the repository at this point in the history
This removes the xNVMe Cython bindings and header. The intent is to
provide a single set of Python bindings, and that these are based on a
minimal toolchain requirements, that is, the built-in ctypes FFI.

The "xnvme-core" is moved into "bindings".

* add clang2py include-path for macos

The tests implemented using the Cython bindings are "ported" to use the
ctypes-bindings instead, and placed in "tests".

Changes to "tests" during porting:

* Change definition of NULL to None
  - This was defined using a class, however, ctypes represent and treat
    the Python None type as NULL, thus, no need to wrap it, however, for
    readability NULL might be preferred, thus keeping it as an alias for
    None
* Remove duplicate definitions of NULL and UINT16_MAX
  - These now only reside in tests/utils.py
* conftest
  - Rename variables retrieved via environment variables
  - Assert that environment variables are defined, inform when they are
    not
  - Embed 'opts' fixture inside 'dev' fixture
  - Use xnvme_opts_set_defaults() instead of xnvme_opts_default()

Signed-off-by: Simon A. F. Lund <simon.lund@samsung.com>
  • Loading branch information
safl committed May 17, 2023
1 parent bd78d2d commit 5b7ed96
Show file tree
Hide file tree
Showing 52 changed files with 113 additions and 2,309 deletions.
123 changes: 16 additions & 107 deletions .github/workflows/verify.yml
Original file line number Diff line number Diff line change
Expand Up @@ -336,33 +336,22 @@ jobs:
run: |
apt-get install -qy libclang-13-dev
- name: Python sdist-packages, build(xnvme-core, xnvme-cy-header, and xnvme-cy-bindings)
- name: Python sdist-packages, build bindings
run: |
pushd python/xnvme-core
pushd python/bindings
make clean build
popd
pushd python/xnvme-cy-header
make clean build
popd
pushd python/xnvme-cy-bindings
# Explicitly avoid bdist build
make clean .build-py-env build-sdist
popd
- name: Python sdist-packages, find(...)
run: |
find python/ -name *tar.gz
- name: Python sdist-packages, upload(xnvme-core, xnvme-cy-header, xnvme-cy-bindings)
- name: Python sdist-packages, upload bindings
uses: actions/upload-artifact@v3.1.1
with:
name: python-xnvme-pkg
path: |
python/xnvme-core/dist/xnvme-core-*.tar.gz
python/xnvme-cy-bindings/dist/xnvme-cy-bindings-*.tar.gz
python/xnvme-cy-header/dist/xnvme-cy-header-*.tar.gz
python/bindings/dist/xnvme-core-*.tar.gz
if-no-files-found: error

#
Expand Down Expand Up @@ -483,53 +472,15 @@ jobs:
run: |
cijoe -r
- name: Python-bindings (ctypes), install and test
if: (!contains('centos7 opensuse buster focal 15.4 15.3 gentoo stream8', matrix.container.ver))
run: |
python3 -m pipx inject cijoe xnvme-core/dist/xnvme-core-*.tar.gz
pytest --pyargs xnvme.ctypes_bindings
- name: Cython-header, inject xnvme-cy-header (libxnvme.pxd) and reqs. in pipx-venv
if: (!contains('centos7 opensuse buster focal 15.4 15.3 gentoo stream8', matrix.container.ver))
run: |
python3 -m pipx inject cijoe cython
python3 -m pipx inject cijoe "pytest-cython-collect>=0.2"
python3 -m pipx inject cijoe "setuptools>=40.1"
python3 -m pipx inject cijoe xnvme-cy-header/dist/xnvme-cy-header-*.tar.gz
- name: Cython-header, install xnvme-core and xnvme-cy-header (libxnvme.pxd) system-wide
if: (!contains('centos7 opensuse buster focal 15.4 15.3 gentoo stream8', matrix.container.ver))
run: |
python3 -m pip install xnvme-core/dist/xnvme-core-*.tar.gz || python3 -m pip install xnvme-core/dist/xnvme-core-*.tar.gz --break-system-packages
python3 -m pip install xnvme-cy-header/dist/xnvme-cy-header-*.tar.gz --user || python3 -m pip install xnvme-cy-header/dist/xnvme-cy-header-*.tar.gz --user --break-system-packages
# This also requires installing 'xnvme/cython_header/tests' from the tarball/repository
# The tests that are run here only tests "dummy", which means without an NVMe device.
#
# A PKG_CONFIG_PATH is set, as it is needed on Arch, CentOS 7/8 and Gentoo
# Didn't this get fixes previously? Why is this a problem again?
- name: Cython, install xnvme-cy-header test-requirements system-wide
if: (!contains('centos7 opensuse buster focal 15.4 15.3 gentoo stream8', matrix.container.ver))
- name: Foo
run: |
cd python/xnvme-cy-header/xnvme/cython_header/tests/
python3 -m pip install -r requirements.txt --user || python3 -m pip install -r requirements.txt --user --break-system-packages
PKG_CONFIG_PATH="/usr/local/lib64/pkgconfig:/usr/local/lib/pkgconfig:/usr/local/lib/x86_64-linux-gnu/pkgconfig" python3 setup.py build_ext --inplace
pytest --cython-collect test_cython.pyx::test_dummy -v -s
find . -name "*.tar.gz"
# Why is this needed: "python/xnvme-cy-bindings/requirements.txt --user" ?
# Should't build-requirements take care of this?
#
# --no-build-isolation, normally packages a build in a venv without access to other installed
# packages, in to build the xnvme-cy-bindings the 'xnvme-core' and 'xnvme-cy-header' packages
# are needed.
# Perhaps no longer the 'xnvme-core'?
- name: Python-Bindings (Cython), Cython-implementation, Install and test xNVMe Python bindings (Cython)
- name: Python-bindings (ctypes), install and test
if: (!contains('centos7 opensuse buster focal 15.4 15.3 gentoo stream8', matrix.container.ver))
run: |
python3 -m pip install -r python/xnvme-cy-bindings/requirements.txt --user || python3 -m pip install -r python/xnvme-cy-bindings/requirements.txt --user --break-system-packages
python3 -m pip install xnvme-cy-bindings/dist/xnvme-cy-bindings-*.tar.gz --user -vvv --no-build-isolation || python3 -m pip install xnvme-cy-bindings/dist/xnvme-cy-bindings-*.tar.gz --user -vvv --no-build-isolation --break-system-packages
python3 -m pip install -r python/xnvme-cy-bindings/xnvme/cython_bindings/tests/requirements.txt --user || python3 -m pip install -r python/xnvme-cy-bindings/xnvme/cython_bindings/tests/requirements.txt --user --break-system-packages
python3 -m pytest --pyargs xnvme.cython_bindings -k 'test_version' -s
python3 -m pipx inject cijoe ./xnvme-core-*.tar.gz
pytest --pyargs xnvme.ctypes_bindings
#
# Ramdisk testing
Expand Down Expand Up @@ -645,53 +596,15 @@ jobs:
run: |
cijoe -r
- name: Python-bindings (ctypes), install and test
if: (!contains('centos7 opensuse buster focal 15.4 15.3 gentoo stream8', matrix.container.ver))
run: |
python3 -m pipx inject cijoe xnvme-core/dist/xnvme-core-*.tar.gz
pytest --pyargs xnvme.ctypes_bindings
- name: Cython-header, inject xnvme-cy-header (libxnvme.pxd) and reqs. in pipx-venv
if: (!contains('centos7 opensuse buster focal 15.4 15.3 gentoo stream8', matrix.container.ver))
- name: Foo
run: |
python3 -m pipx inject cijoe cython
python3 -m pipx inject cijoe "pytest-cython-collect>=0.2"
python3 -m pipx inject cijoe "setuptools>=40.1"
python3 -m pipx inject cijoe xnvme-cy-header/dist/xnvme-cy-header-*.tar.gz
- name: Cython-header, install xnvme-core and xnvme-cy-header (libxnvme.pxd) system-wide
if: (!contains('centos7 opensuse buster focal 15.4 15.3 gentoo stream8', matrix.container.ver))
run: |
python3 -m pip install xnvme-core/dist/xnvme-core-*.tar.gz || python3 -m pip install xnvme-core/dist/xnvme-core-*.tar.gz --break-system-packages
python3 -m pip install xnvme-cy-header/dist/xnvme-cy-header-*.tar.gz --user || python3 -m pip install xnvme-cy-header/dist/xnvme-cy-header-*.tar.gz --user --break-system-packages
# This also requires installing 'xnvme/cython_header/tests' from the tarball/repository
# The tests that are run here only tests "dummy", which means without an NVMe device.
#
# A PKG_CONFIG_PATH is set, as it is needed on Arch, CentOS 7/8 and Gentoo
# Didn't this get fixes previously? Why is this a problem again?
- name: Cython, install xnvme-cy-header test-requirements system-wide
if: (!contains('centos7 opensuse buster focal 15.4 15.3 gentoo stream8', matrix.container.ver))
run: |
cd python/xnvme-cy-header/xnvme/cython_header/tests/
python3 -m pip install -r requirements.txt --user || python3 -m pip install -r requirements.txt --user --break-system-packages
PKG_CONFIG_PATH="/usr/local/lib64/pkgconfig:/usr/local/lib/pkgconfig:/usr/local/lib/x86_64-linux-gnu/pkgconfig" python3 setup.py build_ext --inplace
pytest --cython-collect test_cython.pyx::test_dummy -v -s
find . -name "*.tar.gz"
# Why is this needed: "python/xnvme-cy-bindings/requirements.txt --user" ?
# Should't build-requirements take care of this?
#
# --no-build-isolation, normally packages a build in a venv without access to other installed
# packages, in to build the xnvme-cy-bindings the 'xnvme-core' and 'xnvme-cy-header' packages
# are needed.
# Perhaps no longer the 'xnvme-core'?
- name: Python-Bindings (Cython), Cython-implementation, Install and test xNVMe Python bindings (Cython)
- name: Python-bindings (ctypes), install and test
if: (!contains('centos7 opensuse buster focal 15.4 15.3 gentoo stream8', matrix.container.ver))
run: |
python3 -m pip install -r python/xnvme-cy-bindings/requirements.txt --user || python3 -m pip install -r python/xnvme-cy-bindings/requirements.txt --user --break-system-packages
python3 -m pip install xnvme-cy-bindings/dist/xnvme-cy-bindings-*.tar.gz --user -vvv --no-build-isolation || python3 -m pip install xnvme-cy-bindings/dist/xnvme-cy-bindings-*.tar.gz --user -vvv --no-build-isolation --break-system-packages
python3 -m pip install -r python/xnvme-cy-bindings/xnvme/cython_bindings/tests/requirements.txt --user || python3 -m pip install -r python/xnvme-cy-bindings/xnvme/cython_bindings/tests/requirements.txt --user --break-system-packages
python3 -m pytest --pyargs xnvme.cython_bindings -k 'test_version' -s
python3 -m pipx inject cijoe ./xnvme-core-*.tar.gz
pytest --pyargs xnvme.ctypes_bindings
#
# Build on Windows
Expand Down Expand Up @@ -809,9 +722,7 @@ jobs:
find /tmp -name "*.tar.gz"
rm -r /tmp/artifacts || true
mkdir /tmp/artifacts
mv xnvme-core/dist/xnvme-core-*.tar.gz /tmp/artifacts/xnvme-core.tar.gz
mv xnvme-cy-header/dist/xnvme-cy-header-*.tar.gz /tmp/artifacts/xnvme-cy-header.tar.gz
mv xnvme-cy-bindings/dist/xnvme-cy-bindings-*.tar.gz /tmp/artifacts/xnvme-cy-bindings.tar.gz
mv xnvme-core-*.tar.gz /tmp/artifacts/xnvme-core.tar.gz
mv xnvme-*.*.*.tar.gz /tmp/artifacts/xnvme.tar.gz
find /tmp/artifacts -name "*.tar.gz"
Expand Down Expand Up @@ -923,9 +834,7 @@ jobs:
find /tmp -name "*.tar.gz"
rm -r /tmp/artifacts || true
mkdir /tmp/artifacts
mv xnvme-core/dist/xnvme-core-*.tar.gz /tmp/artifacts/xnvme-core.tar.gz
mv xnvme-cy-header/dist/xnvme-cy-header-*.tar.gz /tmp/artifacts/xnvme-cy-header.tar.gz
mv xnvme-cy-bindings/dist/xnvme-cy-bindings-*.tar.gz /tmp/artifacts/xnvme-cy-bindings.tar.gz
mv xnvme-core-*.tar.gz /tmp/artifacts/xnvme-core.tar.gz
mv xnvme-*.*.*.tar.gz /tmp/artifacts/xnvme.tar.gz
find /tmp/artifacts -name "*.tar.gz"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,53 +1,51 @@
import xnvme.cython_bindings as xnvme
import pytest

import xnvme

def test_version():
from ..conftest import xnvme_parametrize


def test_version(cijoe):
# Very simple test that checks the bindings without an NVMe disk
assert xnvme.xnvme_ver_major() == 0, "Has major version been updated?"
assert xnvme.xnvme_ver_minor() >= 3
assert xnvme.xnvme_ver_patch() >= 0


def test_libconf():
fd = xnvme.FILE()
fd.tmpfile()
xnvme.xnvme_libconf_fpr(fd, xnvme.XNVME_PR_DEF)
data = fd.getvalue()
print(data.decode())
assert len(data) != 0, "libconf file-stream is empty!"
def test_xnvme_ver_fpr(cijoe):
count = xnvme.xnvme_ver_pr(xnvme.XNVME_PR_DEF)
assert count, "version file-stream is empty!"


def test_xnvme_ver_fpr():
fd = xnvme.FILE()
fd.tmpfile()
xnvme.xnvme_ver_fpr(fd, xnvme.XNVME_PR_DEF)
data = fd.getvalue()
print(data.decode())
assert len(data) != 0, "version file-stream is empty!"
def test_libconf(cijoe):
count = xnvme.xnvme_libconf_pr(xnvme.XNVME_PR_DEF)

assert count, "libconf file-stream is empty!"

def test_dev(dev):

def test_dev(cijoe, dev):
assert dev, xnvme.xnvme_dev_pr(dev, xnvme.XNVME_PR_DEF)


def test_ident(dev):
def test_ident(cijoe, dev):
ident = xnvme.xnvme_dev_get_ident(dev)

assert ident.nsid == 1
assert ident.to_dict()


def test_pointers(dev):
def test_pointers(cijoe, dev):
# A random xNVMe object which is easy to create
ctx = xnvme.xnvme_cmd_ctx_from_dev(dev)

# Verify that we are pointing to the struct cmd embedded at the origin of ctx (they will share address)
assert ctx.void_pointer == ctx.cmd.void_pointer
assert ctx == ctx.cmd

# Verify that we are pointing to the struct cpl embedded at the origin of ctx + the size of cmd
assert ctx.void_pointer + ctx.cmd.sizeof == ctx.cpl.void_pointer
assert ctx + ctx.cmd.sizeof == ctx.cpl

# Check pointers are well defined and stable -- i.e. not generated at each call
assert ctx.dev.void_pointer == ctx.dev.void_pointer
assert ctx.dev == ctx.dev

# Check that a pointer from two places to the same struct shows the same address
assert ctx.dev.void_pointer == xnvme.xnvme_dev_get_geo(ctx.dev).void_pointer
assert ctx.dev == xnvme.xnvme_dev_get_geo(ctx.dev)
59 changes: 0 additions & 59 deletions cijoe/tests/bindings/test_cython.py

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
"""
This test the cli-tool-examples implemented in Python:
* xpy_enumerate
* xpy_libconf
* xpy_dev_open
"""
import pytest

from ..conftest import xnvme_parametrize
Expand Down
Loading

0 comments on commit 5b7ed96

Please sign in to comment.