ziggy-pydust icon indicating copy to clipboard operation
ziggy-pydust copied to clipboard

Allow third party Zig modules

Open ivansantiagojr opened this issue 9 months ago • 8 comments

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 .

ivansantiagojr avatar Jun 28 '25 21:06 ivansantiagojr

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Jun 28 '25 21:06 CLAassistant

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 avatar Jun 28 '25 22:06 ivansantiagojr

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 avatar Aug 13 '25 02:08 ivansantiagojr

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

ivansantiagojr avatar Aug 27 '25 12:08 ivansantiagojr

@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.

jburgy avatar Aug 27 '25 13:08 jburgy

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.

ivansantiagojr avatar Aug 27 '25 13:08 ivansantiagojr

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

robert3005 avatar Sep 29 '25 00:09 robert3005

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!

ivansantiagojr avatar Sep 29 '25 01:09 ivansantiagojr