Skip to content

Conversation

ivansantiagojr
Copy link

Changes

Use @cImport because translate-c does not work with files outside of the source directory, and pydust.build.zig depends on it to work.

The original changes were added in the commit 9fb5721, and that also removed usingnamespace, so I was careful I did not re-add those.

Notes

Discussions about this happened in the PR #500, where I got stuck with the usage of translate-c, so @jburgy listed 2 ways to fix that in this comment .

I decided to revert the change and use @cImport again because ziglang/translate-c does not have tagged releases and it targes latest Zig, even when unstable. It is worth keeping an eye on this library, though, we might want to use that in the future Zig releases.

This because translate-c does not work with files outside of the source directory, and pydust.build.zig depends on it
@ivansantiagojr
Copy link
Author

Note: I made these changes in a single commit so it is easier to re-revert, but if need, I am happy to split into multiple commits!

@robert3005
Copy link
Member

I am confused by this change - isn't @cimport going to go away and translate_c is going to be the only way forward? I would prefer to figure out a way to adjust the build script instead of relying on deprecated functionality. TranslateC is shipped as part of Zig compiler itself. I think the standard management practice for any c style dependencies is to import the header file you code against and then link as approriate against local build or a shared library.

@robert3005
Copy link
Member

I see, had to read the upstream zig issue. We can merge this and go back to translatec with zig 0.16

@ivansantiagojr
Copy link
Author

I see, had to read the upstream zig issue. We can merge this and go back to translatec with zig 0.16

Agreed, @robert3005. I believe this is the way to go in this case.

@ivansantiagojr
Copy link
Author

I am investigating the problem in the CI, it looks like Python libraries are not properly installed maybe? I will research the diff between ci.yml and find out what's going on.

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.

2 participants