Skip to content

std.Build: Remove anonymousDependency #18590

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

Merged
merged 7 commits into from
Apr 7, 2024

Conversation

castholm
Copy link
Contributor

@castholm castholm commented Jan 16, 2024

Closes #17238.

Removes std.Build.anonymousDependency and makes std.Build.dependencyInner private.

Standalone and linker tests depended heavily on b.anonymousDependency, so the bulk effort of this PR revolves around converting them into to proper (relative path) packages that can be resolved through b.dependency. This also means that the Zig compiler tests now also actually dogfoods package management; prior to this, there weren't actually any references to b.dependency or (non-example) build.zig.zon files in the compiler codebase.

I preserved the order of the original test case definitions and didn't fix any of the commented-out entries. "Simple" standalone test cases were moved into their own directory for clarity.

@castholm castholm force-pushed the move-standalone-test-cases branch from 9c97ea8 to 30301b1 Compare January 16, 2024 18:31
@nektro
Copy link
Contributor

nektro commented Jan 16, 2024

was the addition of the simple directory intentional?

@castholm
Copy link
Contributor Author

was the addition of the simple directory intentional?

Yes, I moved all of the simple test cases to their own directory to make it a bit more clear that each of the other directories below test/standalone/ are packages consumed by test/standalone/build.zig(.zon).

@castholm castholm marked this pull request as draft February 1, 2024 22:41
@castholm castholm force-pushed the move-standalone-test-cases branch from 215aac8 to 4c5a0ef Compare February 17, 2024 17:18
@castholm castholm changed the title Promote standalone test cases to packages std.Build: Remove anonymousDependency Feb 17, 2024
@castholm castholm marked this pull request as ready for review February 17, 2024 17:26
@castholm castholm force-pushed the move-standalone-test-cases branch from 4c5a0ef to aa40a9a Compare February 29, 2024 22:15
@plajjan
Copy link

plajjan commented Mar 7, 2024

I'm currently using anonymousDependency and I'm interested to understand the impact of this PR. My use case of build.zig is as the low level compilation step for another programming language, so things like dependencies are not directly written by a human but rather generated by the compiler and fed in to build.zig as arguments or discovered by other means. I quite like the fact that build.zig is a fully featured zig program since it enables me to write code to do most of anything.

From this PR it looks like I now have to specify the path to a dependency in the .zon file. Is that correct? Is there an alternative in code? I'd really prefer not to have to generate a .zon file.

Perhaps I should document our use of build.zig as part of the low level compile in this language...

@castholm
Copy link
Contributor Author

castholm commented Mar 7, 2024

@plajjan Could you provide an example of how you generate these dependencies?

Assuming https://github.com/actonlang/acton is the project you're talking about, I had a quick look at it. Am I understanding it correctly that the main complication preventing you from moving your vendored dependencies to .path field in build.zig.zon is that you allow passing a -Dsyspath option to override the directory it uses as its build root?

