Coder Social home page Coder Social logo

Comments (8)

ndmitchell avatar ndmitchell commented on April 18, 2024 1

I've just put up an internal diff for anon_targets implemented natively. It should make it to the outside world tomorrow after diff review.

After reading everything though, I'm not sure anon targets are the best way to go. Given you have the dependency list statically, my approach would have been to generate a separate named target for each named entry, and pass it the deps explicitly. That basically reflects the static graph into a Buck2 static graph, and everything just works from there. There are lots of restrictions on anon targets, and its harder to introspect the graph, whereas if they are real named targets its much easier.

from buck2.

thoughtpolice avatar thoughtpolice commented on April 18, 2024 1

Actually the dependency list doesn't need to be static, I just did it that way to make my first approach as simple as possible and make it work. In fact, I could generate the dependency list on-demand by using nix store path-info --json -r, which is basically what I do "out of band" right now to generate toolchains.bzl. I don't actually need toolchains.bzl at all — I can generate the root hash (i.e. one of the hashes in nix_toolchains) and the dependency graph of that hash (i.e. the graph in nix_toolchains_dep_graph) all on demand, purely from a name like "rustc-stable". So, the static approach is just useful here for me getting started. I just wasn't sure of how to really structure things and decided to try this approach instead, and figured anonymous targets "sounded like the right thing".

So now I see two sorts of approaches here:

1) Continue with a static pre-computed graph, and encode target dependencies explicitly.

This basically sounds like what you recommend. Instead of generating toolchains.bzl, I can generate a TARGETS file that instead would encode this in more or less the same way. So, something like:

load("@prelude//:nix.bzl", "build_toolchain_store_path_rule")

build_toolchain_store_path_rule(
  name = "rust-stable"
  hash = "5jfg0xr0nkii0jr7v19ri9zl9fnb8cx8-rust-default-1.65.0"
  deps = [
      "sdsqayp3k5w5hqraa3bkp1bys613q7dc-libunistring-1.0",
      ...
      "2dvg4fgb0lvsvr9i8qlljqj34pk2aydd-rust-docs-1.65.0-x86_64-unknown-linux-gnu",
      "0fv17zk01p08zh6bi17m61zlfh93fcwj-rust-std-1.65.0-x86_64-unknown-linux-gnu",
    ]
)

Users then ask for the nix://:rustc-stable toolchain or whatever as a dependency, and the rest "just works"

The downside of this approach is that I must manually regenerate the TARGETS file "out of band" and it can become desynchronized from the Nix code I use to create these paths (i.e. I update the nix code but forget to regenerate.)

2) Go fully dynamic

Based on what I previously said: I can generate the dependency list for a "root hash" on demand, then toposort it. This has an advantage that it keeps the toolchains.bzl file simpler. I can go a step further and generate the hash on demand, too, actually, purely from a name like "rustc-stable". This step is more expensive (because it requires evaluating Nix code), but because Buck can do early-cut off, re-evaluating isn't always a big loss (i.e if "rustc-nightly" changes its hash, but "rustc-stable" doesn't, then re-evaluating the Nix code and computing the hash for "rustc-stable" is OK, since early cut off will kick in, as it's the same.)

Going all the way actually has a big advantage: it means I never need to compute the set of root hashes, or generate a TARGETS file, like above. This eliminates a source of errors where a developer updates the set of "root hashes", but forgets to update the corresponding TARGETS file. While this can be caught in CI or something, it's nice to eliminate. I've actually already done this on accident once, but it's not a dealbreaker either.

The downside of dynamism is that graph introspection becomes worse and likely other things I don't know about.


I get the feeling that doing 2 in Shake would have been pretty easy, but my attempts at using dynamic_output have so far been much more modest in Buck. Do you have any inputs on which approach might be better? I don't exactly know how this will impact Buck's other features; its design still remains much of a mystery to me...

from buck2.

ndmitchell avatar ndmitchell commented on April 18, 2024 1

Note that because the TARGETS file is just calls to predefined functions, you could keep your toolchains.bzl exactly as it was, create a function interpret_toolchains that "generates" the TARGETS file off your data very easily. Up to you whether that's preferable.

The dynamic features in Buck2 are deliberately restricted so that you still have a mostly static graph with just occasional points of dynamic/monadic features. In particular there is no way to make analysis dependent on actions. That's really useful at large scale, but that missing primitive might be exactly what stops integrating with a third-party ecosystem (Nix, but also Cargo/Cabal too). I'm going to do some brainstorming in case there is something we can do to fix that.

As to design advice, I don't think we know right now. Most things are static. As we introduce dynamic features, we tend to find holes which really needed them, but where people worked around them before without really knowing how useful they would be. I hope to write a paper on the dynamic bits we added, why/where, and what their ultimate level of power is. But part of the reason for wanting to write that paper is to figure out those details for myself!

from buck2.

ndmitchell avatar ndmitchell commented on April 18, 2024 1

0bb53f8 defines ctx.actions.anon_targets

from buck2.

ndmitchell avatar ndmitchell commented on April 18, 2024

Great you are enjoying Buck2!

