Skip to content

Conversation

ivansantiagojr
Copy link

This PR makes a suggestion of feature to allow Ziggy Pydust users to add third party modules.

To achieve this, I add a list of imports to PythonModuleOptions and then iterated this list calling lib.addImport for each module. The same was made for libtest.

I also added basic docs giving an example of how to use that, so feel free to consult it and analyze if this suggestion is a good way to address the issue ziglang/zig#474 .

This commit adds support to use external Zig libraries in Ziggy Pydust by passing a list a of imports to
PythonModuleOptions and calling addImport for each module in the lib and libtest.
@CLAassistant
Copy link

CLAassistant commented Jun 28, 2025

CLA assistant check
All committers have signed the CLA.

@ivansantiagojr
Copy link
Author

I open this PR as a draft because I am getting this error when trying to use Ziggy Pydust with these modifications, take a look at the Zig error message:

install
└─ install generated to mdz.abi3.so
   └─ zig build-lib mdz ReleaseSafe native
      └─ translate-c failure
error: error: CacheCheckFailed

error: the following command exited with error code 1:
/tmp/tmpkgmgy72_/.venv/lib/python3.13/site-packages/ziglang/zig translate-c -lc --listen=- -OReleaseSafe -I /usr/include/python3.13 -D Py_LIMITED_API=0x030d05f0 /home/ivan/Documents/personal_git/mdz/pydust/src/ffi.h
Build Summary: 1/5 steps succeeded; 1 failed
install transitive failure
└─ install generated to mdz.abi3.so transitive failure
   └─ zig build-lib mdz ReleaseSafe native transitive failure
      └─ translate-c failure
error: the following build command failed with exit code 1:
.zig-cache/o/1cc39b81d4afaad5bf864b1d780902e9/build /tmp/tmpkgmgy72_/.venv/lib/python3.13/site-packages/ziglang/zig /tmp/tmpkgmgy72_/.venv/lib/python3.13/site-packages/ziglang/lib /home/ivan/Documents/personal_git/mdz .zig-cache /home/ivan/.cache/zig --seed 0x7e74b40b -Z698b40c7d38d9e90 install -Dpython-exe=/tmp/tmpkgmgy72_/.venv/bin/python -Doptimize=ReleaseSafe

And I do not know how to fix that properly, maybe I forgot to add the lib somewhere? Or is it just some Zig detail I let pass (given that I am not very good with Zig)? I would appreciate some help, and would love to help implement this feature.

The project I tried to build and got the error above is this one: https://github.com/ivansantiagojr/mdz/tree/using-fork-ziggy-pydust

@ivansantiagojr ivansantiagojr marked this pull request as ready for review July 29, 2025 13:29
@ivansantiagojr
Copy link
Author

ivansantiagojr commented Aug 13, 2025

To register here (and possibly get help from the community), the compilation error described above only happens in the develop branch. When I checkout the last release's version, it builds and works fine, which means I can use third-party Zig modules with the code additions made in this PR from version 0.25.1.

@ivansantiagojr
Copy link
Author

Workaround:

I noticed that in the generated pydust.build.zig it calls the addTranslateC with the root_source_file = b.path("pydust/src/ffi.h"), the thing is: it tries to look for this file in my project instead of Ziggy Pydust's src, so that's why the code does not compile. I could make it work by creating this path in my project and copying the contents of ffi.h to that place.

How could I make the project get this file from the installed Ziggy Pydust instead of my current project? Any ideas @jburgy @robert3005

@jburgy
Copy link
Contributor

jburgy commented Aug 27, 2025

@ivansantiagojr your issue is likely related to this discord thread (see this minimal repro if you can't access the discord thread). Note that I raised ziglang/translate-c#47 and found a work-around. Reading this reply (as well as ziglang/zig#24497), I understand now that zig has two translate-c implementations and 0.14 gives the clang-based one by default. You probably don't want to wait until 0.16 to have the zig-based implementation. That leaves you with 2 alternatives:

  1. revert that portion of my change and rely on @cImport
  2. use the new translate-c via build.zig.zon

Let me know what you prefer and how I can help you with it.

@ivansantiagojr
Copy link
Author

Oh, I didn't know there was a discord server. Thanks for letting me know and for the tips! I will investigate a little more and try those alternatives you listed locally, probably another PR should be open to solve that.

@robert3005
Copy link
Member

I think if we have #526 we do not need this?

@robert3005 robert3005 closed this Sep 29, 2025
@ivansantiagojr
Copy link
Author

I think if we have #526 we do not need this?

@robert3005, we'll need both, actually. This PR allows us to use third party modules, and #526 only reverts the strategy to use C modules. In order for the code of this branch to work, we need #526 to be merged first. Sorry for not making it clearer earlier!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants