Skip to content

Conversation

@antlilja
Copy link
Contributor

@antlilja antlilja commented Aug 3, 2023

Partly closes #16643

This PR implements dependencyPath, and allows for fetching dependencies without a build.zig.

This PR adds support for non zig dependencies (dependencies without a build.zig). It adds a function path to the Dependency struct which returns a LazyPath to a path inside the dependency.

It does not currently fetch these dependencies lazily, it instead just uses the current way of fetching dependencies before running the build. Doing actual lazy fetching should probably be implemented separately together with lazy dependency fetching a zig dependencies. Right now it would require duplicating a lot of work from the existing dependency fetching.

I also created a proof of concept project based on Andrews SDL which downloads a tagged release of SDL from the SDL website and builds a static library: https://github.com/antlilja/zig-sdl.

@antlilja antlilja marked this pull request as draft August 4, 2023 00:07
@mlugg
Copy link
Member

mlugg commented Aug 4, 2023

IMO this should be a method on Dependency, with the LazyPath variant storing a *Dependency instead of directly storing the build root. (This makes the usage (b.dependency("foo").path("foo/bar.txt").)

@antlilja antlilja marked this pull request as ready for review August 5, 2023 12:15
@antlilja
Copy link
Contributor Author

antlilja commented Aug 5, 2023

IMO this should be a method on Dependency, with the LazyPath variant storing a *Dependency instead of directly storing the build root. (This makes the usage (b.dependency("foo").path("foo/bar.txt").)

I tried implementing it like this instead and I agree, at least the example I provided becomes much cleaner when not having a seperate dependencyPath function.

Copy link
Member

@andrewrk andrewrk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this - this is going to make a lot of people happy. I love your SDL proof of concept too. A couple requests-

@mlugg
Copy link
Member

mlugg commented Aug 6, 2023

Oh also, wrt lazy dep fetching: this is planned to be implemented in future. I've discussed it a little with Andrew, and the basic idea is this:

  • initially, we fetch no dependencies, and run the build script
  • when your build script calls dependency for a dep which isn't currently downloaded, it communicates this requirement to the build runner
  • the build runner fetches that package, and re-runs the build script with that dependency in place
  • rinse and repeat until we have all deps we need

Obviously this recompiles the build script a bunch, so we really want to wait for the self-hosted backends to be ready for this to make those rebuilds fast.

Anyways, the point here is that optional deps won't be specific to path uses or anything, so it's fine for this code to not touch them: once we implement that it should apply to dependencies of any kind.

@antlilja
Copy link
Contributor Author

antlilja commented Aug 6, 2023

My first implementation was a bit hacky so I've rewritten it a bit. Dependencies which don't contain a build.zig file now don't create package, they are instead just added to build_root_source. I think this makes a bit more sense and cleans up the previous implementation quite a lot (for example not requiring changes in Compilation.zig).

@ikskuh
Copy link
Contributor

ikskuh commented Aug 18, 2023

Dis gonna be gud!

@antlilja antlilja force-pushed the dependencyPath branch 2 times, most recently from a8f2506 to 96155cf Compare September 23, 2023 10:41
@antlilja antlilja requested a review from mlugg September 23, 2023 15:21
Copy link
Member

@mlugg mlugg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - a couple minor points.

Dependencies no longer require a build.zig file.

Adds path function to Dependency struct which
returns a LazyPath into a dependency.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ability to construct a LazyPath out of any file from a dependency package

4 participants