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

Warning: crane cannot find name/version attribute in Cargo.toml #281

Closed
fasterthanlime opened this issue Mar 28, 2023 · 13 comments · Fixed by #282
Closed

Warning: crane cannot find name/version attribute in Cargo.toml #281

fasterthanlime opened this issue Mar 28, 2023 · 13 comments · Fixed by #282

Comments

@fasterthanlime
Copy link
Contributor

fasterthanlime commented Mar 28, 2023

I think these are triggered because crane parses my top-level, workspace Cargo.toml which lacks these attributes:

trace: warning: crane cannot find name attribute in /nix/store/cfr3q64698bi17i84m66q3jjh6xfi3a3-source/Cargo.toml, consider setting `pname = "...";` explicitly
trace: warning: crane cannot find version attribute in /nix/store/cfr3q64698bi17i84m66q3jjh6xfi3a3-source/Cargo.toml, consider setting `version = "...";` explicitly

That top-level Cargo.toml file only has:

[workspace]
members = [
	"crates/*",
]

[workspace.dependencies]
# omitted

[profile.dev]
debug = 1
incremental = true

[profile.release]
debug = 1
incremental = true

This happens for me on 72fa295 still.

@dpc
Copy link
Contributor

dpc commented Mar 28, 2023

Yeah, I get the same thing since some time ago, and I've added fake version = to my workspace builds. Most (all?) of the crane-based derivations are building either whole-workspace or multiple packages, so I'm not sure what crane could use instead, so I didn't report it as an issue.

@ipetkov
Copy link
Owner

ipetkov commented Mar 29, 2023

think these are triggered because crane parses my top-level, workspace Cargo.toml which lacks these attributes:

@fasterthanlime yep that's exactly the reason for that warning! It is an intentional design decision that crane will not try to guess which Cargo.toml represents the derivation itself beyond looking at the root Cargo.toml. If the attributes are not found a placeholder will be used, but the feedback kept pointing to folks being confused why their derivations had placeholder names which led to this warning being added.

There's a few way to silence it:

  • explicitly set pname = "..."; and/or version = "..."; in the derivation arguments
  • set package.name/package.version/workspace.package.name/workspace.package.version in the root Cargo.toml
  • explicitly call crateNameFromCargoToml { cargoToml = ./path/to/Cargo.toml; } and append its results to the derivation arguments

It's also probably worth making this warning message slightly more friendly in terms of suggesting a concrete solution

@fasterthanlime
Copy link
Contributor Author

There's a few way to silence it:

Do any of these impact cacheability? A workspace of mine takes longer to build than I think it should (but maybe that's due to the newly-introduced crane-utils? also cargo tree | wc -l outputs a number > 1500, lolsob)

  • explicitly call crateNameFromCargoToml { cargoToml = ./path/to/Cargo.toml; } and append its results to the derivation arguments

Does this also set the version?

  • set package.name/package.version/workspace.package.name/workspace.package.version in the root Cargo.toml

Is this cargo-compliant or just something cargo happens to ignore?

@ipetkov
Copy link
Owner

ipetkov commented Mar 29, 2023

Do any of these impact cacheability?

They do since they affect what the resulting Nix path would be. Come to think of it we should probably be making buildDepsOnly ignore the crate version (at least) and hard code its own string so that you don't need to rebuild all dependencies when bumping your own crate version 🤔

(reopening to track this)

A workspace of mine takes longer to build than I think it should (but maybe that's due to the newly-introduced crane-utils?

Do you have a lot of derivations using a lot of different rust toolchains by any chance? Current crane-utils is built by whatever toolchain craneLib is overrided with (though I suppose we could pin it to the nixpkgs version to reduce cache invalidation?). It's a pretty small crate, basically a script to handle vendoring git deps, it shouldn't contribute that much to the overall build time but if you have any observations to share I'd be happy to look into it!

Does this also set the version?

It will if the specified Cargo.toml has a version attribute. Note this attribute cannot be inherited from the root workspace (crane does not re-implement cargo's workspace traversal logic).

Is this cargo-compliant or just something cargo happens to ignore?

Fully cargo compliant. The [workspace.package] table is used for inheritance which includes inheriting a default package name and package version

@ipetkov ipetkov reopened this Mar 29, 2023
@fasterthanlime
Copy link
Contributor Author

Fully cargo compliant. The [workspace.package] table is used for inheritance which includes inheriting a default package name and package version

Ah, that's good to know! This feels like the correct fix then.

They do since they affect what the resulting Nix path would be. Come to think of it we should probably be making buildDepsOnly ignore the crate version (at least) and hard code its own string so that you don't need to rebuild all dependencies when bumping your own crate version 🤔

That seems like a good idea - perhaps better suited for a separate issue on this? I'm not sure why you re-opened that issue, if it's just to answer my questions, consider them answered!

@ipetkov
Copy link
Owner

ipetkov commented Apr 2, 2023

perhaps better suited for a separate issue on this? I'm not sure why you re-opened that issue, if it's just to answer my questions, consider them answered!

I wanted to come back and revisit my earlier observation with a fresh head (before forgetting about it) but I'll open a new issue for it, thanks!

@torhovland
Copy link

torhovland commented May 24, 2023

I am getting the version warning on the current master version, even though I think I have set things correctly:

trace: warning: crane will use a placeholder value since `version` cannot be found in /nix/store/8h0vgdvc3zf2ijp91zmb4jfr93b074rf-Cargo.toml
to silence this warning consider one of the following:
- setting `version = "...";` in the derivation arguments explicitly
- setting `package.version = "..."` or `workspace.package.version = "..."` in the root Cargo.toml
- explicitly looking up the values from a different Cargo.toml via 
  `craneLib.crateNameFromCargoToml { cargoToml = ./path/to/Cargo.toml; }`

But my root Cargo.toml has this:

[workspace.package]
version = "0.2.1"

And my other Cargo.toml files (including the Nix store one mentioned in the warning) have this:

[package]
version.workspace = true

If I change the latter to the following, the warning goes away, but that is not ideal:

[package]
version = "0.2.1"

@ipetkov
Copy link
Owner

ipetkov commented May 25, 2023

@torhovland Looks like you are explicitly setting the path to cargoToml which means that's the only file that's being looked at for the package version. You can look at the contents of /nix/store/8h0vgdvc3zf2ijp91zmb4jfr93b074rf-Cargo.toml to confirm, but if that's the case, you'll need to either continue using your workaround, or manually compute and pass in the version attribute

@torhovland
Copy link

You are right, that's a good point. I have now removed all instances of setting cargoToml, and I could remove my workaround too.

I don't think the workspace handling is perfect yet, though. If I set workspace.package.name Cargo gives me a warning that it is unused, because I still have to set package.name explicitly in all the child Cargo.toml files. So I end up having to set pname in the derivations.

@fasterthanlime
Copy link
Contributor Author

If I set workspace.package.name Cargo gives me a warning that it is unused, because I still have to set package.name explicitly in all the child Cargo.toml files

Same here - I've been maintaining something on and off for weeks and I keep ping-ponging back between cargo's warning and crane's warning.

@dpc
Copy link
Contributor

dpc commented May 25, 2023

Would using metadata fields (like [package.metadata.cargo-udeps.ignore] I have around) be a desirable solution here?

@ipetkov
Copy link
Owner

ipetkov commented May 27, 2023

I don't think the workspace handling is perfect yet, though. If I set workspace.package.name Cargo gives me a warning that it is unused, because I still have to set package.name explicitly in all the child Cargo.toml files. So I end up having to set pname in the derivations.

crane cannot support arbitrary crawling of a directory to figure out the exact workspace hierarchy (trying to delegate to cargo quickly requires IFD which has its own set of headaches). Peeking at the root Cargo.toml started out as a convenience to not have to keep the package name and version in sync in the actual derivation. The warning was later added due to feedback where folks were surprised why their package name or version was being replaced with a placeholder...

It's been really hard trying to find a balance where we don't throw out the baby with the bathwater in terms of usefulness and not ever seeing a warning in the build logs...

@torhovland
Copy link

Sure, no problem. Just wanted to report it as a minor inconvenience. It would be nice if there was a simple way to sort it out.

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 a pull request may close this issue.

4 participants