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

Fail if crates.io has the same version as in Cargo.toml #803

Open
nyurik opened this issue Jun 4, 2024 · 5 comments
Open

Fail if crates.io has the same version as in Cargo.toml #803

nyurik opened this issue Jun 4, 2024 · 5 comments
Labels
C-enhancement Category: raise the bar on expectations

Comments

@nyurik
Copy link

nyurik commented Jun 4, 2024

This amazing tool's documentation starts with

The action is designed to be used right before cargo publish.

but I think this tool is missing one important test - it does not fail in case the current code version has already been published. So unless CI does some magical version bump automatically and commits it to the repo before doing cargo publish, I think this tool should prevent cargo publish from even running? This might be an optional flag though - so that if some repo prefers to keep the version in Cargo.toml the same as latest published, they can?

@obi1kenobi
Copy link
Owner

Many projects use the "bump the version right before publishing" workflow, including most of my own. So definitely not a good default.

But I could be convinced to have an option for this. I'm curious though: how would you use such a flag? Could you point me to a project where you'd use it in such a way? Asking so I can figure out why you're suggesting this is better as a cargo-semver-checks feature instead of using something like cargo read-manifest | jq .version to extract the manifest version number and then compare it against cargo search <your-crate-name>.

For example, cargo search <your-crate-name> | grep "<your-crate-name> = $(cargo read-manifest | jq .version)" would exit non-zero if the current version in Cargo.toml is not the same as the one on crates.io. To me, this seems easier than figuring out how to use a cargo-semver-checks flag, but I might be wrong — I'd love to get your thoughts!

@obi1kenobi obi1kenobi added the C-enhancement Category: raise the bar on expectations label Jun 4, 2024
@nyurik
Copy link
Author

nyurik commented Jun 4, 2024

Thanks for such a quick reply! I use it in sqlite-hashes and sqlite-compressions (they are nearly identical).

I would like to avoid the following scenario:

  • the main branch of my repo has a passing CI
  • I start a new release (by clicking "draft a new release" on the github releases page) and clicking publish
  • the CI would pass all steps including semver check and the ones to publish release files
  • the very last step to do cargo publish would fail because this version has already been published.
  • Now I have to cleanup the release, confuse users who are subscribed to notifications, bump the version, and restart the whole process.

Instead, I would prefer for my repos to always be in the "ready-to-be-published" state - i.e. just clicking "publish new release" button should be certain to pass including cargo publishing

I did implement a test similar to your suggestion, but it seems ugly, only tests the latest version (what if somehow I have older version in the repo?), and you already do most of the steps to make this test much easier.

@nyurik
Copy link
Author

nyurik commented Jun 19, 2024

@obi1kenobi do you think that maybe this should be moved to the cargo-semver-checks instead? Or any other feedback? Thx!

@obi1kenobi
Copy link
Owner

I've been thinking about this for a few days now, and I came to the following conclusions:

  • This can't be implemented in the action alone. If it's going to exist, it has to be a feature of cargo-semver-checks itself.
  • If it's a feature of cargo-semver-checks itself, it has to have sensible and useful behavior in all situations where cargo-semver-checks is used. For example:
    • in a workspace with a single crate
    • in a workspace with multiple crates
    • in a workspace with multiple crates where some crates are publish = false
  • Implementing the above would require a substantial amount of design work, tests, and careful documentation. Meanwhile, I'm not sure how large the benefit is — it's possible there are more people with similar workflows to yours that would love this feature, or maybe there aren't that many at all.

Given all this, I don't think I can justify the effort of building this feature in the place of any other feature I could dedicate an equivalent amount of time to.

If you're interested in pushing this forward and can commit the ~2-3 weeks of work I estimate it would take to implement and nail down + test all the edge cases, I'd be happy to support you in that and point you in the right direction. Of course, even this is not a guarantee the work would be merged in the end — the code would have to be of satisfactory quality at my sole discretion, since I'd have to maintain it ~indefinitely going forward.

Alternatively, if this feature is something your employer wants and can justify dedicating some budget to, I'd be happy to work out a commercial build & support contract for it with you.

Otherwise, I'd be happy to move this issue to the cargo-semver-checks repo, mark it as a feature request, and re-evaluate in 6 months to see if it has become a highly-requested feature that I should prioritize. (And again, even that is not a guarantee that I'd build it either.)

I'm sorry — I know this isn't the answer you were hoping for. Unfortunately, open-source is a resource-scarce environment and I'm often forced to make unfortunate decisions like this for the sake of the longevity of the project.

@nyurik
Copy link
Author

nyurik commented Jun 24, 2024

@obi1kenobi thanks for an in-depth look at this request! Please move it to the upstream repo, and lets reevaluate in 6m. Thx for the awesome work!

@obi1kenobi obi1kenobi transferred this issue from obi1kenobi/cargo-semver-checks-action Jun 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: raise the bar on expectations
Projects
None yet
Development

No branches or pull requests

2 participants