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: publish crates directly #34794

Merged
merged 1 commit into from
Jan 22, 2024
Merged

Conversation

yihau
Copy link
Member

@yihau yihau commented Jan 16, 2024

Problem

  • we don't raise errors when publishing crates
  • we try to verify a crate even if it fails to publish
  • the verification seems a little bit cumbersome

a related convo: https://discord.com/channels/428295358100013066/560503042458517505/1196572613523279942

Summary of Changes

  1. return an error when the crate fails to publish
    => sometimes we receive reports about missing crates. I think we can detect these issues earlier and take appropriate actions (like re-publish or move to next tag)

  2. use the check which is already in cargo publish instead of crate test
    => I guess we use crate-test for ensuring the successful publishing of the next crate 🤔 maybe we can just retry cargo publish. this command will tell us if anything goes wrong. (like dependences are not ready or building error)

these are big changes to our existing publish pipeline. let me know what you think. btw, I will only backport it to v1.17. I think our v1.16 will never pass this one atm🙈

@yihau yihau marked this pull request as ready for review January 16, 2024 11:27
@CriesofCarrots
Copy link
Contributor

Linking this for history/context: #5136
So to be explicit (please confirm), this changeset's approach to the delay issue is that we iterate on the actual cargo publish, instead of only firing that off once. So a later crate that depends on an earlier one might fail to publish a time or two if it can't find the earlier crate yet, but then should succeed.

This seems like a decent approach to me, although there might be some value in retaining the REST api check. Let's wait to hear from @t-nelson, though, as I know he was also exploring some options.

Copy link
Contributor

@t-nelson t-nelson left a comment

Choose a reason for hiding this comment

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

:shipit:

@yihau yihau added the v1.17 PRs that should be backported to v1.17 label Jan 22, 2024
@yihau yihau merged commit f9bfb60 into solana-labs:master Jan 22, 2024
17 checks passed
@yihau yihau deleted the update-publish-crate branch January 22, 2024 02:43
Copy link
Contributor

mergify bot commented Jan 22, 2024

Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis.

mergify bot pushed a commit that referenced this pull request Jan 22, 2024
yihau added a commit that referenced this pull request Jan 28, 2024
ci: publish crates directly (#34794)

(cherry picked from commit f9bfb60)

Co-authored-by: Yihau Chen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v1.17 PRs that should be backported to v1.17
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants