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

Add workflow to publish Cargo automatically #14202

Merged
merged 1 commit into from
Jul 13, 2024

Conversation

pietroalbini
Copy link
Member

One of the last manual pieces of the Rust release process is publishing Cargo, and as it happens after the release itself it is often forgotten (I am often guilty of that). This PR aims to fully automate that.

Nowadays tagging Cargo happens automatically, as the release process creates and pushes a signed tag for the release. So the only piece remaining is automatically executing the publish.py script. This PR adds a workflow triggered by the tag push to run publish.py with a rust-lang-owner scoped token.

The secret is stored in the release environment, which can only be used during tag pushes.

cc @Mark-Simulacrum

@rustbot
Copy link
Collaborator

rustbot commented Jul 6, 2024

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 6, 2024
Comment on lines +1 to +3
# Publish Cargo to crates.io whenever a new tag is pushed. Tags are pushed by
# the Rust release process (https://github.com/rust-lang/promote-release),
# which will cause this workflow to run.
Copy link
Member

Choose a reason for hiding this comment

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

I believe I am allowed to manually create tags. Would that also trigger this workflow and how should we prevent it?

image

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes you creating tags would trigger the workflow. I guess we could setup a ruleset only allowing promote-release to create tags in the repository, but I'm not sure if it's worth it.

