-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
introduce file system watching features to the zig build system #20580
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andrewrk
added
zig build system
std.Build, the build runner, `zig build` subcommand, package management
release notes
This PR should be mentioned in the release notes.
breaking
Implementing this issue could cause existing code to no longer compile or have different behavior.
labels
Jul 10, 2024
This direction is not quite right because it mutates shared state in a threaded context, so the next commit will need to fix this.
* Delete existing `FAN` struct in favor of a `fanotify` struct which has type-safe bindings (breaking). * Add name_to_handle_at syscall wrapper. * Add file_handle * Add kernel_fsid_t * Add fsid_t * Add and update std.posix wrappers.
Helpful methods when using one of these structs as a hash table key.
I'm still learning how the fanotify API works but I think after playing with it in this commit, I finally know how to implement it, at least on Linux. This commit does not accomplish the goal but I want to take the code in a different direction and still be able to reference this point in time by viewing a source control diff. I think the move is going to be saving the file_handle for the parent directory, which combined with the dirent names is how we can correlate the events back to the Step instances that have registered file system inputs. I predict this to be similar to implementations on other operating systems.
So far, only implemented for InstallFile steps. Default debounce interval bumped to 50ms. I think it should be configurable. Next I have an idea to simplify the fanotify implementation, but other OS implementations might want to refer back to this commit before I make those changes.
and make failed steps always be invalidated and make steps that don't need to be reevaluated marked as cached
by obtaining the stderr lock when printing the build summary
This has been planned for quite some time; this commit finally does it. Also implements file system watching integration in the make() implementation for UpdateSourceFiles and fixes the reporting of step caching for both. WriteFile does not yet have file system watching integration.
The goal is to move towards using `std.Build.Cache.Path` instead of absolute path names. This was helpful for implementing file watching integration to the InstallDir Step
And use it to implement InstallDir Step watch integration. I'm not seeing any events triggered when I run `mkdir` in the watched directory, however, and I have not yet figured out why.
Otherwise it reports EBADF.
This happens when deleting watched directories and is harmless.
This makes mkdir/rmdir events show up.
Since I spent a couple minutes debugging this, hopefully this saves someone some future trouble doing the same.
and deprecate `addFile`. Part of an effort to move towards using `std.Build.Cache.Path` abstraction in more places, which makes it easier to avoid absolute paths and path resolution.
and add file system watching integration. `addDirectoryWatchInput` now returns a `bool` which helps remind the caller to 1. call addDirectoryWatchInputFromPath on any derived paths 2. but only if the dependency is not already captured by a step dependency edge. The make function now recursively walks all directories and adds the found files to the cache hash rather than incorrectly only adding the directory name to the cache hash. closes #20571
The cache hash already has the zig version in there, so it's not really needed.
The cache hash already has the zig version in there, so it's not really needed.
This function previously wrote a trailing directory separator, but that's not correct if the path refers to a file.
Updates the build runner to unconditionally require a zig lib directory parameter. This parameter is needed in order to correctly understand file system inputs from zig compiler subprocesses, since they will refer to "the zig lib directory", and the build runner needs to place file system watches on directories in there. The build runner's fanotify file watching implementation now accounts for when two or more Cache.Path instances compare unequal but ultimately refer to the same directory in the file system. Breaking change: std.Build no longer has a zig_lib_dir field. Instead, there is the Graph zig_lib_directory field, and individual Compile steps can still have their zig lib directories overridden. I think this is unlikely to break anyone's build in practice. The compiler now sends a "file_system_inputs" message to the build runner which shares the full set of files that were added to the cache system with the build system, so that the build runner can watch properly and redo the Compile step. This is implemented for whole cache mode but not yet for incremental cache mode.
These are also used for whole cache mode in the case that any compile errors are emitted.
need to add another field to initialize now
it's not advertised in the usage and only available in debug builds of the compiler. Makes it easier to test changes to the build runner that might affect targets differently.
Makes the build runner compile successfully for non-linux targets; printing an error if you ask for --watch rather than making build scripts fail to compile.
22 tasks
This was referenced Jul 13, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
breaking
Implementing this issue could cause existing code to no longer compile or have different behavior.
release notes
This PR should be mentioned in the release notes.
zig build system
std.Build, the build runner, `zig build` subcommand, package management
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Feature Explanation
Uses the build system's perfect knowledge of all file system inputs to the pipeline to keep the build runner alive after completion, watching the minimal number of directories in order to trigger re-running only the dirty steps from the graph.
Default debounce time is 50ms but this is configurable. It helps prevent wasted rebuilds when source files are changed in rapid succession, for example when saving with vim it does not do an atomic rename into place but actually deletes the destination file before writing it again, causing a brief period of invalid state, which would cause a build failure without debouncing (it would be followed by a successful build, but it's annoying to experience the temporary build failure regardless).
The purpose of this feature is to reduce latency between editing and debugging in the development cycle. In large projects, the cache system must call
fstat
on a large number of files even when it is a cache hit. File system watching allows more efficient detection of stale pipeline steps.Mainly this is motivated by incremental compilation landing soon, so that we can keep the compiler running and responding to source code changes as fast as possible. In this case, also keeping the rest of the build pipeline up-to-date is table stakes.
This also paves the road towards #68. A
Run
step combined with--watch
connects file system updates directly to new code inside an already-running executable. It takes steps closer to more advanced use cases as well:zig build
as a child process, speaking a compiler protocol (Implement a server in the compiler that serves information about the compilation #615) to learn about type information, perform refactors, request rebuilds, receive errors, etc. The protocol will multiplex between an arbitrary number ofCompile
steps, each with a running instance of the compiler.Run step asciinema demo This demo only shows 1/2 terminals used, but in the other window I'm editing assembly files with vim and saving them.
Compile step asciinema demo - getting quick compile error feedback. Incremental compilation is not done yet, this is full compiler rebuilds, but skipping codegen.
unit test workflow asciinema demo
Migration Guide
If you were using
WriteFile
for its ability to update source files, that functionality has been extracted into a separate step calledUpdateSourceFiles
. Everything else is the same, so migration looks like this:If you were using a
RemoveDir
step, it now takes aLazyPath
instead of[]const u8
. Probably your migration looks like this:However, please consider not choosing a temporary path at configure time as this is somewhat brittle when it comes to running your build pipeline concurrently.
Follow-Up Work
Compile
steps keep the compiler alive #20600--watch
does not pick up sub-Compilation file system inputs such as compiler_rt, glibc, or musl source files #20601zig build --watch
detect modifications to the build script and rerun itself #20602zig build test-cases --watch
does not pick up changes to source files #20606we still know the set of files that trigger recompilation.