Skip to content

Conversation

@helsaawy
Copy link
Contributor

@helsaawy helsaawy commented Sep 18, 2025

GO_FLAGS_EXTRA behavior is bugged and cannot be used to pass extra Go flags to the build command.
E.g., make GO_BUILD_TAGS=rego GO_FLAGS_EXTRA=-trimpath out/delta-snp.tar.gz builds the Go binaries with only the -trimpath flag, and not -tags=rego:

GOOS=linux CGO_ENABLED=0 go build -ldflags "-s -w"  -trimpath -o bin/cmd/gcs # ...

Specifying Make variables via command line takes precedence over and overrides all assignments to it.
This causes issues with GO_FLAGS_EXTRA, which may be updated to include -tags an -mod flags, as well as used to pass extra Go flags via command line (e.g., make GO_FLAGS_EXTRA=-trimpath). Update Makefile to update GO_FLAGS instead, and leave GO_FLAGS_EXTRA unmodified.

Move Makefile variables intended to be overridden in the command line to dedicated blocks, and add comments to them.

Additionally, add fix for new Microsoft Go behavior (starting in go1.25) that requires CGo by default for crypto libraries.

See:

@helsaawy helsaawy requested a review from a team as a code owner September 18, 2025 17:15
Specifying Make variables via command line takes precedence over and
overrides all assignments to it.
This causes issues with `GO_FLAGS_EXTRA`, which may be updated to include
`-tags` an `-mod` flags, as well as used to pass extra Go flags via
command line (e.g., `make GO_FLAGS_EXTRA=-trimpath).
Update `Makefile` to update `GO_FLAGS` instead, and leave
`GO_FLAGS_EXTRA` unmodified.

Move `Makefile` variables intended to be overridden in the command line
to dedicated blocks, and add comments to them.

Additionally, add fix for new Microsoft Go behavior (starting in go1.25)
that requires CGo by default for crypto libraries.

See:
 - https://www.gnu.org/software/make/manual/html_node/Overriding.html
 - https://github.com/microsoft/go/blob/microsoft/main/eng/doc/MigrationGuide.md#disabling-systemcrypto

Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
Copy link
Contributor

@ambarve ambarve left a comment

Choose a reason for hiding this comment

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

One minor suggestion, but LGTM otherwise.

@msscotb msscotb assigned ambarve and unassigned msscotb Oct 25, 2025
@helsaawy helsaawy merged commit 79b4311 into microsoft:main Oct 27, 2025
17 checks passed
@helsaawy helsaawy deleted the make-var branch October 27, 2025 15:22
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.

4 participants