build_runner: collect c macro definitions from build.zig

fixes #2110
zigtools · Jan 16, 2025 · c6a70b0 · c6a70b0
1 parent 3426c68
commit c6a70b0
Show file tree

Hide file tree

Showing 11 changed files with 111 additions and 24 deletions.
diff --git a/src/DocumentStore.zig b/src/DocumentStore.zig
@@ -1489,6 +1489,34 @@ pub fn collectIncludeDirs(
     return collected_all;
 }
 
+/// returns `true` if all c macro definitions could be collected
+/// may return `false` because macros from a build.zig may not have been resolved already
+/// **Thread safe** takes a shared lock
+pub fn collectCMacros(
+    store: *DocumentStore,
+    allocator: std.mem.Allocator,
+    handle: *Handle,
+    c_macros: *std.ArrayListUnmanaged([]const u8),
+) !bool {
+    const collected_all = switch (try handle.getAssociatedBuildFileUri2(store)) {
+        .none => true,
+        .unresolved => false,
+        .resolved => |build_file_uri| blk: {
+            const build_file = store.getBuildFile(build_file_uri).?;
+            const build_config = build_file.tryLockConfig() orelse break :blk false;
+            defer build_file.unlockConfig();
+
+            try c_macros.ensureUnusedCapacity(allocator, build_config.c_macros.len);
+            for (build_config.c_macros) |c_macro| {
+                c_macros.appendAssumeCapacity(try allocator.dupe(u8, c_macro));
+            }
+            break :blk true;
+        },
+    };
+
+    return collected_all;
+}
+
 /// returns the document behind `@cImport()` where `node` is the `cImport` node
 /// if a cImport can't be translated e.g. requires computing a
 /// comptime value `resolveCImport` will return null
@@ -1533,10 +1561,24 @@ pub fn resolveCImport(self: *DocumentStore, handle: *Handle, node: Ast.Node.Inde
         return null;
     };
 
