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.Step.Run: avoid file system watches when already tracked by step dependency edges #20631

Open
andrewrk opened this issue Jul 15, 2024 · 0 comments
Labels
enhancement Solving this issue will likely involve adding new logic or components to the codebase. optimization zig build system std.Build, the build runner, `zig build` subcommand, package management
Milestone

Comments

@andrewrk
Copy link
Member

Extracted from #20580.

The make function of std.Build.Step.Run:

fn make(step: *Step, prog_node: std.Progress.Node) !void {

Uses the cache system (std.Build.Cache) to track file system inputs, and then reports those to the build runner via cacheHitAndWatch and writeManifestAndWatch.

This has a couple of shortcomings.

Firstly, a failed run step fails to report its file system inputs, meaning that those files will not be watched. Instead, even a failed run step needs to register its file inputs to the cache system so that they can be tracked.

Secondly, it is common for Run steps to be given build system outputs as inputs. For example,

                nasm_run.addFileArg(config_asm.getOutput());

In this case, there is already a dependency edge between the config_asm Step and the nasm_run Step, making the file system watch placed on the output of config_asm a waste of resources.

As part of this issue, I suggest to take note of all file system watch directories, and eliminate cases where zig-cache/o/ directories are watched, because those will always be handled instead by Step dependency edges.

This is even true for incremental cache mode which will result in mutations to those files within the o directory. As a follow-up issue to this one, incremental cache mode should use a different subdirectory than o so that o can be used exclusively for artifacts that are never mutated.

As part of implementing this issue, I suggest to revisit the std.Build.Cache implementation, particularly with an eye for using the std.Build.Cache.Path abstraction in order to fully eliminate absolute paths from manifest files. In particular I noticed files relative to the p/ directory inside the global zig cache were being stored in absolute path form.

@andrewrk andrewrk added enhancement Solving this issue will likely involve adding new logic or components to the codebase. optimization zig build system std.Build, the build runner, `zig build` subcommand, package management labels Jul 15, 2024
@andrewrk andrewrk added this to the 0.14.0 milestone Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Solving this issue will likely involve adding new logic or components to the codebase. optimization zig build system std.Build, the build runner, `zig build` subcommand, package management
Projects
None yet
Development

No branches or pull requests

1 participant