Skip to content

ci: fix mod-check#1146

Open
starius wants to merge 2 commits into
lightninglabs:masterfrom
starius:go-mod-fix
Open

ci: fix mod-check#1146
starius wants to merge 2 commits into
lightninglabs:masterfrom
starius:go-mod-fix

Conversation

@starius
Copy link
Copy Markdown
Collaborator

@starius starius commented May 23, 2026

GitLab probably blocked GitHub's CI runner IP address. So we can't download packages from GitLab directly during this check. Workaround applied: download these particular packages from proxy.golang.org which works in CI.

Pull Request Checklist

  • Update release_notes.md if your PR contains major features, breaking changes or bugfixes

@gemini-code-assist
Copy link
Copy Markdown

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses CI failures caused by network restrictions on GitLab-hosted modules. By pre-fetching these dependencies through the public Go proxy before running the module check, the build process becomes more resilient to external network blocking.

Highlights

  • CI Reliability: Introduced a pre-fetching step in the 'mod-check' Makefile target to download problematic GitLab-hosted modules via the public Go proxy.
  • Dependency Management: Added a comprehensive list of GitLab and modernc.org modules to the Makefile to ensure CI runners can successfully resolve dependencies despite network restrictions.
New Features

🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a pre-fetching mechanism in the Makefile to seed specific GitLab and modernc.org modules from the public Go proxy, which helps prevent CI runner traffic rejections. Feedback highlights that maintaining a hardcoded list of modules and versions is a significant maintenance burden and suggests using a proxy fallback instead. Additionally, the sequential download loop in the Makefile is inefficient and should be replaced with a single parallel command. Finally, an inconsistency was noted in the go.mod file where an indirect dependency tag was removed without moving the dependency to the direct block.

Comment thread Makefile Outdated
Comment thread Makefile Outdated
Comment thread go.mod
@starius
Copy link
Copy Markdown
Collaborator Author

starius commented May 23, 2026

@claude review this

@claude
Copy link
Copy Markdown

claude Bot commented May 23, 2026

Code Review

Overview

This PR works around GitLab blocking GitHub CI runner IPs during the mod-check target. It prefetches GitLab-backed and modernc.org modules through proxy.golang.org before the GOPROXY=direct go mod tidy calls that would otherwise fail. A secondary unrelated change removes the // indirect annotation from a google.golang.org/genproto entry in go.mod.


Makefile changes

What works well

  • Pragmatic and minimal: seed the proxy cache for the specific modules that cannot be fetched directly, then let the rest of the tidy check run unchanged.
  • Scanning all four go.mod/go.sum pairs (root, swapserverrpc/, looprpc/, tools/) is thorough and catches transitive deps that only appear in go.sum.
  • The sort -u dedup is correct and necessary since the same module/version pair can appear in multiple files.

Potential issues

  1. Empty MOD_CHECK_PREFETCH edge case. The variable is computed via $(shell ...) at Makefile parse time. If the awk/sort pipeline ever produces no output (e.g. all problematic modules are eventually removed), the resulting line is GOPROXY=https://proxy.golang.org go mod download with no arguments, which downloads ALL modules in the current module. That is slow and unintentional. Wrapping the download line with an ifneq guard would prevent this:

    ifneq ($(MOD_CHECK_PREFETCH),)
            @GOPROXY=https://proxy.golang.org $(GOMOD) download $(MOD_CHECK_PREFETCH)
    endif
  2. Stale versions from go.sum. The script scans go.sum, which contains entries for older module versions needed only for dependency graph resolution (e.g. modernc.org/ccgo/v3 v3.0.0-20220428*). Passing those to go mod download fetches the full module ZIP, not just the go.mod metadata that go mod tidy actually needs for those old versions. Low severity since proxy.golang.org caches everything, but it is unnecessary extra work.

  3. Awk host filter is implicit. The regex matches only gitlab.com/ and modernc.org/. If a future dependency from a similarly restrictive host is added, maintainers will need to remember to extend this list. A brief comment explaining why these specific hosts are listed (CI runner IPs blocked upstream) would help future readers.


go.mod change

The // indirect comment is removed from google.golang.org/genproto/googleapis/rpc. This change is correct: cmd/loop/session_recorder.go directly imports google.golang.org/genproto/googleapis/rpc/status, so go mod tidy correctly drops the annotation. However, it is unrelated to the stated CI fix. Consider calling it out in the PR description, or splitting it into its own commit (go.mod: drop incorrect indirect annotation) to keep history clean.


Summary

  • Correctness: The core approach is sound and should fix the CI issue.
  • Main risk: Empty-prefetch edge case accidentally triggers go mod download with no args (low probability, easy to guard with ifneq).
  • Minor nit: Unrelated go.mod annotation fix not mentioned in the PR description.

The fix is small and focused with blast radius limited to CI. Guarding the empty-list case would make it more robust, but is not blocking.

@starius starius marked this pull request as ready for review May 24, 2026 03:00
@starius starius requested review from alexbosworth and hieblmi May 24, 2026 03:00
Copy link
Copy Markdown
Collaborator

@hieblmi hieblmi left a comment

Choose a reason for hiding this comment

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

Thank you for the fix 🎆

I think 1. of Claude's review makes sense to address.

GitLab probably blocked GitHub's CI runner IP address. So we can't download
packages from GitLab directly during this check. Workaround applied: download
these particular packages from proxy.golang.org which works in CI.
@starius
Copy link
Copy Markdown
Collaborator Author

starius commented May 24, 2026

Addressed Claude's 1st finding.

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.

2 participants