Never purposefully fail to import crc32c

This behaviour was still lingering from the old 1.x series, so let's
remove it. Instead of failing at import time, we can still choose to
fail at runtime, which is a better design, since it makes importing the
module much less cumbersome. And while it's a backwards-incompatible
change, nobody will miss it (and if anyone does, they can easily
replicate it on their end).

The biggest repercussion this change had was in our own tests, which
relied on the import working or not to skip tests conditionally. This
logic has now been updated, and the supporting code has been redesigned
as fixtures and markers, more in line with pytest's (and therefore, many
users and readers) expectations.

Signed-off-by: Rodrigo Tobar <rtobar@icrar.org>

CHANGELOG.md

@@ -5,6 +5,12 @@
 * Added a `gil_relese_mode` parameter to the `CRC32CHash` constructor
   used by all underlying `crc32c` function calls (#51).
 * Added `CRC32CHash.checksum` auxiliary property.
+* The ``crc32c`` module doesn't fail to import when ``CRC32C_SW_MODE`` is ``none``
+  and no hardware acceleration is found,
+  making the act of importing the module less ackward.
+  Now a ``RuntimeWarning`` is issued at import time instead,
+  and a``RuntimeError`` is raised any time
+  a checksum calculation is attempted.
 * Skip hardware probing if the `CRC32C_SKIP_HW_PROBE` environment variable
   is set to `1`.
 

pytest.ini

@@ -0,0 +1,3 @@
+[pytest]
+markers =
+  calculates_crc32c: Mark a test as needing crc32c working

src/crc32c/ext/_crc32c.c

@@ -58,6 +58,14 @@ PyObject* crc32c_crc32c(PyObject *self, PyObject *args, PyObject *kwargs) {
 	/* In python 3 we accept only bytes-like objects */
 	const char *format ="y*|Ii:crc32";
 
+	if (!crc_fn) {
+		PyErr_SetString(
+		    PyExc_RuntimeError,
+		    "crc32c: software mode disabled and no hardware acceleration found, can't calculate checksum"
+		);
+		return NULL;
+	}
+
 	if (!PyArg_ParseTupleAndKeywords(args, kwargs, format, kwlist, &pbin, &crc, &gil_release_mode) )
 		return NULL;
 
@@ -128,15 +136,16 @@ static PyMethodDef CRC32CMethods[] = {
 	{NULL, NULL, 0, NULL}        /* Sentinel */
 };
 
-static const char *import_error_msg = "\n\n"
+static const char *no_hw_or_sw_error_msg = "\n\n"
 "Hardware extensions providing a crc32c hardware instruction are not available in\n"
 "your processor. This package comes with a software implementation, but this\n"
 "support has been opted out because the CRC32C_SW_MODE environment variable is\n"
-"set to \"none\". CRC32C_SW_MODE can take one of the following values:\n"
+"set to \"none\", and therefore any checksum calculation will result in a\n"
+"RuntimeError. CRC32C_SW_MODE can take one of the following values:\n"
 " * If unset: use the software implementation if no hardware support is found\n"
 " * 'auto': as above, but will eventually be discontinued\n"
 " * 'force': use software implementation regardless of hardware support.\n"
-" * 'none': fail if no hardware support is found (this error).\n";
+" * 'none': fail if no hardware support is found.\n";
 
 static struct PyModuleDef moduledef = {PyModuleDef_HEAD_INIT, "_crc32c", "crc32c implementation in hardware and software", -1, CRC32CMethods};
 
@@ -176,8 +185,12 @@ PyMODINIT_FUNC PyInit__crc32c(void)
 		hardware_based = Py_False;
 	}
 	else if (sw_mode == NONE) {
-		PyErr_SetString(PyExc_ImportError, import_error_msg);
-		return NULL;
+		if (PyErr_WarnEx(PyExc_RuntimeWarning,
+		                 no_hw_or_sw_error_msg,
+		                 1) == -1) {
+			return NULL;
+		}
+		hardware_based = Py_False;
 	}
 
 	is_big_endian = (*(const char *)(&n) == 0);

test/conftest.py

@@ -24,20 +24,38 @@
 
 import pytest
 
-try:
-    import crc32c
-except:
-    crc32c = None  # type: ignore[assignment]
+import crc32c
 
 
-def pytest_sessionstart(session: pytest.Session) -> None:
-    if not crc32c:
-        print("crc32c module not imported, can't show general diagnostics")
-        return
+def _crc32c_is_available() -> bool:
+    try:
+        crc32c.crc32c(b"dummy")
+        return True
+    except RuntimeError:
+        return False
+
+
+@pytest.fixture
+def crc32c_is_available() -> bool:
+    return _crc32c_is_available()
+
 
+@pytest.fixture(autouse=True)
+def skip_if_crc32c_unavailable(
+    request: pytest.FixtureRequest, crc32c_is_available: bool
+) -> None:
+    if request.node.get_closest_marker("calculates_crc32c") and not crc32c_is_available:
+        pytest.skip("crc32c is not available on this platform")
+
+
+def pytest_sessionstart(session: pytest.Session) -> None:
     print("crc32c is big endian? ", crc32c.big_endian)
     print("crc32c is hardware based? ", crc32c.hardware_based)
 
+    if not _crc32c_is_available():
+        print("crc32c can't run, no performance diagnostic issued")
+        return
+
     # We run 1GB in total
     data = b" " * int(1e8)
     n = 10

test/test_crc32c.py

@@ -29,16 +29,16 @@
 
 import pytest
 
-try:
-    import crc32c
+import crc32c
 
+
+def test_software_mode(crc32c_is_available: bool) -> None:
+    """Check that the CRC32C_SW_MODE has intended consequences"""
     sw_mode = os.environ.get("CRC32C_SW_MODE")
-    if sw_mode == "none" and not crc32c.hardware_based:
-        raise RuntimeError('"none" should force hardware support')
-    elif sw_mode == "force" and crc32c.hardware_based:
-        raise RuntimeError('"force" should force software support')
-except ImportError:
-    crc32c = None  # type: ignore[assignment]
+    if sw_mode == "force":
+        assert not crc32c.hardware_based
+    if sw_mode == "none" and crc32c_is_available:
+        assert crc32c.hardware_based
 
 
 def ulonglong_as_bytes(x: int) -> bytes:
@@ -68,7 +68,7 @@ def as_individual_bytes(x: bytes) -> Generator[bytes, None, None]:
         yield bytes([b])
 
 
-@pytest.mark.skipif(crc32c is None, reason="no crc32c support in this platform")
+@pytest.mark.calculates_crc32c
 class TestMisc:
 
     def test_zero(self) -> None:
@@ -134,7 +134,7 @@ class CRCTestValue(NamedTuple):
 ]
 
 
-@pytest.mark.skipif(crc32c is None, reason="no crc32c support in this platform")
+@pytest.mark.calculates_crc32c
 class TestCRC32CHash:
     def test_misc(self) -> None:
         crc32c_hash = crc32c.CRC32CHash()
@@ -192,6 +192,7 @@ def test_all(self, data: bytes, crc: int) -> None:
             self._check_values(crc32c.CRC32CHash(data), crc)
 
 
+@pytest.mark.calculates_crc32c
 class Crc32cChecks:
     checksum: int
     val: bytes
@@ -223,11 +224,10 @@ def test_by_different_memory_offsets(self) -> None:
 
 
 # Generate the actual test classes for each of the testing values
-if crc32c is not None:
-    for value in test_values:
-        classname = "Test%s" % value.name
-        locals()[classname] = type(
-            classname,
-            (Crc32cChecks,),
-            {"val": value.data, "checksum": value.crc},
-        )
+for value in test_values:
+    classname = "Test%s" % value.name
+    locals()[classname] = type(
+        classname,
+        (Crc32cChecks,),
+        {"val": value.data, "checksum": value.crc},
+    )