search

RIOT-OS / RIOT

RIOT - The friendly OS for IoT
https://riot-os.org
GNU Lesser General Public License v2.1
4.88k stars 1.98k forks source link

make: use of immediate value of variables before they have their final value #8913

Open cladmi opened 6 years ago

cladmi commented 6 years ago

Description

Some makefiles use variables with their immediate value before they get the final one. It's not problematic for build flags but for variables containing files/path.

The root of the problem is that the makefiles do not follow a pattern of defining all variables first/including sub makefiles, and then only after define the rules using the variables as file target or file dependency.

This issue will be the central one to reference sub pull requests to fix this uses.

Debugging

The problem can be seen by using $(info debug VARIABLE $(VARIABLE)) before usage.

TODO list:

board/cpu Makefile.include

Some boards and cpus Makefile.include have different behaviors depending on the modules.

git grep -e USEPKG -e USEMODULE -e FEATURES boards/ cpu/ | grep Makefile.include | grep -e ifeq -e ifneq
boards/common/nrf52xxxdk/Makefile.include:ifneq (,$(filter nordic_softdevice_ble,$(USEPKG)))
boards/native/Makefile.include:ifneq (,$(filter netdev_default gnrc_netdev_default,$(USEMODULE)))
cpu/esp8266/Makefile.include:ifneq (, $(filter pthread, $(USEMODULE)))
cpu/esp8266/Makefile.include:ifneq (, $(filter esp_sw_timer, $(USEMODULE)))
cpu/esp8266/Makefile.include:ifneq (, $(filter esp_sdk, $(USEMODULE)))
cpu/esp8266/Makefile.include:ifneq (, $(filter esp_gdbstub, $(USEMODULE)))
cpu/esp8266/Makefile.include:ifneq (, $(filter esp_gdb, $(USEMODULE)))
cpu/esp8266/Makefile.include:ifneq (, $(filter esp_spiffs, $(USEMODULE)))
cpu/esp8266/Makefile.include:ifneq (, $(filter esp_sdk, $(USEMODULE)))
cpu/esp8266/Makefile.include:    ifneq (, $(filter esp_now, $(USEMODULE)))
cpu/stm32_common/Makefile.include:ifneq (,$(filter periph_flashpage periph_eeprom,$(FEATURES_REQUIRED)))

However, modules dependencies are only resolved after including these files.

Some even include Makefile.dep to fix issues (other for no reason)

https://github.com/RIOT-OS/RIOT/blob/master/boards/common/nrf52xxxdk/Makefile.include#L21-L27

git grep Makefile.dep boards | grep 'Makefile.include:'
boards/common/arduino-atmega/Makefile.include:include $(RIOTBOARD)/common/arduino-atmega/Makefile.dep
boards/common/arduino-mkr/Makefile.include:include $(RIOTBOARD)/$(BOARD)/Makefile.dep
boards/common/nrf52xxxdk/Makefile.include:include $(RIOTBOARD)/$(BOARD)/Makefile.dep
boards/common/nrf52xxxdk/Makefile.include:include $(RIOTBOARD)/$(BOARD)/Makefile.dep
boards/feather-m0/Makefile.include:include $(RIOTBOARD)/$(BOARD)/Makefile.dep
boards/sensebox_samd21/Makefile.include:include $(RIOTBOARD)/$(BOARD)/Makefile.dep
boards/sodaq-explorer/Makefile.include:include $(RIOTBOARD)/$(BOARD)/Makefile.dep
boards/sodaq-one/Makefile.include:include $(RIOTBOARD)/$(BOARD)/Makefile.dep

Dependencies need to be processed before including any other Makefile.include.

Also referenced in https://github.com/RIOT-OS/RIOT/issues/9811

pkg/ PKG_BUILDDIR

Some packages makefile use $(PKG_BUILDDIR) with its immediate value when defining targets before it is defined by including pkg/pkg.mk.

It is mitigated because then they are treated as non file targets, and because the command repeats the variable and does not use the automatic make variables like $@. https://github.com/RIOT-OS/RIOT/pull/8509#discussion_r178037265

Done

Makefile.include

In Makefile.include, all uses of $(BASELIBS), except when defining _LINK, are using the immediate > value of $(BASELIBS) which does not have its final value as this is done by including modules.inc.mk 200 lines below.

All the uses are currently solved by hidden "tricks"

  • $(ELFFILE): $(BASELIBS) which works because there is a $(ELFFILE): FORCE rule, so it will be rebuilt anyway, and _LINK is using the deferred value of BASELIBS.
  • link: $(BASELIBS) when in the RIOTNOLINK case, where BASELIBS is missing $(USEPKG:%=$(BINDIR)/%.a) files, but as $(BINDIR)/$(APPLICATION_MODULE).a has a dependency to $(USEPKG:%=$(BINDIR)/%.a) they are still build anyway.
  • The clean before building trick for parallel execution is using $(BASELIBS) also without the $(USEPKG:%=$(BINDIR)/%.a) files. But as there is the RIOTPKG/%/Makefile.include target also depending on clean, the packages are built after clean.
cladmi commented 5 years ago

Includes is also evaluated directly https://github.com/RIOT-OS/RIOT/blob/1f7ec9b2082eec7419f50a7f35eefc84b26a6e90/makefiles/libc/newlib.mk#L89 even if CFLAGS_FPU can be changed later by pkg/nordic_softdevice_ble which would change the INCLUDES when using llvm https://github.com/RIOT-OS/RIOT/blob/1f7ec9b2082eec7419f50a7f35eefc84b26a6e90/makefiles/toolchain/llvm.inc.mk#L70

In that case it is not breaking as llvm is said incompatible but it shows an issue with these immediate evaluations.

cladmi commented 5 years ago

This one is also related, where a file that should be put in BUILDDEPS is declared by a makefile included after using BUILDDEPS: https://github.com/RIOT-OS/RIOT/pull/10492#issuecomment-444108243

It goes with many definitions orders in our Makefiles that should be addressed in a deterministic way at some point.