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.

Sign up for GitHub

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

build: detect and abort build when getPath is called outside of any make #21541

Closed
wants to merge 1 commit into from
Closed

build: detect and abort build when getPath is called outside of any make #21541

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 27 additions & 33 deletions lib/std/Build.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ pub const Module = @import("Build/Module.zig");
pub const Watch = @import("Build/Watch.zig");
pub const Fuzz = @import("Build/Fuzz.zig");

const ThreadLocalData = struct {
running_step: ?*Step = null,
};
pub threadlocal var thread_local: ThreadLocalData = .{};

/// Shared state among all Build instances.
graph: *Graph,
install_tls: TopLevelStep,
Expand Down Expand Up @@ -2524,7 +2529,7 @@ pub const LazyPath = union(enum) {
.src_path => |sp| .{ .src_path = .{
.owner = sp.owner,
.sub_path = dirnameAllowEmpty(sp.sub_path) orelse {
dumpBadDirnameHelp(null, null, "dirname() attempted to traverse outside the build root\n", .{}) catch {};
dumpBadDirnameHelp(null, "dirname() attempted to traverse outside the build root\n", .{}) catch {};
@panic("misconfigured build script");
},
} },
Expand All @@ -2545,14 +2550,14 @@ pub const LazyPath = union(enum) {
// In either case, the build script tried to go too far
// and we should panic.
if (fs.path.isAbsolute(rel_path)) {
dumpBadDirnameHelp(null, null,
dumpBadDirnameHelp(null,
\\dirname() attempted to traverse outside the root.
\\No more directories left to go up.
\\
, .{}) catch {};
@panic("misconfigured build script");
} else {
dumpBadDirnameHelp(null, null,
dumpBadDirnameHelp(null,
\\dirname() attempted to traverse outside the current working directory.
\\
, .{}) catch {};
Expand All @@ -2563,7 +2568,7 @@ pub const LazyPath = union(enum) {
.dependency => |dep| .{ .dependency = .{
.dependency = dep.dependency,
.sub_path = dirnameAllowEmpty(dep.sub_path) orelse {
dumpBadDirnameHelp(null, null,
dumpBadDirnameHelp(null,
\\dirname() attempted to traverse outside the dependency root.
\\
, .{}) catch {};
Expand Down Expand Up @@ -2619,20 +2624,15 @@ pub const LazyPath = union(enum) {

/// Deprecated, see `getPath3`.
pub fn getPath(lazy_path: LazyPath, src_builder: *Build) []const u8 {
return getPath2(lazy_path, src_builder, null);
}

/// Deprecated, see `getPath3`.
pub fn getPath2(lazy_path: LazyPath, src_builder: *Build, asking_step: ?*Step) []const u8 {
const p = getPath3(lazy_path, src_builder, asking_step);
const p = getPath3(lazy_path, src_builder);
return src_builder.pathResolve(&.{ p.root_dir.path orelse ".", p.sub_path });
}

/// Intended to be used during the make phase only.
///
/// `asking_step` is only used for debugging purposes; it's the step being
/// run that is asking for the path.
pub fn getPath3(lazy_path: LazyPath, src_builder: *Build, asking_step: ?*Step) Cache.Path {
pub fn getPath3(lazy_path: LazyPath, src_builder: *Build) Cache.Path {
if (thread_local.running_step == null) @panic(
"getPath called on LazyPath outside of any step's make function",
);
switch (lazy_path) {
.src_path => |sp| return .{
.root_dir = sp.owner.build_root,
Expand All @@ -2651,7 +2651,7 @@ pub const LazyPath = union(enum) {
.sub_path = gen.file.path orelse {
std.debug.lockStdErr();
const stderr = std.io.getStdErr();
dumpBadGetPathHelp(gen.file.step, stderr, src_builder, asking_step) catch {};
dumpBadGetPathHelp(stderr, src_builder) catch {};
std.debug.unlockStdErr();
@panic("misconfigured build script");
},
Expand All @@ -2665,7 +2665,7 @@ pub const LazyPath = union(enum) {
if (mem.eql(u8, file_path.sub_path, cache_root_path)) {
// If we hit the cache root and there's still more to go,
// the script attempted to go too far.
dumpBadDirnameHelp(gen.file.step, asking_step,
dumpBadDirnameHelp(gen.file.step,
\\dirname() attempted to traverse outside the cache root.
\\This is not allowed.
\\
Expand All @@ -2677,7 +2677,7 @@ pub const LazyPath = union(enum) {
// dirname will return null only if we're at root.
// Typically, we'll stop well before that at the cache root.
file_path.sub_path = fs.path.dirname(file_path.sub_path) orelse {
dumpBadDirnameHelp(gen.file.step, asking_step,
dumpBadDirnameHelp(gen.file.step,
\\dirname() reached root.
\\No more directories left to go up.
\\
Expand Down Expand Up @@ -2723,7 +2723,6 @@ pub const LazyPath = union(enum) {

fn dumpBadDirnameHelp(
fail_step: ?*Step,
asking_step: ?*Step,
comptime msg: []const u8,
args: anytype,
) anyerror!void {
Expand All @@ -2744,12 +2743,12 @@ fn dumpBadDirnameHelp(
s.dump(stderr);
}

if (asking_step) |as| {
if (thread_local.running_step) |running_step| {
tty_config.setColor(w, .red) catch {};
try stderr.writer().print(" The step '{s}' that is missing a dependency on the above step was created by this stack trace:\n", .{as.name});
try stderr.writer().print(" The step '{s}' that is missing a dependency on the above step was created by this stack trace:\n", .{running_step.name});
tty_config.setColor(w, .reset) catch {};

as.dump(stderr);
running_step.dump(stderr);
}

tty_config.setColor(w, .red) catch {};
Expand All @@ -2759,34 +2758,29 @@ fn dumpBadDirnameHelp(

/// In this function the stderr mutex has already been locked.
pub fn dumpBadGetPathHelp(
s: *Step,
stderr: fs.File,
src_builder: *Build,
asking_step: ?*Step,
) anyerror!void {
const w = stderr.writer();
try w.print(
\\getPath() was called on a GeneratedFile that wasn't built yet.
\\ source package path: {s}
\\ Is there a missing Step dependency on step '{s}'?
\\
, .{
src_builder.build_root.path orelse ".",
s.name,
});

const tty_config = std.io.tty.detectConfig(stderr);
tty_config.setColor(w, .red) catch {};
try stderr.writeAll(" The step was created by this stack trace:\n");
tty_config.setColor(w, .reset) catch {};

s.dump(stderr);
if (asking_step) |as| {
if (thread_local.running_step) |running_step| {
try w.print(
"Is there a missing Step dependency on step '{s}'?\n",
.{running_step.name},
);
tty_config.setColor(w, .red) catch {};
try stderr.writer().print(" The step '{s}' that is missing a dependency on the above step was created by this stack trace:\n", .{as.name});
try stderr.writeAll(" The step was created by this stack trace:\n");
tty_config.setColor(w, .reset) catch {};

as.dump(stderr);
running_step.dump(stderr);
}
tty_config.setColor(w, .red) catch {};
try stderr.writeAll(" Hope that helps. Proceeding to panic.\n");
Expand Down
18 changes: 8 additions & 10 deletions lib/std/Build/Module.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,23 +169,22 @@ pub const IncludeDir = union(enum) {
include_dir: IncludeDir,
b: *std.Build,
zig_args: *std.ArrayList([]const u8),
asking_step: ?*Step,
) !void {
switch (include_dir) {
.path => |include_path| {
try zig_args.appendSlice(&.{ "-I", include_path.getPath2(b, asking_step) });
try zig_args.appendSlice(&.{ "-I", include_path.getPath(b) });
},
.path_system => |include_path| {
try zig_args.appendSlice(&.{ "-isystem", include_path.getPath2(b, asking_step) });
try zig_args.appendSlice(&.{ "-isystem", include_path.getPath(b) });
},
.path_after => |include_path| {
try zig_args.appendSlice(&.{ "-idirafter", include_path.getPath2(b, asking_step) });
try zig_args.appendSlice(&.{ "-idirafter", include_path.getPath(b) });
},
.framework_path => |include_path| {
try zig_args.appendSlice(&.{ "-F", include_path.getPath2(b, asking_step) });
try zig_args.appendSlice(&.{ "-F", include_path.getPath(b) });
},
.framework_path_system => |include_path| {
try zig_args.appendSlice(&.{ "-iframework", include_path.getPath2(b, asking_step) });
try zig_args.appendSlice(&.{ "-iframework", include_path.getPath(b) });
},
.other_step => |other| {
if (other.generated_h) |header| {
Expand Down Expand Up @@ -540,7 +539,6 @@ pub fn addCMacro(m: *Module, name: []const u8, value: []const u8) void {
pub fn appendZigProcessFlags(
m: *Module,
zig_args: *std.ArrayList([]const u8),
asking_step: ?*Step,
) !void {
const b = m.owner;

Expand Down Expand Up @@ -605,22 +603,22 @@ pub fn appendZigProcessFlags(
}

for (m.include_dirs.items) |include_dir| {
try include_dir.appendZigProcessFlags(b, zig_args, asking_step);
try include_dir.appendZigProcessFlags(b, zig_args);
}

try zig_args.appendSlice(m.c_macros.items);

try zig_args.ensureUnusedCapacity(2 * m.lib_paths.items.len);
for (m.lib_paths.items) |lib_path| {
zig_args.appendAssumeCapacity("-L");
zig_args.appendAssumeCapacity(lib_path.getPath2(b, asking_step));
zig_args.appendAssumeCapacity(lib_path.getPath(b));
}

try zig_args.ensureUnusedCapacity(2 * m.rpaths.items.len);
for (m.rpaths.items) |rpath| switch (rpath) {
.lazy_path => |lp| {
zig_args.appendAssumeCapacity("-rpath");
zig_args.appendAssumeCapacity(lp.getPath2(b, asking_step));
zig_args.appendAssumeCapacity(lp.getPath(b));
},
.special => |bytes| {
zig_args.appendAssumeCapacity("-rpath");
Expand Down
12 changes: 12 additions & 0 deletions lib/std/Build/Step.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,18 @@ pub fn init(options: StepOptions) Step {
pub fn make(s: *Step, options: MakeOptions) error{ MakeFailed, MakeSkipped }!void {
const arena = s.owner.allocator;

const old_step = Build.thread_local.running_step;
const panic_on_nested_makes = true;
if (old_step) |old| if (panic_on_nested_makes) std.debug.panic(
"make was called on step '{s}' while inside make for step '{s}'",
.{ s.name, old.name },
);
Build.thread_local.running_step = s;
defer {
std.debug.assert(Build.thread_local.running_step == s);
Build.thread_local.running_step = old_step;
}

s.makeFn(s, options) catch |err| switch (err) {
error.MakeFailed => return error.MakeFailed,
error.MakeSkipped => return error.MakeSkipped,
Expand Down
2 changes: 1 addition & 1 deletion lib/std/Build/Step/CheckFile.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ fn make(step: *Step, options: Step.MakeOptions) !void {
const check_file: *CheckFile = @fieldParentPtr("step", step);
try step.singleUnchangingWatchInput(check_file.source);

const src_path = check_file.source.getPath2(b, step);
const src_path = check_file.source.getPath(b);
const contents = fs.cwd().readFileAlloc(b.allocator, src_path, check_file.max_bytes) catch |err| {
return step.fail("unable to read '{s}': {s}", .{
src_path, @errorName(err),
Expand Down
Loading