Skip to content

Commit

Permalink
std.Build.Module.addShallowDependencies: fix missing dependencies
Browse files Browse the repository at this point in the history
De-syncing here was not exposed before because old artifact creation API
always called `addShallowDependencies` before user could call
`addCSourceFile` etc., and their logic constantly overwrote this one.

Example of this de-syncing: `addShallowDependencies` depended only
on `RcSourceFile.file` LazyPath, but did not depend on
`RcSourceFile.include_paths` LazyPaths. Meanwhile,
`addWin32ResourceFile` depended on both.

This means that user could not call `addWin32ResourceFile` this way:
```zig
const mod = b.createModule(...);
mod.addWin32ResourceFile(...);
const exe = b.addExecutable2(...);
```

Since `addShallowDependencies` will be called on 1 and 3 line, with
incomplete logic, and Step.Compile will miss `include_paths` dependency.

I initially tried to do this in standalone test "windows_resources",
but it caused errors about file not being generated yet:

```zig
getPath() was called on a GeneratedFile that wasn't built yet.
  source package path: /home/bratishkaerik/github.com/zig/test/standalone/windows_resources
  Is there a missing Step dependency on step 'WriteFile generated.h'?
    The step was created by this stack trace:
name: 'WriteFile generated.h'. creation stack trace:
```

Now `addShallowDependencies` is synced with all functions and dependecies
are same no matter if user calls them before b.addExecutable2 or after it.

Signed-off-by: Eric Joldasov <[email protected]>
  • Loading branch information
BratishkaErik committed Nov 1, 2024
1 parent ed0662f commit d873dd4
Showing 1 changed file with 73 additions and 12 deletions.
85 changes: 73 additions & 12 deletions lib/std/Build/Module.zig
Original file line number Diff line number Diff line change
Expand Up @@ -324,28 +324,89 @@ pub fn addImport(m: *Module, name: []const u8, module: *Module) void {
/// dependencies on `m`'s `depending_steps`.
fn addShallowDependencies(m: *Module, dependee: *Module) void {
if (dependee.root_source_file) |lazy_path| addLazyPathDependencies(m, dependee, lazy_path);
for (dependee.lib_paths.items) |lib_path| addLazyPathDependencies(m, dependee, lib_path);

// Note: all code below should be synced with respective functions
// in regards to lazy path and step dependencies.
//
// With old API (`b.addExecutable` etc.), root module was created
// during `Step.Compile.create` call, and this function was immediately
// called with `depending_steps` > 0. Even if the logic here is de-synced,
// user can still call `addCSourceFile`, `addIncludePath` etc. peacefully
// since they are called later that this function and add required
// dependencies by themselves.
//
// Now, with the new API (`b.addExecutable2` etc.), root module is created
// by user before creating artifact, and in between these 2 moments
// they can call same `addCSourceFile` etc. functions. But this time,
// this function is called both earlier and later that them:
// during module creation (`depending_steps` == 0), and during artifact
// creation (`depending_steps` > 0). Which means not all dependencies will be
// added.
for (dependee.include_dirs.items) |include_dir| switch (include_dir) {
// Sync with:
// * `addIncludePath`,
// * `addAfterIncludePath`,
// * `addSystemIncludePath`,
// * `addFrameworkPath`,
// * and `addSystemFrameworkPath`.
.path,
.path_after,
.path_system,
.framework_path,
.framework_path_system,
=> |lp| addLazyPathDependencies(m, dependee, lp),

// Sync with `link_objects` for-loop below and `linkLibraryOrObject`.
.other_step => |compile| {
addStepDependencies(m, dependee, &compile.step);
// We don't need to depend on
addLazyPathDependenciesOnly(m, compile.getEmittedIncludeTree());
},

// Sync with `addConfigHeader`.
.config_header_step => |config_header| addStepDependencies(m, dependee, &config_header.step),
};

for (dependee.lib_paths.items) |directory_path|
// Sync with `addLibraryPath`.
addLazyPathDependencies(m, dependee, directory_path);

for (dependee.rpaths.items) |rpath| switch (rpath) {
// Sync with `addRPath`.
.lazy_path => |lp| addLazyPathDependencies(m, dependee, lp),

// Sync with `addRPathSpecial`.
.special => {},
};

for (dependee.link_objects.items) |link_object| switch (link_object) {
.other_step => |compile| {
addStepDependencies(m, dependee, &compile.step);
addLazyPathDependenciesOnly(m, compile.getEmittedIncludeTree());
// Sync with `addObjectFile`.
.static_path => |lp| addLazyPathDependencies(m, dependee, lp),

// Sync with `include_paths` for-loop above and `linkLibraryOrObject`.
.other_step => |_| {
// Dependency on the compile step and `compile.getEmittedIncludeTree()`
// is already added by mentioned for-loop.
},

.static_path,
.assembly_file,
=> |lp| addLazyPathDependencies(m, dependee, lp),
// Sync with `linkSystemLibrary`.
.system_lib => {},

// Sync with `addAssemblyFile`.
.assembly_file => |lp| addLazyPathDependencies(m, dependee, lp),

.c_source_file => |x| addLazyPathDependencies(m, dependee, x.file),
.win32_resource_file => |x| addLazyPathDependencies(m, dependee, x.file),
// Sync with `addCSourceFile`.
.c_source_file => |source| addLazyPathDependencies(m, dependee, source.file),
// Sync with `addCSourceFiles`.
.c_source_files => |source_files| addLazyPathDependencies(m, dependee, source_files.root),

.c_source_files,
.system_lib,
=> {},
// Sync with `addWin32ResourceFile`.
.win32_resource_file => |source| {
addLazyPathDependencies(m, dependee, source.file);
for (source.include_paths) |include_path| {
addLazyPathDependencies(m, dependee, include_path);
}
},
};
}

Expand Down

0 comments on commit d873dd4

Please sign in to comment.