rustup: retry rustup-mirror on transient network failures

[yaoge123]

Summary

Pass --retries "${RUSTUP_RETRIES:-3}" through to rustup-mirror, enabling its built-in per-file retry for transient download failures.

Why

A single connection timeout to static.rust-lang.org fails the whole rustup sync run, and the job then shows as failed until the next scheduled attempt. We see this intermittently on mirror.nju.edu.cn, where direct connections to static.rust-lang.org time out.

rustup-mirror 0.12.0 (jiegec/rustup-mirror@c0c6fcb) retries transient per-file download failures with exponential backoff and exposes --retries (default 3). Per-file retry is the right layer for this: a wrapper loop around the whole invocation would re-run the manifest walk and --gc on every attempt, multiply with the inner backoff, and retry permanent errors N times — so the wrapper loop this PR started with was dropped per review.

Environment variables

Variable	Default	Meaning
RUSTUP_RETRIES	3	per-file download attempts, passed to rustup-mirror --retries

(MIRROR_BASE_URL and RUSTUP_GC behave as before.)

Testing

Running in production on mirror.nju.edu.cn, where direct connections to static.rust-lang.org time out intermittently; with per-file retry the job completes reliably.

Deployment

Requires rustup-mirror ≥ 0.12.0 in the image — earlier versions have no --retries flag and would fail on the unknown option. The image builds from crates.io max_stable_version, so the next image rebuild picks up 0.12.0.

[Copilot]

Copilot review overview

🟡 Changes recommended

Avoid logging and sleeping after the final failed attempt.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Avoid retry delay after final failed sync attempt · New

What changed in this PR

Adds retry handling to rustup-mirror for transient network failures.

Changes:

• Retries synchronization up to three times.
• Waits 60 seconds between attempts.
• Preserves failure status after all attempts fail.

File	Description
rustup.sh	Adds retry handling around rustup-mirror.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[yaoge123]

Verified: the Copilot review item is already resolved on this branch. The final failed attempt logs giving up and exits without the extra sleep (fixed in 8f313ba; the guard was carried over when retries were raised to 30x10s in f3f3594). No further changes needed.

[Copilot]

Copilot review overview

🟡 Changes recommended

Align the retry loop with the advertised attempt and delay policy.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Retry loop exceeds advertised attempts and interval · New

Resolved since last review (1)

• Avoid retry delay after final failed sync attempt

[yaoge123]

Round-2 review item addressed: the retry policy is now named constants RETRY_ATTEMPTS=30 / RETRY_DELAY=10 at the top of the script, used by the loop bound, the guard, and the sleep (de7fbab). The PR description has been updated to state the actual policy (30 attempts, 10s apart) instead of the original 3x60s.

[happyaron]

rustup-mirror v0.12.0 (released today, includes jiegec/rustup-mirror@c0c6fcb) now retries transient download failures per file with exponential backoff instead of panicking, and exposes --retries (default 3). Since our image builds from crates.io max_stable_version, the next image rebuild picks this up.

Given that, could we drop the whole-run retry loop and instead pass the flag through, e.g. --retries "${RUSTUP_RETRIES:-3}", so NJU can raise it via env? A per-file retry avoids re-running the manifest walk and --gc up to 30 times, and doesn't multiply with the inner backoff. It also won't retry permanent errors 30 times.

If you still see failures on 0.12.0 that the per-file retry doesn't cover, happy to revisit the outer loop. In that case please make RETRY_ATTEMPTS/RETRY_DELAY env-overridable like RUSTUP_GC.

[Copilot]

Copilot review overview

🟢 Approval recommended

No unresolved review issues were identified.

Review effort: Lite
Findings: 1

Open (1)

• Retry loop exceeds advertised attempts and interval

[yaoge123]

@happyaron Agreed and done in b57386a: the wrapper retry loop is dropped and the script now passes --retries "${RUSTUP_RETRIES:-3}" straight through, so the policy is env-overridable like RUSTUP_GC. Per-file retry with backoff is clearly the better layer for this — no repeated manifest walks, no retrying permanent errors 30 times. The PR description has been updated to match.

One deployment note from our side: the current production image still ships rustup-mirror 0.11.0 (no --retries flag), so we'll hold this version of the script on our production copy until the image is rebuilt with 0.12.0; the loop version stays there in the meantime.

[yaoge123]

Branch cleanup note: this branch has been squashed to a single commit, dd924bd. All commit SHAs referenced earlier in the review threads (bdbde69, 8f313ba, f3f3594, de7fbab, b57386a) are now orphaned commits — the links still open, but please rely on the current diff, whose tree is byte-identical to the previous head b57386a.

Review items → how they were addressed (all included in the current diff)

• Retry loop slept 60 s after the final failed attempt (Copilot round 1) → fixed at the time; moot now because the loop itself is gone.
• Loop policy (30×10 s) did not match the described 3×60 s (Copilot round 2) → superseded: the wrapper loop was dropped entirely, so there is no loop policy left to misdescribe.
• Drop the whole-run retry loop and pass --retries through instead (@happyaron) → done exactly as suggested: the script now passes --retries "${RUSTUP_RETRIES:-3}" to rustup-mirror, env-overridable like RUSTUP_GC. Per-file retry with backoff avoids re-running the manifest walk and --gc on every attempt and does not retry permanent errors N times.

Deployment note (unchanged): the script now requires rustup-mirror ≥ 0.12.0 in the image; our production copy keeps the loop version until the image is rebuilt.

Some earlier in-thread replies described intermediate states of the branch; please rely on the current diff and this summary.