pub fn build(b: *std.Build) void {
    // ...
    const syspath = b.option([]const u8, "syspath", "") orelse "";
    // ...
    const dep_libpcre2 = b.anonymousDependency(joinPath(b.allocator, dots_to_root, syspath, "deps/pcre2"), @import("deps/pcre2/build.zig"), .{
        .target = target,
        .optimize = optimize,
    });

I don't think using a build root that is different from the directory in which the dependency's build.zig resides was ever an intended use of anonymousDependency judging by the doc comments:

    /// The path to the directory containing the dependency's build.zig file,
    /// relative to the current package's build.zig.
    relative_build_root: []const u8,
    /// A direct `@import` of the build.zig of the dependency.
    comptime build_zig: type,

For vendored dependencies, build.zig.zon is the intended way to reference them moving forward.

For the -Dsyspath option I don't have an easy suggested fix. If you don't mind getting a little dirty from poking at implementation details of std.Build that might break in future releases of Zig, you could declare your own function that replicates the same logic anonymousDependency uses to construct a new child builder instance yourself.

@plajjan
Copy link

plajjan commented Mar 7, 2024

@castholm Acton is indeed the project I'm referring to. Thanks so much for taking a peek! First off, there's the current solution in place today and the future that I imagine. Currently, Acton (it's rather young still) does not support dependencies on external projects, so if you create an Acton project today, you can write your own modules in that project and you can also import things from stdlib, but you cannot have a dependency on any other Acton project. What this translates into for build.zig is that all the zig dependencies are just on Acton "base" (which is builtins + stdlib + rts) and some statically known C libraries (libuv, utf8proc etc). Thus the build.zig can be rather static and that's what you see in https://github.com/actonlang/acton/blob/main/builder/build.zig (an acton project main build.zig) and https://github.com/actonlang/acton/blob/main/base/build.zig (the Acton base build.zig).

In the near future I am attempting to build support for dependencies on other projects, which on the build.zig level would translate to a dynamic list of anonymousDependency. I am in the process of coding this up but have not yet reached a level where it works, so maybe I've just missed something and it's just not possible the way I imagine it. But anyhow, the way I think of it is that I have a acton project, say in the folder foo. The acton compiler will manage fetching dependencies from the internet and it could store them in a deps folder, so dependency bar would be stored at foo/deps/bar. Each project has its .act source in src and the generated C files go in out/types/*.c, so we get something like:

  • foo/
    • deps/
      • bar/
        • out/types/
          • bar_a.c
        • src/
          • bar_a.act
    • src/
      • foo.act

Now the build.zig in foo project will see that there is a bar dependency and thus create something like

    var dep_path = function_to_compute_path(blargh); // dep_path will be like deps/bar
    const dep = b.anonymousDependency(dep_path, @import("build.zig"), .{
        .target = target,
        .optimize = optimize,
    });

since the dependency project is just another well-formed Acton project, it can reuse the same build.zig that "we", the main level "foo" project, is using. Then we'd have a list of dep build.zig as per above and we'd link in their artifacts. Does that (my explanation) make sense? Do you see how the equivalent can be achieved with this patch?

I'd really prefer if Zig would support my use case in a sort of official capacity. Like if @andrewrk & co think that build.zig should be a viable option for roughly the use case in Acton, as a low level compilation apparatus, that would be ideal! I don't mind going a little deep into the internal details (with some help) but it's really rather tough if my use case is just never an intended one and I'll always have to be fighting changes that try to rip the rug from under my feet.

@castholm
Copy link
Contributor Author

castholm commented Mar 10, 2024

@plajjan
I do believe there are multiple of ways of supporting your use case without anonymousDependency, but which strategy to suggest depends a lot on the exact details of how Acton dependencies are managed.

Are dependencies always fetched and copied to deps/ or is deps/ committed to revision control and meant to be able to contain vendored dependencies? What if a main project depends on alfa, bravo and charlie, but bravo also depends on charlie, will there be both deps/charlie/ and deps/bravo/deps/charlie/ or does the package manager deduplicate them?

Another thing you might want to consider is making the Acton package manager a wrapper around the Zig package manager to get all the caching, deduplication and hash integrity checking for free.

You say you don't want to generate build.zig.zon files, but is there any particular reason why? Is it because of aesthetic preferences and wanting to hide implementation details from the user? One approach that uses the Zig build system and package manager "as intended" might be to have the Acton package manager generate build.zig and build.zig.zon files (with relative paths) when you add/remove Acton dependencies or fetch the dependency tree for a checked-out project the first time.

main/
+- <acton package manifest>
+- build.zig      ; generated, not committed to revision control
+- build.zig.zon  ; generated, not committed to revision control
+- src/
+- out/
+- deps/ ; not committed to revision control
   +- abc/
   |  +- build.zig     ; generated
   |  +- build.zig.zon ; generated
   |  +- src/
   |  +- out/
   +- xyz/
      +- build.zig     ; generated
      +- build.zig.zon ; generated
      +- src/
      +- out/

The build.zig.zon files would reference dependencies using relative paths and generally look something like this:

.{
    .name = "main",
    .version = "0.0.0",
    .dependencies = .{
        .abc = .{
            .path = "deps/abc/",
        },
        .xyz = .{
            .path = "deps/xyz/,
        },
    },
    .paths = .{""},
}

Assuming there is a base build.zig template that all well-formed Acton projects must adhere to and that the only thing that might differ between them are lines of code that reference dependencies, generating all of these files should be fairly trivial.

(Also a disclaimer: I am just a random contributor not affiliated with the core Zig team, so I am not speaking in any official capacity and only suggesting what I know given my understanding of the build system.)

@plajjan
Copy link

plajjan commented Mar 14, 2024

@castholm again thanks for your time, I'm really grateful for your feedback! :)

I realize now while reading through this thread again and composing this reply that the file you looked at initially and the dependencies it has are our vendored C dependencies. That wasn't my intended focus, I am mostly concerned with dependencies between Acton projects. Such dependencies are not committed to git. I imagined they would be populated in the top level deps/ directory by doing like acton fetch or similar, so that's sort of deduplication although it would probably be ignorant of versions so couldn't properly handle diamond dependencies, but that's OK for now. The longer term goal for Acton is to use content hashing which will solve this much more elegantly.

Anyhow, since I started commenting here, I actually went ahead and implemented support for dependencies in the Acton compiler and low level build system (which is essentially build.zig). Yes, I prefer the aesthetics of not materializing build.zig files in the project directories since I consider it implementation internal. Since all Acton project are well-formed and have the same form, I can use the same build.zig files for the top level project as well as the dependencies projects. The relevant part in our build.zig is at https://github.com/actonlang/acton/blob/main/builder/build.zig#L321, included here:

            // Link project dependencies
            const deps_path = joinPath(b.allocator, buildroot_path, "deps");
            const deps_dir = std.fs.cwd().openDir(deps_path, .{ .iterate = true });
            if (deps_dir) |dir| {
                //defer dir.close();
                var deps_walker = dir.iterate();
                while (deps_walker.next() catch unreachable) |dep_entry| {
                    if (dep_entry.kind == .directory) {
                        // Process sub-directory. For example, print its name.
                        std.debug.print("Found sub-directory: {s}\n", .{dep_entry.name});
                        const dep_path = joinPath(b.allocator, deps_path, dep_entry.name);
                        libActonProject.addIncludePath(.{ .path = dep_path });
                        executable.addIncludePath(.{ .path = dep_path });
                        const dep_path_rel = joinPath(b.allocator, "deps", dep_entry.name);
                        const dep_dep = b.anonymousDependency(dep_path_rel, @import("build.zig"), .{
                            .target = target,
                            .optimize = optimize,
                            .only_lib = true,
                            .syspath = syspath,
                        });
                        executable.linkLibrary(dep_dep.artifact("ActonProject"));
                    }
                }
            } else |err| {
                std.debug.print("Failed to open directory: {}\n", .{err});
            }

...

so as you can see, we do a anonymousDependency( pointing to the dependency directory, like deps/foo and then we use @import("build.zig") which is the same build.zig as this file, so we just reuse the same build.zig for all dependencies. I think that's quite neat. This way seems neater than having to materialize lots of build.zig and build.zig.zon files on disk, no?

Vendored C libraries are somewhat of a different story since we have much fewer of them. I don't really see the benefit of having a build.zig.zon. I don't think I need the Zig package manager. All the source code for those vendored libraries are part of the Acton distribution itself, so there is no need to download anything or use similar features in the Zig package manager...

I really like Zig and the package manager. For C libraries and such system level libraries I think Zig does such a great job, filling gaps that we have with C (like no package manager). But I think my use of the Zig build system in Acton is just a different use case and what is the best and most elegant solution for the Zig package manager when used in a Zig/C system library project, where build.zig(.zon) files are directly maintained by a human who also directly invokes zig build, might not be what is most appropriate for the use case where zig build is called by another compiler.

It is of course valid for Zig to say that my use case is not the primary or even intended one and shouldn't be explicitly supported or maybe only on some best-effort basis ("if it works it works"). I would hope that the Zig folks (Andrew!?) thinks that "yes, zig should be a formidable choice for use as low level build system for higher level langauges that compile to Zig/C/C++" and thus offer a build.zig design that also caters to this use case. I think that includes having something like anonymousDependency() which I can use without needing a build.zig.zon (or am I just missing what value it brings me?). As you suggested in your original reply, maybe I can poke at some internals and implement my own anonymousDependency just like I want it, but again, I'd much prefer if build.zig offered me an interface that allows me to do this without hacking at internals :)

@castholm
Copy link
Contributor Author

@plajjan

The relevant part in our build.zig is at https://github.com/actonlang/acton/blob/main/builder/build.zig#L321, included here:

It might be worth noting that build.zig's build function is intended to be declaractive and that (at least in the current iteration) doing things like directly accessing the file system or spawning child processes is considered poor form and should be generally avoided. In the future build.zig may be sandboxed for security reasons and not have imperative access to the host system at all.

So whether or not you use b.anonymousDependency (or a custom function replicating its logic once removed) or build.zig.zon + b.dependency, ideally you want to explicitly list all dependencies in code. This means updating that list when dependencies are added or removed.

Vendored C libraries are somewhat of a different story since we have much fewer of them. I don't really see the benefit of having a build.zig.zon. I don't think I need the Zig package manager.

Just to clarify, when I say "package manager" I don't necessarily imply remote packages that are downloaded over network. The package manager is also used for vendored dependencies. build.zig.zon is just a package manifest listing dependencies and it is perfectly fine for all dependencies to be local. If you have a Zig package (a root directory with a build.zig file and optionally a build.zig.zon file) that depends on other Zig packages on disk, you add the relative paths to those packages to the dependencies.<name>.path fields of the depending package's build.zig.zon. This is intended to be the only way of expressing dependencies between packages, regardless of whether they are local or remote.

@andrewrk andrewrk force-pushed the move-standalone-test-cases branch from 4cd2940 to e5bf1a5 Compare April 7, 2024 23:06
@andrewrk andrewrk merged commit 1b27146 into ziglang:master Apr 7, 2024
@andrewrk andrewrk added breaking Implementing this issue could cause existing code to no longer compile or have different behavior. zig build system std.Build, the build runner, `zig build` subcommand, package management release notes This PR should be mentioned in the release notes. labels Apr 7, 2024
@andrewrk
Copy link
Member

andrewrk commented Apr 7, 2024

Thanks for the discussion, @plajjan. I have made the decision to merge this for the 0.12 release in order to prevent the accumulation of more users of this API, however, I would like to invite you to open up a new issue and continue the discussion of this use case. I think the zig build system should be able to support your project, and if it cannot do that satisfactorily, that may be a good reason to revert the decision.

@plajjan
Copy link

plajjan commented Apr 8, 2024

@andrewrk I opened up #19305 in an attempt to take a step back and look at the overall design for the way I'm using zig build as the low level build system in Acton. I would really appreciate your feedback on that. If I'm trying to do things in weird ways I suspect I'll keep running into weird issues because "I'm holding it wrong", so we really got to start with the basic design I think :)

@andrewrk
Copy link
Member

andrewrk commented Apr 8, 2024

@plajjan thanks!

@castholm castholm deleted the move-standalone-test-cases branch April 8, 2024 14:37
@andrewrk andrewrk added this to the 0.12.0 milestone Apr 18, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proposal: remove anonymousDependency once relative paths are supported in build.zig.zon
4 participants