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

ability to fetch only needed dependencies #14597

Closed
andrewrk opened this issue Feb 8, 2023 · 8 comments · Fixed by #18778
Closed

ability to fetch only needed dependencies #14597

andrewrk opened this issue Feb 8, 2023 · 8 comments · Fixed by #18778
Labels
accepted This proposal is planned. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. zig build system std.Build, the build runner, `zig build` subcommand, package management
Milestone

Comments

@andrewrk
Copy link
Member

andrewrk commented Feb 8, 2023

This is a competing proposal to #14591.

Inside of a build.zig file, there may be conditional logic that decides whether a dependency is needed. For example:

const std = @import("std");

pub fn build(b: *std.build.Builder) void {
    const target = b.standardTargetOptions(.{});
    const optimize = b.standardOptimizeOption(.{});

    const enable_libmp3lame = b.option(bool, "enable-libmp3lame",
        "support MP3 decoding via libmp3lame") orelse true;

    const lib = b.addStaticLibrary(.{
        .name = "ffmpeg",
        .target = target,
        .optimize = optimize,
    });
    lib.linkLibrary(libz_dep.artifact("z"));
    if (enable_libmp3lame) {
        const libmp3lame_dep = b.dependency("libmp3lame", .{
            .target = target,
            .optimize = optimize,
        });
        lib.linkLibrary(libmp3lame_dep.artifact("mp3lame"));
    }
    lib.linkLibC();
    lib.addIncludePath(".");
}

Key point here being that b.dependency("mp3lame") is only called if the enable_libmp3lame option is set to true. This means that when that option is not set, libmp3lame is in fact not a dependency.

This proposal is to embrace arbitrary logic deciding what is or isn't a dependency. Instead of marking categories of dependencies in the build.zig.zon file, Zig would have the heuristic choice to pre-fetch some, all, or none of the dependencies. When running build.zig, if any packages referenced with dependency() were not fetched already, the build runner would exit in a special manner and communicate the set of dependency packages that were missing. Zig would fetch them and then re-execute the build runner.

Along with this proposal would come a new flag to zig build: --fetch. Note that this is still part of the zig build subcommand - indeed it would support all of the same flags and build options as a standard zig build operation. However, in this case, after running the build.zig script, it would not actually perform the make(). Instead it would only fetch missing packages for the given set of flags. In most typical cases, no additional flags would be used, but one could imagine passing something like this: zig build --fetch --Denable-libmp3lame=false to avoid fetching libmp3lame in the above example.

Related: #14283

@andrewrk andrewrk added proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. zig build system std.Build, the build runner, `zig build` subcommand, package management labels Feb 8, 2023
@andrewrk andrewrk added this to the 0.11.0 milestone Feb 8, 2023
@star-tek-mb
Copy link
Contributor

Question, how can we fetch dependency only if we run some command?

const exe = b.addExecutable(.{
    .name = "ffmpeg",
    .target = target,
    .optimize = optimize,
});
exe.linkLibrary(libz_dep.artifact("z"));
const benchmark_cmd = exe.run();
const benchmark_step = b.step("benchmark", "Benchmark");
benchmark_step.dependOn(&benchmark_cmd.step);

How can we build exe (and fetch its dependencies) only if:

zig build benchmark

has been invoken.

@InKryption
Copy link
Contributor

InKryption commented Feb 8, 2023

I imagine that could be done by only calling std.Build.dependency in a build step, which is depended on by benchmark.

@dweiller
Copy link
Contributor

dweiller commented Feb 9, 2023

If I understand correctly, the proposal here is to not add functionality to cover optional dependencies or those only needed for a package's development as the conditional use of dependency() will always be possible. The issue I see with relying on this type of conditional use is it requires all transitive dependees of a package to correctly pass arguments to child builders to prevent unneeded (transitive) dependencies from being fetched.

For example if libmp3lame above has some static website generator as a dependency because the developers publish documentation via zig build publish-docs, then either ffmpeg needs to realise this and make sure it's disabled in all branches that depend on libmp3lame, or they need to expose an option to disable it for their dependees and give the decision to them. This could end up complicating build.zigs quite a bit and definitely gets away from the declarative style that they often have.

I think either declaring dependencies as explicitly optional/dev-only as in #14591 or just fetching lazily (as suggested in comments there) would helpful for reducing this complication in build.zig - it should (at least in common cases) simplify the job of both package developers and their dependees, and if there are cases it can't handle well it won't preclude using arbitrary logic as suggested in this issue.

The --fetch flag behaviour can work independently of how optional dependencies handled and I think is a good idea regardless.

@kuon
Copy link
Contributor

kuon commented Feb 11, 2023

Lazy fetching should be the default, and we should have a fetch command that fetch everything (for example to prepare a source tarball with all dependencies, so it can be built on a machine with no network).

If we provide an explicit fetch command, I do not see any downside to lazy fetching, and it would cover the concern that @dweiller pointed.

@atweiden
Copy link

atweiden commented Aug 5, 2023

