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

[CI] Convert crate publish readiness checks to Rust #2590

Closed
tjtelan opened this issue Aug 25, 2022 · 4 comments
Closed

[CI] Convert crate publish readiness checks to Rust #2590

tjtelan opened this issue Aug 25, 2022 · 4 comments
Labels
CI DX/GENERAL Developer Experience General enhancement New feature or request good first issue Good for newcomers help wanted Good issue for community involvement no-issue-activity Test Infrastructure Testing infrastructure value/high

Comments

@tjtelan
Copy link
Contributor

tjtelan commented Aug 25, 2022

These tests together are pretty useful for ensuring release doesn't surprise use with crate version number related noise, but doesn't give enough feedback for new or external contributors.


For context, my ideal solution is to use something like https://github.com/obi1kenobi/cargo-semver-check, since it provides feedback about what kind of version bump to perform.

After a (very) quick check, I did not find a single tool that handles the edge cases that we look for before during publishing.

Examples of changes

  • Valid, but trivial file changes, such as control flow logic, comments, whitespace, variable renames and no version change
  • Valid, but Cargo.toml only changes, like for updating dependencies and no version change

This conversion will help close the gap for the ability to contribute to a community-supported crate that will handle our edge cases.

@tjtelan tjtelan added enhancement New feature or request help wanted Good issue for community involvement good first issue Good for newcomers CI Test Infrastructure Testing infrastructure Priority/low value/high DX/GENERAL Developer Experience General labels Aug 25, 2022
@mfdorst
Copy link
Contributor

mfdorst commented Aug 26, 2022

So, to be clear, this issue is only proposing to translate the functionality that currently exists in test-crate-version.sh and check-publish-crates.sh into Rust, not to add any additional functionality?

@tjtelan
Copy link
Contributor Author

tjtelan commented Aug 27, 2022

I suppose this is a little open ended, as long as it improves the overall reliability of the automated release process. However, the minimum is to cover the example cases I mentioned, which the scripts currently addresses during CI.

These scripts are admittedly naive, so I'm open to solutions that aren't exact ports, as long as there's justification. If you have other ideas, I'm open to hearing you out.

Ideally, no one should need to think too hard about resolving these issues if they arise. I think this will be addressed by giving clear feedback to contributors.

@mfdorst
Copy link
Contributor

mfdorst commented Aug 27, 2022

From what I can tell, test-crate-version.sh downloads https://static.crates.io/db-dump.tar.gz (a fairly large file) for the sole purpose of checking if the crate is published. There is a much faster way - just request https://crates.io/api/v1/crates/{crate_name}/versions and if it returns 404, the crate is not published. I'm using that in my rewrite unless there's some other reason to download the whole db-dump archive.

@tjtelan tjtelan linked a pull request Sep 8, 2022 that will close this issue
10 tasks
bors bot pushed a commit that referenced this issue Sep 9, 2022
**Notable changes**
+ Does not download `https://static.crates.io/db-dump.tar.gz` to check if crates are published (very slow)
   + Instead requests `https://crates.io/api/v1/crates/{crate_name}/versions` - 404 indicates that the crate is not published

**TODO**
+ [x] Test source diffing
+ [x] Display source changes
+ [x] Test manifest diffing
  + [x] Decide whether deleted manifest keys should be ignored or flagged
+ [x] Display manifest changes
+ [x] Download crates without an external tool (currently using `cargo-download`)
+ [ ] ~~Diff source without an external tool (currently using `diff`) - maybe? Might not be worth the effort.~~
+ [x] Add a `--verbose` mode
+ [x] Download crates concurrently
+ [x] Colorize diff

Related: #2590
@github-actions
Copy link

Stale issue message

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI DX/GENERAL Developer Experience General enhancement New feature or request good first issue Good for newcomers help wanted Good issue for community involvement no-issue-activity Test Infrastructure Testing infrastructure value/high
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants