Conversation
|
Current states: |
|
This seems suboptimal, but it compiles and runs now! |
| }); | ||
| pub usingnamespace c; | ||
|
|
||
| // Re-export the C symbols we need |
There was a problem hiding this comment.
Is the re-export necessary now?
There was a problem hiding this comment.
I do not have requisite experience or knowledge to answer this question
There was a problem hiding this comment.
I'm currently taking the baton here for making this work on 0.15 (and 0.16 to release soon).
This does not need to be re-exported. While usingnamespace was essentially removed entirely, we can just directly re-export via an alias:
pub const nc = @cImport({
@cInclude("notcurses/notcurses.h");
});Then using the lib is just a matter of:
const nc = @import("notcurses.zig").nc;When 0.16 or 0.17 rolls around (depending on the issue status), @cImport will disappear in favour of direct declartion in the build script as a zig module instead (ziglang/zig#20630). So, that will essentially allow direct import like @import("notcurses") or something similar.
|
I might not have time to test and merge this. I am a bit fatigued from Zig changes, my plan is to wait for 1.0. We could perhaps add a link to your fork to the readme? |
|
eh maybe just leave it. I literally just started hacking on something for a weekend project, I have no idea what I'm doing. Unlikely I'll be paying attention to this long term. ... my initial impression has too be leaning towards "wait for 1.0". Was pretty rough coming in and finding all libs broken (took me a bit to figure out that 0.15 only came out end of August, probably should have downgraded.) |
| "-std=gnu11", | ||
| "-D_GNU_SOURCE", // to make memory management work, see sys/mman.h | ||
| "-DUSE_MULTIMEDIA=none", | ||
| "-DUSE_QRCODEGEN=OFF", |
There was a problem hiding this comment.
You'll want to drop this define, since the C files make use of it with #ifdef without a value check.
| .optimize = optimize, | ||
| }); | ||
| exe_module.linkLibrary(notcurses); | ||
| exe_module.linkSystemLibrary("qrcodegen", .{}); |
There was a problem hiding this comment.
Drop this link as well if the previously mentioned define is removed
| exe.addIncludePath(.{ .path = notcurses_source_path ++ "/include" }); | ||
| exe.linkLibrary(notcurses); | ||
| exe.linkSystemLibrary("notcurses-core"); | ||
| exe.addObjectFile(b.path(notcurses_source_path ++ "/build/libnotcurses-core.a")); |
There was a problem hiding this comment.
Neither of seem to be needed as we are compiling the code into a Zig module, not building it externally into a static lib.
| # In case of errors, try `git checkout v3.0.9` and re-run cmake as I tested it with this version. | ||
|
|
||
| # 0.15 UPDATE: The current build script requires make to built libncurses-core | ||
| make |
There was a problem hiding this comment.
This isn't exactly true, if we're required to have an externally built static lib then sure, but since we are compiling the code into a zig module this is not necessary.
| .optimize = optimize, | ||
| // notcurses has saome undefined benavior which makes the demo crash with | ||
| // illegal instruction, disabling UBSAN to make it work (-fno-sanitize-c) | ||
| .sanitize_c = std.zig.SanitizeC.off, |
There was a problem hiding this comment.
Can add .link_libc = true, and remove the deprecated notcurses.linkLibC() usage below
| exe_module.linkSystemLibrary("ncurses", .{}); | ||
| exe_module.linkSystemLibrary("readline", .{}); | ||
| exe_module.linkSystemLibrary("unistring", .{}); | ||
| exe_module.linkSystemLibrary("z", .{}); |
There was a problem hiding this comment.
These should be attached to the notcurses_module instead of exe_module
3 participants
Attempting to address #5
Isn't linking ncurses properly yet :(