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

Add Cargo package autodiscovery #1094

Merged
merged 30 commits into from
Feb 2, 2023
Merged

Add Cargo package autodiscovery #1094

merged 30 commits into from
Feb 2, 2023

Conversation

loispostula
Copy link
Contributor

@loispostula loispostula commented Jan 17, 2023

Implement cargo package auto-discovery

Dependson:

  • Support autodiscovery of private crates
  • Support autodiscovery of crate specifying their version inline

Support autodiscovery of crate specifying their version inline

Cargo dependencies can be specified in two ways:

[dependencies]
rand = "0.1.0"

or

[dependencies]
[dependencies.rand]
version = "0.1.0"

Due to limitation in the current toml parsing capabilities, it's not possible to perform such complex queries on the Cargo.toml files. This means that autodiscovery will not work for dependencies not specifies using the full spec

Test

To test this pull request, you can run the following commands:

cd pkg/plugins/autodiscovery/cargo
go test

Additional Information

Tradeoff

Potential improvement

  • Allow to restrict version update to a specific type such as only major/minor/patch update e if semver is used
  • Target executing ``cargo generate-lockfilein dry-run mode shouldn't not change the fileCargo.lock`
  • Leverage google/osv.dev to detect CVEs

@olblak
Copy link
Member

olblak commented Jan 18, 2023

Due to limitation in the current toml parsing capabilities, it's not possible to perform such complex queries on the Cargo.toml files. This means that autodiscovery will not work for dependencies not specifies using the full spec

Do you have example of queries that do no work?
Because the two example that you provide are working.

sources:
  example1:
    kind: toml
    spec:
      file: '/tmp/example2.toml'
      key: 'dependencies.rand'

  example2:
    kind: toml
    spec:
      file: '/tmp/example.toml'
      key: 'dependencies.rand.version'

@loispostula loispostula marked this pull request as ready for review January 18, 2023 13:49
@olblak olblak added this to the 0.44.0 milestone Jan 23, 2023
Copy link
Member

@olblak olblak left a comment

Choose a reason for hiding this comment

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

I made some comments and spot some breaking changes that should be avoided

@loispostula
Copy link
Contributor Author

loispostula commented Jan 24, 2023

I've update according to the review, I think the remaining items are:

  • Is it ok to provide all scmid
  • Should we move the test repos to an updateclitest org
  • Deprecation and cargo manifest upgrade changes. I've implemented those in the New resource code.

@olblak olblak added autodiscovery All things related to the autodiscovery feature resource-cargo Related to Rust crate enhancement New feature or request labels Jan 26, 2023
@olblak olblak changed the title [enh] Bootstrap support for autodiscovery of cargo package Add Cargo package autodiscovery Jan 27, 2023
@loispostula
Copy link
Contributor Author

@olblak Thanks for #1071 and #1067

I've added thoses key to the manifest, I also add to make sure that the shell resource inherited the path environment variable. And it's now working fine

@olblak
Copy link
Member

olblak commented Jan 27, 2023

I'll run some tests over the weekend

loispostula and others added 4 commits January 30, 2023 18:12
* fix: Replace tab by space in generated yaml
* fix: Switch back to `cargo generate-lockfile` as it updates the lock
  file without changing version constrains specified in Cargo.toml such
  as 1.0
* fix: test with the new target name
Signed-off-by: Olblak <me@olblak.com>
@olblak
Copy link
Member

olblak commented Jan 31, 2023

@loispostula In my last commit, I disable updating the Cargo.toml if it's not strict semver. So 1.0.0 should be updated to >=1.0.0 while "1.0" should only update "Cargo.lock" by running cargo generate-lockfile

I have mixed feeling about overidding version constraint in Cargo.toml (and I am open for discussion)

@ghost
Copy link

ghost commented Feb 2, 2023

👇 Click on the image for a new way to code review
  • Make big changes easier — review code in small groups of related files

  • Know where to start — see the whole change at a glance

  • Take a code tour — explore the change with an interactive tour

  • Make comments and review — all fully sync’ed with github

    Try it now!

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map Legend

@olblak olblak merged commit 2762a7d into main Feb 2, 2023
@olblak olblak deleted the cargo-autodiscovery branch February 2, 2023 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autodiscovery All things related to the autodiscovery feature enhancement New feature or request resource-cargo Related to Rust crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants