std.Build.LazyPath: store Build relative information#19589
Closed
lacc97 wants to merge 14 commits intoziglang:masterfrom
Closed
std.Build.LazyPath: store Build relative information#19589lacc97 wants to merge 14 commits intoziglang:masterfrom
lacc97 wants to merge 14 commits intoziglang:masterfrom
Conversation
Contributor
Author
|
Not sure why the CI tests failed, the test suite on my linux dev machine passes. |
Contributor
|
might to to rebase/merge main to get the changes from #19485 |
This frees up the `path` identifier for a container-scope function.
These functions let a std.Build construct a LazyPath. At the moment this is mostly pointless relative to the old way of doing things but once the internal structure of LazyPath changes it will make it significantly easier to make local changes without having to rewrite them everywhere else that uses a LazyPath.
This adds a `root` field to LazyPath which contains the actual union data. This means the LazyPath is now a struct instead of a union itself.
This function won't be possible to implement once LazyPath requires a pointer to std.Build that owns it.
We can simply use the LazyPath's owner.
I couldn't find any other place where Dependency.path() was being used, so I added a standalone test that uses it, in addition to some of the other Dependency functions for a somewhat more thorough test of how dependencies might be used.
The use case is subsumed by the `path` union field.
This commit also renames the union fields `path` -> `build` and `cwd_relative` -> `cwd`. Now, the struct field `root` represents from whence the `path` field should be evaluated. At the moment, I haven't touched the `generated` and `generated_dirname` fields so LazyPath looks a bit awkward at the moment. The other fields will likely need a bit more work so they will go as a separate commit.
This slightly simplifies the implementation.
andrewrk
requested changes
Apr 9, 2024
| /// system API, it was allowed to be absolute. Absolute paths should use `cwd_relative`. | ||
| path: []const u8, | ||
| pub const LazyPath = struct { | ||
| owner: *std.Build, |
Member
There was a problem hiding this comment.
- Should not be stored for a GeneratedFile since it is already accessible via
generated_file.step.owner - Should not be stored for cwd_relative, since those are not associated with any
*std.Buildinstance.
| }; | ||
|
|
||
| pub fn path(b: *Build, p: []const u8) LazyPath { | ||
| // TODO(lacc97): absolute paths |
Member
|
Too much breaking. Please see #19597 |
Contributor
Author
Sorry about that 😅, got a bit carried away. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Implements #19313.