-
-
Notifications
You must be signed in to change notification settings - Fork 346
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
Build.mk changes to allow the use of CPPFLAGS, CFLAGS and CXXFLAGS. #2065
Conversation
…d if the following is observed - CPPFLAGS - common flags for C and C++ - CFLAGS - C only flags - CXXFLAGS - C++ only flags In addition some toolchains require special standard for their C or C++ headers. Therefore every architecture should be responsible in the Arch/*/build.mk to set the C standard as a CFLAG.
…FLAGS, CFLAGS and CXXFLAGS.
ff48223
to
14857ab
Compare
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.
Looks good, just a couple of points.
Sming/Arch/Esp8266/build.mk
Outdated
@@ -4,7 +4,8 @@ | |||
# | |||
############## | |||
|
|||
CFLAGS += -DARCH_ESP8266 | |||
CPPFLAGS += -DARCH_ESP8266 | |||
CFLAGS += -std=c11 |
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.
Should C coding standard be set in main build.mk file, rather than repeating for arch. ?
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.
Should C coding standard be set in main build.mk file, rather than repeating for arch. ?
We could set a default one in the main build.mk file but allow every architecture to change it if needed. For Arch Esp32 we need c99 standard for the c files otherwise too may problems pop-up.
Sming/building.rst
Outdated
.. envvar:: COMPONENT_CFLAGS | ||
|
||
Used to provide **ONLY** C++ flags that are applied globally. |
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.
Remove - already listed above (lines 599 - 605).
@@ -116,7 +116,7 @@ CFLAGS_COMMON = \ | |||
-ffunction-sections |
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.
CFLAGS_COMMON
can probably just be rolled into CPPFLAGS, it's not used anywhere else.
Before this PR it was impossible to set easily C only flag because CFLAGS was used to set both C and C++ flags.
With this PR the common C/C++ flags should be set using the CPPFLAGS, as in a standard makefile.
With this change CFLAGS will be further on used ONLY for C compilation flags.
This PR is part of the changes needed to allow the compilation of the upcoming ESP32 architecture.
With this PR CPPFLAGS, CFLAGS and CXXFLAGS are allowed only in the Sming/Arch/*/build.mk files and should not be used in a component or application makefile.