Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds CI/build configuration and small code adjustments to ship self-contained manylinux wheels that bundle a deliberately minimal, statically linked Gammu build, while keeping source builds available for full-featured installations.
Changes:
- Add
cibuildwheelconfiguration + a GitHub Actions wheel build matrix for x86_64 and aarch64 manylinux_2_28 wheels. - Add a build script that fetches and compiles a minimal static Gammu/gsmsd for embedding into wheels, and update
setup.pyto build against it viaGAMMU_PATH. - Add a targeted SMSD test and document the feature matrix/limitations of the Linux wheels.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
test/test_smsd.py |
Adds a test to validate the presence of SMSD “files” and “null” services expected in minimal Linux wheels. |
setup.py |
Adjusts link library order and supports non-pkg-config builds by using include/library dirs from GAMMU_PATH. |
README.rst |
Documents Linux wheel availability, bundled Gammu source link/license, and feature limitations. |
pyproject.toml |
Adds cibuildwheel build/test configuration for manylinux wheels. |
gammu/src/gammu.c |
Introduces a BEGIN_PHONE_COMM_DEALLOC macro and uses it in StateMachine_dealloc. |
.github/workflows/setup.yml |
Splits sdist vs wheel jobs, adds Linux wheel matrix, and validates wheel contents in the check job. |
.github/scripts/build-gammu-wheel.sh |
New script to download/verify/build/install a minimal static Gammu for wheel builds. |
.github/renovate.json |
Adds Renovate regex managers to keep bundled Gammu version/SHA and README link in sync. |
Comments suppressed due to low confidence (1)
gammu/src/gammu.c:105
- Same as the threaded branch: raising an exception from
tp_deallocis unsafe. UsePyErr_WriteUnraisable()+PyErr_Clear()before returning to avoid leaving an exception set in the current thread state.
if (self->in_callback) { \
PyErr_SetString(PyExc_RuntimeError, "Can not call Gammu functions from within callback"); \
return; \
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc7312e417
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| install -m 0644 "${build_root}/build/include/"*.h \ | ||
| "${GAMMU_PREFIX}/include/gammu/" |
There was a problem hiding this comment.
During every cibuildwheel build, this copies only CMake's binary-tree generated headers, while Gammu's public headers such as gammu.h and gammu-smsd.h remain in the extracted source tree. Because setup.py points the compiler at ${GAMMU_PREFIX}/include/gammu, compiling gammu/src/gammu.c will fail with a missing-header error. Copy the source-tree headers as well, or use Gammu's install target to populate the prefix.
Useful? React with 👍 / 👎.
Ship self-contained manylinux wheels without pulling optional Gammu dependencies into every installation. This lets Linux users install without a system Gammu package while preserving source builds for full functionality.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (3)
.github/renovate.json:21
- Same issue here:
managerFilePatternsshould be a plain regex string without/.../delimiters, otherwise Renovate will look for literal slashes and won't matchREADME.rst.
"customType": "regex",
"managerFilePatterns": ["/^README\\.rst$/"],
"matchStrings": [
gammu/src/gammu.c:84
BEGIN_PHONE_COMM_DEALLOCsets a Python exception and then returns fromtp_dealloc. Exceptions raised during deallocation are not reliably reportable, and leaving an exception set can leak into unrelated Python code later. If you want diagnostics here, write it as an unraisable and clear the error indicator before returning.
#define END_PHONE_COMM \
PyThread_release_lock(self->mutex); \
Py_END_ALLOW_THREADS \
CheckIncomingEvents(self);
gammu/src/gammu.c:105
- Same dealloc error-handling issue in the non-threaded macro definition: returning from
tp_deallocwith an active exception can leave the interpreter error state set. Consider writing the error as unraisable and clearing it before returning.
/* ----------------------------------------------------- */
/* Declarations for objects of type StateMachine */
typedef struct {
| @pytest.mark.parametrize("service", ["files", "null"]) | ||
| def test_builtin_service(tmp_path: Path, service: str) -> None: | ||
| """Verify SMSD services included in the minimal Linux wheels.""" |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
test/test_smsd.py:50
- This test is Linux-wheel specific (it hardcodes /dev/null and asserts minimal SMSD services). As written it will also run on non-Linux platforms when developers run the test suite locally, where it is likely to fail for reasons unrelated to the wheel contents. Consider skipping it unless running on Linux.
@pytest.mark.parametrize("service", ["files", "null"])
def test_builtin_service(tmp_path: Path, service: str) -> None:
"""Verify SMSD services included in the minimal Linux wheels."""
| if (self->in_callback) { | ||
| PyObject *error_type = NULL; | ||
| PyObject *error_value = NULL; | ||
| PyObject *error_traceback = NULL; | ||
|
|
||
| PyErr_Fetch(&error_type, &error_value, &error_traceback); | ||
| PyErr_SetString(PyExc_RuntimeError, "Can not call Gammu functions from within callback"); | ||
| PyErr_WriteUnraisable((PyObject *)self); | ||
| PyErr_Clear(); | ||
| PyErr_Restore(error_type, error_value, error_traceback); | ||
| return; | ||
| } |
Ship self-contained manylinux wheels without pulling optional Gammu dependencies into every installation. This lets Linux users install without a system Gammu package while preserving source builds for full functionality.