fix(makefile): work OOTB on macOS #2137

dronenb · 2025-11-07T19:34:08Z

The README instructs to install heimdall via brew to build on macOS. However, this alone is insufficient. This change detects if you are building on macOS and have the gssapi tag, and if so, determines your brew prefix,, and includes the heimdall headers

Fixes #2136

Signed-off-by: Ben Dronen <[email protected]>

openshift-ci · 2025-11-07T19:34:23Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dronenb
Once this PR has been reviewed and has the lgtm label, please assign ardaguclu for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

coderabbitai · 2025-11-07T19:34:26Z

Walkthrough

The PR addresses a macOS build failure by adding Darwin-specific compiler configuration that sets Heimdal header paths when the gssapi tag is used, and reformats a Makefile build flag assignment to a single-line form.

Changes

Cohort / File(s)	Change Summary
macOS gssapi configuration `Makefile`	Introduces a conditional block that sets `HOMEBREW_PREFIX` and exports `CGO_CFLAGS` with Heimdal include paths when building on Darwin (`uname -s = Darwin`) with the gssapi tag present in `GO_BUILD_FLAGS`. Also consolidates multi-line `GO_LD_EXTRAFLAGS` assignment into single-line format.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Verify the Darwin detection logic (uname -s = Darwin) and conditional flag parsing for gssapi tag
Confirm HOMEBREW_PREFIX and CGO_CFLAGS settings align with Heimdal installation expectations on macOS
Validate that the reformatted GO_LD_EXTRAFLAGS preserves all original flag values

Assessment against linked issues

Objective	Addressed	Explanation
Fix `make oc` compilation failure on macOS with gssapi tag [`#2136`]	✅

Out-of-scope changes

No out-of-scope changes identified. The reformatting of GO_LD_EXTRAFLAGS is a trivial formatting change that does not alter functionality.

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 1c5f490 and 711f483.

📒 Files selected for processing (1)

