Can we have Cargo.lock in the repository?

[thoughtpolice]

This question was bound to come up but, basically, title.

In the quest to add a package to the Nix flake.nix file in the repo, which would allow people to consume or install Buck2 fairly easily, I have come to the cross roads where I need a committed Cargo.lock file for the build to work. The reason is pretty simple: Cargo's whole design requires the lockfile, not the Toml file, to be the deterministic source of truth. There is no guarantee cargo generate-lockfile will generate the same lockfile when run twice in a row, so a committed one is necessary if you want to e.g. keep a stable hash of all dependencies needed and downloading them, like Nix does. Assuming this exists, well, adding the Buck package to the flake file is easy!

But Meta doesn't use Cargo lockfiles, and there are reasons for that. But without getting into it: is this a hard rule? Could we add a Cargo.lock in the top-level directory, and start supporting it? Some other Meta projects such as Hermit for example have lockfiles, while Sapling doesn't — so I'm not sure how hard of a rule this is, or if both can be easily supported.

I don't think it needs to be like a hard blocker for pushing, obviously. Frankly the OSS CI build seems to fail a bit more than would be ideal, but that's life and I understand concerns about velocity, etc. :) But we could just:

• Commit it, keep it working on a best effort basis.
• Put it in the OSS CI build and make sure it gets built on every commit.
• If there are some intervening moments where things are broken, that's sometimes life.

I'm pretty sure at some point this is more or less going to be a requirement in the long run for non-Meta users, people will be very easy about bucking(!!) the trend here and generating their own lockfiles. We actually require this in Nixpkgs more or less too, so we're going to have to either help contribute one or ship one ourselves, eventually (like we already do for Sapling, and like I already do for buck2-nix.)

In the long run as Buck2 evolves and sees more outside usage, the best-effort CI policy can be reconsidered, stuff like that. And assuming this can be all worked out, I can easily submit a change for adding it soon. But I wanted to ask, since I figured if there isn't one already, that's a pretty good sign it should be discussed first...

[thoughtpolice]

FWIW, I already have a working commit for this here, which adds a lockfile, but it ONLY works for the Nix flake, and it adds it under a name outside the toplevel directory so people aren't mislead: https://github.com/thoughtpolice/buck2, see latest commit.

With this, any Nix user on macOS or Linux could now install buck2 via Nix just by saying:

nix profile install github:thoughtpolice/buck2

and they don't need to do anything else, which is pretty cool! Not the most important requirement in the grand scheme or anything, just figured I'd show the motivating example.

[stepancheg]

I like lock files: if we use lock files we make sure internal builds and external use exactly the same dependency version (internally we use vendored dependencies).

However, it may be not trivial to implement, because we modify files when exporting to github, like here (internally this line is not commented out):

buck2/Cargo.toml

Lines 281 to 282 in 23efaf5

	fbinit.version = "0.1"
	# @oss-disable: fbinit.path = "../common/rust/shed/fbinit"

so likely it needs to be a separate lock file for open source, separate machinery to generate it etc.

CC @ndmitchell who IIRC opposed lock files a while ago.

[thoughtpolice]

Yeah, I've seen the @oss-disable stuff sprinkled around and figured that the magical version control server probably had some impact on how this could be pulled off. :) I personally like lockfiles too, and in this particular case I think they're important since you're really installing an application and that's what 99% of people are going to use it for, and they want to know that the version the developers tested is what they're installing, including transitive dependencies.