While Buck2 has been in use internally for a while, anon_targets are fresh off the press. I think what you ran into is the bug fixed in 37dbffd (2 days ago). Can you see if that fixes your issue? Note that we do have an extensive internal test suite for things like anon targets, but unfortunately our test suite has both pure tests (like anon targets) and tests that certain internal dependencies and targets work. We need to unpick that before we open source it. (I'll respond to the rest slightly later, but that fix might be useful for you right now!)

from buck2.

thoughtpolice avatar thoughtpolice commented on April 18, 2024

Yes, that worked! I've been updating Buck every couple of days but missed that change in the history (it moves pretty fast!) And now the first version of my Nix-dependency fetcher works! Thank you! I still need to make the anon_targets share their own dependencies I think ("Problem 2"), but that's not a correctness issue for now.

Here's my first cut of this working, using transitive sets in order to build the shared topological graph, given a toolchains.bzl file like above. Note that this isn't the prime use case for transitive sets, I know that — but it's a great way to get a toposort done inside of Starlark itself; otherwise I'd have to write more jq/python/bash code to create a pre-toposorted toolchains.bzl. This keeps the bash automation simpler while not needing extra stuff in the mix.

NixStoreOutputInfo = provider(fields = [ "path" ])

NixTransitiveSets = transitive_set()

def __nix_build_tset(ctx, start_name):
    __nix_tset_cache = {}
    def k(ctx, name):
        deps = __nix_toolchain_dep_graph__[name]["r"]
        if len(deps) == 0:
            if name not in __nix_tset_cache:
                __nix_tset_cache[name] = ctx.actions.tset(NixTransitiveSets, value = name)
            return name
        else:
            for d in deps:
                if d not in __nix_tset_cache:
                    k(ctx, d)

            children = [__nix_tset_cache[d] for d in deps]
            __nix_tset_cache[name] = ctx.actions.tset(NixTransitiveSets, value = name, children = children)
        return name

    return __nix_tset_cache[k(ctx, start_name)]

def __mk_nix_build_cmd(ctx, hash, deps=[]):
    out = ctx.actions.declare_output("{}".format(hash))
    storepath = "/nix/store/{}".format(hash)
    args = cmd_args([
        "nix", "build",
        "--extra-substituters", "https://buck2-nix-cache.aseipp.dev/",
        "--trusted-public-keys", "cache.nixos.org-1:6NCHdD59X431o0gWypbMrAURkbJ16ZPMQFGspcDShjY= buck2-nix-preview.aseipp.dev-1:sLpXPuuXpJdk7io25Dr5LrE9CIY1TgGQTPC79gkFj+o=",
        "--out-link", out.as_output(),
        storepath,
    ]).hidden(deps)
    ctx.actions.run(args, category = "nix")
    return [out, storepath]

def __nix_build_hash_0(ctx):
    [out, storepath] = __mk_nix_build_cmd(ctx, ctx.attrs.hash)
    return [ DefaultInfo(default_outputs = [out]), NixStoreOutputInfo(path = out) ]

__nix_build_hash = rule(
    impl = __nix_build_hash_0,
    attrs = { "hash": attrs.string() },
)

def __anon_targets(ctx, xs, k=None):
    def f(hs, ps):
        if len(hs) == 0:
            return k(ctx, ps) if k else ps
        else:
            return ctx.actions.anon_target(__nix_build_hash, hs[0]).map(
                lambda p: f(hs[1:], ps+[p])
            )
    return f(xs, [])

def __nix_build_toolchain_store_path_impl(ctx: "context"):
    if ctx.label.name not in __nix_toolchains__:
        fail("Bad toolchain name!")
    toolchain_info = __nix_toolchains__[ctx.label.name]
    hash = toolchain_info["out"]
    tset = __nix_build_tset(ctx, hash)

    deps = []
    for t in reversed(tset.traverse(ordering = "topological")):
        deps.append(t)

    def k(ctx, ps):
        deps = [p[NixStoreOutputInfo].path for p in ps]
        [out, storepath] = __mk_nix_build_cmd(ctx, hash, deps)
        return [ DefaultInfo(default_outputs = deps + [out]), NixStoreOutputInfo(path = out) ]

    return __anon_targets(ctx, [{"hash": d} for d in deps], k)

build_toolchain_store_path_rule = rule(impl = __nix_build_toolchain_store_path_impl, attrs = {})

Given the following TARGETS file:

load("@prelude//:nix.bzl", "build_toolchain_store_path_rule")

build_toolchain_store_path_rule(name = "rust-stable")

I can have Buck provision any Nix store path described in toolchains.bzl and have each dependency in the graph tracked. This is great!

from buck2.

thoughtpolice avatar thoughtpolice commented on April 18, 2024

I've now implemented a static build graph with a pre-computed TARGETS file here, based on what I described: https://github.com/thoughtpolice/buck2-nix/blob/621ad6dac683ebc1ce5b8f891e8de9ac6c149ec6/buck/nix/toolchains/TARGETS

This really is working great, and the downsides aren't so bad at the moment. With this, you can simply cd && buck build ... and everything will be provided on demand, including necessary Nix closures!

It would still be nice to think more about this, but for now I consider this done. The previous code is a fair approximation of what I attempted before, so I think it's a good record of fact. Feel free to address other things/close this as you wish.

from buck2.

thoughtpolice avatar thoughtpolice commented on April 18, 2024

My new solution has been working really well and is now based on the interpret_toolchains approach you mentioned, and anon_targets being included by default is great; so I'm going to close this as solved for now.

from buck2.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.