Skip to content

Avoid Docker push when the image did not change - #160434

Open
Kobzol wants to merge 1 commit into
rust-lang:mainfrom
Kobzol:docker-download
Open

Avoid Docker push when the image did not change#160434
Kobzol wants to merge 1 commit into
rust-lang:mainfrom
Kobzol:docker-download

Conversation

@Kobzol

@Kobzol Kobzol commented Aug 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

This was bugging me for a long time. Could the solution be so easy this whole time? :)

CI runs:

r? @marcoieni

@rustbot rustbot added A-CI Area: Our Github Actions CI A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Aug 3, 2026
@Kobzol

Kobzol commented Aug 3, 2026

Copy link
Copy Markdown
Member Author

@bors try jobs=x86_64-rust-for-linux

@rust-bors

This comment has been minimized.

Sign in to view
rust-bors Bot pushed a commit that referenced this pull request Aug 3, 2026
@bors
Auto merge of #160434 - Kobzol:docker-download, r=<try>
7d773ca 
Avoid Docker push when the image did not change


try-job: x86_64-rust-for-linux
@Kobzol

Kobzol commented Aug 3, 2026

Copy link
Copy Markdown
Member Author

@bors try jobs=x86_64-rust-for-linux

@rust-bors

This comment has been minimized.

Sign in to view
rust-bors Bot pushed a commit that referenced this pull request Aug 3, 2026
@bors
Auto merge of #160434 - Kobzol:docker-download, r=<try>
fd1b16e 
Avoid Docker push when the image did not change


try-job: x86_64-rust-for-linux
@Kobzol

Kobzol commented Aug 3, 2026

Copy link
Copy Markdown
Member Author

Looks like invalidation works, the last CI run rebuilt Docker.

@Kobzol
Kobzol marked this pull request as ready for review August 3, 2026 12:52
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 3, 2026
@Kobzol

Kobzol commented Aug 3, 2026

Copy link
Copy Markdown
Member Author

Also CC @Mark-Simulacrum

marcoieni
marcoieni reviewed Aug 3, 2026
View reviewed changes
Comment thread src/ci/docker/scripts/rfl-build.sh Outdated
Comment thread src/ci/docker/run.sh Outdated
Comment thread src/ci/docker/run.sh Outdated
Comment thread src/ci/docker/run.sh Outdated
@rust-bors

rust-bors Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: fd1b16e (fd1b16e862b17e637cb53ecd1406f92e01f0eca0)
Base parent: 7c329d6 (7c329d6c76e11ca40c5673818ab0439c1be8962c)

@marcoieni

marcoieni commented Aug 3, 2026

Copy link
Copy Markdown
Member

The problem of this approach is that we don't get dockerhub base images updates (which might also include security fixes).
Docker recommends rebuilding images often.

One solution might be detecting the image manifest digest (with the inspect command) and rebuild the image only if the digest changed.

@Kobzol

Kobzol commented Aug 3, 2026

Copy link
Copy Markdown
Member Author

I'm not sure if I understand 🤔 Which base images are you talking about? Are you suggesting that when we have FROM ghcr.io/rust-lang/centos:7, you want to get a rebuild when that image changes? Because that doesn't really happen even today, right? Because then some builds would suddenly take ~6 hours out of nowhere, we would notice that. Or can Docker(x) reuse the rest of the build when the base image changes?

@Kobzol

Kobzol commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

@bors r+

I'm going to self-approve this and try to include it with a modified version of #160482, to see if it can unblock CI. With 40 approved PRs, I don't think that we can afford to depend on external mirrors and the Docker cache being thrashed all the time.

@rust-bors

rust-bors Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

📌 Commit bf2fa69 has been approved by Kobzol

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 10. This pull request will be tested once the tree is reopened.

Reason for tree closure: manually handling queue due to backlog

@rust-bors rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 4, 2026
Kobzol added a commit to Kobzol/rust that referenced this pull request Aug 4, 2026
@Kobzol
Rollup merge of rust-lang#160434 - Kobzol:docker-download, r=Kobzol
a6e5f43 
Avoid Docker push when the image did not change

This was bugging me for a long time. Could the solution be so easy this whole time? :)

CI runs:
- [Image pulled](https://github.com/rust-lang/rust/actions/runs/30814436863/job/91688737728) (I manually cancelled the job after the download)
- [Image built](https://github.com/rust-lang/rust/actions/runs/30815010995/job/91690611647)

r? @marcoieni
@Kobzol Kobzol mentioned this pull request Aug 4, 2026
Open
rust-bors Bot pushed a commit that referenced this pull request Aug 4, 2026
@bors
Auto merge of #160493 - Kobzol:rollup-gSPQQor, r=Kobzol
0b63def 
Rollup of 29 pull requests

Successful merges:

 - #153749 (Account for ownership mismatch on argument that doesn't meet bound)
 - #159130 (a bit optimize four-digit chunks in integer formatting)
 - #159326 (Deny multiple EII impls on a single item)
 - #159535 (Optimize slice::contains for bytewise types)
 - #159595 (Promote loongarch32-unknown-none* to Tier 2)
 - #160184 (Add -Zinstrument-mcount={fentry-nop-record,fentry-record})
 - #160320 (point at trait definition when it is used as a derive macro)
 - #160369 (When suggesting method names, prefer *exact* doc aliases over similar names)
 - #160406 (`DepKind` cleanups)
 - #160424 (Use `thread::available_parallelism` as the default limit for backend parallelism)
 - #160434 (Avoid Docker push when the image did not change)
 - #159303 (Fix ICE for direct inline const generic defaults)
 - #159977 (Add regression test for bool indexing codegen)
 - #160011 (remove InterpError::map_err_info)
 - #160165 (reject `...` without pattern post-expansion)
 - #160172 (tests: Enable `feature(stmt_expr_attributes)` in `2229_closure_analysis` tests)
 - #160295 (Fix rustdoc ICE when checking if a generic arg can be elided)
 - #160305 (Linkify C-SKY targets in `platform-support.md`)
 - #160314 (fix borrowck ICE for consts with fn pointer type)
 - #160322 (ElaborateBoxDeref: remove unnecessary projection)
 - #160338 (Add regression test for supertrait associated type normalization through dyn)
 - #160340 (Add regression test for unused_parens on contract clauses)
 - #160371 (Add doc aliases for transpositions `read_exact_buf` and `read_exact_buf_at`)
 - #160384 (Add PR body notes for Cargo lock file maintenance)
 - #160412 (Move duplicate-names check for #[rustc_must_implement_one_of] to attribute parser)
 - #160427 (Run try builds on EC2 by default)
 - #160435 (bump tracing-tree)
 - #160449 (Fix lookup of object files)
 - #160477 (Update the tracking issue for `borrowed_buf_init`)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@marcoieni marcoieni marcoieni left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@marcoieni marcoieni

Labels

A-CI Area: Our Github Actions CI A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Kobzol @marcoieni @rustbot