Skip to content

Dockerfile: retry add-apt-repository#22125

Merged
MikeMcQuaid merged 4 commits intomainfrom
dockerfile-retry-add-apt-repository
May 3, 2026
Merged

Dockerfile: retry add-apt-repository#22125
MikeMcQuaid merged 4 commits intomainfrom
dockerfile-retry-add-apt-repository

Conversation

@p-linnane
Copy link
Copy Markdown
Member

@p-linnane p-linnane commented May 2, 2026

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them? Performance claims (e.g. "this is faster") must include Hyperfine benchmarks.
  • Have you written new tests (excluding integration tests) for your changes? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) with your changes locally?
  • AI was used to generate or assist with generating this PR. Please specify below how you used AI to help you, and what steps you have taken to manually verify the changes. Non-maintainers may only have one AI-assisted/generated PR open at a time.

Retry add-apt-repository to work around intermittent 504s from Launchpad's API during the ongoing Canonical infrastructure DDoS (https://www.theregister.com/2026/05/01/canonical_confirms_ubuntu_infrastructure_under/).

Mirrors the existing apt-get update retry on the line above.

@p-linnane
Dockerfile: retry add-apt-repository
8c292b7 
Signed-off-by: Patrick Linnane <patrick@linnane.io>
Copilot AI review requested due to automatic review settings May 2, 2026 17:05
Copilot AI reviewed May 2, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates Homebrew’s Linux Docker image build to make repository setup more resilient during transient Launchpad failures. Specifically, it adds retry logic around the git-core PPA registration step in the Dockerfile, which supports the CI/container environments built from this image.

Changes:

  • Wrap add-apt-repository -y ppa:git-core/ppa in a 5-attempt retry loop.
  • Keep the existing architecture guard so the PPA is still skipped on aarch64.
  • Align the new retry behavior with the nearby apt-get update retry pattern already used in the Dockerfile.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Dockerfile Outdated
MikeMcQuaid
MikeMcQuaid approved these changes May 2, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Looks good when comment addressed!

ZhongRuoyu added 2 commits May 3, 2026 20:35
@ZhongRuoyu
Dockerfile: fail after all retry attempts fail
2aece26
@ZhongRuoyu
Dockerfile: do not let apt-get update errors slip through
520dc06 
When `apt-get update` fails to fetch from a repository due to a
transient network error, its default behavior is to issue a warning and
continue [^1]. When the git-core PPA is unavailable (which is currently
the case due to an ongoing DDoS incident), this causes `apt-get install`
to install an outdated version of `git` from the default Ubuntu
repositories.

To avoid this, let's set `--error-on=any` for `apt-get update`, which
causes it to error out on such transient network errors [^2]. This
option is available since Ubuntu 21.04 and is equivalent to the APT
option `APT::Update::Error-Mode=any` [^3].

