Skip to content

fix(makefile): work OOTB on macOS #2137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(makefile): work OOTB on macOS #2137

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 20 additions & 10 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,16 @@ include $(addprefix ./vendor/github.com/openshift/build-machinery-go/make/, \
KUBE_GIT_MINOR_VERSION := "34"
KUBE_GIT_VERSION := "v1.34.1"

GO_LD_EXTRAFLAGS :=-X k8s.io/component-base/version.gitMajor="1" \
-X k8s.io/component-base/version.gitMinor=$(KUBE_GIT_MINOR_VERSION) \
-X k8s.io/component-base/version.gitVersion=$(KUBE_GIT_VERSION) \
-X k8s.io/component-base/version.gitCommit="$(SOURCE_GIT_COMMIT)" \
-X k8s.io/component-base/version.buildDate="$(shell date -u +'%Y-%m-%dT%H:%M:%SZ')" \
-X k8s.io/component-base/version.gitTreeState="clean" \
-X k8s.io/client-go/pkg/version.gitVersion="$(SOURCE_GIT_TAG)" \
-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" \
Copy link
Member

@ardaguclu ardaguclu Nov 12, 2025

Choose a reason for hiding this comment

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

We don't need the changes in here.

-X k8s.io/component-base/version.gitMinor=$(KUBE_GIT_MINOR_VERSION) \
-X k8s.io/component-base/version.gitVersion=$(KUBE_GIT_VERSION) \
-X k8s.io/component-base/version.gitCommit="$(SOURCE_GIT_COMMIT)" \
-X k8s.io/component-base/version.buildDate="$(shell date -u +'%Y-%m-%dT%H:%M:%SZ')" \
-X k8s.io/component-base/version.gitTreeState="clean" \
-X k8s.io/client-go/pkg/version.gitVersion="$(SOURCE_GIT_TAG)" \
-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_BUILD_PACKAGES :=$(strip \
./cmd/... \
Expand All @@ -37,6 +37,16 @@ GO_BUILD_FLAGS_DARWIN :=-tags 'include_gcs include_oss containers_image_openpgp'
GO_BUILD_FLAGS_WINDOWS :=-tags 'include_gcs include_oss containers_image_openpgp'
GO_BUILD_FLAGS_LINUX_CROSS :=-tags 'include_gcs include_oss containers_image_openpgp'

# 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)
Copy link
Member

@ardaguclu ardaguclu Nov 12, 2025

Choose a reason for hiding this comment

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

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.

ifneq (,$(findstring gssapi,$(GO_BUILD_FLAGS)))
HOMEBREW_PREFIX := $(shell brew --prefix)
export CGO_CFLAGS := -I$(HOMEBREW_PREFIX)/opt/heimdal/include
Copy link
Member

@ardaguclu ardaguclu Nov 12, 2025

Choose a reason for hiding this comment

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

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

endif
endif

Copy link
Contributor

@tchap tchap Nov 11, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@tchap tchap Nov 11, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Author

@dronenb dronenb Nov 11, 2025

Choose a reason for hiding this comment

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

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...

Copy link
Member

@ardaguclu ardaguclu Nov 12, 2025

Choose a reason for hiding this comment

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

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

Copy link
Contributor

@tchap tchap Nov 12, 2025

Choose a reason for hiding this comment

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

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.

OUTPUT_DIR :=_output
CROSS_BUILD_BINDIR :=$(OUTPUT_DIR)/bin
RPM_VERSION :=$(shell set -o pipefail && echo '$(SOURCE_GIT_TAG)' | sed -E 's/v([0-9]+\.[0-9]+\.[0-9]+)-.*/\1/')
Expand Down