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

[source][condition] Add Cargo Package support #1081

Merged
merged 15 commits into from
Jan 12, 2023
Merged

[source][condition] Add Cargo Package support #1081

merged 15 commits into from
Jan 12, 2023

Conversation

loispostula
Copy link
Contributor

@loispostula loispostula commented Jan 11, 2023

Implement a new plugin to support cargo packages

This PR allows using Cargo Package as a source and a condition.

By default, it will use crates.io index repository, but a private repository can be used as well

Test

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

cd pkg/plugins/resources/cargopackage
go test

Additional Information

Tradeoff

Potential improvement

@olblak
Copy link
Member

olblak commented Jan 11, 2023

Waow nice, I am glad to see you manage to create a new resource without reaching out for help :D
I'll find some time later today or this week to review your PR

@loispostula
Copy link
Contributor Author

No problem, I've remove all the git handling, I had not understood the scm concept fully. I'm adapting the description as well to remove the tradeoff

@olblak
Copy link
Member

olblak commented Jan 11, 2023

We use this linter https://golangci-lint.run/ that detect a bunch of minor errors

@olblak
Copy link
Member

olblak commented Jan 11, 2023

Coud I also ask you to add an manifest to e2e/updatecli.d/success.d/crate.yaml?
The manifest cannot return any error or warning message as define on https://github.com/updatecli/updatecli/blob/main/e2e/venom.d/test_diff.yaml

@olblak
Copy link
Member

olblak commented Jan 11, 2023

I think it's missing the part where you dowload the index from the crate.io repository.

I just tested the following manifest

sources:
  test:
    kind: cargopackage
    spec:
      package: syn

and I get

SOURCES
=======

test1
-----
ERROR: something went wrong while opening the package file "open /home/olblak/Projects/Updatecli/updatecli/3/s/syn: no such file or directory"
ERROR: ✗ open /home/olblak/Projects/Updatecli/updatecli/3/s/syn: no such file or directory
Pipeline "CRATE.YAML" failed
Skipping due to:
	sources stage:	"open /home/olblak/Projects/Updatecli/updatecli/3/s/syn: no such file or directory"

This makes me think you may want to download the repository in the temporary directory created by Updatecli
https://github.com/updatecli/updatecli/blob/main/pkg/core/tmp/main.go#L25

@loispostula
Copy link
Contributor Author

loispostula commented Jan 11, 2023

You mean that we should default to use and clone the crates.io repository, if no IndexUrl or scmId is defined?

Yes I think so. clone the crate.io repository in the temporary directory created by Updatecli
I don't know how heavy it is. Maybe we can pair on this one

@olblak olblak added enhancement New feature or request resource-cargo Related to Rust crate labels Jan 11, 2023
@olblak
Copy link
Member

olblak commented Jan 12, 2023

Excepted a few minor remark, I think this pullrequest is really great.
Once the default "clone" operation uses the Updatecli scm snippet so it clones the repository in the Updatecli temporary location and we ensure that we do not modify the spec data, I think it can be merged

Not in this pull request but a follow up effort would be to improve the git clone operation . I think some benchmarking is needed to speed up. I never realize how slow it could be for large repositories

@loispostula
Copy link
Contributor Author

I've adapted the behaviour regarding the default index. The package now support the following spec:

  • IndexDir: specifies the url of the index to use to check version, If set, package version will be checked against the API
  • IndexUrl: specifies the directory of the index to use to check version, If set, package version will be checked using the file structure

The behaviour to fetch a package data becomes:

  1. If IndexUrl is set, we hit that API Url to get package data
  2. If IndexDir is set, we use that directory to get package data
  3. If scmid is set, we use the scm directory to get package data
  4. If none is set, we use the default crate API to get package data

@olblak olblak changed the title [enh] Implement new resource for Cargo Package [source][condition] Add Cargo Package support Jan 12, 2023
@olblak
Copy link
Member

olblak commented Jan 12, 2023

Super excited to see this pullrequest.
I did some testing and it works great.
I am waiting for the tests to pass before merging

@olblak olblak enabled auto-merge (squash) January 12, 2023 22:54
@olblak olblak merged commit a7f39a2 into updatecli:main Jan 12, 2023
@loispostula loispostula deleted the add_cargo_package branch January 13, 2023 05:15
@loispostula loispostula mentioned this pull request Jan 23, 2023
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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