+    var c_macros: std.ArrayListUnmanaged([]const u8) = .empty;
+    defer {
+        for (c_macros.items) |c_macro| {
+            self.allocator.free(c_macro);
+        }
+        c_macros.deinit(self.allocator);
+    }
+
+    const collected_all_c_macros = self.collectCMacros(self.allocator, handle, &c_macros) catch |err| {
+        log.err("failed to resolve include paths: {}", .{err});
+        return null;
+    };
+
     const maybe_result = translate_c.translate(
         self.allocator,
         self.config,
         include_dirs.items,
+        c_macros.items,
         source,
     ) catch |err| switch (err) {
         error.OutOfMemory => |e| return e,
@@ -1547,7 +1589,7 @@ pub fn resolveCImport(self: *DocumentStore, handle: *Handle, node: Ast.Node.Inde
     };
     var result = maybe_result orelse return null;
 
-    if (result == .failure and !collected_all_include_dirs) {
+    if (result == .failure and (!collected_all_include_dirs or !collected_all_c_macros)) {
         result.deinit(self.allocator);
         return null;
     }

diff --git a/src/build_runner/master.zig b/src/build_runner/master.zig
@@ -1026,13 +1026,19 @@ fn extractBuildInformation(
             name: []const u8,
             packages: *Packages,
             include_dirs: *std.StringArrayHashMapUnmanaged(void),
+            c_macros: *std.StringArrayHashMapUnmanaged(void),
         ) !void {
             if (module.root_source_file) |root_source_file| {
                 _ = try packages.addPackage(name, root_source_file.getPath(module.owner));
             }
 
             if (compile) |exe| {
-                try processPkgConfig(allocator, include_dirs, exe);
+                try processPkgConfig(allocator, include_dirs, c_macros, exe);
+            }
+
+            try c_macros.ensureUnusedCapacity(allocator, module.c_macros.items.len);
+            for (module.c_macros.items) |c_macro| {
+                c_macros.putAssumeCapacity(c_macro, {});
             }
 
             for (module.include_dirs.items) |include_dir| {
@@ -1119,6 +1125,9 @@ fn extractBuildInformation(
     var include_dirs: std.StringArrayHashMapUnmanaged(void) = .{};
     defer include_dirs.deinit(gpa);
 
+    var c_macros: std.StringArrayHashMapUnmanaged(void) = .{};
+    defer c_macros.deinit(gpa);
+
     var packages: Packages = .{ .allocator = gpa };
     defer packages.deinit();
 
@@ -1127,20 +1136,20 @@ fn extractBuildInformation(
         for (steps.keys()) |step| {
             const compile = step.cast(Step.Compile) orelse continue;
             const graph = compile.root_module.getGraph();
-            try helper.processItem(gpa, compile.root_module, compile, "root", &packages, &include_dirs);
+            try helper.processItem(gpa, compile.root_module, compile, "root", &packages, &include_dirs, &c_macros);
             for (graph.modules) |module| {
                 for (module.import_table.keys(), module.import_table.values()) |name, import| {
-                    try helper.processItem(gpa, import, null, name, &packages, &include_dirs);
+                    try helper.processItem(gpa, import, null, name, &packages, &include_dirs, &c_macros);
                 }
             }
         }
 
         for (b.modules.values()) |root_module| {
             const graph = root_module.getGraph();
-            try helper.processItem(gpa, root_module, null, "root", &packages, &include_dirs);
+            try helper.processItem(gpa, root_module, null, "root", &packages, &include_dirs, &c_macros);
             for (graph.modules) |module| {
                 for (module.import_table.keys(), module.import_table.values()) |name, import| {
-                    try helper.processItem(gpa, import, null, name, &packages, &include_dirs);
+                    try helper.processItem(gpa, import, null, name, &packages, &include_dirs, &c_macros);
                 }
             }
         }
@@ -1190,6 +1199,7 @@ fn extractBuildInformation(
             .include_dirs = include_dirs.keys(),
             .top_level_steps = b.top_level_steps.keys(),
             .available_options = available_options,
+            .c_macros = c_macros.keys(),
         },
         .{
             .whitespace = .indent_2,
@@ -1201,6 +1211,7 @@ fn extractBuildInformation(
 fn processPkgConfig(
     allocator: std.mem.Allocator,
     include_dirs: *std.StringArrayHashMapUnmanaged(void),
+    c_macros: *std.StringArrayHashMapUnmanaged(void),
     exe: *Step.Compile,
 ) !void {
     for (exe.root_module.link_objects.items) |link_object| {
@@ -1209,7 +1220,7 @@ fn processPkgConfig(
 
         if (system_lib.use_pkg_config == .no) continue;
 
-        getPkgConfigIncludes(allocator, include_dirs, exe, system_lib.name) catch |err| switch (err) {
+        const args = copied_from_zig.runPkgConfig(exe, system_lib.name) catch |err| switch (err) {
             error.PkgConfigInvalidOutput,
             error.PkgConfigCrashed,
             error.PkgConfigFailed,
@@ -1218,31 +1229,25 @@ fn processPkgConfig(
             => switch (system_lib.use_pkg_config) {
                 .yes => {
                     // pkg-config failed, so zig will not add any include paths
+                    continue;
                 },
                 .force => {
                     std.log.warn("pkg-config failed for library {s}", .{system_lib.name});
+                    continue;
                 },
                 .no => unreachable,
             },
             else => |e| return e,
         };
-    }
-}
-
-fn getPkgConfigIncludes(
-    allocator: std.mem.Allocator,
-    include_dirs: *std.StringArrayHashMapUnmanaged(void),
-    exe: *Step.Compile,
-    name: []const u8,
-) !void {
-    if (copied_from_zig.runPkgConfig(exe, name)) |args| {
         for (args) |arg| {
             if (std.mem.startsWith(u8, arg, "-I")) {
                 const candidate = arg[2..];
                 try include_dirs.put(allocator, candidate, {});
+            } else if (std.mem.startsWith(u8, arg, "-D")) {
+                try c_macros.put(allocator, arg, {});
             }
         }
-    } else |err| return err;
+    }
 }
 
 // TODO: Having a copy of this is not very nice

diff --git a/src/build_runner/shared.zig b/src/build_runner/shared.zig
@@ -11,6 +11,7 @@ pub const BuildConfig = struct {
     include_dirs: []const []const u8,
     top_level_steps: []const []const u8,
     available_options: std.json.ArrayHashMap(AvailableOption),
+    c_macros: []const []const u8 = &.{},
 
     pub const DepsBuildRoots = Package;
     pub const Package = struct {

diff --git a/src/translate_c.zig b/src/translate_c.zig
@@ -118,6 +118,7 @@ pub fn translate(
     allocator: std.mem.Allocator,
     config: Config,
     include_dirs: []const []const u8,
+    c_macros: []const []const u8,
     source: []const u8,
 ) !?Result {
     const tracy_zone = tracy.trace(@src());
@@ -166,7 +167,7 @@ pub fn translate(
         "--listen=-",
     };
 
-    const argc = base_args.len + 2 * include_dirs.len + 1;
+    const argc = base_args.len + 2 * include_dirs.len + c_macros.len + 1;
     var argv: std.ArrayListUnmanaged([]const u8) = try .initCapacity(allocator, argc);
     defer argv.deinit(allocator);
 
@@ -177,6 +178,8 @@ pub fn translate(
         argv.appendAssumeCapacity(include_dir);
     }
 
+    argv.appendSliceAssumeCapacity(c_macros);
+
     argv.appendAssumeCapacity(file_path);
 
     var process: std.process.Child = .init(argv.items, allocator);

diff --git a/tests/build_runner_cases/add_module.json b/tests/build_runner_cases/add_module.json
@@ -11,5 +11,6 @@
     "install",
     "uninstall"
   ],
-  "available_options": {}
+  "available_options": {},
+  "c_macros": []
 }
diff --git a/tests/build_runner_cases/define_c_macro.json b/tests/build_runner_cases/define_c_macro.json
@@ -0,0 +1,18 @@
+{
+  "deps_build_roots": [],
+  "packages": [
+    {
+      "name": "root",
+      "path": "root.zig"
+    }
+  ],
+  "include_dirs": [],
+  "top_level_steps": [
+    "install",
+    "uninstall"
+  ],
+  "available_options": {},
+  "c_macros": [
+    "-Dkey=value"
+  ]
+}
diff --git a/tests/build_runner_cases/define_c_macro.zig b/tests/build_runner_cases/define_c_macro.zig
@@ -0,0 +1,8 @@
+const std = @import("std");
+
+pub fn build(b: *std.Build) void {
+    const foo = b.addModule("foo", .{
+        .root_source_file = b.path("root.zig"),
+    });
+    foo.addCMacro("key", "value");
+}
diff --git a/tests/build_runner_cases/empty.json b/tests/build_runner_cases/empty.json
@@ -6,5 +6,6 @@
     "install",
     "uninstall"
   ],
-  "available_options": {}
+  "available_options": {},
+  "c_macros": []
 }
diff --git a/tests/build_runner_cases/module_self_import.json b/tests/build_runner_cases/module_self_import.json
@@ -15,5 +15,6 @@
     "install",
     "uninstall"
   ],
-  "available_options": {}
+  "available_options": {},
+  "c_macros": []
 }
diff --git a/tests/build_runner_cases/multiple_module_import_names.json b/tests/build_runner_cases/multiple_module_import_names.json
@@ -35,5 +35,6 @@
     "install",
     "uninstall"
   ],
-  "available_options": {}
+  "available_options": {},
+  "c_macros": []
 }
diff --git a/tests/language_features/cimport.zig b/tests/language_features/cimport.zig
@@ -109,7 +109,13 @@ fn testTranslate(c_source: []const u8) !translate_c.Result {
     var ctx: Context = try .init();
     defer ctx.deinit();
 
-    var result = (try translate_c.translate(allocator, zls.DocumentStore.Config.fromMainConfig(ctx.server.config), &.{}, c_source)).?;
+    var result = (try translate_c.translate(
+        allocator,
+        zls.DocumentStore.Config.fromMainConfig(ctx.server.config),
+        &.{},
+        &.{},
+        c_source,
+    )).?;
     errdefer result.deinit(allocator);
 
     switch (result) {