How might this proposal handle a hypothetical “build dependencies” feature? Cargo offers a [build-dependencies] section in its manifest to enable using crates in build scripts. I’d like to be able to do the same in build.zig with dependencies declared in build.zig.zon, e.g.

// build.zig.zon
.{
    .name = "egg",
    .version = "0.0.1",
    .dependencies = .{
        .ziglua = .{
            .url = "https://github.com/natecraddock/ziglua/archive/1f8a76825d3102928903918d6f413fc9613b9927.tar.gz",
            .hash = "1220ba95132772f7e484cca975e7892addf64a596bfffd10159a9d62ae22b91a2d21",
        },
    }
}
// build.zig
const std = @import("std");

pub fn build(b: *std.Build) void {
    const target = b.standardTargetOptions(.{});
    const optimize = b.standardOptimizeOption(.{});
    ...
    useLua(b.allocator);
    ...
}

// Not possible to use ziglua library during build:
//fn useLua(allocator: std.mem.Allocator) void {
//    const ziglua = @import("ziglua");
//    const Lua = ziglua.Lua;
//    var lua = Lua.init(allocator) catch unreachable;
//    defer lua.deinit();
//    lua.openLibs();
//    lua.doString("return 100") catch unreachable;
//    std.debug.print("{d}\n", .{lua.checkNumber(-1)});
//}

// Possible to reference publicly accessible declarations in ziglua’s `build.zig`.
fn useLua(_: std.mem.Allocator) void {
    const ziglua = @import("ziglua");
    for (std.enums.values(ziglua.LuaVersion)) |v| std.debug.print("{}\n", .{v});
}
zig build
build.LuaVersion.lua_51
build.LuaVersion.lua_52
build.LuaVersion.lua_53
build.LuaVersion.lua_54

@nathany
Copy link

nathany commented Sep 27, 2023

I'm trying to imagine a larger example with platform-specific dependencies and dependencies only needed for development/tests. 🤔

On the surface, #14591 appears more declarative and less verbose, but also less flexible. Or would the build.zig file for #14591 look basically the same as the one here?

@slimsag
Copy link
Contributor

slimsag commented Nov 7, 2023

This would solve two challenges we have with using the package manager for platform-specific binaries (#17914) as well as fetching source vs. binaries depending on a build flag (hexops/mach#901) - so would be a great win for us.

@slimsag
Copy link
Contributor

slimsag commented Nov 7, 2023

One slight challenge here: normally dependencies in build.zig may be imported e.g. using @import("foo").someBuildHelper - but with dynamic dependencies like this I'm unsure how that would be done ahead of time.

@andrewrk andrewrk modified the milestones: 0.13.0, 0.12.0 Feb 2, 2024
@andrewrk andrewrk added the accepted This proposal is planned. label Feb 2, 2024
BratishkaErik added a commit to BratishkaErik/zig that referenced this issue May 15, 2024
`--fetch` flag now has additional optional parameter, which specifies
how lazy dependencies should be fetched:
 * `lazy` — lazy dependencies are fetched only if they are required
   for current build configuration to work. Default and works same
   as old `--fetch` flag.
 * `all` — lazy dependencies are always fetched. If `--system` flag
   is used after that, it's guaranteed that **any** build configuration
   will not require additional download of dependencies during build.
   Helpful for distro packagers and CI systems:
   ziglang#14597 (comment)

If none is passed, behaviour is same as if `lazy` was passed.

Signed-off-by: Eric Joldasov <bratishkaerik@landless-city.net>
BratishkaErik added a commit to BratishkaErik/zig that referenced this issue May 15, 2024
`--fetch` flag now has additional optional parameter, which specifies
how lazy dependencies should be fetched:
 * `lazy` — lazy dependencies are fetched only if they are required
   for current build configuration to work. Default and works same
   as old `--fetch` flag.
 * `all` — lazy dependencies are always fetched. If `--system` flag
   is used after that, it's guaranteed that **any** build configuration
   will not require additional download of dependencies during build.
   Helpful for distro packagers and CI systems:
   ziglang#14597 (comment)

If none is passed, behaviour is same as if `lazy` was passed.

Signed-off-by: Eric Joldasov <bratishkaerik@landless-city.net>
BratishkaErik added a commit to BratishkaErik/zig that referenced this issue Jun 17, 2024
`--fetch` flag now has additional optional parameter, which specifies
how lazy dependencies should be fetched:
 * `lazy` — lazy dependencies are fetched only if they are required
   for current build configuration to work. Default and works same
   as old `--fetch` flag.
 * `all` — lazy dependencies are always fetched. If `--system` flag
   is used after that, it's guaranteed that **any** build configuration
   will not require additional download of dependencies during build.
   Helpful for distro packagers and CI systems:
   ziglang#14597 (comment)

If none is passed, behaviour is same as if `lazy` was passed.

Signed-off-by: Eric Joldasov <bratishkaerik@landless-city.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted This proposal is planned. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. zig build system std.Build, the build runner, `zig build` subcommand, package management
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

8 participants