make: Place most configuration macros in a separate file instead of on the command line

[jnohlgard]

Fixes #5092
Additionally fixes the known issue that a change to USEMODULE in the Makefile requires a manual make clean to build correctly (I couldn't find a Github issue for this though). Whenever USEMODULE (or CFLAGS) is modified in the Makefile, then all object files will be rebuilt.

The implementation checks the old file against the new file and only replaces the old file if the new file is different, this allows make to handle dependencies as usual by looking at the file timestamps.

[kaspar030]

Just wondering - instead of parsing the variable, can't you just call the preprocessor (gcc -nostdi c -E)?

[jnohlgard]

@kaspar030 that was a good idea, though it defines all builtins as well, not just command line:

# 1 "/dev/null"
# 1 "/data/riotbuild/riotproject/examples/gnrc_networking//"
# 1 "<built-in>"
#define __STDC__ 1
#define __STDC_VERSION__ 199901L
#define __STDC_UTF_16__ 1
#define __STDC_UTF_32__ 1
#define __STDC_HOSTED__ 1
#define __GNUC__ 5
#define __GNUC_MINOR__ 2
#define __GNUC_PATCHLEVEL__ 1
#define __VERSION__ "5.2.1 20151202 (release) [ARM/embedded-5-branch revision 231848]"
#define __ATOMIC_RELAXED 0
#define __ATOMIC_SEQ_CST 5
#define __ATOMIC_ACQUIRE 2
#define __ATOMIC_RELEASE 3
#define __ATOMIC_ACQ_REL 4
#define __ATOMIC_CONSUME 1
#define __OPTIMIZE_SIZE__ 1
#define __OPTIMIZE__ 1
...

[kaspar030]

With gcc I got it down to two extra defines:

$ echo "" | gcc -DDEVELHELP -DRIOT_VERSION=1234 -nostdinc -dM -undef -ansi -E -
#define __STDC_HOSTED__ 1
#define RIOT_VERSION 1234
#define DEVELHELP 1
#define __STDC__ 1
[kaspar@booze gnrc_border_router (ethos_br_hack)]$

[kaspar030]

"clang" is similar:

$ echo "" | clang -DDEVELHELP -DRIOT_VERSION=1234 -nostdinc -dM -undef -ansi -E -
#define DEVELHELP 1
#define RIOT_VERSION 1234
#define __STDC_HOSTED__ 1
#define __STDC_UTF_16__ 1
#define __STDC_UTF_32__ 1
#define __STDC__ 1
[kaspar@booze gnrc_border_router (ethos_br_hack)]$

[jnohlgard]

@kaspar030 I don't see the point when the result of using gcc gives additional cruft and when the current script works well enough and only uses echo and sh. There are no extra dependencies.
The checksumming is done using md5sum, but that would be the same regardless if the file is generated using gcc or using echo.

[kaspar030]

@gebart Makes sense.

I'll test as soon as I find time, unfortunately not today anymore.

[miri64]

Additionally fixes the known issue that a change to USEMODULE in the Makefile requires a manual make clean to build correctly (I couldn't find a Github issue for this though). Whenever USEMODULE (or CFLAGS) is modified in the Makefile, then all object files will be rebuilt.

Awesome! That will probably also remove the need for -B at last (which I saw that some contributers still need it).

[jnohlgard]

I don't understand what Murdock is doing here, it seems like a race condition between clean and genconfigheader

[jnohlgard]

OK, managed to reproduce the error locally now

[jnohlgard]

make clean all -j9999 now works as expected.
@kaspar030 OK to squash?

[kaspar030]

Yes, please squash!

[jnohlgard]

rebased, squashed

[kaspar030]

Yes, please squash!

[jnohlgard]

Squashed, waiting for Murdock

[jnohlgard]

Murdock is green. @kaspar030 do you mind doing a review and ACK?

[BytesGalore]

$(BINDIR)/riotbuild/riotbuild.h
         ^

I guess this slash can be removed

[BytesGalore]

really nice 👍, the generated header gives a clean and handy overview of the set defines.
(just a reminder, we should calm the output of the generation before merging)

[BytesGalore]

Is there a specific reason for switching these lines? I'm just curious.

[jnohlgard]

I don't remember, I think I just deleted them and rewrote them, and when I
did, I put them in alphabetical order.
Den 28 jun 2016 07:38 skrev "BytesGalore" notifications@github.com:

In Makefile.modules
#5097 (comment):

@@ -6,9 +6,9 @@ USEMODULE := $(filter-out $(filter-out $(FEATURES_PROVIDED), $(FEATURES_OPTIONAL
INCLUDES += -I$(RIOTBASE)/core/include -I$(RIOTBASE)/drivers/include -I$(RIOTBASE)/sys/include
INCLUDES += -I$(RIOTCPU)/$(CPU)/include
INCLUDES += -I$(RIOTBOARD)/$(BOARD)/include
-ED = $(patsubst %,-DMODULE_%,$(subst -,,$(USEMODULE) $(USEPKG)))
-ED += $(patsubst %,-DFEATURE%,$(subst -,,$(filter $(FEATURES_PROVIDED), $(FEATURES_REQUIRED))))
-EXTDEFINES = $(shell echo $(sort $(ED))|tr 'a-z' 'A-Z')
+ED = $(addprefix FEATURE,$(sort $(filter $(FEATURES_PROVIDED), $(FEATURES_REQUIRED))))
+ED += $(addprefix MODULE_,$(sort $(USEMODULE) $(USEPKG)))

Is there a specific reason for switching these lines? I'm just curious.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/RIOT-OS/RIOT/pull/5097/files/a2d9ffeba8d582a741f4da3a7b3e1585606a94b8#r68699946,
or mute the thread
https://github.com/notifications/unsubscribe/AATYQmr0RMmiUgvmnymDHGi24ynq0Oh8ks5qQLNRgaJpZM4Hy4H3
.

[BytesGalore]

The script works as intended, I tested it for native and for samr21-xpro with the example/gnrc_networking.
So if the output of the generation is calmed I give an ACK.

[jnohlgard]

@BytesGalore addressed comments

[BytesGalore]

Looks good and works as intended 👍.
Please add @ here to suppress the output. I guess you can squash directly.
Then ACK from my side.

[jnohlgard]

I added a check for QUIET=1 as well, to silence the script messages.

[jnohlgard]

squashed

[Kijewski]

Could you please test if the FORCE target in tests/unittests/Makefile can then be omitted? Your PR should make it obsolete, because a changed -DTEST_SUITES="…" will force the recompilation in its own.

[jnohlgard]

@Kijewski I tried removing the FORCE part of the unittest Makefile and it seems to still pick up the changed arguments when changing the command line between different test suites. Thanks for the hint!

[jnohlgard]

ACK holds?

[BytesGalore]

yup ACK holds.

[jnohlgard]

& go!