ipetkov / crane

A Nix library for building cargo projects. Never build twice thanks to incremental artifact caching.
https://crane.dev
MIT License
962 stars 92 forks source link

Source workspace name from `workspace.metadata.name` #584

Closed dpc closed 7 months ago

dpc commented 7 months ago

The problem with workspace.package.name is that unlike workspace.package.version nothing really needs to inherit it, as packages usually have more specific names.

So when setting it just so crane can pick up workspace build "name" one will keep getting:

warning: /home/dpc/lab/fedimint/Cargo.toml: unused manifest key: workspace.package.name

on every cargo command.

Would be nice if crane could fallback to workspace.metadata.name if workspace.package.name is not available which cargo doesn't mind being unused.

ipetkov commented 7 months ago

The automatic name/version selection was intended to be a quality of life improvement to not need to keep the attributes in sync with the derivation by hand, but since then I've come to realize that cargo workspaces are a lot more common these days than solo crates.

Given that crateNameFromCargoToml is already a standalone helper, I'm debating whether we should deprecate the whole automatic name selection "feature" and leave it up to the project maintainer to decide which Cargo.toml to use (if at all). Sure we can tack on extra logic to look at workspace.metadata.name but that's yet another thing one would have to fill out just to make it work with our default selection? Perhaps getting rid of it would reduce confusion and therefore friction

dpc commented 7 months ago

In our project, which has > 10 binaries, we compile everything as one workspace. And there is no crate that will have a name that fits the whole workspace. What is the best invocation to set the name + version for the workspace handling derivations then?

I really like the current approach, and if cargo would just shut up about that unused manifest key: workspace.package.name, everything would be perfect. :D

ipetkov commented 7 months ago

Are you building each binary as its own separate derivation? If yes, I'd imagine you could name each derivation after it (e.g. if you have cargoExtraArgs = "-p foo"; you can also set pname = "foo";).

dpc commented 7 months ago

Are you building each binary as its own separate derivation?

Nope. Usually everything as one big build.

ipetkov commented 7 months ago

I really like the current approach, and if cargo would just shut up about that unused manifest key: workspace.package.name, everything would be perfect. :D

Looking at this a bit more closely, I just realized workspace.package.name isn't actually documented anywhere in cargo's behavior and I can't tell if I might have made it up since workspace.package.version is supported :sweat_smile:

With that in mind maybe we should just peek at {workspace,package}.metadata.crane.{name,version} first and then fallback to workspace.package.version or package.{name,version} :thinking:

ipetkov commented 7 months ago

@dpc this will be fixed with https://github.com/ipetkov/crane/pull/606!