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

[ANE-1659] update cargo metadata ID parser #1416

Merged
merged 12 commits into from
Apr 25, 2024

Conversation

spatten
Copy link
Contributor

@spatten spatten commented Apr 20, 2024

Overview

Cargo changed the format of project IDs in the output of cargo metadata.

Prior to cargo 1.77.0 they looked like this:

adler 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)

For 1.77.0 and greater, they look like this, as defined in https://doc.rust-lang.org/nightly/cargo/reference/pkgid-spec.html

registry+https://github.com/rust-lang/crates.io-index#adler@1.0.2

For path dependencies, I've seen them with and without the package name in the fragment:

path+file:///Users/scott/projects/health-data/health_data#package_name@0.1.0

or

path+file:///Users/scott/projects/health-data/health_data#0.1.0

Acceptance criteria

  • We can parse the output of cargo metadata for new versions of cargo
  • We can parse the output of cargo metadata with older versions of cargo

Testing plan

Use rustup to install an old and a new version of cargo:

rustup update # This is currently 1.77.2
rustup install 1.76.0

Now, analyze a cargo project with both the old version:

rustup default 1.77.2
cabal run fossa -- analyze <path to rust project> --output > /tmp/output-1.77.2.json
rustup default 1.76.0
cabal run fossa -- analyze <path to rust project> --output > /tmp/output-1.76.0.json

Clean up those output files (you sometimes get some cruft from cabal run at the beginning) and then run them through jq or otherwise prettify them.

You should now see no difference when you diff the output.

diff /tmp/output-1.77.2.json /tmp/output-1.76.0.json

Chris: I did this with foundation which has a mix of path deps and regular ones and got no differences:

make install-local
rustup default stable
./fossa analyze --output ~/devel/foundation | jq . > 1_77_out.json
rustup install 1.76
rustup default 1.76
./fossa analyze --output ~/devel/foundation | jq . > 1_76_out.json
diff -u 1_77_out.json 1_76_out.json

Risks

Metrics

References

https://fossa.atlassian.net/browse/ANE-1659

https://teamfossa.slack.com/archives/C043EM3L96Z/p1713558253884749

Checklist

  • I added tests for this PR's change (or explained in the PR description why tests don't make sense).
    - [ ] If this PR introduced a user-visible change, I added documentation into docs/.
    - [ ] If this PR added docs, I added links as appropriate to the user manual's ToC in docs/README.ms and gave consideration to how discoverable or not my documentation is.
  • If this change is externally visible, I updated Changelog.md. If this PR did not mark a release, I added my changes into an # Unreleased section at the top.
    - [ ] If I made changes to .fossa.yml or fossa-deps.{json.yml}, I updated docs/references/files/*.schema.json AND I have updated example files used by fossa init command. You may also need to update these if you have added/removed new dependency type (e.g. pip) or analysis target type (e.g. poetry).
    - [ ] If I made changes to a subcommand's options, I updated docs/references/subcommands/<subcommand>.md.

@csasarak csasarak marked this pull request as ready for review April 24, 2024 17:59
@csasarak csasarak requested a review from a team as a code owner April 24, 2024 17:59
Copy link
Contributor

@JeffreyHuynh1 JeffreyHuynh1 left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +485 to +500
-- Prior to Cargo 1.77.0, package IDs looked like this:
-- package version (source URL)
-- adler 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)
--
-- For 1.77.0 and later, they look like this:
-- registry source URL with a fragment of package@version
-- registry+https://github.com/rust-lang/crates.io-index#adler@1.0.2
-- or
-- path source URL with a fragment of package@version
-- path+file:///Users/scott/projects/health-data/health_data#package_name@0.1.0
-- or
-- path source URL with a fragment of version
-- In this case we grab the last entry in the path to use for the package name
-- path+file:///Users/scott/projects/health-data/health_data#0.1.0
--
-- Package Spec: https://doc.rust-lang.org/cargo/reference/pkgid-spec.html
Copy link
Contributor

Choose a reason for hiding this comment

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

The comments you added in this file are great!! It makes it really easy to follow and understand the implementation logic.

Copy link
Contributor

Choose a reason for hiding this comment

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

Scott wrote the big one and made the testing setup, so thank you @spatten .

src/Strategy/Cargo.hs Outdated Show resolved Hide resolved
src/Strategy/Cargo.hs Outdated Show resolved Hide resolved
csasarak and others added 2 commits April 25, 2024 11:00
Co-authored-by: Jeffrey Huynh <jeffrey@fossa.com>
@csasarak csasarak enabled auto-merge (squash) April 25, 2024 16:50
@csasarak csasarak merged commit 2940925 into master Apr 25, 2024
16 checks passed
@csasarak csasarak deleted the ANE-1659-update-cargo-metadata-id-parser branch April 25, 2024 17:03
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.

None yet

3 participants