Skip to content

chore(ci): make Rustup mirror configurable via build args#92

Merged
fslongjin merged 1 commit intoTencentCloud:masterfrom
fslongjin:jinlong-config-rustup-mirror-via-build-args
Apr 27, 2026
Merged

chore(ci): make Rustup mirror configurable via build args#92
fslongjin merged 1 commit intoTencentCloud:masterfrom
fslongjin:jinlong-config-rustup-mirror-via-build-args

Conversation

@fslongjin
Copy link
Copy Markdown
Collaborator

  • Add RUSTUP_DIST_SERVER and RUSTUP_UPDATE_ROOT as ARG in Dockerfile
  • Change hardcoded ENV values to reference ARG variables
  • Allow CI workflows to override with official Rust servers ( https://static.rust-lang.org))

- Add RUSTUP_DIST_SERVER and RUSTUP_UPDATE_ROOT as ARG in Dockerfile
- Change hardcoded ENV values to reference ARG variables
- Allow CI workflows to override with official Rust servers (
[https://static.rust-lang.org)](https://static.rust-lang.org))

Signed-off-by: jinlong <jinlong@tencent.com>
@fslongjin fslongjin merged commit 2c3ba28 into TencentCloud:master Apr 27, 2026
9 checks passed
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 27, 2026

Claude finished @fslongjin's task in 2m 26s —— View job


PR #92 Review: Make Rustup mirror configurable via build args

  • Gather context and read changed files
  • Check DCO compliance
  • Review code quality, bugs, security, and performance
  • Post inline review comments
  • Final summary

DCO Check

The commit includes Signed-off-by: jinlong <jinlong@tencent.com> — DCO compliant.

Summary

This is a clean, well-structured PR that makes the Rustup mirror configuration overridable via Docker build args. The approach is correct and follows Docker best practices:

  • Default values in the Dockerfile preserve backward compatibility (defaults to rsproxy.cn mirrors)
  • CI overrides use the official static.rust-lang.org servers, which makes sense for GitHub Actions runners that have direct internet access
  • Minimal, focused changes — only what's needed, no unnecessary refactoring

Findings

Category Finding Severity
Behavior The rustup installer (sh.rustup.rs) on line 110-111 runs before the ENV instruction, so it won't use the custom mirror. This is fine because --default-toolchain none means no toolchain is downloaded at install time — the heavy downloads happen in the subsequent RUN block which correctly sees the ENV vars. See inline comment for a future-proofing tip. Info
Style The two standalone ENV lines (113-114) could be folded into the existing multi-line ENV block for consistency, though keeping them separate has readability benefits too. Info
Security No concerns — URLs are hardcoded in both Dockerfile and workflow files, no credentials involved. N/A
Performance No impact — the same number of downloads occurs, just from different servers. N/A

Overall: This PR looks good. No blocking issues found.

Comment thread docker/Dockerfile.builder
Comment on lines 113 to 114
| sh -s -- -y --profile minimal --default-toolchain none

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor style note: the ENV block above (lines 17-30) uses the multi-line \ continuation format. These two standalone ENV lines could be folded into that block for consistency, since they're all just setting environment variables:

ENV LANG=C.UTF-8 \
    LC_ALL=C.UTF-8 \
    ... \
    RUSTUP_DIST_SERVER="${RUSTUP_DIST_SERVER}" \
    RUSTUP_UPDATE_ROOT="${RUSTUP_UPDATE_ROOT}"

This is purely a style preference — the current approach is also valid and keeps the rustup-related config visually separate, which has its own readability benefit. Just flagging for consideration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant