-
Notifications
You must be signed in to change notification settings - Fork 79
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
Respect CC/CXX/CFLAGS/CXXFLAGS/LDFLAGS. #72
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Piotr Sikora <[email protected]>
cc @rossberg |
|
||
# Linking C / C++ example | ||
.PRECIOUS: ${EXAMPLES:%=${EXAMPLE_OUT}/%-c} | ||
${EXAMPLE_OUT}/%-c: ${EXAMPLE_OUT}/%-c.o ${WASM_C_O} | ||
${CC_COMP} ${C_FLAGS} ${LD_FLAGS} $< -o $@ \ | ||
${CXX} ${CXXFLAGS} ${LDFLAGS} $< -o $@ \ |
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.
Note: I changed CFLAGS
to CXXFLAGS
here, since CXX
is used to link those targets, so it seemed more correct.
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.
Makes sense.
Ping. |
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.
To be honest, I'm not a big fan of inheriting configuration flags from the environment, since they are easily clobbered by accident that way (dynamic scoping sucks!). Do you have a use case where it is not enough to set them from the command line (make VAR=X
instead of VAR=X make
)?
|
||
C_COMP = clang | ||
ifeq ($(origin CC),default) | ||
CC = clang |
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.
Why not simply CC ?= clang
?
CC = clang | ||
endif | ||
ifeq ($(origin CXX),default) | ||
CXX = clang++ |
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.
Same here.
Of course, moving this here loses the ability to set both compilers with just one command line var.
|
||
# Linking C / C++ example | ||
.PRECIOUS: ${EXAMPLES:%=${EXAMPLE_OUT}/%-c} | ||
${EXAMPLE_OUT}/%-c: ${EXAMPLE_OUT}/%-c.o ${WASM_C_O} | ||
${CC_COMP} ${C_FLAGS} ${LD_FLAGS} $< -o $@ \ | ||
${CXX} ${CXXFLAGS} ${LDFLAGS} $< -o $@ \ |
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.
Makes sense.
Signed-off-by: Piotr Sikora [email protected]