Skip to content

Conversation

@vvoland
Copy link
Contributor

@vvoland vvoland commented Aug 5, 2025

No description provided.

@vvoland vvoland self-assigned this Aug 5, 2025
@vvoland vvoland requested a review from thaJeztah August 5, 2025 10:28

ENV GOPROXY=https://proxy.golang.org|direct
ENV GO111MODULE=off
ENV GO111MODULE=on
Copy link
Member

Choose a reason for hiding this comment

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

Would auto work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure, according to https://go.dev/ref/mod

If GO111MODULE=auto, the go command runs in module-aware mode if a go.mod file is present in the current directory or any parent directory. In Go 1.15 and lower, this was the default behavior. go mod subcommands and go install with a version query run in module-aware mode even if no go.mod file is present.

This isn't really the same as the dynamic choice of on vs off.

@vvoland
Copy link
Contributor Author

vvoland commented Aug 5, 2025

Actually, I've simplified the version check to just checking if vendor.mod/go.mod exists.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Copy link

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 enables Go modules across all Docker build environments by changing GO111MODULE from off to on in Dockerfiles and build scripts. The change modernizes the build process to use Go's module system instead of the legacy GOPATH-based approach.

Key changes include:

  • Updated all Docker build environments to enable Go modules
  • Added dynamic Go module detection logic in build scripts
  • Maintained backward compatibility by checking for both go.mod and vendor.mod files

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
rpm/rhel-9/Dockerfile Enable Go modules in RHEL 9 build environment
rpm/rhel-8/Dockerfile Enable Go modules in RHEL 8 build environment
rpm/fedora-42/Dockerfile Enable Go modules in Fedora 42 build environment
rpm/fedora-41/Dockerfile Enable Go modules in Fedora 41 build environment
rpm/centos-9/Dockerfile Enable Go modules in CentOS 9 build environment
rpm/centos-10/Dockerfile Enable Go modules in CentOS 10 build environment
rpm/SPECS/docker-ce.spec Add dynamic Go module detection logic to RPM spec file
deb/ubuntu-plucky/Dockerfile Enable Go modules in Ubuntu Plucky build environment
deb/ubuntu-oracular/Dockerfile Enable Go modules in Ubuntu Oracular build environment
deb/ubuntu-noble/Dockerfile Enable Go modules in Ubuntu Noble build environment
deb/ubuntu-jammy/Dockerfile Enable Go modules in Ubuntu Jammy build environment
deb/raspbian-bullseye/Dockerfile Enable Go modules in Raspbian Bullseye build environment
deb/raspbian-bookworm/Dockerfile Enable Go modules in Raspbian Bookworm build environment
deb/debian-trixie/Dockerfile Enable Go modules in Debian Trixie build environment
deb/debian-bullseye/Dockerfile Enable Go modules in Debian Bullseye build environment
deb/debian-bookworm/Dockerfile Enable Go modules in Debian Bookworm build environment
deb/common/rules Add dynamic Go module detection logic to Debian build rules

elif [ -f go.mod ]; then
GOMOD=on
else
echo "No go.mod or vendor.mod found in engine directory"
Copy link

Copilot AI Aug 5, 2025

Choose a reason for hiding this comment

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

The error message should use proper exit status convention. Consider using 'exit 1' consistently and ensure the error is logged to stderr using 'echo ... >&2'.

Suggested change
echo "No go.mod or vendor.mod found in engine directory"
echo "No go.mod or vendor.mod found in engine directory" >&2

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Other echos inside this file do not output to stderr, so I'm keeping it the same.

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

we should have a tracking ticket to remove this as soon as we can; putting this in the deb/common/rules and Spec files feels a bit kludge-y 🙈

@vvoland vvoland merged commit 10f5969 into docker:master Aug 5, 2025
37 checks passed
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