-
Notifications
You must be signed in to change notification settings - Fork 647
Add a background job for squashing the index #3592
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
Conversation
☔ The latest upstream changes (presumably #3591) made this pull request unmergeable. Please resolve the merge conflicts. |
From a glance the code looks correct, but I'm really worried about force pushing without lease. I'd rather prefer to shell out to git just for this operation rather than doing a force push without lease. |
This adds a background job that squashes the index into a single commit. The current plan is to manually enqueue this job on a 6 week schedule, roughly aligning with new `rustc` releases. Before deploying this, will need to make sure that the SSH key is allowed to do a force push to the protected master branch. This job is derived from a [script] that was periodically run by the cargo team. There are a few minor differences relative to the original script: * The push of the snapshot branch is no longer forced. The job will fail if run more than once on the same day. (If the first attempt fails before pushing a new root commit upstream, then retries should succeed as long as the snapshot can be fast-forwarded.) * The push of the new root commit to the origin no longer uses `--force-with-lease` to reject the force push if new commits have been pushed there in parallel. Other than the occasional manual changes to the index (such as deleting crates), background jobs have exclusive write access to the index while running. Given that such manual changes are rare, this job completes quickly, and such manual tasks should be automated too, this is low risk. The alternative is to shell out to git because `libgit2` (and thus the `git2` crate) do not yet support this portion of the protocol. [script]: rust-lang/crates-io-cargo-teams#47 (comment)
Password authentication is not supported as we do not use that in production or staging. The SSH key is written to a temporary file which is deleted after the push operation completes. The SSH host key is automatically accepted as long as no conflicting key already exists.
@pietroalbini I've updated the background job to now shell out to git for the Password authentication is not supported as we do not use that in production or staging. The SSH key is written to a temporary file which is deleted after the push operation completes. Finally, the SSH host key is automatically accepted as long as no conflicting key already exists. I've tested this on staging. The production deploy key will need to enable forced pushes to |
This avoids persisting production SSH keys to disk. This commit also avoids manually opening the file, as the `tempfile::Builder` already does this.
I've addressed the comments and tested the new version on staging. |
Thanks! In the future we should add tests for the whole git module, but let's not block this on that implementation as it's not going to be trivial (as we'll need to mock a git server for tests). @bors r+ |
📌 Commit 68bf2e7 has been approved by |
☀️ Test successful - checks-actions |
This adds a background job that squashes the index into a single commit.
The current plan is to manually enqueue this job on a 6 week schedule,
roughly aligning with new
rustc
releases. Before deploying this, willneed to make sure that the SSH key is allowed to do a force push to the
protected master branch.
This job is derived from a script that was periodically run by the
cargo team. Relative to the original script, the push of the snapshot
branch is no longer forced. The job will fail if run more than once on
the same day. (If the first attempt fails before pushing a new root
commit upstream, then retries should succeed as long as the snapshot
can be fast-forwarded.)