See below how `--error-on=any` turns the warning into an error:

    linuxbrew@c557e2acccfe:~$ sudo apt-get update
    Hit:1 https://cli.github.com/packages stable InRelease
    Hit:2 http://archive.ubuntu.com/ubuntu jammy InRelease
    Hit:3 http://security.ubuntu.com/ubuntu jammy-security InRelease
    Hit:4 http://archive.ubuntu.com/ubuntu jammy-updates InRelease
    Hit:5 http://archive.ubuntu.com/ubuntu jammy-backports InRelease
    Ign:6 https://ppa.launchpadcontent.net/git-core/ppa/ubuntu jammy InRelease
    Ign:6 https://ppa.launchpadcontent.net/git-core/ppa/ubuntu jammy InRelease
    Ign:6 https://ppa.launchpadcontent.net/git-core/ppa/ubuntu jammy InRelease
    Err:6 https://ppa.launchpadcontent.net/git-core/ppa/ubuntu jammy InRelease
      Could not connect to ppa.launchpadcontent.net:443 (185.125.190.80), connection timed out
    Reading package lists... Done
    W: Failed to fetch https://ppa.launchpadcontent.net/git-core/ppa/ubuntu/dists/jammy/InRelease  Could not connect to ppa.launchpadcontent.net:443 (185.125.190.80), connection timed out
    W: Some index files failed to download. They have been ignored, or old ones used instead.
    linuxbrew@c557e2acccfe:~$ sudo apt-get update --error-on=any
    Hit:1 https://cli.github.com/packages stable InRelease
    Hit:2 http://archive.ubuntu.com/ubuntu jammy InRelease
    Hit:3 http://security.ubuntu.com/ubuntu jammy-security InRelease
    Hit:4 http://archive.ubuntu.com/ubuntu jammy-updates InRelease
    Hit:5 http://archive.ubuntu.com/ubuntu jammy-backports InRelease
    Ign:6 https://ppa.launchpadcontent.net/git-core/ppa/ubuntu jammy InRelease
    Ign:6 https://ppa.launchpadcontent.net/git-core/ppa/ubuntu jammy InRelease
    Ign:6 https://ppa.launchpadcontent.net/git-core/ppa/ubuntu jammy InRelease
    Err:6 https://ppa.launchpadcontent.net/git-core/ppa/ubuntu jammy InRelease
      Could not connect to ppa.launchpadcontent.net:443 (185.125.190.80), connection timed out
    Reading package lists... Done
    E: Failed to fetch https://ppa.launchpadcontent.net/git-core/ppa/ubuntu/dists/jammy/InRelease  Could not connect to ppa.launchpadcontent.net:443 (185.125.190.80), connection timed out
    E: Some index files failed to download. They have been ignored, or old ones used instead.

[^1]: https://github.com/Debian/apt/blob/6b128124271e94bdb0f4e7850d9286170d712b04/apt-pkg/update.cc#L136-L139
[^2]: https://manpages.debian.org/trixie/apt/apt-get.8.en.html
[^3]: https://lists.ubuntu.com/archives/ubuntu-devel/2021-February/041374.html
@ZhongRuoyu ZhongRuoyu force-pushed the dockerfile-retry-add-apt-repository branch from 3633dbe to 520dc06 Compare May 3, 2026 12:35
@ZhongRuoyu
Copy link
Copy Markdown
Member

ZhongRuoyu commented May 3, 2026 •
edited
Loading

Sorry for hijacking the PR; I fixed Copilot's review comment in 2aece26 and added another fix in 520dc06 that prevents apt-get update errors from slipping through (see e.g. https://github.com/Homebrew/brew/actions/runs/25271495452/job/74094600035#step:7:966), which made us end up not getting git from the PPA. The commit message of 520dc06 has a more detailed explanation.

MikeMcQuaid
MikeMcQuaid approved these changes May 3, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Thanks!

@ZhongRuoyu
Dockerfile: work around a ShellCheck error
cbd36bc 
ShellCheck sees Dockerfile as a regular shell script, so it cannot
understand it when the function definition `retry()` comes after the
`RUN` command (it sees it as a single command `RUN retry() ...`). As
this causes a parsing error, it cannot be worked around with a
`# shellcheck disable` comment. Instead, let's add a no-op `:` command
before the function definition, so that ShellCheck can happily parse the
file.
@ZhongRuoyu ZhongRuoyu force-pushed the dockerfile-retry-add-apt-repository branch from 90a8857 to cbd36bc Compare May 3, 2026 13:27
@MikeMcQuaid MikeMcQuaid added this pull request to the merge queue May 3, 2026
Merged via the queue into main with commit 478f80a May 3, 2026
33 of 35 checks passed
@MikeMcQuaid MikeMcQuaid deleted the dockerfile-retry-add-apt-repository branch May 3, 2026 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ZhongRuoyu ZhongRuoyu ZhongRuoyu left review comments

Copilot code review Copilot Copilot left review comments

@MikeMcQuaid MikeMcQuaid MikeMcQuaid approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@p-linnane @ZhongRuoyu @MikeMcQuaid