From 0b7123f41d66bdda4da29d59623299d47b29aefb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carl=20=C3=85stholm?= Date: Sat, 2 Mar 2024 22:59:00 +0100 Subject: [PATCH 01/11] std.Build: correct behavior of `Step.Compile.installHeader` Previously, `Step.Compile.installHeader` and friends would incorrectly modify the default `install` top-level step, when the intent was for headers to get bundled with and installed alongside an artifact. This change set implements the intended behavior. This carries with it some breaking changes; `installHeader` and `installConfigHeader` both have new signatures, and `installHeadersDirectory` and `installHeadersDirectoryOptions` have been merged into `installHeaders`. --- lib/std/Build/Module.zig | 30 +++-- lib/std/Build/Step/Compile.zig | 157 +++++++++++++++---------- lib/std/Build/Step/InstallArtifact.zig | 95 +++++++++++---- 3 files changed, 183 insertions(+), 99 deletions(-) diff --git a/lib/std/Build/Module.zig b/lib/std/Build/Module.zig index 46c6a19578..b8b6560019 100644 --- a/lib/std/Build/Module.zig +++ b/lib/std/Build/Module.zig @@ -265,8 +265,8 @@ fn addShallowDependencies(m: *Module, dependee: *Module) void { for (dependee.link_objects.items) |link_object| switch (link_object) { .other_step => |compile| { addStepDependencies(m, dependee, &compile.step); - for (compile.installed_headers.items) |install_step| - addStepDependenciesOnly(m, install_step); + for (compile.installed_headers.items) |header| + addLazyPathDependenciesOnly(m, header.source.path()); }, .static_path, @@ -691,20 +691,19 @@ pub fn appendZigProcessFlags( }, .other_step => |other| { if (other.generated_h) |header| { - try zig_args.append("-isystem"); - try zig_args.append(std.fs.path.dirname(header.path.?).?); - } - if (other.installed_headers.items.len > 0) { - try zig_args.append("-I"); - try zig_args.append(b.pathJoin(&.{ - other.step.owner.install_prefix, "include", - })); + try zig_args.appendSlice(&.{ "-isystem", std.fs.path.dirname(header.getPath()).? }); } + for (other.installed_headers.items) |header| switch (header.source) { + .file => |lp| { + try zig_args.appendSlice(&.{ "-I", std.fs.path.dirname(lp.getPath2(b, asking_step)).? }); + }, + .directory => |dir| { + try zig_args.appendSlice(&.{ "-I", dir.path.getPath2(b, asking_step) }); + }, + }; }, .config_header_step => |config_header| { - const full_file_path = config_header.output_file.path.?; - const header_dir_path = full_file_path[0 .. full_file_path.len - config_header.include_path.len]; - try zig_args.appendSlice(&.{ "-I", header_dir_path }); + try zig_args.appendSlice(&.{ "-I", std.fs.path.dirname(config_header.output_file.getPath()).? }); }, } } @@ -752,9 +751,8 @@ fn linkLibraryOrObject(m: *Module, other: *Step.Compile) void { m.link_objects.append(allocator, .{ .other_step = other }) catch @panic("OOM"); m.include_dirs.append(allocator, .{ .other_step = other }) catch @panic("OOM"); - for (other.installed_headers.items) |install_step| { - addStepDependenciesOnly(m, install_step); - } + for (other.installed_headers.items) |header| + addLazyPathDependenciesOnly(m, header.source.path()); } fn requireKnownTarget(m: *Module) std.Target { diff --git a/lib/std/Build/Step/Compile.zig b/lib/std/Build/Step/Compile.zig index 0c37769e9c..caa3a9e34a 100644 --- a/lib/std/Build/Step/Compile.zig +++ b/lib/std/Build/Step/Compile.zig @@ -59,7 +59,7 @@ test_runner: ?[]const u8, test_server_mode: bool, wasi_exec_model: ?std.builtin.WasiExecModel = null, -installed_headers: ArrayList(*Step), +installed_headers: ArrayList(InstalledHeader), // keep in sync with src/Compilation.zig:RcIncludes /// Behavior of automatic detection of include directories when compiling .rc files. @@ -249,6 +249,70 @@ pub const Kind = enum { @"test", }; +pub const InstalledHeader = struct { + source: Source, + dest_rel_path: []const u8, + + pub const Source = union(enum) { + file: LazyPath, + directory: Directory, + + pub const Directory = struct { + path: LazyPath, + options: Directory.Options, + + pub const Options = struct { + /// File paths which end in any of these suffixes will be excluded + /// from installation. + exclude_extensions: []const []const u8 = &.{}, + /// Only file paths which end in any of these suffixes will be included + /// in installation. + /// `null` means all suffixes will be included. + /// `exclude_extensions` takes precedence over `include_extensions` + include_extensions: ?[]const []const u8 = &.{".h"}, + + pub fn dupe(self: Directory.Options, b: *std.Build) Directory.Options { + return .{ + .exclude_extensions = b.dupeStrings(self.exclude_extensions), + .include_extensions = if (self.include_extensions) |incs| + b.dupeStrings(incs) + else + null, + }; + } + }; + + pub fn dupe(self: Directory, b: *std.Build) Directory { + return .{ + .path = self.path.dupe(b), + .options = self.options.dupe(b), + }; + } + }; + + pub fn path(self: Source) LazyPath { + return switch (self) { + .file => |lp| lp, + .directory => |dir| dir.path, + }; + } + + pub fn dupe(self: Source, b: *std.Build) Source { + return switch (self) { + .file => |lp| .{ .file = lp.dupe(b) }, + .directory => |dir| .{ .directory = dir.dupe(b) }, + }; + } + }; + + pub fn dupe(self: InstalledHeader, b: *std.Build) InstalledHeader { + return .{ + .source = self.source.dupe(b), + .dest_rel_path = b.dupePath(self.dest_rel_path), + }; + } +}; + pub fn create(owner: *std.Build, options: Options) *Compile { const name = owner.dupe(options.name); if (mem.indexOf(u8, name, "/") != null or mem.indexOf(u8, name, "\\") != null) { @@ -308,7 +372,7 @@ pub fn create(owner: *std.Build, options: Options) *Compile { .out_lib_filename = undefined, .major_only_filename = null, .name_only_filename = null, - .installed_headers = ArrayList(*Step).init(owner.allocator), + .installed_headers = ArrayList(InstalledHeader).init(owner.allocator), .zig_lib_dir = null, .exec_cmd_args = null, .filters = options.filters, @@ -380,78 +444,47 @@ pub fn create(owner: *std.Build, options: Options) *Compile { return self; } -pub fn installHeader(cs: *Compile, src_path: []const u8, dest_rel_path: []const u8) void { - const b = cs.step.owner; - const install_file = b.addInstallHeaderFile(src_path, dest_rel_path); - b.getInstallStep().dependOn(&install_file.step); - cs.installed_headers.append(&install_file.step) catch @panic("OOM"); -} - -pub const InstallConfigHeaderOptions = struct { - install_dir: InstallDir = .header, - dest_rel_path: ?[]const u8 = null, -}; - -pub fn installConfigHeader( +pub fn installHeader( cs: *Compile, - config_header: *Step.ConfigHeader, - options: InstallConfigHeaderOptions, -) void { - const dest_rel_path = options.dest_rel_path orelse config_header.include_path; - const b = cs.step.owner; - const install_file = b.addInstallFileWithDir( - .{ .generated = &config_header.output_file }, - options.install_dir, - dest_rel_path, - ); - install_file.step.dependOn(&config_header.step); - b.getInstallStep().dependOn(&install_file.step); - cs.installed_headers.append(&install_file.step) catch @panic("OOM"); -} - -pub fn installHeadersDirectory( - a: *Compile, - src_dir_path: []const u8, + source: LazyPath, dest_rel_path: []const u8, ) void { - return installHeadersDirectoryOptions(a, .{ - .source_dir = .{ .path = src_dir_path }, - .install_dir = .header, - .install_subdir = dest_rel_path, - }); + const b = cs.step.owner; + cs.installed_headers.append(.{ + .source = .{ .file = source.dupe(b) }, + .dest_rel_path = b.dupePath(dest_rel_path), + }) catch @panic("OOM"); + source.addStepDependencies(&cs.step); } -pub fn installHeadersDirectoryOptions( +pub fn installHeaders( cs: *Compile, - options: std.Build.Step.InstallDir.Options, + source: LazyPath, + dest_rel_path: []const u8, + options: InstalledHeader.Source.Directory.Options, ) void { const b = cs.step.owner; - const install_dir = b.addInstallDirectory(options); - b.getInstallStep().dependOn(&install_dir.step); - cs.installed_headers.append(&install_dir.step) catch @panic("OOM"); + cs.installed_headers.append(.{ + .source = .{ .directory = .{ + .path = source.dupe(b), + .options = options.dupe(b), + } }, + .dest_rel_path = b.dupePath(dest_rel_path), + }) catch @panic("OOM"); + source.addStepDependencies(&cs.step); } -pub fn installLibraryHeaders(cs: *Compile, l: *Compile) void { - assert(l.kind == .lib); +pub fn installConfigHeader(cs: *Compile, config_header: *Step.ConfigHeader) void { + cs.installHeader(.{ .generated = &config_header.output_file }, config_header.include_path); +} + +pub fn installLibraryHeaders(cs: *Compile, lib: *Compile) void { + assert(lib.kind == .lib); const b = cs.step.owner; - const install_step = b.getInstallStep(); - // Copy each element from installed_headers, modifying the builder - // to be the new parent's builder. - for (l.installed_headers.items) |step| { - const step_copy = switch (step.id) { - inline .install_file, .install_dir => |id| blk: { - const T = id.Type(); - const ptr = b.allocator.create(T) catch @panic("OOM"); - ptr.* = step.cast(T).?.*; - ptr.dest_builder = b; - break :blk &ptr.step; - }, - else => unreachable, - }; - cs.installed_headers.append(step_copy) catch @panic("OOM"); - install_step.dependOn(step_copy); + for (lib.installed_headers.items) |header| { + cs.installed_headers.append(header.dupe(b)) catch @panic("OOM"); + header.source.path().addStepDependencies(&cs.step); } - cs.installed_headers.appendSlice(l.installed_headers.items) catch @panic("OOM"); } pub fn addObjCopy(cs: *Compile, options: Step.ObjCopy.Options) *Step.ObjCopy { diff --git a/lib/std/Build/Step/InstallArtifact.zig b/lib/std/Build/Step/InstallArtifact.zig index 3ef69f5975..5f77487823 100644 --- a/lib/std/Build/Step/InstallArtifact.zig +++ b/lib/std/Build/Step/InstallArtifact.zig @@ -77,12 +77,10 @@ pub fn create(owner: *std.Build, artifact: *Step.Compile, options: Options) *Ins }, .h_dir = switch (options.h_dir) { .disabled => null, - // https://github.com/ziglang/zig/issues/9698 - .default => null, - //.default => switch (artifact.kind) { - // .lib => .header, - // else => null, - //}, + .default => switch (artifact.kind) { + .lib => .header, + else => null, + }, .override => |o| o, }, .implib_dir = switch (options.implib_dir) { @@ -113,7 +111,8 @@ pub fn create(owner: *std.Build, artifact: *Step.Compile, options: Options) *Ins if (self.dest_dir != null) self.emitted_bin = artifact.getEmittedBin(); if (self.pdb_dir != null) self.emitted_pdb = artifact.getEmittedPdb(); - if (self.h_dir != null) self.emitted_h = artifact.getEmittedH(); + // https://github.com/ziglang/zig/issues/9698 + //if (self.h_dir != null) self.emitted_h = artifact.getEmittedH(); if (self.implib_dir != null) self.emitted_implib = artifact.getEmittedImplib(); return self; @@ -122,14 +121,14 @@ pub fn create(owner: *std.Build, artifact: *Step.Compile, options: Options) *Ins fn make(step: *Step, prog_node: *std.Progress.Node) !void { _ = prog_node; const self: *InstallArtifact = @fieldParentPtr("step", step); - const dest_builder = step.owner; + const b = step.owner; const cwd = fs.cwd(); var all_cached = true; if (self.dest_dir) |dest_dir| { - const full_dest_path = dest_builder.getInstallPath(dest_dir, self.dest_sub_path); - const full_src_path = self.emitted_bin.?.getPath2(step.owner, step); + const full_dest_path = b.getInstallPath(dest_dir, self.dest_sub_path); + const full_src_path = self.emitted_bin.?.getPath2(b, step); const p = fs.Dir.updateFile(cwd, full_src_path, cwd, full_dest_path, .{}) catch |err| { return step.fail("unable to update file from '{s}' to '{s}': {s}", .{ full_src_path, full_dest_path, @errorName(err), @@ -145,8 +144,8 @@ fn make(step: *Step, prog_node: *std.Progress.Node) !void { } if (self.implib_dir) |implib_dir| { - const full_src_path = self.emitted_implib.?.getPath2(step.owner, step); - const full_implib_path = dest_builder.getInstallPath(implib_dir, fs.path.basename(full_src_path)); + const full_src_path = self.emitted_implib.?.getPath2(b, step); + const full_implib_path = b.getInstallPath(implib_dir, fs.path.basename(full_src_path)); const p = fs.Dir.updateFile(cwd, full_src_path, cwd, full_implib_path, .{}) catch |err| { return step.fail("unable to update file from '{s}' to '{s}': {s}", .{ full_src_path, full_implib_path, @errorName(err), @@ -156,8 +155,8 @@ fn make(step: *Step, prog_node: *std.Progress.Node) !void { } if (self.pdb_dir) |pdb_dir| { - const full_src_path = self.emitted_pdb.?.getPath2(step.owner, step); - const full_pdb_path = dest_builder.getInstallPath(pdb_dir, fs.path.basename(full_src_path)); + const full_src_path = self.emitted_pdb.?.getPath2(b, step); + const full_pdb_path = b.getInstallPath(pdb_dir, fs.path.basename(full_src_path)); const p = fs.Dir.updateFile(cwd, full_src_path, cwd, full_pdb_path, .{}) catch |err| { return step.fail("unable to update file from '{s}' to '{s}': {s}", .{ full_src_path, full_pdb_path, @errorName(err), @@ -167,14 +166,68 @@ fn make(step: *Step, prog_node: *std.Progress.Node) !void { } if (self.h_dir) |h_dir| { - const full_src_path = self.emitted_h.?.getPath2(step.owner, step); - const full_h_path = dest_builder.getInstallPath(h_dir, fs.path.basename(full_src_path)); - const p = fs.Dir.updateFile(cwd, full_src_path, cwd, full_h_path, .{}) catch |err| { - return step.fail("unable to update file from '{s}' to '{s}': {s}", .{ - full_src_path, full_h_path, @errorName(err), - }); + if (self.emitted_h) |emitted_h| { + const full_src_path = emitted_h.getPath2(b, step); + const full_h_path = b.getInstallPath(h_dir, fs.path.basename(full_src_path)); + const p = fs.Dir.updateFile(cwd, full_src_path, cwd, full_h_path, .{}) catch |err| { + return step.fail("unable to update file from '{s}' to '{s}': {s}", .{ + full_src_path, full_h_path, @errorName(err), + }); + }; + all_cached = all_cached and p == .fresh; + } + + for (self.artifact.installed_headers.items) |header| switch (header.source) { + .file => |lp| { + const full_src_path = lp.getPath2(b, step); + const full_h_path = b.getInstallPath(h_dir, header.dest_rel_path); + const p = fs.Dir.updateFile(cwd, full_src_path, cwd, full_h_path, .{}) catch |err| { + return step.fail("unable to update file from '{s}' to '{s}': {s}", .{ + full_src_path, full_h_path, @errorName(err), + }); + }; + all_cached = all_cached and p == .fresh; + }, + .directory => |dir| { + const full_src_dir_path = dir.path.getPath2(b, step); + const full_h_prefix = b.getInstallPath(h_dir, header.dest_rel_path); + + var src_dir = b.build_root.handle.openDir(full_src_dir_path, .{ .iterate = true }) catch |err| { + return step.fail("unable to open source directory '{s}': {s}", .{ + full_src_dir_path, @errorName(err), + }); + }; + defer src_dir.close(); + + var it = try src_dir.walk(b.allocator); + next_entry: while (try it.next()) |entry| { + for (dir.options.exclude_extensions) |ext| { + if (std.mem.endsWith(u8, entry.path, ext)) continue :next_entry; + } + if (dir.options.include_extensions) |incs| { + for (incs) |inc| { + if (std.mem.endsWith(u8, entry.path, inc)) break; + } else { + continue :next_entry; + } + } + const full_src_entry_path = b.pathJoin(&.{ full_src_dir_path, entry.path }); + const full_dest_path = b.pathJoin(&.{ full_h_prefix, entry.path }); + switch (entry.kind) { + .directory => try cwd.makePath(full_dest_path), + .file => { + const p = fs.Dir.updateFile(cwd, full_src_entry_path, cwd, full_dest_path, .{}) catch |err| { + return step.fail("unable to update file from '{s}' to '{s}': {s}", .{ + full_src_entry_path, full_dest_path, @errorName(err), + }); + }; + all_cached = all_cached and p == .fresh; + }, + else => continue, + } + } + }, }; - all_cached = all_cached and p == .fresh; } step.result_cached = all_cached; From ff0bec60b73a4698cda39588ec98ef06b0ecb50e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carl=20=C3=85stholm?= Date: Sat, 2 Mar 2024 23:32:01 +0100 Subject: [PATCH 02/11] Remove `dest_builder` field from `InstallDir/File` This is no longer needed after the installed headers refactoring. --- lib/std/Build/Step/InstallDir.zig | 23 +++++++++-------------- lib/std/Build/Step/InstallFile.zig | 11 +++-------- 2 files changed, 12 insertions(+), 22 deletions(-) diff --git a/lib/std/Build/Step/InstallDir.zig b/lib/std/Build/Step/InstallDir.zig index 96ab920531..f29a6ef2b8 100644 --- a/lib/std/Build/Step/InstallDir.zig +++ b/lib/std/Build/Step/InstallDir.zig @@ -8,9 +8,6 @@ const InstallDirStep = @This(); step: Step, options: Options, -/// This is used by the build system when a file being installed comes from one -/// package but is being installed by another. -dest_builder: *std.Build, pub const base_id = .install_dir; @@ -55,7 +52,6 @@ pub fn create(owner: *std.Build, options: Options) *InstallDirStep { .makeFn = make, }), .options = options.dupe(owner), - .dest_builder = owner, }; options.source_dir.addStepDependencies(&self.step); return self; @@ -63,15 +59,14 @@ pub fn create(owner: *std.Build, options: Options) *InstallDirStep { fn make(step: *Step, prog_node: *std.Progress.Node) !void { _ = prog_node; + const b = step.owner; const self: *InstallDirStep = @fieldParentPtr("step", step); - const dest_builder = self.dest_builder; - const arena = dest_builder.allocator; - const dest_prefix = dest_builder.getInstallPath(self.options.install_dir, self.options.install_subdir); - const src_builder = self.step.owner; - const src_dir_path = self.options.source_dir.getPath2(src_builder, step); - var src_dir = src_builder.build_root.handle.openDir(src_dir_path, .{ .iterate = true }) catch |err| { + const arena = b.allocator; + const dest_prefix = b.getInstallPath(self.options.install_dir, self.options.install_subdir); + const src_dir_path = self.options.source_dir.getPath2(b, step); + var src_dir = b.build_root.handle.openDir(src_dir_path, .{ .iterate = true }) catch |err| { return step.fail("unable to open source directory '{}{s}': {s}", .{ - src_builder.build_root, src_dir_path, @errorName(err), + b.build_root, src_dir_path, @errorName(err), }); }; defer src_dir.close(); @@ -104,20 +99,20 @@ fn make(step: *Step, prog_node: *std.Progress.Node) !void { .file => { for (self.options.blank_extensions) |ext| { if (mem.endsWith(u8, entry.path, ext)) { - try dest_builder.truncateFile(dest_path); + try b.truncateFile(dest_path); continue :next_entry; } } const prev_status = fs.Dir.updateFile( - src_builder.build_root.handle, + b.build_root.handle, src_sub_path, cwd, dest_path, .{}, ) catch |err| { return step.fail("unable to update file from '{}{s}' to '{s}': {s}", .{ - src_builder.build_root, src_sub_path, dest_path, @errorName(err), + b.build_root, src_sub_path, dest_path, @errorName(err), }); }; all_cached = all_cached and prev_status == .fresh; diff --git a/lib/std/Build/Step/InstallFile.zig b/lib/std/Build/Step/InstallFile.zig index ca5a986fd1..1ad9fa7d5d 100644 --- a/lib/std/Build/Step/InstallFile.zig +++ b/lib/std/Build/Step/InstallFile.zig @@ -11,9 +11,6 @@ step: Step, source: LazyPath, dir: InstallDir, dest_rel_path: []const u8, -/// This is used by the build system when a file being installed comes from one -/// package but is being installed by another. -dest_builder: *std.Build, pub fn create( owner: *std.Build, @@ -34,7 +31,6 @@ pub fn create( .source = source.dupe(owner), .dir = dir.dupe(owner), .dest_rel_path = owner.dupePath(dest_rel_path), - .dest_builder = owner, }; source.addStepDependencies(&self.step); return self; @@ -42,11 +38,10 @@ pub fn create( fn make(step: *Step, prog_node: *std.Progress.Node) !void { _ = prog_node; - const src_builder = step.owner; + const b = step.owner; const self: *InstallFile = @fieldParentPtr("step", step); - const dest_builder = self.dest_builder; - const full_src_path = self.source.getPath2(src_builder, step); - const full_dest_path = dest_builder.getInstallPath(self.dir, self.dest_rel_path); + const full_src_path = self.source.getPath2(b, step); + const full_dest_path = b.getInstallPath(self.dir, self.dest_rel_path); const cwd = std.fs.cwd(); const prev = std.fs.Dir.updateFile(cwd, full_src_path, cwd, full_dest_path, .{}) catch |err| { return step.fail("unable to update file from '{s}' to '{s}': {s}", .{ From ce71eb31dadd255b0560e4c7a837e255e0617a91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carl=20=C3=85stholm?= Date: Sat, 2 Mar 2024 23:32:44 +0100 Subject: [PATCH 03/11] Update `addInstallHeaderFile` to take a `LazyPath` --- lib/std/Build.zig | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/std/Build.zig b/lib/std/Build.zig index 5db70d5491..f0ef8b9a4a 100644 --- a/lib/std/Build.zig +++ b/lib/std/Build.zig @@ -1569,22 +1569,22 @@ pub fn addObjCopy(b: *Build, source: LazyPath, options: Step.ObjCopy.Options) *S } ///`dest_rel_path` is relative to install prefix path -pub fn addInstallFile(self: *Build, source: LazyPath, dest_rel_path: []const u8) *Step.InstallFile { - return self.addInstallFileWithDir(source.dupe(self), .prefix, dest_rel_path); +pub fn addInstallFile(b: *Build, source: LazyPath, dest_rel_path: []const u8) *Step.InstallFile { + return b.addInstallFileWithDir(source, .prefix, dest_rel_path); } ///`dest_rel_path` is relative to bin path -pub fn addInstallBinFile(self: *Build, source: LazyPath, dest_rel_path: []const u8) *Step.InstallFile { - return self.addInstallFileWithDir(source.dupe(self), .bin, dest_rel_path); +pub fn addInstallBinFile(b: *Build, source: LazyPath, dest_rel_path: []const u8) *Step.InstallFile { + return b.addInstallFileWithDir(source, .bin, dest_rel_path); } ///`dest_rel_path` is relative to lib path -pub fn addInstallLibFile(self: *Build, source: LazyPath, dest_rel_path: []const u8) *Step.InstallFile { - return self.addInstallFileWithDir(source.dupe(self), .lib, dest_rel_path); +pub fn addInstallLibFile(b: *Build, source: LazyPath, dest_rel_path: []const u8) *Step.InstallFile { + return b.addInstallFileWithDir(source, .lib, dest_rel_path); } -pub fn addInstallHeaderFile(b: *Build, src_path: []const u8, dest_rel_path: []const u8) *Step.InstallFile { - return b.addInstallFileWithDir(.{ .path = src_path }, .header, dest_rel_path); +pub fn addInstallHeaderFile(b: *Build, source: LazyPath, dest_rel_path: []const u8) *Step.InstallFile { + return b.addInstallFileWithDir(source, .header, dest_rel_path); } pub fn addInstallFileWithDir( @@ -1593,7 +1593,7 @@ pub fn addInstallFileWithDir( install_dir: InstallDir, dest_rel_path: []const u8, ) *Step.InstallFile { - return Step.InstallFile.create(self, source.dupe(self), install_dir, dest_rel_path); + return Step.InstallFile.create(self, source, install_dir, dest_rel_path); } pub fn addInstallDirectory(self: *Build, options: Step.InstallDir.Options) *Step.InstallDir { From 2c7be4f8dd55653be6cbf5d06f8f9c47e8d9276e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carl=20=C3=85stholm?= Date: Sun, 3 Mar 2024 16:22:11 +0100 Subject: [PATCH 04/11] Create an include tree of installed headers for dependent modules --- lib/std/Build/Module.zig | 17 +-- lib/std/Build/Step/Compile.zig | 180 ++++++++++++++----------- lib/std/Build/Step/InstallArtifact.zig | 12 +- 3 files changed, 113 insertions(+), 96 deletions(-) diff --git a/lib/std/Build/Module.zig b/lib/std/Build/Module.zig index b8b6560019..8937f21e88 100644 --- a/lib/std/Build/Module.zig +++ b/lib/std/Build/Module.zig @@ -265,8 +265,7 @@ fn addShallowDependencies(m: *Module, dependee: *Module) void { for (dependee.link_objects.items) |link_object| switch (link_object) { .other_step => |compile| { addStepDependencies(m, dependee, &compile.step); - for (compile.installed_headers.items) |header| - addLazyPathDependenciesOnly(m, header.source.path()); + addLazyPathDependenciesOnly(m, compile.getEmittedIncludeTree()); }, .static_path, @@ -693,14 +692,9 @@ pub fn appendZigProcessFlags( if (other.generated_h) |header| { try zig_args.appendSlice(&.{ "-isystem", std.fs.path.dirname(header.getPath()).? }); } - for (other.installed_headers.items) |header| switch (header.source) { - .file => |lp| { - try zig_args.appendSlice(&.{ "-I", std.fs.path.dirname(lp.getPath2(b, asking_step)).? }); - }, - .directory => |dir| { - try zig_args.appendSlice(&.{ "-I", dir.path.getPath2(b, asking_step) }); - }, - }; + if (other.installed_headers_include_tree) |include_tree| { + try zig_args.appendSlice(&.{ "-I", include_tree.generated_directory.getPath() }); + } }, .config_header_step => |config_header| { try zig_args.appendSlice(&.{ "-I", std.fs.path.dirname(config_header.output_file.getPath()).? }); @@ -751,8 +745,7 @@ fn linkLibraryOrObject(m: *Module, other: *Step.Compile) void { m.link_objects.append(allocator, .{ .other_step = other }) catch @panic("OOM"); m.include_dirs.append(allocator, .{ .other_step = other }) catch @panic("OOM"); - for (other.installed_headers.items) |header| - addLazyPathDependenciesOnly(m, header.source.path()); + addLazyPathDependenciesOnly(m, other.getEmittedIncludeTree()); } fn requireKnownTarget(m: *Module) std.Target { diff --git a/lib/std/Build/Step/Compile.zig b/lib/std/Build/Step/Compile.zig index caa3a9e34a..2f9821e12d 100644 --- a/lib/std/Build/Step/Compile.zig +++ b/lib/std/Build/Step/Compile.zig @@ -59,7 +59,13 @@ test_runner: ?[]const u8, test_server_mode: bool, wasi_exec_model: ?std.builtin.WasiExecModel = null, -installed_headers: ArrayList(InstalledHeader), +installed_headers: ArrayList(HeaderInstallation), + +/// This step is used to create an include tree that dependent modules can add to their include +/// search paths. Installed headers are copied to this step. +/// This step is created the first time a module links with this artifact and is not +/// created otherwise. +installed_headers_include_tree: ?*Step.WriteFile = null, // keep in sync with src/Compilation.zig:RcIncludes /// Behavior of automatic detection of include directories when compiling .rc files. @@ -249,66 +255,62 @@ pub const Kind = enum { @"test", }; -pub const InstalledHeader = struct { - source: Source, - dest_rel_path: []const u8, +pub const HeaderInstallation = union(enum) { + file: File, + directory: Directory, - pub const Source = union(enum) { - file: LazyPath, - directory: Directory, + pub const File = struct { + source: LazyPath, + dest_rel_path: []const u8, - pub const Directory = struct { - path: LazyPath, - options: Directory.Options, - - pub const Options = struct { - /// File paths which end in any of these suffixes will be excluded - /// from installation. - exclude_extensions: []const []const u8 = &.{}, - /// Only file paths which end in any of these suffixes will be included - /// in installation. - /// `null` means all suffixes will be included. - /// `exclude_extensions` takes precedence over `include_extensions` - include_extensions: ?[]const []const u8 = &.{".h"}, - - pub fn dupe(self: Directory.Options, b: *std.Build) Directory.Options { - return .{ - .exclude_extensions = b.dupeStrings(self.exclude_extensions), - .include_extensions = if (self.include_extensions) |incs| - b.dupeStrings(incs) - else - null, - }; - } - }; - - pub fn dupe(self: Directory, b: *std.Build) Directory { - return .{ - .path = self.path.dupe(b), - .options = self.options.dupe(b), - }; - } - }; - - pub fn path(self: Source) LazyPath { - return switch (self) { - .file => |lp| lp, - .directory => |dir| dir.path, - }; - } - - pub fn dupe(self: Source, b: *std.Build) Source { - return switch (self) { - .file => |lp| .{ .file = lp.dupe(b) }, - .directory => |dir| .{ .directory = dir.dupe(b) }, + pub fn dupe(self: File, b: *std.Build) File { + return .{ + .source = self.source.dupe(b), + .dest_rel_path = b.dupePath(self.dest_rel_path), }; } }; - pub fn dupe(self: InstalledHeader, b: *std.Build) InstalledHeader { - return .{ - .source = self.source.dupe(b), - .dest_rel_path = b.dupePath(self.dest_rel_path), + pub const Directory = struct { + source: LazyPath, + dest_rel_path: []const u8, + options: Directory.Options, + + pub const Options = struct { + /// File paths that end in any of these suffixes will be excluded from installation. + exclude_extensions: []const []const u8 = &.{}, + /// Only file paths that end in any of these suffixes will be included in installation. + /// `null` means that all suffixes will be included. + /// `exclude_extensions` takes precedence over `include_extensions`. + include_extensions: ?[]const []const u8 = &.{".h"}, + + pub fn dupe(self: Directory.Options, b: *std.Build) Directory.Options { + return .{ + .exclude_extensions = b.dupeStrings(self.exclude_extensions), + .include_extensions = if (self.include_extensions) |incs| b.dupeStrings(incs) else null, + }; + } + }; + + pub fn dupe(self: Directory, b: *std.Build) Directory { + return .{ + .source = self.source.dupe(b), + .dest_rel_path = b.dupePath(self.dest_rel_path), + .options = self.options.dupe(b), + }; + } + }; + + pub fn getSource(self: HeaderInstallation) LazyPath { + return switch (self) { + inline .file, .directory => |x| x.source, + }; + } + + pub fn dupe(self: HeaderInstallation, b: *std.Build) HeaderInstallation { + return switch (self) { + .file => |f| f.dupe(b), + .directory => |d| d.dupe(b), }; } }; @@ -372,7 +374,7 @@ pub fn create(owner: *std.Build, options: Options) *Compile { .out_lib_filename = undefined, .major_only_filename = null, .name_only_filename = null, - .installed_headers = ArrayList(InstalledHeader).init(owner.allocator), + .installed_headers = ArrayList(HeaderInstallation).init(owner.allocator), .zig_lib_dir = null, .exec_cmd_args = null, .filters = options.filters, @@ -444,49 +446,71 @@ pub fn create(owner: *std.Build, options: Options) *Compile { return self; } -pub fn installHeader( - cs: *Compile, - source: LazyPath, - dest_rel_path: []const u8, -) void { +pub fn installHeader(cs: *Compile, source: LazyPath, dest_rel_path: []const u8) void { const b = cs.step.owner; - cs.installed_headers.append(.{ - .source = .{ .file = source.dupe(b) }, + const installation: HeaderInstallation = .{ .file = .{ + .source = source.dupe(b), .dest_rel_path = b.dupePath(dest_rel_path), - }) catch @panic("OOM"); - source.addStepDependencies(&cs.step); + } }; + cs.installed_headers.append(installation) catch @panic("OOM"); + cs.addHeaderInstallationToIncludeTree(installation); + installation.getSource().addStepDependencies(&cs.step); } pub fn installHeaders( cs: *Compile, source: LazyPath, dest_rel_path: []const u8, - options: InstalledHeader.Source.Directory.Options, + options: HeaderInstallation.Directory.Options, ) void { const b = cs.step.owner; - cs.installed_headers.append(.{ - .source = .{ .directory = .{ - .path = source.dupe(b), - .options = options.dupe(b), - } }, + const installation: HeaderInstallation = .{ .directory = .{ + .source = source.dupe(b), .dest_rel_path = b.dupePath(dest_rel_path), - }) catch @panic("OOM"); - source.addStepDependencies(&cs.step); + .options = options.dupe(b), + } }; + cs.installed_headers.append(installation) catch @panic("OOM"); + cs.addHeaderInstallationToIncludeTree(installation); + installation.getSource().addStepDependencies(&cs.step); } pub fn installConfigHeader(cs: *Compile, config_header: *Step.ConfigHeader) void { - cs.installHeader(.{ .generated = &config_header.output_file }, config_header.include_path); + cs.installHeader(config_header.getOutput(), config_header.include_path); } pub fn installLibraryHeaders(cs: *Compile, lib: *Compile) void { assert(lib.kind == .lib); - const b = cs.step.owner; - for (lib.installed_headers.items) |header| { - cs.installed_headers.append(header.dupe(b)) catch @panic("OOM"); - header.source.path().addStepDependencies(&cs.step); + for (lib.installed_headers.items) |installation| { + cs.installed_headers.append(installation) catch @panic("OOM"); + cs.addHeaderInstallationToIncludeTree(installation); + installation.getSource().addStepDependencies(&cs.step); } } +fn addHeaderInstallationToIncludeTree(cs: *Compile, installation: HeaderInstallation) void { + if (cs.installed_headers_include_tree) |wf| switch (installation) { + .file => |file| { + _ = wf.addCopyFile(file.source, file.dest_rel_path); + }, + .directory => |dir| { + _ = dir; // TODO + }, + }; +} + +pub fn getEmittedIncludeTree(cs: *Compile) LazyPath { + if (cs.installed_headers_include_tree) |wf| return wf.getDirectory(); + const b = cs.step.owner; + const wf = b.addWriteFiles(); + cs.installed_headers_include_tree = wf; + for (cs.installed_headers.items) |installation| { + cs.addHeaderInstallationToIncludeTree(installation); + } + // The compile step itself does not need to depend on the write files step, + // only dependent modules do. + return wf.getDirectory(); +} + pub fn addObjCopy(cs: *Compile, options: Step.ObjCopy.Options) *Step.ObjCopy { const b = cs.step.owner; var copy = options; diff --git a/lib/std/Build/Step/InstallArtifact.zig b/lib/std/Build/Step/InstallArtifact.zig index 5f77487823..7ebe8fdaf0 100644 --- a/lib/std/Build/Step/InstallArtifact.zig +++ b/lib/std/Build/Step/InstallArtifact.zig @@ -177,10 +177,10 @@ fn make(step: *Step, prog_node: *std.Progress.Node) !void { all_cached = all_cached and p == .fresh; } - for (self.artifact.installed_headers.items) |header| switch (header.source) { - .file => |lp| { - const full_src_path = lp.getPath2(b, step); - const full_h_path = b.getInstallPath(h_dir, header.dest_rel_path); + for (self.artifact.installed_headers.items) |installation| switch (installation) { + .file => |file| { + const full_src_path = file.source.getPath2(b, step); + const full_h_path = b.getInstallPath(h_dir, file.dest_rel_path); const p = fs.Dir.updateFile(cwd, full_src_path, cwd, full_h_path, .{}) catch |err| { return step.fail("unable to update file from '{s}' to '{s}': {s}", .{ full_src_path, full_h_path, @errorName(err), @@ -189,8 +189,8 @@ fn make(step: *Step, prog_node: *std.Progress.Node) !void { all_cached = all_cached and p == .fresh; }, .directory => |dir| { - const full_src_dir_path = dir.path.getPath2(b, step); - const full_h_prefix = b.getInstallPath(h_dir, header.dest_rel_path); + const full_src_dir_path = dir.source.getPath2(b, step); + const full_h_prefix = b.getInstallPath(h_dir, dir.dest_rel_path); var src_dir = b.build_root.handle.openDir(full_src_dir_path, .{ .iterate = true }) catch |err| { return step.fail("unable to open source directory '{s}': {s}", .{ From e16db2988777e18c56925b4a458914b20a183c12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carl=20=C3=85stholm?= Date: Sun, 3 Mar 2024 17:14:02 +0100 Subject: [PATCH 05/11] Implement `WriteFile.addCopyDirectory` --- lib/std/Build/Step/Compile.zig | 5 +- lib/std/Build/Step/WriteFile.zig | 131 ++++++++++++++++++++++++++++++- 2 files changed, 131 insertions(+), 5 deletions(-) diff --git a/lib/std/Build/Step/Compile.zig b/lib/std/Build/Step/Compile.zig index 2f9821e12d..c046619b0b 100644 --- a/lib/std/Build/Step/Compile.zig +++ b/lib/std/Build/Step/Compile.zig @@ -493,7 +493,10 @@ fn addHeaderInstallationToIncludeTree(cs: *Compile, installation: HeaderInstalla _ = wf.addCopyFile(file.source, file.dest_rel_path); }, .directory => |dir| { - _ = dir; // TODO + _ = wf.addCopyDirectory(dir.source, dir.dest_rel_path, .{ + .exclude_extensions = dir.options.exclude_extensions, + .include_extensions = dir.options.include_extensions, + }); }, }; } diff --git a/lib/std/Build/Step/WriteFile.zig b/lib/std/Build/Step/WriteFile.zig index d0ac68377a..2ed7b1344c 100644 --- a/lib/std/Build/Step/WriteFile.zig +++ b/lib/std/Build/Step/WriteFile.zig @@ -15,9 +15,11 @@ const ArrayList = std.ArrayList; const WriteFile = @This(); step: Step, -/// The elements here are pointers because we need stable pointers for the -/// GeneratedFile field. + +// The elements here are pointers because we need stable pointers for the GeneratedFile field. files: std.ArrayListUnmanaged(*File), +directories: std.ArrayListUnmanaged(*Directory), + output_source_files: std.ArrayListUnmanaged(OutputSourceFile), generated_directory: std.Build.GeneratedFile, @@ -33,6 +35,33 @@ pub const File = struct { } }; +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. + exclude_extensions: []const []const u8 = &.{}, + /// Only file paths that end in any of these suffixes will be included in copying. + /// `null` means that all suffixes will be included. + /// `exclude_extensions` takes precedence over `include_extensions`. + include_extensions: ?[]const []const u8 = &.{".h"}, + + pub fn dupe(self: Options, b: *std.Build) Options { + return .{ + .exclude_extensions = b.dupeStrings(self.exclude_extensions), + .include_extensions = if (self.include_extensions) |incs| b.dupeStrings(incs) else null, + }; + } + }; + + pub fn getPath(self: *Directory) std.Build.LazyPath { + return .{ .generated = &self.generated_dir }; + } +}; + pub const OutputSourceFile = struct { contents: Contents, sub_path: []const u8, @@ -53,6 +82,7 @@ pub fn create(owner: *std.Build) *WriteFile { .makeFn = make, }), .files = .{}, + .directories = .{}, .output_source_files = .{}, .generated_directory = .{ .step = &wf.step }, }; @@ -96,6 +126,28 @@ pub fn addCopyFile(wf: *WriteFile, source: std.Build.LazyPath, sub_path: []const return file.getPath(); } +pub fn addCopyDirectory( + wf: *WriteFile, + source: std.Build.LazyPath, + sub_path: []const u8, + options: Directory.Options, +) std.Build.LazyPath { + const b = wf.step.owner; + const gpa = b.allocator; + const dir = gpa.create(Directory) catch @panic("OOM"); + dir.* = .{ + .source = source.dupe(b), + .sub_path = b.dupePath(sub_path), + .options = options.dupe(b), + .generated_dir = .{ .step = &wf.step }, + }; + wf.directories.append(gpa, dir) catch @panic("OOM"); + + wf.maybeUpdateName(); + source.addStepDependencies(&wf.step); + return dir.getPath(); +} + /// A path relative to the package root. /// Be careful with this because it updates source files. This should not be /// used as part of the normal build process, but as a utility occasionally @@ -130,11 +182,16 @@ pub fn getDirectory(wf: *WriteFile) std.Build.LazyPath { } fn maybeUpdateName(wf: *WriteFile) void { - if (wf.files.items.len == 1) { + if (wf.files.items.len == 1 and wf.directories.items.len == 0) { // First time adding a file; update name. if (std.mem.eql(u8, wf.step.name, "WriteFile")) { wf.step.name = wf.step.owner.fmt("WriteFile {s}", .{wf.files.items[0].sub_path}); } + } else if (wf.directories.items.len == 1 and wf.files.items.len == 0) { + // First time adding a directory; update name. + if (std.mem.eql(u8, wf.step.name, "WriteFile")) { + wf.step.name = wf.step.owner.fmt("WriteFile {s}", .{wf.directories.items[0].sub_path}); + } } } @@ -209,6 +266,12 @@ fn make(step: *Step, prog_node: *std.Progress.Node) !void { }, } } + for (wf.directories.items) |dir| { + man.hash.addBytes(dir.source.getPath2(b, step)); + man.hash.addBytes(dir.sub_path); + for (dir.options.exclude_extensions) |ext| man.hash.addBytes(ext); + if (dir.options.include_extensions) |incs| for (incs) |inc| man.hash.addBytes(inc); + } if (try step.cacheHit(&man)) { const digest = man.final(); @@ -233,6 +296,8 @@ fn make(step: *Step, prog_node: *std.Progress.Node) !void { }; defer cache_dir.close(); + const cwd = fs.cwd(); + for (wf.files.items) |file| { if (fs.path.dirname(file.sub_path)) |dirname| { cache_dir.makePath(dirname) catch |err| { @@ -252,7 +317,7 @@ fn make(step: *Step, prog_node: *std.Progress.Node) !void { .copy => |file_source| { const source_path = file_source.getPath(b); const prev_status = fs.Dir.updateFile( - fs.cwd(), + cwd, source_path, cache_dir, file.sub_path, @@ -279,6 +344,64 @@ fn make(step: *Step, prog_node: *std.Progress.Node) !void { cache_path, file.sub_path, }); } + for (wf.directories.items) |dir| { + const full_src_dir_path = dir.source.getPath2(b, step); + const dest_dirname = dir.sub_path; + + if (dest_dirname.len != 0) { + cache_dir.makePath(dest_dirname) catch |err| { + return step.fail("unable to make path '{}{s}{c}{s}': {s}", .{ + b.cache_root, cache_path, fs.path.sep, dest_dirname, @errorName(err), + }); + }; + } + + var src_dir = b.build_root.handle.openDir(full_src_dir_path, .{ .iterate = true }) catch |err| { + return step.fail("unable to open source directory '{s}': {s}", .{ + full_src_dir_path, @errorName(err), + }); + }; + defer src_dir.close(); + + var it = try src_dir.walk(b.allocator); + next_entry: while (try it.next()) |entry| { + for (dir.options.exclude_extensions) |ext| { + if (std.mem.endsWith(u8, entry.path, ext)) continue :next_entry; + } + if (dir.options.include_extensions) |incs| { + for (incs) |inc| { + if (std.mem.endsWith(u8, entry.path, inc)) break; + } else { + continue :next_entry; + } + } + const full_src_entry_path = b.pathJoin(&.{ full_src_dir_path, entry.path }); + const dest_path = b.pathJoin(&.{ dest_dirname, entry.path }); + switch (entry.kind) { + .directory => try cache_dir.makePath(dest_path), + .file => { + const prev_status = fs.Dir.updateFile( + cwd, + full_src_entry_path, + cache_dir, + dest_path, + .{}, + ) catch |err| { + return step.fail("unable to update file from '{s}' to '{}{s}{c}{s}': {s}", .{ + full_src_entry_path, + b.cache_root, + cache_path, + fs.path.sep, + dest_path, + @errorName(err), + }); + }; + _ = prev_status; + }, + else => continue, + } + } + } try step.writeManifest(&man); } From 27c8f895ebeed9f124d32f6a96eaea92e06825bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carl=20=C3=85stholm?= Date: Sun, 3 Mar 2024 19:55:41 +0100 Subject: [PATCH 06/11] Add standalone tests for `Compile.installHeaders` Co-authored-by: Abhinav Gupta --- test/standalone.zig | 4 + test/standalone/install_headers/build.zig | 83 +++++++++++++++++++ .../install_headers/check_exists.zig | 37 +++++++++ test/standalone/install_headers/include/a.h | 1 + .../install_headers/include/ignore_me.txt | 1 + .../install_headers/include/sub_dir/b.h | 1 + .../include/sub_dir/ignore_me.h | 1 + 7 files changed, 128 insertions(+) create mode 100644 test/standalone/install_headers/build.zig create mode 100644 test/standalone/install_headers/check_exists.zig create mode 100644 test/standalone/install_headers/include/a.h create mode 100644 test/standalone/install_headers/include/ignore_me.txt create mode 100644 test/standalone/install_headers/include/sub_dir/b.h create mode 100644 test/standalone/install_headers/include/sub_dir/ignore_me.h diff --git a/test/standalone.zig b/test/standalone.zig index 495468eb24..2f1b7d1339 100644 --- a/test/standalone.zig +++ b/test/standalone.zig @@ -266,6 +266,10 @@ pub const build_cases = [_]BuildCase{ .build_root = "test/standalone/depend_on_main_mod", .import = @import("standalone/depend_on_main_mod/build.zig"), }, + .{ + .build_root = "test/standalone/install_headers", + .import = @import("standalone/install_headers/build.zig"), + }, }; const std = @import("std"); diff --git a/test/standalone/install_headers/build.zig b/test/standalone/install_headers/build.zig new file mode 100644 index 0000000000..20fab2aaa6 --- /dev/null +++ b/test/standalone/install_headers/build.zig @@ -0,0 +1,83 @@ +const std = @import("std"); + +pub fn build(b: *std.Build) void { + const test_step = b.step("test", "Test"); + b.default_step = test_step; + + const libfoo = b.addStaticLibrary(.{ + .name = "foo", + .target = b.resolveTargetQuery(.{}), + .optimize = .Debug, + }); + libfoo.addCSourceFile(.{ .file = b.addWriteFiles().add("empty.c", "") }); + + const exe = b.addExecutable(.{ + .name = "exe", + .target = b.resolveTargetQuery(.{}), + .optimize = .Debug, + .link_libc = true, + }); + exe.addCSourceFile(.{ .file = b.addWriteFiles().add("main.c", + \\#include + \\#include + \\#include + \\#include + \\#include + \\int main(void) { + \\ printf(FOO_A FOO_B FOO_D FOO_CONFIG_1 FOO_CONFIG_2); + \\ return 0; + \\} + ) }); + + libfoo.installHeaders(.{ .path = "include" }, "foo", .{ .exclude_extensions = &.{".ignore_me.h"} }); + libfoo.installHeader(b.addWriteFiles().add("d.h", + \\#define FOO_D "D" + \\ + ), "foo/d.h"); + + if (libfoo.installed_headers_include_tree != null) std.debug.panic("include tree step was created before linking", .{}); + + // Link before we have registered all headers for installation, + // to verify that the lazily created write files step is properly taken into account. + exe.linkLibrary(libfoo); + + if (libfoo.installed_headers_include_tree == null) std.debug.panic("include tree step was not created after linking", .{}); + + libfoo.installConfigHeader(b.addConfigHeader(.{ + .style = .blank, + .include_path = "foo/config.h", + }, .{ + .FOO_CONFIG_1 = "1", + .FOO_CONFIG_2 = "2", + })); + + const run_exe = b.addRunArtifact(exe); + run_exe.expectStdOutEqual("ABD12"); + test_step.dependOn(&run_exe.step); + + const install_exe = b.addInstallArtifact(libfoo, .{ + .dest_dir = .{ .override = .{ .custom = "custom" } }, + .h_dir = .{ .override = .{ .custom = "custom/include" } }, + .implib_dir = .disabled, + .pdb_dir = .disabled, + }); + const check_exists = b.addExecutable(.{ + .name = "check_exists", + .root_source_file = .{ .path = "check_exists.zig" }, + .target = b.resolveTargetQuery(.{}), + .optimize = .Debug, + }); + const run_check_exists = b.addRunArtifact(check_exists); + run_check_exists.addArgs(&.{ + "custom/include/foo/a.h", + "!custom/include/foo/ignore_me.txt", + "custom/include/foo/sub_dir/b.h", + "!custom/include/foo/sub_dir/c.ignore_me.h", + "custom/include/foo/d.h", + "custom/include/foo/config.h", + }); + run_check_exists.setCwd(.{ .cwd_relative = b.getInstallPath(.prefix, "") }); + run_check_exists.expectExitCode(0); + run_check_exists.step.dependOn(&install_exe.step); + test_step.dependOn(&run_check_exists.step); +} diff --git a/test/standalone/install_headers/check_exists.zig b/test/standalone/install_headers/check_exists.zig new file mode 100644 index 0000000000..da07af1028 --- /dev/null +++ b/test/standalone/install_headers/check_exists.zig @@ -0,0 +1,37 @@ +const std = @import("std"); + +/// Checks the existence of files relative to cwd. +/// A path starting with ! should not exist. +pub fn main() !void { + var arena_state = std.heap.ArenaAllocator.init(std.heap.page_allocator); + defer arena_state.deinit(); + + const arena = arena_state.allocator(); + + var arg_it = try std.process.argsWithAllocator(arena); + _ = arg_it.next(); + + const cwd = std.fs.cwd(); + const cwd_realpath = try cwd.realpathAlloc(arena, ""); + + while (arg_it.next()) |file_path| { + if (file_path.len > 0 and file_path[0] == '!') { + errdefer std.log.err( + "excluded file check '{s}{c}{s}' failed", + .{ cwd_realpath, std.fs.path.sep, file_path[1..] }, + ); + if (std.fs.cwd().statFile(file_path[1..])) |_| { + return error.FileFound; + } else |err| switch (err) { + error.FileNotFound => {}, + else => return err, + } + } else { + errdefer std.log.err( + "included file check '{s}{c}{s}' failed", + .{ cwd_realpath, std.fs.path.sep, file_path }, + ); + _ = try std.fs.cwd().statFile(file_path); + } + } +} diff --git a/test/standalone/install_headers/include/a.h b/test/standalone/install_headers/include/a.h new file mode 100644 index 0000000000..570548abb7 --- /dev/null +++ b/test/standalone/install_headers/include/a.h @@ -0,0 +1 @@ +#define FOO_A "A" diff --git a/test/standalone/install_headers/include/ignore_me.txt b/test/standalone/install_headers/include/ignore_me.txt new file mode 100644 index 0000000000..592fd2594b --- /dev/null +++ b/test/standalone/install_headers/include/ignore_me.txt @@ -0,0 +1 @@ +ignore me diff --git a/test/standalone/install_headers/include/sub_dir/b.h b/test/standalone/install_headers/include/sub_dir/b.h new file mode 100644 index 0000000000..f16019c96f --- /dev/null +++ b/test/standalone/install_headers/include/sub_dir/b.h @@ -0,0 +1 @@ +#define FOO_B "B" diff --git a/test/standalone/install_headers/include/sub_dir/ignore_me.h b/test/standalone/install_headers/include/sub_dir/ignore_me.h new file mode 100644 index 0000000000..11e5ba752b --- /dev/null +++ b/test/standalone/install_headers/include/sub_dir/ignore_me.h @@ -0,0 +1 @@ +#error "ignore me" From 8ce3a8b6041d2dd2eea1e9fcb55bc9d602469667 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carl=20=C3=85stholm?= Date: Sun, 3 Mar 2024 20:20:25 +0100 Subject: [PATCH 07/11] `WriteFile.addCopyDirectory` should include all files by default --- lib/std/Build/Step/WriteFile.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/std/Build/Step/WriteFile.zig b/lib/std/Build/Step/WriteFile.zig index 2ed7b1344c..f0ff6cfa70 100644 --- a/lib/std/Build/Step/WriteFile.zig +++ b/lib/std/Build/Step/WriteFile.zig @@ -47,7 +47,7 @@ pub const Directory = struct { /// Only file paths that end in any of these suffixes will be included in copying. /// `null` means that all suffixes will be included. /// `exclude_extensions` takes precedence over `include_extensions`. - include_extensions: ?[]const []const u8 = &.{".h"}, + include_extensions: ?[]const []const u8 = null, pub fn dupe(self: Options, b: *std.Build) Options { return .{ From 7b1a6a93a4e94ee859ab2a41c7865b7859d19e62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carl=20=C3=85stholm?= Date: Sun, 3 Mar 2024 22:13:38 +0100 Subject: [PATCH 08/11] Fix install_headers test on macOS (and possibly Linux) --- test/standalone/install_headers/check_exists.zig | 6 +++--- .../include/sub_dir/{ignore_me.h => c.ignore_me.h} | 0 2 files changed, 3 insertions(+), 3 deletions(-) rename test/standalone/install_headers/include/sub_dir/{ignore_me.h => c.ignore_me.h} (100%) diff --git a/test/standalone/install_headers/check_exists.zig b/test/standalone/install_headers/check_exists.zig index da07af1028..62706749aa 100644 --- a/test/standalone/install_headers/check_exists.zig +++ b/test/standalone/install_headers/check_exists.zig @@ -12,12 +12,12 @@ pub fn main() !void { _ = arg_it.next(); const cwd = std.fs.cwd(); - const cwd_realpath = try cwd.realpathAlloc(arena, ""); + const cwd_realpath = try cwd.realpathAlloc(arena, "."); while (arg_it.next()) |file_path| { if (file_path.len > 0 and file_path[0] == '!') { errdefer std.log.err( - "excluded file check '{s}{c}{s}' failed", + "exclusive file check '{s}{c}{s}' failed", .{ cwd_realpath, std.fs.path.sep, file_path[1..] }, ); if (std.fs.cwd().statFile(file_path[1..])) |_| { @@ -28,7 +28,7 @@ pub fn main() !void { } } else { errdefer std.log.err( - "included file check '{s}{c}{s}' failed", + "inclusive file check '{s}{c}{s}' failed", .{ cwd_realpath, std.fs.path.sep, file_path }, ); _ = try std.fs.cwd().statFile(file_path); diff --git a/test/standalone/install_headers/include/sub_dir/ignore_me.h b/test/standalone/install_headers/include/sub_dir/c.ignore_me.h similarity index 100% rename from test/standalone/install_headers/include/sub_dir/ignore_me.h rename to test/standalone/install_headers/include/sub_dir/c.ignore_me.h From 5af4e91e0036a5d83889d20ee1477a366c6c12ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carl=20=C3=85stholm?= Date: Mon, 4 Mar 2024 01:38:29 +0100 Subject: [PATCH 09/11] Oops, forgot to dupe installations in `installLibraryHeaders` Added test coverage for `installLibraryHeaders` --- lib/std/Build/Step/Compile.zig | 12 +++++++----- test/standalone/install_headers/build.zig | 22 +++++++++++++++++++--- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/lib/std/Build/Step/Compile.zig b/lib/std/Build/Step/Compile.zig index c046619b0b..d7e8ec8ed3 100644 --- a/lib/std/Build/Step/Compile.zig +++ b/lib/std/Build/Step/Compile.zig @@ -309,8 +309,8 @@ pub const HeaderInstallation = union(enum) { pub fn dupe(self: HeaderInstallation, b: *std.Build) HeaderInstallation { return switch (self) { - .file => |f| f.dupe(b), - .directory => |d| d.dupe(b), + .file => |f| .{ .file = f.dupe(b) }, + .directory => |d| .{ .directory = d.dupe(b) }, }; } }; @@ -480,10 +480,12 @@ pub fn installConfigHeader(cs: *Compile, config_header: *Step.ConfigHeader) void pub fn installLibraryHeaders(cs: *Compile, lib: *Compile) void { assert(lib.kind == .lib); + const b = cs.step.owner; for (lib.installed_headers.items) |installation| { - cs.installed_headers.append(installation) catch @panic("OOM"); - cs.addHeaderInstallationToIncludeTree(installation); - installation.getSource().addStepDependencies(&cs.step); + const installation_copy = installation.dupe(b); + cs.installed_headers.append(installation_copy) catch @panic("OOM"); + cs.addHeaderInstallationToIncludeTree(installation_copy); + installation_copy.getSource().addStepDependencies(&cs.step); } } diff --git a/test/standalone/install_headers/build.zig b/test/standalone/install_headers/build.zig index 20fab2aaa6..69206fb59d 100644 --- a/test/standalone/install_headers/build.zig +++ b/test/standalone/install_headers/build.zig @@ -4,12 +4,14 @@ pub fn build(b: *std.Build) void { const test_step = b.step("test", "Test"); b.default_step = test_step; + const empty_c = b.addWriteFiles().add("empty.c", ""); + const libfoo = b.addStaticLibrary(.{ .name = "foo", .target = b.resolveTargetQuery(.{}), .optimize = .Debug, }); - libfoo.addCSourceFile(.{ .file = b.addWriteFiles().add("empty.c", "") }); + libfoo.addCSourceFile(.{ .file = empty_c }); const exe = b.addExecutable(.{ .name = "exe", @@ -23,8 +25,9 @@ pub fn build(b: *std.Build) void { \\#include \\#include \\#include + \\#include \\int main(void) { - \\ printf(FOO_A FOO_B FOO_D FOO_CONFIG_1 FOO_CONFIG_2); + \\ printf(FOO_A FOO_B FOO_D FOO_CONFIG_1 FOO_CONFIG_2 BAR_X); \\ return 0; \\} ) }); @@ -51,8 +54,20 @@ pub fn build(b: *std.Build) void { .FOO_CONFIG_2 = "2", })); + const libbar = b.addStaticLibrary(.{ + .name = "bar", + .target = b.resolveTargetQuery(.{}), + .optimize = .Debug, + }); + libbar.addCSourceFile(.{ .file = empty_c }); + libbar.installHeader(b.addWriteFiles().add("bar.h", + \\#define BAR_X "X" + \\ + ), "bar.h"); + libfoo.installLibraryHeaders(libbar); + const run_exe = b.addRunArtifact(exe); - run_exe.expectStdOutEqual("ABD12"); + run_exe.expectStdOutEqual("ABD12X"); test_step.dependOn(&run_exe.step); const install_exe = b.addInstallArtifact(libfoo, .{ @@ -75,6 +90,7 @@ pub fn build(b: *std.Build) void { "!custom/include/foo/sub_dir/c.ignore_me.h", "custom/include/foo/d.h", "custom/include/foo/config.h", + "custom/include/bar.h", }); run_check_exists.setCwd(.{ .cwd_relative = b.getInstallPath(.prefix, "") }); run_check_exists.expectExitCode(0); From d99e44a157dee9204c38e66bf7eba00c051690ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carl=20=C3=85stholm?= Date: Sun, 10 Mar 2024 12:17:33 +0100 Subject: [PATCH 10/11] Document added/updated functions Also renames `addHeaders` to `addHeadersDirectory` for clarity. --- lib/std/Build.zig | 7 ++++--- lib/std/Build/Step/Compile.zig | 14 +++++++++++++- lib/std/Build/Step/WriteFile.zig | 3 +++ test/standalone/install_headers/build.zig | 2 +- 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/lib/std/Build.zig b/lib/std/Build.zig index f0ef8b9a4a..50aa5c504c 100644 --- a/lib/std/Build.zig +++ b/lib/std/Build.zig @@ -1568,21 +1568,22 @@ pub fn addObjCopy(b: *Build, source: LazyPath, options: Step.ObjCopy.Options) *S return Step.ObjCopy.create(b, source, options); } -///`dest_rel_path` is relative to install prefix path +/// `dest_rel_path` is relative to install prefix path pub fn addInstallFile(b: *Build, source: LazyPath, dest_rel_path: []const u8) *Step.InstallFile { return b.addInstallFileWithDir(source, .prefix, dest_rel_path); } -///`dest_rel_path` is relative to bin path +/// `dest_rel_path` is relative to bin path pub fn addInstallBinFile(b: *Build, source: LazyPath, dest_rel_path: []const u8) *Step.InstallFile { return b.addInstallFileWithDir(source, .bin, dest_rel_path); } -///`dest_rel_path` is relative to lib path +/// `dest_rel_path` is relative to lib path pub fn addInstallLibFile(b: *Build, source: LazyPath, dest_rel_path: []const u8) *Step.InstallFile { return b.addInstallFileWithDir(source, .lib, dest_rel_path); } +/// `dest_rel_path` is relative to header path pub fn addInstallHeaderFile(b: *Build, source: LazyPath, dest_rel_path: []const u8) *Step.InstallFile { return b.addInstallFileWithDir(source, .header, dest_rel_path); } diff --git a/lib/std/Build/Step/Compile.zig b/lib/std/Build/Step/Compile.zig index d7e8ec8ed3..2883992c73 100644 --- a/lib/std/Build/Step/Compile.zig +++ b/lib/std/Build/Step/Compile.zig @@ -446,6 +446,9 @@ pub fn create(owner: *std.Build, options: Options) *Compile { return self; } +/// Marks the specified header for installation alongside this artifact. +/// When a module links with this artifact, all headers marked for installation are added to that +/// module's include search path. pub fn installHeader(cs: *Compile, source: LazyPath, dest_rel_path: []const u8) void { const b = cs.step.owner; const installation: HeaderInstallation = .{ .file = .{ @@ -457,7 +460,10 @@ pub fn installHeader(cs: *Compile, source: LazyPath, dest_rel_path: []const u8) installation.getSource().addStepDependencies(&cs.step); } -pub fn installHeaders( +/// Marks headers from the specified directory for installation alongside this artifact. +/// When a module links with this artifact, all headers marked for installation are added to that +/// module's include search path. +pub fn installHeadersDirectory( cs: *Compile, source: LazyPath, dest_rel_path: []const u8, @@ -474,10 +480,16 @@ pub fn installHeaders( installation.getSource().addStepDependencies(&cs.step); } +/// Marks the specified config header for installation alongside this artifact. +/// When a module links with this artifact, all headers marked for installation are added to that +/// module's include search path. pub fn installConfigHeader(cs: *Compile, config_header: *Step.ConfigHeader) void { cs.installHeader(config_header.getOutput(), config_header.include_path); } +/// Forwards all headers marked for installation from `lib` to this artifact. +/// When a module links with this artifact, all headers marked for installation are added to that +/// module's include search path. pub fn installLibraryHeaders(cs: *Compile, lib: *Compile) void { assert(lib.kind == .lib); const b = cs.step.owner; diff --git a/lib/std/Build/Step/WriteFile.zig b/lib/std/Build/Step/WriteFile.zig index f0ff6cfa70..310decdfe7 100644 --- a/lib/std/Build/Step/WriteFile.zig +++ b/lib/std/Build/Step/WriteFile.zig @@ -126,6 +126,9 @@ pub fn addCopyFile(wf: *WriteFile, source: std.Build.LazyPath, sub_path: []const return file.getPath(); } +/// Copy files matching the specified exclude/include patterns to the specified subdirectory +/// relative to this step's generated directory. +/// The returned value is a lazy path to the generated subdirectory. pub fn addCopyDirectory( wf: *WriteFile, source: std.Build.LazyPath, diff --git a/test/standalone/install_headers/build.zig b/test/standalone/install_headers/build.zig index 69206fb59d..965239aac5 100644 --- a/test/standalone/install_headers/build.zig +++ b/test/standalone/install_headers/build.zig @@ -32,7 +32,7 @@ pub fn build(b: *std.Build) void { \\} ) }); - libfoo.installHeaders(.{ .path = "include" }, "foo", .{ .exclude_extensions = &.{".ignore_me.h"} }); + libfoo.installHeadersDirectory(.{ .path = "include" }, "foo", .{ .exclude_extensions = &.{".ignore_me.h"} }); libfoo.installHeader(b.addWriteFiles().add("d.h", \\#define FOO_D "D" \\ From eee5400b7dc37845ea5f42e0841320953e7852b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carl=20=C3=85stholm?= Date: Fri, 15 Mar 2024 19:59:02 +0100 Subject: [PATCH 11/11] Account for dependency boundaries when duping headers This is a temporary workaround that can be revered if/when 'path' lazy paths are updated to encode which build root they are relative to. --- lib/std/Build/Step/Compile.zig | 31 ++++++++++++++++++++--- test/standalone/install_headers/build.zig | 4 +-- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/lib/std/Build/Step/Compile.zig b/lib/std/Build/Step/Compile.zig index 2883992c73..4a5364176a 100644 --- a/lib/std/Build/Step/Compile.zig +++ b/lib/std/Build/Step/Compile.zig @@ -264,8 +264,20 @@ pub const HeaderInstallation = union(enum) { dest_rel_path: []const u8, pub fn dupe(self: File, b: *std.Build) File { + // 'path' lazy paths are relative to the build root of some step, inferred from the step + // in which they are used. This means that we can't dupe such paths, because they may + // come from dependencies with their own build roots and duping the paths as is might + // cause the build script to search for the file relative to the wrong root. + // As a temporary workaround, we convert build root-relative paths to absolute paths. + // If/when the build-root relative paths are updated to encode which build root they are + // relative to, this workaround should be removed. + const duped_source: LazyPath = switch (self.source) { + .path => |root_rel| .{ .cwd_relative = b.pathFromRoot(root_rel) }, + else => self.source.dupe(b), + }; + return .{ - .source = self.source.dupe(b), + .source = duped_source, .dest_rel_path = b.dupePath(self.dest_rel_path), }; } @@ -293,8 +305,20 @@ pub const HeaderInstallation = union(enum) { }; pub fn dupe(self: Directory, b: *std.Build) Directory { + // 'path' lazy paths are relative to the build root of some step, inferred from the step + // in which they are used. This means that we can't dupe such paths, because they may + // come from dependencies with their own build roots and duping the paths as is might + // cause the build script to search for the file relative to the wrong root. + // As a temporary workaround, we convert build root-relative paths to absolute paths. + // If/when the build-root relative paths are updated to encode which build root they are + // relative to, this workaround should be removed. + const duped_source: LazyPath = switch (self.source) { + .path => |root_rel| .{ .cwd_relative = b.pathFromRoot(root_rel) }, + else => self.source.dupe(b), + }; + return .{ - .source = self.source.dupe(b), + .source = duped_source, .dest_rel_path = b.dupePath(self.dest_rel_path), .options = self.options.dupe(b), }; @@ -492,9 +516,8 @@ pub fn installConfigHeader(cs: *Compile, config_header: *Step.ConfigHeader) void /// module's include search path. pub fn installLibraryHeaders(cs: *Compile, lib: *Compile) void { assert(lib.kind == .lib); - const b = cs.step.owner; for (lib.installed_headers.items) |installation| { - const installation_copy = installation.dupe(b); + const installation_copy = installation.dupe(lib.step.owner); cs.installed_headers.append(installation_copy) catch @panic("OOM"); cs.addHeaderInstallationToIncludeTree(installation_copy); installation_copy.getSource().addStepDependencies(&cs.step); diff --git a/test/standalone/install_headers/build.zig b/test/standalone/install_headers/build.zig index 965239aac5..4c9bbb501a 100644 --- a/test/standalone/install_headers/build.zig +++ b/test/standalone/install_headers/build.zig @@ -70,7 +70,7 @@ pub fn build(b: *std.Build) void { run_exe.expectStdOutEqual("ABD12X"); test_step.dependOn(&run_exe.step); - const install_exe = b.addInstallArtifact(libfoo, .{ + const install_libfoo = b.addInstallArtifact(libfoo, .{ .dest_dir = .{ .override = .{ .custom = "custom" } }, .h_dir = .{ .override = .{ .custom = "custom/include" } }, .implib_dir = .disabled, @@ -94,6 +94,6 @@ pub fn build(b: *std.Build) void { }); run_check_exists.setCwd(.{ .cwd_relative = b.getInstallPath(.prefix, "") }); run_check_exists.expectExitCode(0); - run_check_exists.step.dependOn(&install_exe.step); + run_check_exists.step.dependOn(&install_libfoo.step); test_step.dependOn(&run_check_exists.step); }