Skip to content

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

Merged
merged 4 commits into from
Jun 26, 2021

Conversation

jtgeibel
Copy link
Member

@jtgeibel jtgeibel commented May 6, 2021

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. 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.)

@Turbo87 Turbo87 added C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works A-backend ⚙️ labels May 16, 2021
@bors
Copy link
Contributor

bors commented May 18, 2021

☔ The latest upstream changes (presumably #3591) made this pull request unmergeable. Please resolve the merge conflicts.

@pietroalbini
Copy link
Member

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.

jtgeibel added 2 commits June 22, 2021 14:54
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.
@jtgeibel
Copy link
Member Author

@pietroalbini I've updated the background job to now shell out to git for the --force-with-lease option.

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 master before this can be used on production.

@jtgeibel jtgeibel marked this pull request as ready for review June 23, 2021 00:39
jtgeibel added 2 commits June 25, 2021 23:46
This avoids persisting production SSH keys to disk. This commit also
avoids manually opening the file, as the `tempfile::Builder` already
does this.
@jtgeibel
Copy link
Member Author

I've addressed the comments and tested the new version on staging.

@pietroalbini
Copy link
Member

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+

@bors
Copy link
Contributor

bors commented Jun 26, 2021

📌 Commit 68bf2e7 has been approved by pietroalbini

@bors
Copy link
Contributor

bors commented Jun 26, 2021

⌛ Testing commit 68bf2e7 with merge 2b95219...

@bors
Copy link
Contributor

bors commented Jun 26, 2021

☀️ Test successful - checks-actions
Approved by: pietroalbini
Pushing 2b95219 to master...

@bors bors merged commit 2b95219 into rust-lang:master Jun 26, 2021
@jtgeibel jtgeibel deleted the squash-index branch June 26, 2021 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-backend ⚙️ C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants