Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

translate-c build step lacks a way to link system libraries #20649

Open
Tracked by #20630
andrewrk opened this issue Jul 16, 2024 · 1 comment
Open
Tracked by #20630

translate-c build step lacks a way to link system libraries #20649

andrewrk opened this issue Jul 16, 2024 · 1 comment
Labels
enhancement Solving this issue will likely involve adding new logic or components to the codebase. translate-c C to Zig source translation feature (@cImport) zig build system std.Build, the build runner, `zig build` subcommand, package management
Milestone

Comments

@andrewrk
Copy link
Member

Example of not being able to import certain .h files due to limitations of the build system step:

diff --git a/build.zig b/build.zig
index 738bc4f..52d6f01 100644
--- a/build.zig
+++ b/build.zig
@@ -5,6 +5,12 @@ pub fn build(b: *std.Build) void {
     const optimize = b.standardOptimizeOption(.{});
     const use_llvm = b.option(bool, "use-llvm", "use the LLVM backend");
 
+    const translate_c = b.addTranslateC(.{
+        .root_source_file = b.path("src/c.h"),
+        .target = target,
+        .optimize = optimize,
+        .link_libc = true,
+    });
     const exe = b.addExecutable(.{
         .name = "tetris",
         .root_source_file = b.path("src/main.zig"),
@@ -13,10 +19,10 @@ pub fn build(b: *std.Build) void {
         .use_llvm = use_llvm,
         .use_lld = use_llvm,
     });
-
-    exe.linkLibC();
+    exe.root_module.addImport("c", translate_c.createModule());
     exe.linkSystemLibrary("glfw");
     exe.linkSystemLibrary("epoxy");
+
     b.installArtifact(exe);
 
     const play = b.step("play", "Play the game");
diff --git a/src/c.zig b/src/c.zig
deleted file mode 100644
index 7a9f0a2..0000000
--- a/src/c.zig
+++ /dev/null
@@ -1,8 +0,0 @@
-pub usingnamespace @cImport({
-    @cInclude("stdio.h");
-    @cInclude("math.h");
-    @cInclude("time.h");
-    @cInclude("stdlib.h");
-    @cInclude("epoxy/gl.h");
-    @cInclude("GLFW/glfw3.h");
-});
diff --git a/src/main.zig b/src/main.zig
index c3b8498..2ca6c56 100644
--- a/src/main.zig
+++ b/src/main.zig
@@ -8,7 +8,7 @@ const Tetris = @import("tetris.zig").Tetris;
 const std = @import("std");
 const assert = std.debug.assert;
 const bufPrint = std.fmt.bufPrint;
-const c = @import("c.zig");
+const c = @import("c");
 const debug_gl = @import("debug_gl.zig");
 const AllShaders = @import("all_shaders.zig").AllShaders;
 const StaticGeometry = @import("static_geometry.zig").StaticGeometry;
[nix-shell:~/dev/tetris]$ zig build
warning: file_system_inputs empty
install
└─ install tetris
   └─ zig build-exe tetris Debug native
      └─ translate-c 1 errors
/home/andy/dev/tetris/src/c.h:5:10: error: 'epoxy/gl.h' file not found
#include <epoxy/gl.h>
         ^
error: the following command failed with 1 compilation errors:
/home/andy/local/bin/zig translate-c -lc --listen=- /home/andy/dev/tetris/src/c.h 
Build Summary: 0/4 steps succeeded; 1 failed
install transitive failure
└─ install tetris transitive failure
   └─ zig build-exe tetris Debug native transitive failure
      └─ translate-c 1 errors
error: the following build command failed with exit code 1:
/home/andy/dev/tetris/.zig-cache/o/7fddf9d37ec7fba8c98bb526aad30ae4/build /home/andy/local/bin/zig /home/andy/local/lib/zig /home/andy/dev/tetris /home/andy/dev/tetris/.zig-cache /home/andy/.cache/zig --seed 0xf1bb8c15 -Z7d71b57141fa9992

[nix-shell:~/dev/tetris]$ zig build
/home/andy/dev/tetris/build.zig:14:16: error: no field or member function named 'linkSystemLibrary' in 'Build.Step.TranslateC'
    translate_c.linkSystemLibrary("glfw");
    ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
/home/andy/local/lib/zig/std/Build/Step/TranslateC.zig:1:1: note: struct declared here
const std = @import("std");
^~~~~
referenced by:
    runBuild__anon_10304: /home/andy/local/lib/zig/std/Build.zig:2140:33
    main: /home/andy/local/lib/zig/compiler/build_runner.zig:314:29
    3 reference(s) hidden; use '-freference-trace=5' to see all references

In the compiler, zig translate-c is handled with generally the same code path as build-obj, build-exe, and build-lib. It branches off near the end. std.Build.Step.TranslateC either needs to be absorbed by, or duplicate a large portion of the API of std.Build.Step.Compile.

Furthermore, there is interest in reducing the amount of unique C imports. Perhaps the build system API should provide some mechanism that can help modules from different packages coordinate about this.

Related to #7687.

@andrewrk andrewrk added enhancement Solving this issue will likely involve adding new logic or components to the codebase. translate-c C to Zig source translation feature (@cImport) zig build system std.Build, the build runner, `zig build` subcommand, package management labels Jul 16, 2024
@andrewrk andrewrk added this to the 0.14.0 milestone Jul 16, 2024
@BratishkaErik
Copy link
Contributor

BratishkaErik commented Jul 16, 2024

Maybe Step.TranslateC can have it's own module as the input like in #20388 , and then all this linking logic will not need to be added to TranslateC since Module already has them. You would have two modules in this system, one with C source files, libraries etc. that is passed to addTranslateC, and it returns another module with converted Zig source files.

const c_sources = b.createModule(.{
    .root_source_file = b.path("src/c.h"),
    .target = target,
    .optimize = optimize,
});
c_sources.linkSystemLibrary("glfw");
// ...
const translate_c = b.addTranslateC(.{
    .root_module = c_sources, // input to translate
});
//...
main_mod.addImport("c", translate_c.createModule()); // translated output

Together with that PR, user would be able to easily swap between addExecutable from C files and addTranslateC just by changing one line, in case they want to migrate step-by-step to Zig language. Or call both functions...

upd: implementation should be relatively simple, all defineCMacro, addIncludeDir are alfready included in Module so they can be removed from TranslateC, and step would need some binding logic to module like Compile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Solving this issue will likely involve adding new logic or components to the codebase. translate-c C to Zig source translation feature (@cImport) zig build system std.Build, the build runner, `zig build` subcommand, package management
Projects
None yet
Development

No branches or pull requests

2 participants