In the other repositories we have auto-publish configured in (rust-lang/infra-team#117) we let everyone with write access to the repo publish releases. Also, having the Cargo team be able to issue new patch releases of Cargo if problems with the crates.io-published versions are discovered (like a wrong dependency) could reduce friction.

@Mark-Simulacrum do you have any concerns?

Copy link
Member

Choose a reason for hiding this comment

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

Ideally, I think across all of our repositories we would require 2 people to approve release issuance. For most of the repositories we have that's hard though. (And 2 people is still somewhat weak in terms of mechanism).

Maybe we can at least restrict publication to just cargo team members? e.g., triagebot needs write access, but ideally would not be able to publish a release (at least trivially). If we gated it behind an environment with manual approval (similar to what we're planning to do for bors production deployments), that could be just the Cargo team. And I'd feel a little better about the approval being an intentional UI click rather than "just" git push ... with a typo that makes it a tag.

Copy link
Member Author

Choose a reason for hiding this comment

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

So you'd like to require the Cargo team's approval even during normal releases (i.e. when pushed by promote-release)?

Copy link
Member

Choose a reason for hiding this comment

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

No, not necessarily. I'm comfortable with automation making releases, that seems fine to do without approvals (though with if required is likely also ok). But manual publishes I think do need more than just write access.

Copy link
Member Author

Choose a reason for hiding this comment

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

Created a repository rule to only allow promote-release to push tags to this repository.

@weihanglo
Copy link
Member

The secret is stored in the release environment, which can only be used during tag pushes.

BTW I have no permission to read this page (and shouldn't have). So yeah need t-infra people to verity it.

@pietroalbini
Copy link
Member Author

BTW I have no permission to read this page (and shouldn't have). So yeah need t-infra people to verity it.

Current contents of the page:

image

@ehuss
Copy link
Contributor

ehuss commented Jul 6, 2024

Thanks @pietroalbini!

The cargo team has discussed trying to do this in the past. We were stuck on two issues:

  • How to selectively publish individual crates independent of the Rust release process? We had a few different ideas, but didn't really settle on anything.
  • How to guard or otherwise monitor when releases are made? GitHub doesn't really provide a nice way to do this like there is with PRs (which require an independent reviewer, and send out notifications to everyone). We also had some ideas on this, but didn't settle on one. It would be nice if something (crates.io or GitHub) at least sent out a notification.

I don't want to block anything on these, just wanted to let you know what we've been thinking.

@weihanglo
Copy link
Member

  • How to selectively publish individual crates independent of the Rust release process? We had a few different ideas, but didn't really settle on anything.

Perhaps this helps #13397?

  • How to guard or otherwise monitor when releases are made? GitHub doesn't really provide a nice way to do this like there is with PRs (which require an independent reviewer, and send out notifications to everyone). We also had some ideas on this, but didn't settle on one. It would be nice if something (crates.io or GitHub) at least sent out a notification.

if we create a release for each new tag, there is notification for new releases

@ehuss
Copy link
Contributor

ehuss commented Jul 6, 2024

Perhaps this helps #13397?

Not really. We need to be able to do something like "publish home on the master branch, but don't publish cargo". Just looking at the version number doesn't help since we proactively bump cargo's version.

Which makes me think we won't be able to tag releases like home with this PR... 🤔
We might need to consider filtering based on the tag name?

In git2-rs, I use workflow dispatches which have a little GUI that lets you select which crates to publish.

if we create a release for each new tag, there is notification for new releases

Yea, I use that in some other repos and it works pretty well. Something we could consider, though I wouldn't want to block on it.

@pietroalbini
Copy link
Member Author

I would keep publishing other crates as out of scope right now, my main concern here is automating the last bit of the Rust release process :D

Which makes me think we won't be able to tag releases like home with this PR... 🤔
We might need to consider filtering based on the tag name?

In git2-rs, I use workflow dispatches which have a little GUI that lets you select which crates to publish.

Do you plan in the future to tag the release of other crates (like home-1.0.0), while keeping the Cargo tags as just the version number (like 0.80.0)? If so, I can change the trigger rule and the workflow to only apply on Cargo releases, leaving the door open for publishing other crates outside of this process in the future.

@pietroalbini
Copy link
Member Author

OK, I think with the restriction of only allowing promote-release to publish tags I think this is ready to be merged? Assuming we defer the discussion of other crates to a future PR.

Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

Thanks. This is not a one-way-door decision so I am gonna merge it

@weihanglo
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jul 13, 2024

📌 Commit a28baaf has been approved by weihanglo

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 13, 2024
@bors
Copy link
Contributor

bors commented Jul 13, 2024

⌛ Testing commit a28baaf with merge 916c5a4...

@bors
Copy link
Contributor

bors commented Jul 13, 2024

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 916c5a4 to master...

@bors bors merged commit 916c5a4 into rust-lang:master Jul 13, 2024
22 checks passed
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 17, 2024
Update cargo

31 commits in 154fdac39ae9629954e19e9986fd2cf2cdd8d964..a2b58c3dad4d554ba01ed6c45c41ff85390560f2
2024-07-07 01:28:23 +0000 to 2024-07-16 00:52:02 +0000
- chore(ci): bump CI tools (rust-lang/cargo#14257)
- test: migrate fetch and list_availables to snapbox (rust-lang/cargo#14214)
- chore: downgrade to jobserver@0.1.28 (rust-lang/cargo#14254)
- perf(source): Don't `du` on every git source load (rust-lang/cargo#14252)
- fix(source): Don't warn about unreferenced duplicate packages (rust-lang/cargo#14239)
- feat(test): Add cargo_test to test-support prelude (rust-lang/cargo#14243)
- Add workflow to publish Cargo automatically (rust-lang/cargo#14202)
- test: migrate implicit_features to snapbox (rust-lang/cargo#14245)
- test: migrate build-std/main to snapbox (rust-lang/cargo#14241)
- test: migrate check_cfg to snapbox (rust-lang/cargo#14235)
- refactor(source): More RecursivePathSource clean up (rust-lang/cargo#14231)
- Add more profiling traces (rust-lang/cargo#14238)
- fix(overrides): Don't warn on duplicate packages from using '..' (rust-lang/cargo#14234)
- fix(test): Redact elapsed time in the minutes time frame (rust-lang/cargo#14233)
- test: Migrate lto tests to snapbox (rust-lang/cargo#14209)
- fix: Ensure dep/feature activates the dependency on 2024 (rust-lang/cargo#14221)
- chore(docs): update index of reference (rust-lang/cargo#14228)
- test: migrate test to snapbox (rust-lang/cargo#14226)
- chore: remove duplicate words (rust-lang/cargo#14229)
- docs(contrib): Document things I look for in RFCs (rust-lang/cargo#14222)
- docs(ref): Note MSRV for features in the docs (rust-lang/cargo#14224)
- test(progress): Resolve flakiness (rust-lang/cargo#14223)
- fix(test): Reduce over-prescription to the caller (rust-lang/cargo#14217)
- refactor: move get_source_id out of registry (rust-lang/cargo#14218)
- fix: rename to `rustdoc::broken_intra_doc_links` (rust-lang/cargo#14215)
- test: migrate member_errors, multitarget and new to snapbox (rust-lang/cargo#14210)
- test: migrate generate_lockfile and glob_targets to snapbox (rust-lang/cargo#14200)
- test: Ensure --list test works with cargo-bloat (rust-lang/cargo#14213)
- dont make new constant InternedString in hot path (rust-lang/cargo#14211)
- Fix compatible_with_older_cargo test. (rust-lang/cargo#14212)
- test: migrate metabuild, metadata and net_config to snapbox (rust-lang/cargo#14162)
@rustbot rustbot added this to the 1.81.0 milestone Jul 17, 2024
@pietroalbini pietroalbini deleted the pa-publish branch August 2, 2024 19:04
bors added a commit that referenced this pull request Sep 11, 2024
Update docs for how cargo is published

This updates the docs for publishing crates in this repo after the changes from #14202.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants