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

Unable to compile spirv-std-macros with old Cargo.lock. #1053

Closed
dsvensson opened this issue Apr 25, 2023 · 5 comments
Closed

Unable to compile spirv-std-macros with old Cargo.lock. #1053

dsvensson opened this issue Apr 25, 2023 · 5 comments
Labels
t: bug Something isn't working

Comments

@dsvensson
Copy link

dsvensson commented Apr 25, 2023

When trying to get this project working with more recent versions of dependencies I get the following with spirv-std bumped to 0.7.0

error[E0599]: no method named `token` found for struct `LitStr` in the current scope
   --> C:\....\.cargo\registry\src\index.crates.io-6f17d22bba15001f\spirv-std-macros-0.7.0\src\lib.rs:790:65
    |
790 | ...                   new_t.push(TokenTree::Literal(l.token()));
    |                                                       ^^^^^ method not found in `LitStr`

I bumped the rust-toolchain.toml to the same as in this repo fwiw.

@dsvensson dsvensson added the t: bug Something isn't working label Apr 25, 2023
@eddyb
Copy link
Contributor

eddyb commented Apr 26, 2023

Does this go away when you run cargo update -p syn? If so, the issue is that we only require an old version:

syn = { version = "1.0.58", features = ["full", "visit-mut"] }

But some other dependencies are forcing newer versions:

rust-gpu/Cargo.lock

Lines 2386 to 2388 in a64857a

[[package]]
name = "syn"
version = "1.0.105"

Thankfully, this shouldn't affect newer projects, as Cargo will always pick the newest version when creating the Cargo.lock, but it is a mistake on our part and we should be careful with dependencies like that.

This is probably not strong enough, but we should at least run cargo update -Z minimal-versions in CI, to force dependency versions in Cargo.toml files to be large enough to allow building & passing tests (cc @repi).

But I just tried that and it fails to compile anyhow 1.0.0, so that probably means the ecosystem isn't necessarily readily to do something that broad. Also, it would perhaps be nicer if we just forced version bumps in Cargo.lock to be reflected in Cargo.toml, instead, but I can't find a command that would make the two equal.

EDIT: turns out that this can be used by making an isolated package that depends on spirv-std alone.
(as the deps broken by -Z minimal-versions are all on the host side)

@eddyb eddyb changed the title Unable to compile spirv-std-macros on windows Unable to compile spirv-std-macros with old Cargo.lock. Apr 26, 2023
@eddyb
Copy link
Contributor

eddyb commented Apr 26, 2023

@dsvensson I've opened a tentative fix:

But note that the simplest fix for that project is still likely running cargo update -p syn.

@dsvensson
Copy link
Author

dsvensson commented Apr 26, 2023

Aha, doh, so deleting Cargo.lock and rebuilding, should have tried. 🤦‍♂️ Thanks, works now.

@eddyb
Copy link
Contributor

eddyb commented Apr 26, 2023

Aha, doh, so deleting Cargo.lock and rebuilding, should have tried.

No, that's a really bad idea, you want to minimize Cargo.lock changes, not maximize them.
The command I gave you should only update the syn version, not unrelated crates.

When you're trying to fix something, you shouldn't change unrelated things that could cause other complications.

@dsvensson
Copy link
Author

Agree generally, but this is a small toy project almost no deps, and I'm just scouting around various vulkan related projects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants