newlib: remove include order dependency by setting _DEFAULT_SOURCE flag in Makefiles#16010
newlib: remove include order dependency by setting _DEFAULT_SOURCE flag in Makefiles#16010haukepetersen wants to merge 4 commits intoRIOT-OS:masterfrom
_DEFAULT_SOURCE flag in Makefiles#16010Conversation
| @@ -1,2 +1,6 @@ | |||
| MODULE=constfs | |||
|
|
|||
| # Required for strnlen in string.h, when building with -std=c99 | |||
There was a problem hiding this comment.
Since we are building with -std=c11 now, do we even still need this?
There was a problem hiding this comment.
Yes, if you comment these lines out the build will fail, at least on my machine (arm-none-eabi-gcc (GNU Arm Embedded Toolchain 9-2020-q2-update) 9.3.1 20200408 (release))
|
@fjmolinas @benpicco would you mind taking a quick look at this? I would really love to get the |
| USEMODULE += embunit | ||
|
|
||
| # Make some different functions visible between newlib and newlib-nano | ||
| CFLAGS += -D_DEFAULT_SOURCE=1 |
There was a problem hiding this comment.
CFLAGS in the application file are global. I guess this is fine, although I'd rather have the libc test compile the whole tree with the options that are default elsewhere.
Which begs the question - can we make _DEFAULT_SOURCE=1 the default?
There was a problem hiding this comment.
CFLAGS in the application file are global.
Not sure I understand this. Do you mean setting CFLAGS in the Makefile of a module leads to that flag being set globally? As I understand it setting this in the Makefile for a single compilation unit has only effect in that unit - and as such this should have the identical effect than the define in the source file before, right?!
About making _DEFAULT_SOURCE=1 default: I can't tell what the side effects would be, so I can't argue for/against it. I simply fear this would be a lengthy process once more and this would starve #15931 even longer then?!
There was a problem hiding this comment.
About making _DEFAULT_SOURCE=1 default: I can't tell what the side effects would be, so I can't argue for/against it. I simply fear this would be a lengthy process once more and this would starve #15931 even longer then?!
We could split out the global include then? At least the core part would get in, and locally globally including it will be easy while those things are fiddled out.
There was a problem hiding this comment.
I still don't understand...
There was a problem hiding this comment.
I still don't understand...
My comment? It's exactly what you did in #15931 :).
If its the global CFLAGS thing, I think what Kaspar means is that before only those specific headers or compilation units (when it was a pkg) had _DEFAULT_SOURCE=1 so include whatever features that enable. With defining it in the application makefile all compilation units now include those set of features. And since this is a libc set test @kaspar030 would rather have it test with the same configuration used in the whole tree, which is why he suggests a global inclusion if it does not hurt.
There was a problem hiding this comment.
CFLAGS in the application file are global.
Not sure I understand this. Do you mean setting CFLAGS in the Makefile of a module leads to that flag being set globally?
No, setting it in a module makefile will set it for that module and all modules included with DIRS.
But the main application makefile is special. Anything set there will be set for all modules (globally).
There's no straight forward way to set options for just the application "module".
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions. |
Contribution description
The bulid tests for #15931 failed on some platforms due to include order problems for modules that declare the
_DEFAULT_SOURCEflag before including some newlib headers. Reason is, that #15931 includes a specific header file globally, which also pulls some stdlib functionality on some platforms, and thus disabling the effect of the_DEFAULT_SOURCEflag set in subsequent C files.This PR fixes this issue by pulling the
_DEFAULT_SOURCEflag definition out of the corresponding C files and putting it inside the Makefile for that specific compilation unit. This mitigates any include order dependencies for good.This PR also adds slightly more coherency to RIOT, as now all modules that use this flag do use the same way of setting it (
pkg/minmeaalready applied this pattern before...).Testing procedure
Build test would show if anything got broken.
Issues/PRs references
Needed for #15931 to work correctly.