Commit d9f1a952b8

Jonathan Marler <johnnymarler@gmail.com>
2024-07-02 14:15:29
build: fix WriteFile and addCSourceFiles not adding LazyPath deps
Adds a missing call to addLazyPathDependenciesOnly in std.Build.Module.addCSourceFiles. Also fixes an issue in std.Build.Step.WriteFile where it wasn't updating all the GeneratedFile instances for every directory. To fix the second issue, I removed all the GeneratedFile instances and now all files/directories reference the steps main GeneratedFile via sub paths.
1 parent b67caf7
lib/std/Build/Step/WriteFile.zig
@@ -17,8 +17,8 @@ const WriteFile = @This();
 step: Step,
 
 // The elements here are pointers because we need stable pointers for the GeneratedFile field.
-files: std.ArrayListUnmanaged(*File),
-directories: std.ArrayListUnmanaged(*Directory),
+files: std.ArrayListUnmanaged(File),
+directories: std.ArrayListUnmanaged(Directory),
 
 output_source_files: std.ArrayListUnmanaged(OutputSourceFile),
 generated_directory: std.Build.GeneratedFile,
@@ -26,20 +26,14 @@ generated_directory: std.Build.GeneratedFile,
 pub const base_id: Step.Id = .write_file;
 
 pub const File = struct {
-    generated_file: std.Build.GeneratedFile,
     sub_path: []const u8,
     contents: Contents,
-
-    pub fn getPath(file: *File) std.Build.LazyPath {
-        return .{ .generated = .{ .file = &file.generated_file } };
-    }
 };
 
 pub const Directory = struct {
     source: std.Build.LazyPath,
     sub_path: []const u8,
     options: Options,
-    generated_dir: std.Build.GeneratedFile,
 
     pub const Options = struct {
         /// File paths that end in any of these suffixes will be excluded from copying.
@@ -56,10 +50,6 @@ pub const Directory = struct {
             };
         }
     };
-
-    pub fn getPath(dir: *Directory) std.Build.LazyPath {
-        return .{ .generated = .{ .file = &dir.generated_dir } };
-    }
 };
 
 pub const OutputSourceFile = struct {
@@ -92,15 +82,18 @@ pub fn create(owner: *std.Build) *WriteFile {
 pub fn add(write_file: *WriteFile, sub_path: []const u8, bytes: []const u8) std.Build.LazyPath {
     const b = write_file.step.owner;
     const gpa = b.allocator;
-    const file = gpa.create(File) catch @panic("OOM");
-    file.* = .{
-        .generated_file = .{ .step = &write_file.step },
+    const file = File{
         .sub_path = b.dupePath(sub_path),
         .contents = .{ .bytes = b.dupe(bytes) },
     };
     write_file.files.append(gpa, file) catch @panic("OOM");
     write_file.maybeUpdateName();
-    return file.getPath();
+    return .{
+        .generated = .{
+            .file = &write_file.generated_directory,
+            .sub_path = file.sub_path,
+        },
+    };
 }
 
 /// Place the file into the generated directory within the local cache,
@@ -113,9 +106,7 @@ pub fn add(write_file: *WriteFile, sub_path: []const u8, bytes: []const u8) std.
 pub fn addCopyFile(write_file: *WriteFile, source: std.Build.LazyPath, sub_path: []const u8) std.Build.LazyPath {
     const b = write_file.step.owner;
     const gpa = b.allocator;
-    const file = gpa.create(File) catch @panic("OOM");
-    file.* = .{
-        .generated_file = .{ .step = &write_file.step },
+    const file = File{
         .sub_path = b.dupePath(sub_path),
         .contents = .{ .copy = source },
     };
@@ -123,7 +114,12 @@ pub fn addCopyFile(write_file: *WriteFile, source: std.Build.LazyPath, sub_path:
 
     write_file.maybeUpdateName();
     source.addStepDependencies(&write_file.step);
-    return file.getPath();
+    return .{
+        .generated = .{
+            .file = &write_file.generated_directory,
+            .sub_path = file.sub_path,
+        },
+    };
 }
 
 /// Copy files matching the specified exclude/include patterns to the specified subdirectory
@@ -137,18 +133,21 @@ pub fn addCopyDirectory(
 ) std.Build.LazyPath {
     const b = write_file.step.owner;
     const gpa = b.allocator;
-    const dir = gpa.create(Directory) catch @panic("OOM");
-    dir.* = .{
+    const dir = Directory{
         .source = source.dupe(b),
         .sub_path = b.dupePath(sub_path),
         .options = options.dupe(b),
-        .generated_dir = .{ .step = &write_file.step },
     };
     write_file.directories.append(gpa, dir) catch @panic("OOM");
 
     write_file.maybeUpdateName();
     source.addStepDependencies(&write_file.step);
-    return dir.getPath();
+    return .{
+        .generated = .{
+            .file = &write_file.generated_directory,
+            .sub_path = dir.sub_path,
+        },
+    };
 }
 
 /// A path relative to the package root.
@@ -278,11 +277,6 @@ fn make(step: *Step, prog_node: std.Progress.Node) !void {
 
     if (try step.cacheHit(&man)) {
         const digest = man.final();
-        for (write_file.files.items) |file| {
-            file.generated_file.path = try b.cache_root.join(b.allocator, &.{
-                "o", &digest, file.sub_path,
-            });
-        }
         write_file.generated_directory.path = try b.cache_root.join(b.allocator, &.{ "o", &digest });
         return;
     }
@@ -342,10 +336,6 @@ fn make(step: *Step, prog_node: std.Progress.Node) !void {
                 _ = prev_status;
             },
         }
-
-        file.generated_file.path = try b.cache_root.join(b.allocator, &.{
-            cache_path, file.sub_path,
-        });
     }
     for (write_file.directories.items) |dir| {
         const full_src_dir_path = dir.source.getPath2(b, step);
lib/std/Build/Module.zig
@@ -484,6 +484,7 @@ pub fn addCSourceFiles(m: *Module, options: AddCSourceFilesOptions) void {
         .flags = b.dupeStrings(options.flags),
     };
     m.link_objects.append(allocator, .{ .c_source_files = c_source_files }) catch @panic("OOM");
+    addLazyPathDependenciesOnly(m, c_source_files.root);
 }
 
 pub fn addCSourceFile(m: *Module, source: CSourceFile) void {
test/src/Cases.zig
@@ -665,10 +665,12 @@ pub fn lowerToBuildSteps(
         const writefiles = b.addWriteFiles();
         var file_sources = std.StringHashMap(std.Build.LazyPath).init(b.allocator);
         defer file_sources.deinit();
-        for (update.files.items) |file| {
+        const first_file = update.files.items[0];
+        const root_source_file = writefiles.add(first_file.path, first_file.src);
+        file_sources.put(first_file.path, root_source_file) catch @panic("OOM");
+        for (update.files.items[1..]) |file| {
             file_sources.put(file.path, writefiles.add(file.path, file.src)) catch @panic("OOM");
         }
-        const root_source_file = writefiles.files.items[0].getPath();
 
         const artifact = if (case.is_test) b.addTest(.{
             .root_source_file = root_source_file,
test/src/CompareOutput.zig
@@ -81,7 +81,9 @@ pub fn addCase(self: *CompareOutput, case: TestCase) void {
     const b = self.b;
 
     const write_src = b.addWriteFiles();
-    for (case.sources.items) |src_file| {
+    const first_src = case.sources.items[0];
+    const first_file = write_src.add(first_src.filename, first_src.source);
+    for (case.sources.items[1..]) |src_file| {
         _ = write_src.add(src_file.filename, src_file.source);
     }
 
@@ -99,7 +101,7 @@ pub fn addCase(self: *CompareOutput, case: TestCase) void {
                 .target = b.graph.host,
                 .optimize = .Debug,
             });
-            exe.addAssemblyFile(write_src.files.items[0].getPath());
+            exe.addAssemblyFile(first_file);
 
             const run = b.addRunArtifact(exe);
             run.setName(annotated_case_name);
@@ -119,7 +121,7 @@ pub fn addCase(self: *CompareOutput, case: TestCase) void {
 
                 const exe = b.addExecutable(.{
                     .name = "test",
-                    .root_source_file = write_src.files.items[0].getPath(),
+                    .root_source_file = first_file,
                     .optimize = optimize,
                     .target = b.graph.host,
                 });
@@ -145,7 +147,7 @@ pub fn addCase(self: *CompareOutput, case: TestCase) void {
 
             const exe = b.addExecutable(.{
                 .name = "test",
-                .root_source_file = write_src.files.items[0].getPath(),
+                .root_source_file = first_file,
                 .target = b.graph.host,
                 .optimize = .Debug,
             });
test/src/RunTranslatedC.zig
@@ -72,11 +72,13 @@ pub fn addCase(self: *RunTranslatedCContext, case: *const TestCase) void {
     } else if (self.test_filters.len > 0) return;
 
     const write_src = b.addWriteFiles();
-    for (case.sources.items) |src_file| {
+    const first_case = case.sources.items[0];
+    const root_source_file = write_src.add(first_case.filename, first_case.source);
+    for (case.sources.items[1..]) |src_file| {
         _ = write_src.add(src_file.filename, src_file.source);
     }
     const translate_c = b.addTranslateC(.{
-        .root_source_file = write_src.files.items[0].getPath(),
+        .root_source_file = root_source_file,
         .target = b.graph.host,
         .optimize = .Debug,
     });
test/src/StackTrace.zig
@@ -51,10 +51,11 @@ fn addExpect(
         if (mem.indexOf(u8, annotated_case_name, test_filter)) |_| break;
     } else if (self.test_filters.len > 0) return;
 
-    const write_src = b.addWriteFile("source.zig", source);
+    const write_files = b.addWriteFiles();
+    const source_zig = write_files.add("source.zig", source);
     const exe = b.addExecutable(.{
         .name = "test",
-        .root_source_file = write_src.files.items[0].getPath(),
+        .root_source_file = source_zig,
         .optimize = optimize_mode,
         .target = b.graph.host,
         .error_tracing = mode_config.error_tracing,
test/src/TranslateC.zig
@@ -93,12 +93,14 @@ pub fn addCase(self: *TranslateCContext, case: *const TestCase) void {
     } else if (self.test_filters.len > 0) return;
 
     const write_src = b.addWriteFiles();
-    for (case.sources.items) |src_file| {
+    const first_src = case.sources.items[0];
+    const root_source_file = write_src.add(first_src.filename, first_src.source);
+    for (case.sources.items[1..]) |src_file| {
         _ = write_src.add(src_file.filename, src_file.source);
     }
 
     const translate_c = b.addTranslateC(.{
-        .root_source_file = write_src.files.items[0].getPath(),
+        .root_source_file = root_source_file,
         .target = b.resolveTargetQuery(case.target),
         .optimize = .Debug,
     });
test/standalone/dep_lazypath/inc/foo.h
@@ -0,0 +1,1 @@
+#define foo_value 42
test/standalone/dep_lazypath/build.zig
@@ -0,0 +1,37 @@
+const std = @import("std");
+
+pub fn build(b: *std.Build) void {
+    const test_step = b.step("test", "Test it");
+    b.default_step = test_step;
+
+    const optimize: std.builtin.OptimizeMode = .Debug;
+
+    {
+        const write_files = b.addWriteFiles();
+        const generated_main_c = write_files.add("main.c", "");
+        const exe = b.addExecutable(.{
+            .name = "test",
+            .target = b.graph.host,
+            .optimize = optimize,
+        });
+        exe.addCSourceFiles(.{
+            .root = generated_main_c.dirname(),
+            .files = &.{"main.c"},
+        });
+        b.step("csourcefiles", "").dependOn(&exe.step);
+        test_step.dependOn(&exe.step);
+    }
+    {
+        const write_files = b.addWriteFiles();
+        const dir = write_files.addCopyDirectory(b.path("inc"), "", .{});
+        const exe = b.addExecutable(.{
+            .name = "test",
+            .root_source_file = b.path("inctest.zig"),
+            .target = b.graph.host,
+            .optimize = optimize,
+        });
+        exe.addIncludePath(dir);
+        b.step("copydir", "").dependOn(&exe.step);
+        test_step.dependOn(&exe.step);
+    }
+}
test/standalone/dep_lazypath/inctest.zig
@@ -0,0 +1,8 @@
+const std = @import("std");
+const c = @cImport({
+    @cInclude("foo.h");
+});
+comptime {
+    std.debug.assert(c.foo_value == 42);
+}
+pub fn main() void {}
test/standalone/build.zig.zon
@@ -83,6 +83,9 @@
         .dep_shared_builtin = .{
             .path = "dep_shared_builtin",
         },
+        .dep_lazypath = .{
+            .path = "dep_lazypath",
+        },
         .dirname = .{
             .path = "dirname",
         },
test/tests.zig
@@ -783,7 +783,7 @@ pub fn addCliTests(b: *std.Build) *Step {
     if (builtin.os.tag == .linux and builtin.cpu.arch == .x86_64) {
         const tmp_path = b.makeTempPath();
 
-        const writefile = b.addWriteFile("example.zig",
+        const example_zig = b.addWriteFiles().add("example.zig",
             \\// Type your code here, or load an example.
             \\export fn square(num: i32) i32 {
             \\    return num * num;
@@ -804,7 +804,7 @@ pub fn addCliTests(b: *std.Build) *Step {
             "-fno-emit-bin", "-fno-emit-h",
             "-fstrip",       "-OReleaseFast",
         });
-        run.addFileArg(writefile.files.items[0].getPath());
+        run.addFileArg(example_zig);
         const example_s = run.addPrefixedOutputFileArg("-femit-asm=", "example.s");
 
         const checkfile = b.addCheckFile(example_s, .{