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

Fix build and update to syn v2 #518

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft

Conversation

mleonhard
Copy link

This PR fixes #517 . Here's what it does

  • Rolls back dependabot changes which broke the build
  • Update integration_tests__test9_package_with_git_deps.stdout.snap to match a package version in Cargo.lock
  • Migrate from syn v1 to v2 to support new C-style string literals. This fixes the panic: Unrecognized literal: c"".
  • Since Rust 1.77 changed the cargo metadata package-ID format (fix(metadata): Stabilize id format as PackageIDSpec  rust-lang/cargo#12914), cargo-geiger built with a Rust 1.77 toolchain always fails with:
    thread 'scan::scan_tests::list_files_used_but_not_scanned_test::case_1' panicked at /Users/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/krates-0.11.0/src/builder.rs:919:36:
    called `Option::unwrap()` on a `None` value
    
    We work around this by specifying the Rust 1.76 toolchain. The proper fix requires upgrading the cargo_metadata crate to v0.18. This will require major changes to cargo-geiger's code.

Problems in this PR:

  • Some tests are failing.

@mleonhard
Copy link
Author

This PR keeps growing and growing as I fix more tests. I would love to hear suggestions for splitting it up. It would also be great if there was a way to get syn v2 to parse the same way as v1 so expression counts would match.

@pinkforest
Copy link
Collaborator

pinkforest commented Apr 19, 2024

We're re-writing much of the core incl. to break the cargo-reliance so messy PR is ok for now to fix the techdebt np.

Thanks so much for doing this 🚀

@@ -0,0 1 @@
1.76
Copy link
Collaborator

@pinkforest pinkforest Apr 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pinning causes other problems. We can use rust-version in Cargo.toml for MSRV if we are to bump it.

EDIT: I misread your comment - yeah we need to address the core problem really. Thanks for identifying it.

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 this pull request may close these issues.

Unrecognized literal c""
2 participants