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

handle OptionsStep in build_runner #686

Merged
merged 2 commits into from
Sep 29, 2022

Conversation

leecannon
Copy link
Member

@leecannon leecannon commented Sep 28, 2022

This allows generated files from OptionsStep to be resolved.

This is dependant on ziglang/zig#13003 I will leave it as a draft until it is merged.

I'm not 100% happy with this due to:

  • We can't know how costly the dependencies of the step might be so i've decided to only support OptionsSteps that have no dependencies.
    Thankfully this is the most common situation, I don't think i've ever seen an OptionsStep have any dependencies.
  • It's extremely common for the pacakge to be called build_options, this could easily result in multiple packages with the same name pointing to different files.
    As before this is an uncommon issue to hit as multiple OptionsSteps is rare.
    This is one more reason the TODO in build_runner.zig would be nice to fix.
    // TODO: We currently add packages from every LibExeObj step that the install step depends on.
    // Should we error out or keep one step or something similar?
    // We also flatten them, we should probably keep the nested structure.

Before:

$ /home/lee/bin/zig run /home/lee/src/zls/src/special/build_runner.zig --cache-dir /home/lee/.cache/zls --pkg-begin @build@ build.zig --pkg-end -- /home/lee/bin/zig /home/lee/src/fff/ zig-cache ZLS_DONT_CARE
{
    "packages": [
        {
            "name": "args",
            "path": "libraries/zig-args/args.zig"
        },
        {
            "name": "known_folders",
            "path": "libraries/known-folders/known-folders.zig"
        },
        {
            "name": "bestline",
            "path": "libraries/bestline/bestline.zig"
        },
        {
            "name": "tracy",
            "path": "libraries/tracy/tracy.zig"
        },
        {
            "name": "zriscv",
            "path": "zriscv/zriscv.zig"
        },
        {
            "name": "bitjuggle",
            "path": "libraries/zig-bitjuggle/bitjuggle.zig"
        }
    ],
    "include_dirs": [
        "libraries/bestline/bestline"
    ]
}%

After:

$ /home/lee/bin/zig run /home/lee/src/zls/src/special/build_runner.zig --cache-dir /home/lee/.cache/zls --pkg-begin @build@ build.zig --pkg-end -- /home/lee/bin/zig /home/lee/src/fff/ zig-cache ZLS_DONT_CARE
{
    "packages": [
        {
            "name": "build_options",
            "path": "/home/lee/src/zriscv/zig-cache/options/_OHWmub1P7ZxHbsba8NlAXpkS2ryAiTxuWgRR8iH9_AHeEPbLP9T8iSLLH4QQeqm"
        },
        {
            "name": "args",
            "path": "libraries/zig-args/args.zig"
        },
        {
            "name": "known_folders",
            "path": "libraries/known-folders/known-folders.zig"
        },
        {
            "name": "bestline",
            "path": "libraries/bestline/bestline.zig"
        },
        {
            "name": "tracy",
            "path": "libraries/tracy/tracy.zig"
        },
        {
            "name": "zriscv",
            "path": "zriscv/zriscv.zig"
        },
        {
            "name": "bitjuggle",
            "path": "libraries/zig-bitjuggle/bitjuggle.zig"
        }
    ],
    "include_dirs": [
        "libraries/bestline/bestline"
    ]
}%

@leecannon leecannon marked this pull request as ready for review September 29, 2022 11:44
@leecannon
Copy link
Member Author

leecannon commented Sep 29, 2022

ziglang/zig#13003 has been merged, might be best to wait for ziglang download to update before merging this though.

Copy link
Member

@SuperAuguste SuperAuguste left a comment

Choose a reason for hiding this comment

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

This is awesome! :)

This combined with #688 and #683 means we're really stepping up our build.zig completions :P

Btw, about your concerns; I personally believe in Andreas Kling(?)'s mindset that features shouldn't have to be 100% complete or perfect and that many people building them one block at a time is a lot more effective, so either you or someone else will come along and (hopefully :P) address these TODOs.

LGTM!

@SuperAuguste SuperAuguste merged commit 9f2ea75 into zigtools:master Sep 29, 2022
@leecannon leecannon deleted the build_runner_options branch September 29, 2022 18:44
This was referenced Sep 29, 2022
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.

None yet

2 participants