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

std.Build: fix Compile.installHeader behavior, add WriteFile.addCopyDirectory #19167

Merged
merged 11 commits into from
Apr 7, 2024
Merged
25 changes: 13 additions & 12 deletions lib/std/Build.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1568,23 +1568,24 @@ 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
pub fn addInstallFile(self: *Build, source: LazyPath, dest_rel_path: []const u8) *Step.InstallFile {
return self.addInstallFileWithDir(source.dupe(self), .prefix, dest_rel_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
pub fn addInstallBinFile(self: *Build, source: LazyPath, dest_rel_path: []const u8) *Step.InstallFile {
return self.addInstallFileWithDir(source.dupe(self), .bin, dest_rel_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
pub fn addInstallLibFile(self: *Build, source: LazyPath, dest_rel_path: []const u8) *Step.InstallFile {
return self.addInstallFileWithDir(source.dupe(self), .lib, dest_rel_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);
}

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);
/// `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);
}

pub fn addInstallFileWithDir(
Expand All @@ -1593,7 +1594,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 {
Expand Down
21 changes: 6 additions & 15 deletions lib/std/Build/Module.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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) |install_step|
addStepDependenciesOnly(m, install_step);
addLazyPathDependenciesOnly(m, compile.getEmittedIncludeTree());
},

.static_path,
Expand Down Expand Up @@ -691,20 +690,14 @@ 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.?).?);
try zig_args.appendSlice(&.{ "-isystem", std.fs.path.dirname(header.getPath()).? });
}
if (other.installed_headers.items.len > 0) {
try zig_args.append("-I");
try zig_args.append(b.pathJoin(&.{
other.step.owner.install_prefix, "include",
}));
if (other.installed_headers_include_tree) |include_tree| {
try zig_args.appendSlice(&.{ "-I", include_tree.generated_directory.getPath() });
}
},
.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()).? });
},
}
}
Expand Down Expand Up @@ -752,9 +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) |install_step| {
addStepDependenciesOnly(m, install_step);
}
addLazyPathDependenciesOnly(m, other.getEmittedIncludeTree());
}

fn requireKnownTarget(m: *Module) std.Target {
Expand Down
229 changes: 163 additions & 66 deletions lib/std/Build/Step/Compile.zig
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,13 @@ test_runner: ?[]const u8,
test_server_mode: bool,
wasi_exec_model: ?std.builtin.WasiExecModel = null,

installed_headers: ArrayList(*Step),
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.
Expand Down Expand Up @@ -249,6 +255,90 @@ pub const Kind = enum {
@"test",
};

pub const HeaderInstallation = union(enum) {
file: File,
directory: Directory,

pub const File = struct {
source: LazyPath,
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),
};
Comment on lines +267 to +277
Copy link
Contributor Author

@castholm castholm Mar 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this with a simple local project where

  • package a exported a static library lib bundling build root-relative header a.h,
  • package b depended on a, and
  • package b built and installed an executable exe with a main.c root source file that uses symbols from a.h and which did both exe.linkLibrary(lib) and exe.installLibraryHeaders(lib).

That repro failed with an "unable to update file" error prior to this workaround, but passes now.

If #19313 is accepted and implemented, this workaround can be reverted.
(Thanks @MasterQ32 for indirectly bringing this flaw to my attention.)


return .{
.source = duped_source,
.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 {
// '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 = duped_source,
.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| .{ .file = f.dupe(b) },
.directory => |d| .{ .directory = d.dupe(b) },
};
}
};

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) {
Expand Down Expand Up @@ -308,7 +398,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(HeaderInstallation).init(owner.allocator),
.zig_lib_dir = null,
.exec_cmd_args = null,
.filters = options.filters,
Expand Down Expand Up @@ -380,78 +470,85 @@ 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 {
/// 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 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(
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");
}

const installation: HeaderInstallation = .{ .file = .{
.source = source.dupe(b),
.dest_rel_path = b.dupePath(dest_rel_path),
} };
cs.installed_headers.append(installation) catch @panic("OOM");
cs.addHeaderInstallationToIncludeTree(installation);
installation.getSource().addStepDependencies(&cs.step);
}

/// 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(
a: *Compile,
src_dir_path: []const u8,
dest_rel_path: []const u8,
) void {
return installHeadersDirectoryOptions(a, .{
.source_dir = .{ .path = src_dir_path },
.install_dir = .header,
.install_subdir = dest_rel_path,
});
}

pub fn installHeadersDirectoryOptions(
cs: *Compile,
options: std.Build.Step.InstallDir.Options,
source: LazyPath,
dest_rel_path: []const u8,
options: HeaderInstallation.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");
const installation: HeaderInstallation = .{ .directory = .{
.source = source.dupe(b),
.dest_rel_path = b.dupePath(dest_rel_path),
.options = options.dupe(b),
} };
cs.installed_headers.append(installation) catch @panic("OOM");
cs.addHeaderInstallationToIncludeTree(installation);
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);
for (lib.installed_headers.items) |installation| {
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);
}
}

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| {
_ = wf.addCopyDirectory(dir.source, dir.dest_rel_path, .{
.exclude_extensions = dir.options.exclude_extensions,
.include_extensions = dir.options.include_extensions,
});
},
};
}

pub fn installLibraryHeaders(cs: *Compile, l: *Compile) void {
assert(l.kind == .lib);
pub fn getEmittedIncludeTree(cs: *Compile) LazyPath {
if (cs.installed_headers_include_tree) |wf| return wf.getDirectory();
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);
}
cs.installed_headers.appendSlice(l.installed_headers.items) catch @panic("OOM");
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 {
Expand Down
Loading