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

Packages depending on other packages doesn't work #855

Closed
Hejsil opened this issue Mar 24, 2018 · 10 comments
Closed

Packages depending on other packages doesn't work #855

Hejsil opened this issue Mar 24, 2018 · 10 comments
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase. zig build system std.Build, the build runner, `zig build` subcommand, package management
Milestone

Comments

@Hejsil
Copy link
Sponsor Contributor

Hejsil commented Mar 24, 2018

Three file example:
a.zig:

const b = @import("b");
pub fn main() void { b.something(); }

b.zig:

const c = @import("c");
pub fn something() void { c.something(); }

c.zig:

pub fn something() void { }

build.zig:

const std = @import("std");

pub fn build(b: *std.Build) void {
    const optimize = b.standardOptimizeOption(.{});
    const exe = b.addExecutable(.{
        .name = "a",
        .root_source_file = .{ .path = "a.zig" },
        .optimize = optimize,
    });
    const mod_c = b.addModule("c", .{
        .source_file = .{ .path = "c.zig" },
    });
    const mod_b = b.addModule("b", .{
        .source_file = .{ .path = "b.zig" },
        .dependencies = &.{
            .{
                .name = "c",
                .module = mod_c,
            },
        },
    });
    exe.addModule("b", mod_b);

    b.default_step.dependOn(&exe.step);
}

When using zig build I get:

b.zig:1:11: error: unable to find 'c'
const c = @import("c");

I've looked around in build.zig, but it doesn't seem like there is a way to add package path c.zig to b. Actually, it seems that build.zig just passes the info to the zig compiler through command line args:

zig build-exe a.zig --cache-dir zig-cache --output zig-cache/a --name a --pkg-begin b b.zig --pkg-end --pkg-begin c c.zig --pkg-end
@andrewrk
Copy link
Member

andrewrk commented Mar 24, 2018

I think we need to add support for it in std/build.zig but here's the command line options you want (note the recursive --pkg-begin):

zig build-exe a.zig --cache-dir zig-cache --output zig-cache/a --name a --pkg-begin b b.zig --pkg-begin c c.zig  --pkg-end --pkg-end

This puts c as a sub-package in b.

@andrewrk
Copy link
Member

I think this is #353

@andrewrk andrewrk added this to the 0.4.0 milestone Mar 24, 2018
@emekoi
Copy link
Contributor

emekoi commented Feb 7, 2019

should we add a package object similar to the executable and library object? furthermore, does --pkg-begin and --pkg-end allow for specifying build flags unique to that package?

@BenoitJGirard
Copy link
Contributor

As Windows has a maximum size for the command (8K from cmd.exe, 32K from StartProcess()), seems to me that "--pkg-begin" and "--pkg-end" will not scale for large projects with many edges in their package dependency graph.

Unless I am missing something?

Maybe the tree described by "--pkg-begin" and "--pkg-end" should be imported from a file, instead?

@andrewrk
Copy link
Member

@BenoitJGirard you are correct.

How packages work for zig is still in the design phase. The command line interface is one of the things that will be addressed. I'm planning on publishing a detailed design specification proposal for the zig package manager shortly after 0.4.0 is released.

@BenoitJGirard
Copy link
Contributor

@andrewrk Understood.

I like how "--pkg-begin" and "--pkg-end" force the explicit listing of the dependencies all the way down the dependency graph. Policing the dependency graph for large projects is one of the headaches we have at my day job. I hope your upcoming design keeps that mindset.

@andrewrk andrewrk modified the milestones: 0.4.0, 0.5.0 Feb 19, 2019
thejoshwolfe added a commit to thejoshwolfe/legend-of-swarkland that referenced this issue Mar 18, 2019
@ghost ghost mentioned this issue Aug 7, 2019
@SamTebbs33
Copy link
Contributor

The command used is now zig build-exe a.zig --cache-dir zig-cache --name a --pkg-begin b b.zig --pkg-end --pkg-begin c c.zig --pkg-end --cache on and interestingly, re-ordering the calls to addPackagePath doesn't give the same issue.

@andrewrk andrewrk modified the milestones: 0.5.0, 0.6.0 Sep 20, 2019
@andrewrk andrewrk added this to To do in Package Manager Oct 17, 2019
@andrewrk andrewrk modified the milestones: 0.6.0, 0.7.0 Feb 10, 2020
demizer added a commit to demizer/markzig that referenced this issue May 13, 2020
To run tests requires a carful workaround for packages that depend on
packages. See ziglang/zig#855. Basically,
tests cannot be ran using "zig build test" because of package
dependencies.
@andrewrk andrewrk modified the milestones: 0.7.0, 0.8.0 Oct 10, 2020
@andrewrk andrewrk modified the milestones: 0.8.0, 0.9.0 Nov 6, 2020
@SpexGuy SpexGuy added the use case Describes a real use case that is difficult or impossible, but does not propose a solution. label Feb 12, 2021
@andrewrk andrewrk added contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase. and removed use case Describes a real use case that is difficult or impossible, but does not propose a solution. labels Apr 23, 2021
@andrewrk andrewrk added the zig build system std.Build, the build runner, `zig build` subcommand, package management label Apr 23, 2021
@andrewrk
Copy link
Member

This might be working today. Here's what needs to happen for this to be closed:

  • Investigate whether there is still an error trying to do this with the zig build system.
  • Make sure there is test coverage in test/standalone/* (perhaps it already has coverage?)
    • add test coverage if not

@andrewrk andrewrk modified the milestones: 0.9.0, 0.10.0 May 19, 2021
@nektro
Copy link
Contributor

nektro commented Jul 16, 2021

this happened because addPackagePath adds a package with no dependencies. so the tree looks like this

root
  -> b
  -> c

but what you really need is

root
  -> b
    -> c

so what they need to do is

const pkg_c = Pkg{ .name = "c", .path = .{ .path = "c.zig" } };
const pkg_b = Pkg{ .name = "b", .path = .{ .path = "b.zig" }, .dependencies = &[_]Pkg{ pkg_c } };
exe.addPackage(pkg_b);

@andrewrk andrewrk modified the milestones: 0.13.0, 0.11.0 Jan 18, 2023
@andrewrk andrewrk moved this from Uncategorized to QA in Package Manager Jan 18, 2023
@andrewrk
Copy link
Member

This indeed works today, and has test coverage via these files:

$ grep -RI addModule ../test/standalone
../test/standalone/dep_recursive/build.zig:    exe.addModule("foo", foo);
../test/standalone/dep_triangle/build.zig:    exe.addModule("shared", shared);
../test/standalone/dep_mutually_recursive/build.zig:    exe.addModule("foo", foo);
../test/standalone/test_runner_module_imports/build.zig:    t.addModule("module2", module2);

andrewrk added a commit to thejoshwolfe/legend-of-swarkland that referenced this issue Jun 12, 2023
* use the new build system API to avoid hard-coded references to
  zig-cache
  - image generation now runs in parallel to compilation
  - it does caching now to make subsequent builds faster (.d file
    parsing is a TODO in upstream zig however)
* avoid having a file be in multiple modules at once, which was always
  unsound. now everything is properly separated into modules.
* communicate git version via build system options API rather than
  hard-coded dependency on zig-cache directory
* delete deps/zig-sdl in favor of using zig package manager
* remove workaround for ziglang/zig#855
* update to new `@memset`
* update for loop syntax
* update reflection semantics
* update for breaking std lib changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase. zig build system std.Build, the build runner, `zig build` subcommand, package management
Projects
Package Manager
  
Quality Assurance
Development

No branches or pull requests

7 participants