Log a message if the background worker resets HEAD

[jtgeibel]

The first commit refactors some background job logic for the index repository.

The second commit logs a message if the background worker sees a different upstream HEAD than the currently expected commit. Background workers obtain a lock on the registry index so a reset should only occur if the last job modified the index but failed to push its commit, or the index was mutated externally (ex: to delete crates).

r? @Turbo87

[jtgeibel]

@Turbo87 I think something may be broken with the tarpaulin test coverage on the new 1.52.0 stable.

error: Broken pipe (os error 32)
warning: build failed, waiting for other jobs to finish...
error: Broken pipe (os error 32)
error: build failed
May 06 23:47:33.846 ERROR cargo_tarpaulin: Failed to compile tests! Error: cannot find attribute `skip` in this scope
Error: "Failed to compile tests! Error: cannot find attribute `skip` in this scope"
Error: Process completed with exit code 1.

[Turbo87]

on the new 1.52.0 stable.

that's why I prefer to pin the Rust version in CI 😉

something may be broken with the tarpaulin test coverage

hmm, I've noticed before that tarpaulin sometimes swallows the compiler error messages and makes it look as if tarpaulin itself had an issue.

I'm wondering if we could/should adjust the CI pipeline to run the regular cargo test if cargo tarpaulin failed 🤔

[Turbo87]

maybe I'm misunderstanding the logic here, but do we even need to call self.repository.reset() if head == original_head?

[jtgeibel]

I avoided doing an early return here mainly to avoid any unexpected behavior changes to edge cases. It is possible that a previously failed job has staged changes in the index, but failed before the staged changes were committed. In that case, the local and remote HEAD commits would match, but the staged changes would become mingled with new changes from the next job.

I now actually wonder if a reset is always sufficient. I believe a reset will leave any untracked files alone. So it might be possible that the last job created the local file but failed before the new file was staged. In both cases I think these scenarios are extremely unlikely - we're much more likely to fail when accessing the network than for local file I/O - but maybe we should consider the equivalent of a git clean -fdx as well.

[Turbo87]

if we're not sure about these edge cases, would it make sense to unconditionally log the operation?

[jtgeibel]

We could probably do the equivalent of git status and log a message if either the index or working directory isn't clean. Then we could log that as well if there is stale state hanging around. But for this message, I think it makes sense to only log it if the hashes are different.

[Turbo87]

okay, sounds good to me

[Turbo87]

@bors r+

[bors]

📌 Commit d012525 has been approved by Turbo87

[bors]

⌛ Testing commit d012525 with merge ef9b750...

[bors]

☀️ Test successful - checks-actions
Approved by: Turbo87
Pushing ef9b750 to master...