fix(publish): Check remote git registry more than once post-publish #11255
Conversation
|
r? @weihanglo (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
With `publish-timeout` on remote git registries, we were checking once and never checking again. There were 3 layers of guards preventing the cache from being updating - `needs_update`, handled via `invalidate_cache` - `config.updated_sources()`,. handled by removing from the HashSet - `updated`, inaccessible This change collapses `updated` into `config.updated_sources()`, allowing the cache to be updated Tested by publishing `epage-publish-test`. After about 7 registry updates, it succeded. Before, it just hit the timeout of 60s. No tests are added as we don't have the plumbing setup to be able to control when the test remote git registry publishes. All of our tests focus on the remote http registry. Fixes rust-lang#11253
| // To fully invalidate, undo `mark_updated`s work | ||
| self.needs_update = true; |
There was a problem hiding this comment.
Shouldn't cargo remove source from self.config.updated_sources() here?
There was a problem hiding this comment.
I believe the intent of updated_sources was to prevent fetching the repo more than once per session for scenarios like publish (where we do an update for fetching config.json as well as a later update for the verification build).
Maybe it's not a big deal to do an extra fetch here though, since we'll be doing many more fetches with the polling for publish anyway and it would simplify things.
There was a problem hiding this comment.
Thinking some more about this, I feel like more investigation is needed into updated_sources before clearing it here in case there are areas that are relying on this behavior. I feel like that effort is best decoupled from the current effort.
There was a problem hiding this comment.
Sound reasonable. We can deal with it afterward.
|
@bors r=arlosi |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
5 commits in 3ff044334f0567ce1481c78603aeee7211b91623..071eeaf210708219a5a1b2c4728ca2f97df7f2ae 2022-10-17 20:25:00 +0000 to 2022-10-22 01:17:55 +0000 - fix: Remove leading newline in vendor output (rust-lang/cargo#11273) - Fix publishing with a dependency on a sparse registry (rust-lang/cargo#11268) - Add missing edition (rust-lang/cargo#11265) - fix(publish): Check remote git registry more than once post-publish (rust-lang/cargo#11255) - Fix typo (rust-lang/cargo#11258)
Update cargo 5 commits in 3ff044334f0567ce1481c78603aeee7211b91623..071eeaf210708219a5a1b2c4728ca2f97df7f2ae 2022-10-17 20:25:00 +0000 to 2022-10-22 01:17:55 +0000 - fix: Remove leading newline in vendor output (rust-lang/cargo#11273) - Fix publishing with a dependency on a sparse registry (rust-lang/cargo#11268) - Add missing edition (rust-lang/cargo#11265) - fix(publish): Check remote git registry more than once post-publish (rust-lang/cargo#11255) - Fix typo (rust-lang/cargo#11258) r? `@ghost`
With
publish-timeouton remote git registries, we were checking once and never checking again. There were 3 layers of guards preventing the cache from being updatingneeds_update, handled viainvalidate_cacheconfig.updated_sources(),. handled by removing from the HashSetupdated, inaccessibleThis change collapses
updatedintoconfig.updated_sources(), allowing the cache to be updatedTested by publishing
epage-publish-test. After about 7 registry updates, it succeded. Before, it just hit the timeout of 60s. No tests are added as we don't have the plumbing setup to be able to control when the test remote git registry publishes. All of our tests focus on the remote http registry.Fixes #11253