There are also many use cases for buck2 as a library, of course, in which case a lockfile doesn't apply, because you want resolution to be left up to consumers — but it didn't apply anyway in that case (Cargo won't use an upstream lockfile from a library when compiling a downstream app, of course)

[ndmitchell]

The @oss-disable stuff is not very magical - it just moves the comment markers. So we see:

_INCLUDE_EXECUTABLES = True # @oss-disable

And open source we have:

# @oss-disable: _INCLUDE_EXECUTABLES = True 

No code appears or disappears, just some comment markers move around. I've put up an internal patch to document that in HACKING.md.

Ignoring the internal vs open source differences: When I objected to lock files they often went out of sync and we weren't using the vendored dependencies for our internal Cargo builds. Now we are using the vendored dependencies, so I'm a little less concerned. My concern would be whether the lock file will be updated as we update our vendored dependencies, given things like our internal Cargo generation are a bit weak right now. If we can make that work smoothly, I don't mind. But given our track record in generating Cargo files, I'm not vastly optimistic.

I'm not sure we have anything suitable for generating lock files during export. We could generate lock files during a github action, but they wouldn't be in the repo.

[thoughtpolice]

I realized while writing #315 that we could at minimum commit a Cargo.lock file for use by Reindeer in OSS Buck2-on-Buck2 builds, which can also be shared with Nix, right?

At the very least even if the top-level Cargo.lock can't be generated cleanly in a safe way yet, it would be unified somewhere for outside users. Otherwise, we'd basically have three possible dependency resolution outcomes floating around (top level build, Buck2-on-Buck2, and Nix). So for now that might be a workable fix to get a Nix package in the repository?

[ndmitchell]

CC @dtolnay @jsgf about a Cargo.lock for use by Reindeer as they are much more involved in that project.

[dtolnay]

Reindeer respects Cargo.lock if there is one. Feel free to add one in shim/third-party/rust.

Autocargo supports emitting Cargo.lock -- reindeer itself has one generated by autocargo: https://github.com/facebookincubator/reindeer/blob/main/Cargo.lock. This is guaranteed to be consistent with Meta internal builds.

IMO the way to go is maintain TARGETS for shim/third-party/rust, generate Cargo.toml and Cargo.lock from it, and exclude TARGETS from shipping to GitHub.

[thoughtpolice]

Is autocargo a Meta tool? I can absolutely add a Cargo.lock to the shim directory — but if we could get an emitted Cargo.lock file that will be consistent, and have it automatically put under shim/third-party/rust, that would be all I need for the Nix packaging! Why do something when a fleet of servers can do it for me on the daily?

IMO the way to go is maintain TARGETS for shim/third-party/rust, generate Cargo.toml and Cargo.lock from it, and exclude TARGETS from shipping to GitHub.

That's an interesting idea. Right now, e.g. in #321 I wanted the url crate for a simple reason, but I had to add the url dependency in four places: the workspace cargo file, the crate's cargo file, the crate's BUCK file, and in shim/third-party/rust/Cargo.toml. And the version has to be duplicated between the workspace and shim Cargo files. I've done worse dances but that's kind of annoying and really easy to screw up if you're not careful e.g., many people might not test the buck2-on-buck2 build and can overlook it, so the BUCK file is easy to miss (at least until #315 is merged.)

In the long run of things, for consistency between FOSS and Meta's builds, and just general cleanliness among open developers, having a single source of truth for the whole thing, whether it is a Cargo file or a BUCK file, is really important IMO. (I would much prefer the authentic source to be a BUCK file for a simple reason. Why? Because for a project like this, I think it's more honest, and it doesn't let you cut corners, and you can't just say "Well just use cargo..." when something happens. Just my opinion.)

See also some of my musing in #313; maybe if we believe in it, we can one day have a beautiful future where we could just make all the Cargo-based infrastructure (including this, and automatically creating crates.io-compatible packages) completely hidden, generated entirely from the BUCK files for people who need it...

[huwaireb]

👋

It's unfortunate to see the discussions stall here. As although buck2 has a nixpkgs derivation, rust-project doesn't. And for us to use rust-project we have to manually build from source, including generating a Cargo.lock. It's quite problematic.

@ndmitchell, Any chance you'd be open to including a Cargo.lock file in nix/Cargo.lock and have GitHub CI update it? Or shipping one in shims/third-party/rust would also work, but it has to be maintained somehow.

It will also allow nix users to be able to run nix build .#buck2 from the repository root. And include it in projects e.g inputs.buck2.url = "github:facebook/buck2" -> inputs.buck2.packages.${system}.buck2.