-
Notifications
You must be signed in to change notification settings - Fork 672
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Nin Tran <nin.tran52@gmail.com>
@dborovcanin could you take a look |
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>
@dborovcanin I updated |
There was a problem hiding this 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.
I replaced spaces with tabs for the file @dborovcanin |
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?
@unset
with$(eval)
in Makefile rules (check_tls
andcheck_mtls
)GRPC_TLS
andGRPC_MTLS
are properly unset within Makefile's scopeWhich 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.