Makefile (2 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

Makefile

🔇 Additional comments (2)

Makefile (2)

19-28: Clarify GO_LD_EXTRAFLAGS formatting change.

The AI summary states this was "merged into a single-line form," but the visible code shows multi-line format with backslash continuations. Since removed lines aren't shown, I cannot verify the actual transformation. Please confirm whether the change is substantive or merely a whitespace reformatting, and whether any compilation flags were added or removed.

40-48: macOS GSSAPI header path fix is correctly implemented and properly scoped.

Verification confirms the CGO_CFLAGS export mechanism works correctly:

Export propagation: The export CGO_CFLAGS statement at line 46 makes the variable available to all child processes, including the go build command invoked by the build-package function in vendor/github.com/openshift/build-machinery-go/make/targets/golang/build.mk (lines 5-11).

Conditional correctness: The check on line 44 correctly uses $(GO_BUILD_FLAGS) (which includes gssapi) rather than $(GO_BUILD_FLAGS_DARWIN) (which excludes gssapi), ensuring the fix only applies to native macOS builds.

Cross-build isolation: Cross-build targets (lines 126–132) override GO_BUILD_FLAGS with platform-specific variants that exclude gssapi, preventing the Heimdal header path injection for cross-compilation scenarios.

The implementation is sound and handles all build scenarios correctly.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

openshift-ci · 2025-11-07T19:34:29Z

Hi @dronenb. Thanks for your PR.

I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

tchap · 2025-11-10T10:21:20Z

/ok-to-test

openshift-ci · 2025-11-10T14:03:43Z

@dronenb: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

tchap · 2025-11-11T08:43:05Z

Makefile

+export CGO_CFLAGS := -I$(HOMEBREW_PREFIX)/opt/heimdal/include
+endif
+endif
+


This is gonna fail on Windows, right?

I am wondering whether just documenting the issue in README wouldn't suffice TBH. But feel free to come up with a cross-platform way to improve this.

I am actually not sure how much we support Windows TBH 😅 Perhaps @ardaguclu knows better.

I would also like to understand first why my setup worked out of the box. I would prefer for the user to set up their env so that the lib is found just fine instead of us handling the env in the Makefile.

I don't think this would affect Windows at all, since this only applies to Darwin, happy to test it though.

if we wanted users to setup their environment, I imagine that would mean including pkg-config in some capacity? I think this route would be the simplest...

Conditional will only work for Darwin. So this won't affect Windows.

I am honestly bit of a greenhorn regarding Windows, so I was not sure whether uname is even available there. But I guess we require some core UNIX-like utils to be installed. Never mind.

ardaguclu

Thank you for filing and fixing this. Dropped a few comments.

ardaguclu · 2025-11-12T01:18:32Z

Makefile

-                   -X k8s.io/client-go/pkg/version.gitCommit="$(SOURCE_GIT_COMMIT)" \
-                   -X k8s.io/client-go/pkg/version.buildDate="$(shell date -u +'%Y-%m-%dT%H:%M:%SZ')" \
-                   -X k8s.io/client-go/pkg/version.gitTreeState="$(SOURCE_GIT_TREE_STATE)"
+GO_LD_EXTRAFLAGS := -X k8s.io/component-base/version.gitMajor="1" \


We don't need the changes in here.

ardaguclu · 2025-11-12T01:22:56Z

Makefile

+# Fix for macOS GSSAPI compatibility when gssapi tag is used
+# macOS system headers lack RFC 5587 support. Use Homebrew's heimdal for proper headers.
+# See vendor/github.com/apcera/gssapi/name.go for details.
+ifeq ($(shell uname),Darwin)


make oc uses GO_BUILD_FLAGS (line:35) which includes gssapi dependecy. However, if you see darwin build flags (i.e. GO_BUILD_FLAGS_DARWIN, line:36), it does not have gssapi.

Could you please test make oc by removing gssapi from GO_BUILD_FLAGS?. If the build succeeds, it would be great if you can try to use oc login. If oc login works too, I believe that we can modify GO_BUILD_FLAGS by removing gssapi, if the host is darwin.

ardaguclu · 2025-11-12T01:23:18Z

Makefile

+ifeq ($(shell uname),Darwin)
+ifneq (,$(findstring gssapi,$(GO_BUILD_FLAGS)))
+HOMEBREW_PREFIX := $(shell brew --prefix)
+export CGO_CFLAGS := -I$(HOMEBREW_PREFIX)/opt/heimdal/include


I believe that we don't need this at all on macos.

ardaguclu · 2025-11-12T01:23:45Z

Makefile

+export CGO_CFLAGS := -I$(HOMEBREW_PREFIX)/opt/heimdal/include
+endif
+endif
+


Conditional will only work for Darwin. So this won't affect Windows.

fix(makefile): work OOTB on macOS

711f483

Signed-off-by: Ben Dronen <[email protected]>

openshift-ci bot requested review from atiratree and tchap November 7, 2025 19:34

openshift-ci bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Nov 7, 2025

openshift-ci bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 10, 2025

dronenb mentioned this pull request Nov 11, 2025

make oc fails on macOS 26.1 #2136

Open

tchap reviewed Nov 11, 2025

View reviewed changes

ardaguclu reviewed Nov 12, 2025

View reviewed changes

fix(makefile): work OOTB on macOS #2137

Are you sure you want to change the base?

fix(makefile): work OOTB on macOS #2137

Uh oh!

Conversation

dronenb commented Nov 7, 2025

Uh oh!

openshift-ci bot commented Nov 7, 2025

Uh oh!

coderabbitai bot commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Assessment against linked issues

Out-of-scope changes

Uh oh!

openshift-ci bot commented Nov 7, 2025

Uh oh!

tchap commented Nov 10, 2025

Uh oh!

openshift-ci bot commented Nov 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ardaguclu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coderabbitai bot commented Nov 7, 2025 •

edited

Loading