paritytech / psvm

Polkadot SDK Version Manager
Other
31 stars 4 forks source link

feat: get versions from the repo directly #11

Closed patriciobcs closed 2 months ago

patriciobcs commented 3 months ago

Solves: #9

This changes the tool workflow, reducing to almost none its maintenance. Now, it will always fetch the versions directly from the repo using raw files. It seems the rate limit is 5000 queries per hour (ref). Considering the tool will max fetch 2 times (when falling back to Cargo.lock). The rate-limit is totally acceptable. If anyone would want to have a greater limit, in the future this can be replace by direct API usage by using GitHub CLI or API tokens.

rzadp commented 2 months ago

Hey @patriciobcs, thanks for this change!

I wonder, what is the reason to use Plan.toml in the first place, and then Cargo.lock as a fallback - instead of only using Cargo.lock?

I'm asking because I'm having some issues with some of the crates when using Plan.toml in the recent (1.9.0 and 1.10.0 releases) - specifically frame / polkadot-sdk-frame, and substrate-build-script-utils.

In my case, Cargo.lock still works, so I wonder what's the rationale for preferring Plan.toml.

patriciobcs commented 2 months ago

Hey @patriciobcs, thanks for this change!

I wonder, what is the reason to use Plan.toml in the first place, and then Cargo.lock as a fallback - instead of only using Cargo.lock?

I'm asking because I'm having some issues with some of the crates when using Plan.toml in the recent (1.9.0 and 1.10.0 releases) - specifically frame / polkadot-sdk-frame, and substrate-build-script-utils.

In my case, Cargo.lock still works, so I wonder what's the rationale for preferring Plan.toml.

The Cargo.lock file may contain dependencies that are not published in crates.io, and the tool will not be able to filter them out cause it is not possible to determine if a crate is published or not (with this file). If you have a local dependency with a name similar to a crate not published, the tool will overwrite it, so be careful. Adding an extra argument to use directly the Cargo.lock can also be done, but be aware of this issue.

rzadp commented 2 months ago

Thanks @patriciobcs

I see what you mean.

The Cargo.lock file may contain dependencies that are not published in crates.io, and the tool will not be able to filter them out cause it is not possible to determine if a crate is published or not (with this file).

That is true. However, with Plan.toml I'm running into an exactly opposite problem:

The Plan.toml file may contain dependencies that are published in crates.io, but marked as publish = false. And the tool is not able to distinguish them from dependencies that are truly not published (within this file).

How so? The Plan.toml marks publish = false crates that are not supposed to be published - that includes both private crates, and public crates that have not changed (so don't need to be uploaded again without changes).

Example:

psvm filters them both out, which is wrong.


I can think of three solutions to this: