Fix the Python and Lua bindings build and add CI coverage - #2188
Merged
Conversation
|
Ephemeral COPR build failed. @containers/packit-build please check. |
|
@giuseppe works for Arch |
Member
Author
|
do we need a new release or you could just use this diff? |
kolyshkin
reviewed
Aug 13, 2026
kolyshkin
reviewed
Aug 13, 2026
kolyshkin
approved these changes
Aug 13, 2026
kolyshkin
left a comment
kolyshkin
left a comment
Collaborator
There was a problem hiding this comment.
LGTM except for a single nit.
PS hope we don't need to make a new release because of this (and distro packagers can use a patch)
The Python and Lua bindings parse a process JSON blob (in their
update/exec helpers) with parse_json_file() before calling
libcrun_container_exec(). libcrun is built with -fvisibility=hidden and
a version script (libcrun.lds) that only exports symbols matching a few
name patterns, so parse_json_file was not exported and loading either
module failed with:
undefined symbol: parse_json_file
Mark it LIBCRUN_PUBLIC (so it reaches the dynamic symbol table) and add
it to the version script global list (both are required to export it).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Python.h defines _GNU_SOURCE to 1, so a translation unit that includes it before string_map.h (as the Python binding does, transitively via container.h) hits a "_GNU_SOURCE redefined" warning, which is fatal under -Werror. Only define it when it is not already defined. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
libcrun_container_spec() moved from container.h to spec.h, but the Python bindings were never updated and still only included container.h. Building the bindings failed with an implicit-declaration error. Add the missing include. Closes: containers#2187 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
container_start() was defined but never added to the method table, so python_crun had no "start" entry and the function triggered a -Werror=unused-function build failure. Register it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
The method table registered "spec" twice with the same handler. The duplicate is harmless at runtime but confusing; remove it. "spec" remains available via the earlier entry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
giuseppe
force-pushed
the
fix-bindings-build
branch
from
August 13, 2026 20:48
8201007 to
6f79d79
Compare
Member
Author
|
@kolyshkin comments addressed |
I've used this diff to release .1. Thank you! |
kolyshkin
reviewed
Aug 15, 2026
Comment on lines
+114
to
+117
| free ((char *) ctx->id); | ||
| free ((char *) ctx->bundle); | ||
| free ((char *) ctx->state_root); | ||
| free ((char *) ctx->notify_socket); |
Collaborator
There was a problem hiding this comment.
CI errors out on this with:
maint.mk: don't cast free argument
and are these members are defined as const char *
Collaborator
There was a problem hiding this comment.
Amended to use an intermediate variable to drop const, pushed
make_context() created its capsule with a NULL destructor, so the libcrun_context_t and its strdup'd fields leaked whenever a context was garbage collected. It also leaked the freshly allocated context if argument parsing failed. Wire up free_context() as the capsule destructor and free the context on the parse-error path. free_context() now retrieves the pointer with PyCapsule_GetPointer() (the previous cast of the raw PyObject* was wrong for a capsule destructor) and also frees the previously-forgotten bundle field. While here, remove unused local variables from container_status, container_spec and set_verbosity that tripped -Werror. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
libcrun_container_spec() moved from container.h to spec.h, but the Lua bindings still only included container.h. Building the bindings failed with an implicit-declaration error. Add the missing include. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
luaL_error() is not declared as noreturn, so falling off the end of the function after calling it triggered a -Werror=return-type build failure. Return its result (it never actually returns) to satisfy the compiler. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
The language bindings link against internal libcrun symbols but were never built in CI, so API drift (libcrun_container_spec moving to spec.h) and missing symbol exports (parse_json_file) went unnoticed until a build broke. Add a "bindings" test job that configures with --with-python-bindings and --with-lua-bindings under -Werror, builds the modules, and loads each freshly built module to run a smoke test. The smoke tests check that spec generation still works, that every documented entry point is registered, that the verbosity constants exist, and that loading a container resolves all the needed symbols. Ship the new test scripts in the tarball via EXTRA_DIST. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
kolyshkin
force-pushed
the
fix-bindings-build
branch
from
August 15, 2026 23:36
6f79d79 to
9d97f48
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.