Skip to content

SMQ-2831 - Fix Makefile '@unset' incompatibility on macOS #2834

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.

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 8 commits into
base: main
Choose a base branch
from

Conversation

nintran52
Copy link

What type of PR is this?

This is a bug fix because it resolves issue #2831 where using @unset in Makefile caused errors or had no effect depending on the platform.

What does this do?

  • Replaces @unset with $(eval) in Makefile rules (check_tls and check_mtls)
  • Ensures variables like GRPC_TLS and GRPC_MTLS are properly unset within Makefile's scope
  • Makes the logic portable and predictable across Linux, macOS, and Windows environments

Which issue(s) does this PR fix/relate to?

Have you included tests for your changes?

No, because this change affects Makefile logic used at build/config time, which is not covered by unit tests. Manual testing confirms cross-platform compatibility.

Did you document any new/modified feature?

No documentation changes were needed as this is an internal bug fix for script logic.

Notes

This fix is important for developers running make run or related commands in environments like macOS or CI/CD systems. It eliminates a class of non-portable shell commands inside the Makefile.

Signed-off-by: Nin Tran <nin.tran52@gmail.com>
@nintran52 nintran52 requested a review from a team as a code owner April 22, 2025 03:58
@nintran52
Copy link
Author

@dborovcanin could you take a look

@dborovcanin
Copy link
Collaborator

Hello, @nintran52. Thanks for the contribution. I could not test it with MacOS, but I'll trust you with it. :) However, I tested on Linux, and it looks like this PR breaks the Makefile for Linux:

GRPC_TLS=true make run args=-d
Makefile:229: *** missing separator.  Stop.

Can you try something like this, instead?

check_tls:
ifeq ($(GRPC_TLS),true)
	@echo "gRPC TLS is enabled"
	$(eval GRPC_MTLS :=)
else
	$(eval GRPC_TLS :=)
endif

check_mtls:
ifeq ($(GRPC_MTLS),true)
	@echo "gRPC MTLS is enabled"
	$(eval GRPC_TLS :=)
else
	$(eval GRPC_MTLS :=)
endif

This works on Linux, but I can't test it on MacOS.

Signed-off-by: Nin Tran <nin.tran52@gmail.com>
@nintran52
Copy link
Author

@dborovcanin I updated

Copy link
Collaborator

@dborovcanin dborovcanin left a comment

Choose a reason for hiding this comment

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

It looks like you are using spaces, please replace them with tabs since Make only accepts tabs.

@nintran52
Copy link
Author

I replaced spaces with tabs for the file @dborovcanin

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