Build dependencies - processing order issues #9913

cladmi · 2018-09-10T15:25:18Z

There currently are concurrency issues in the order dependencies are defined and used, and cpu dependencies cannot be easily parsed.

I plan to address this by moving the processing of Makefile.dep before including $(BOARD)/Makefile.include

Dependencies resolution uses the following variables

USEMODULE
USEPKG
FEATURES_
BOARD
CPU
CPU_FAM (which depend on files in CPU)
CPU_ARCH (which depend on files in CPU)
CPU_MODEL

Steps

Prerequisites cleanup

mips-malta including common cpu features board/mips-malta: include the cpu features instead of common cpu #10064
boards: always include cpu features boards: always include cpu features #10078
- cpu/esp8266 re-defining CPU cpu/esp8266: remove duplicate 'CPU' variable #10061
- hifive1 not processing CPU features boards/hifive1: include cpu/fe310 features #10063
cpu/fe310: rtc depend on the rtt feature and hifive1 update cpu/fe310: rtc depend on the rtt feature and hifive1 update #10062
static_tests: add build system checks static_tests: add build system checks #10060

And even future improvement to remove multiple duplication between board/board_common cpu/cpu_common path:

With BOARD and CPU recursively including their Makefile.board and Makefile.cpu, define some BOARD_IMPLEMENTATION_DIRS like variables that can be re-used to remove duplication in Makefile, Makefile.deps, Makefile.include, Makefile.features.

Explanation

To summarize, the main Makefile.include does regarding modules:
(a more detailed version is available here https://gist.github.com/cladmi/1c5af63c753635b7cb173cf8311d0b5b)

include $(RIOTBOARD)/$(BOARD)/Makefile.features
    # Define FEATURES_PROVIDED
    # Include with hard written path CPU/Makefile.features

include $(RIOTBOARD)/$(BOARD)/Makefile.include
    # Defines CPU
    # Defines USEMODULE
    # Have specific behavior depending on USEMODULE/USEPKG
include $(RIOTBOARD)/$(CPU)/Makefile.include
    # Define USEMODULE
    # Have specific behavior depending on USEMODULE/USEPKG
    # Define module dependencies

include Makefile.dep
    # Resolve USEMODULE/USEPKG/FEATURES_USED using:
    # * USEMODULE
    # * USEPKG
    # * FEATURES_
    # * BOARD
    # * CPU
    # * CPU_FAM
    # * CPU_ARCH 

include $(RIOTBASE)/sys/Makefile.include
include $(RIOTBASE)/drivers/Makefile.include
-include $(USEPKG:%=$(RIOTPKG)/%/Makefile.include)
include $(RIOTMAKE)/bindist.inc.mk
    # These also add things to USEMODULE

The main Makefile.dep only includes $(BOARD)/Makefile.dep which may sometime also end up including hardwritten_cpu_name/Makefile.dep.

In the same time makefiles/info-global.inc.mk does:

include $(RIOTBOARD)/$(BOARD)/Makefile.features
include $(RIOTBASE)/Makefile.dep

And so never takes into account any of the dependencies defined in $(BOARD)/Makefile.include or any other Makeflie.include files.

The text was updated successfully, but these errors were encountered:

cladmi · 2018-09-11T13:43:32Z

To not re-add another file for nothing, I thought about putting CPU in Makefile.features instead.

It would contain "everything that needs to be defined before being able to process dependencies".

miri64 · 2018-09-11T16:58:04Z

I'm unclear on what the problem is with CPU defined in Makefile.include...

cladmi · 2018-09-12T09:31:31Z

Changing it is a mix of somehow required, it makes sense, do not repeat the same information 3 times, and many things in our makefiles have hard ordering requirements so leads to the proposed solution.

One hint that moving CPU earlier than Makefile.include is a good idea, is that right now the cpu name is already hardcoded in $(BOARD)/Makefile.features (or a common/Makefile.features):

RIOT/boards/samr21-xpro/Makefile.features

Line 15 in 1e6009c

-include $(RIOTCPU)/samd21/Makefile.features

Main issue

The first main issue that is causing me problems, is that board and CPUs need to declare dependencies, and some of their internal modules too. However, this is right now often done in $(BOARD)/Makefile.include and $(CPU)/Makefile.include but the dependencies are resolved later in Makefile.dep. So they use USEMODULE before it is defined.

One example is here:

RIOT/boards/common/nrf52xxxdk/Makefile.include

Lines 21 to 27 in 1e6009c

    
           # The following configuration is dependencies specific 
        
           # but they are resolved later 
        
           # Hack to know now if 'nordic_softdevice_ble' is used 
        
           include $(RIOTBOARD)/$(BOARD)/Makefile.dep 
        
           # special options when using SoftDevice 
        
           ifneq (,$(filter nordic_softdevice_ble,$(USEPKG)))

Another one here:

RIOT/cpu/stm32_common/Makefile.include

Lines 13 to 15 in 1e6009c

    
           ifneq (,$(filter periph_flashpage periph_eeprom,$(FEATURES_REQUIRED))) 
        
             FEATURES_REQUIRED += periph_flash_common 
        
           endif

This one is even using FEATURES_REQUIRED to work because of course USEMODULE is not defined yet. But it fails if the main application only declares a dependency on a module that will add the FEATURES_REQUIRED as does the test in #9892 and the OTA task force branch.

Proposed solution

So for this I want to move the processing of dependencies, before parsing all modules Makefile.include files (boards, cpu, sys, drivers, pkgs).

Constraint

Moving the dependency handling before Makefile.include means to somehow do:

include $(RIOTBOARD)/$(BOARD)/Makefile.dep
include $(RIOTBOARD)/$(CPU)/Makefile.dep

...

include $(RIOTBOARD)/$(BOARD)/Makefile.include

Which would require having CPU defined.

I say somehow, because it could be $(BOARD)/Makefile.dep or one of its $(RIOTBOARD)/common/one_on_the_common/Makefile.dep that does it like we are doing for Makefile.features and directly includes the cpu_name/Makefile.dep

But I am not going to push toward having 3 times the cpu name defined between Makefile.features, Makefile.dep, Makefile.include.

Then on the somehow required:

The CPU variable is used in Makefile.dep

CPU is used in https://github.com/RIOT-OS/RIOT/blob/master/cpu/stm32_common/Makefile.dep
It is not entirely required, because this file is only included in https://github.com/RIOT-OS/RIOT/blob/master/cpu/stm32_common/periph/Makefile and could be replaced by SRC += instead. Here USEMODULE is currently private to this Makefile. So could be moved out of Makefile.dep.

However, it is still concept a dependency that the board depends/uses on this cpu so makes sense to be explicit when resolving Makefile.dep.

cladmi · 2018-09-20T14:58:37Z

We can also see this problem with the esp32 board that need to define CFLAGS depending on included modules. #9426

They could in theory be defined in inline if statement, but it is not a pattern used in other boards, and would be solved by re-ordering the processing.

cladmi · 2018-10-31T11:33:54Z

Found another case showing the issue:

mips-malta is said supported for tests/pipe

make --no-print-directory -C tests/pipe/ info-boards-supported | grep mips-malta
acd52832 airfy-beacon arduino-due arduino-duemilanove arduino-mega2560 arduino-mkr1000 arduino-mkrfox1200 arduino-mkrzero arduino-uno arduino-zero avsextrem b-l072z-lrwan1 b-l475e-iot01a blackpill bluepill calliope-mini cc2538dk cc2650-launchpad cc2650stk chronos ek-lm4f120xl esp32-mh-et-live-minikit esp32-olimex-evb esp32-wemos-lolin-d32-pro esp32-wroom-32 esp32-wrover-kit esp8266-esp-12x esp8266-olimex-mod esp8266-sparkfun-thing f4vi1 feather-m0 firefly fox frdm-k22f frdm-k64f frdm-kw41z hifive1 ikea-tradfri iotlab-a8-m3 iotlab-m3 limifrog-v1 lobaro-lorabox maple-mini mbed_lpc1768 microbit mips-malta msb-430 msb-430h msba2 msbiot mulle native nrf51dongle nrf52840dk nrf52dk nrf6310 nucleo-f030r8 nucleo-f031k6 nucleo-f042k6 nucleo-f070rb nucleo-f072rb nucleo-f091rc nucleo-f103rb nucleo-f207zg nucleo-f302r8 nucleo-f303k8 nucleo-f303re
nucleo-f303ze nucleo-f334r8 nucleo-f401re nucleo-f410rb nucleo-f411re nucleo-f412zg nucleo-f413zh nucleo-f429zi nucleo-f446re nucleo-f446ze nucleo-f722ze nucleo-f746zg nucleo-f767zi nucleo-l031k6 nucleo-l053r8 nucleo-l073rz nucleo-l152re nucleo-l432kc nucleo-l433rc nucleo-l452re nucleo-l476rg nucleo-l496zg nz32-sc151 opencm904 openmote-b openmote-cc2538 pba-d-01-kw2x pic32-clicker pic32-wifire remote-pa remote-reva remote-revb ruuvitag samd21-xpro saml21-xpro samr21-xpro samr30-xpro seeeduino_arch-pro sensebox_samd21 slstk3401a slstk3402a sltb001a slwstk6000b slwstk6220a sodaq-autonomo sodaq-explorer sodaq-one spark-core stk3600 stk3700 stm32f0discovery stm32f3discovery stm32f429i-disc1 stm32f4discovery stm32f769i-disco stm32l476g-disco teensy31 telosb thingy52 ublox-c030-u201 udoo waspmote-pro wsn430-v1_3b wsn430-v1_4 yunjia-nrf51822 z1

But when compiling we get

make --no-print-directory -C tests/pipe/ BOARD=mips-malta
/home/harter/work/git/RIOT/makefiles/toolchain/gnu.inc.mk:18: objcopy not found. Hex file will not be created.
There are unsatisfied feature requirements: periph_uart


EXPECT ERRORS!

Because mips does USEMODULE += newlib in

cpu/mips32r2_common/Makefile.include:export USEMODULE += newlib

cladmi · 2018-11-16T11:24:28Z

@MrKevinWeiss The issue I talk to you about with the dependencies processing

aabadie · 2019-12-07T12:10:10Z

Is any work or progress to move the inclusion of Makefile.dep before Makefile.include?

Thanks for reviving this issue, which is still valid. I just had a look at it and there is still some work to be done, and in many different places. That will require several PRs before Makefile.dep can be included before Makefile.include.

aabadie · 2019-12-07T16:59:30Z

@gschorcht, if you are interested, I have a branch where I started to move cpu dependencies in dedicated Makefile.dep. I'm hesitating to PR it. If you think it's worth, I'll do.

I could already see problems if we include Makefile.dep before board/cpus Makefile.include: some cpu add modules based on variables defined in board/cpu level Makefile.include (for example pic32, esp32). There's also the kinetis-info.mk that should be split I think (maybe @fjmolinas has some ideas here).

I haven't looked at esp.

gschorcht · 2019-12-07T17:13:19Z

The ESP32 wont be a problem. For ESPs, some variables are defined dependent on some modules but not vise versa. Therefore, some module dependencies have to be defined at the very beginning of Makefile.include.

But, once Makefile.dep is included before Makefile.include, it will become possible to move all module dependency definitions to Makefile.dep and all dependent variable definitions to Makefile.include.

I'm hesitating to PR it. If you think it's worth, I'll do.

I think it is worth. It would be further step to cleanup the makefiles.

gschorcht · 2019-12-07T17:15:54Z

@aabadie If you'd like, I can contribute the ESP changes to your branch.

aabadie · 2019-12-07T17:21:39Z

If you'd like, I can contribute the ESP changes to your branch.

My branch is touching almost all CPUs so I think it will be difficult to review it and get it merged fast. Some reviewers may also ask to split it. I would prefer you open a separate PR for esp.

gschorcht · 2019-12-07T17:30:26Z

OK, I see. It seems like a chicken-and-egg problem 😟 It's certainly not possible to split the PRs without affecting the compilability for some CPUs. On the other hand, I can't provide the changes as separate PRs until the include order is changed without affecting the compilability for the EPSs.

aabadie · 2019-12-07T17:34:44Z

I think it is worth

Ok, then it's here: #12898

For ESP, you can still move forward to an intermediate state where non conditional dependencies could be moved to Makefile.dep.

nrf52 includes include $(RIOTBOARD)/$(BOARD)/Makefile.dep to know if `nordic_softdevice_ble` is used, this changes dependency resolution sinnce -include $(APPDIR)/Makefile.board.dep should be resolved before. This can be removed once RIOT-OS#9913 is if `nordic_softdevice` is deprecated.

aabadie · 2020-06-23T13:18:12Z

@fjmolinas @leandrolanzieri, what is the status of this issue ? Are all cpu dependencies addressed ?

fjmolinas · 2020-06-23T13:38:04Z

@fjmolinas @leandrolanzieri, what is the status of this issue ? Are all cpu dependencies addressed ?

I think we are at a point where we could test changing the inclusion order, other than sketch_module I'm not sure what is in the way. Would be worth launching debugdependencies on a PR that changes the inclusion order.

fjmolinas · 2020-06-23T13:39:00Z

I believe:

Move CPU dependencies to $(CPU)/Makefile.dep
Fix CPU dependencies not being parsed easily as reported first in
Move all BOARD/CPU dependencies to $(BOARD)/Makefile.dep and $(CPU)/Makefile.dep

are done, can you think of something that is missing @aabadie @leandrolanzieri?

cladmi added State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Area: build system Area: Build system Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR labels Sep 10, 2018

cladmi self-assigned this Sep 10, 2018

cladmi added the Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) label Sep 10, 2018

cladmi mentioned this issue Sep 10, 2018

Makefile.include: always include board Makefile.features #9915

Merged

This was referenced Sep 11, 2018

pba-d-01-kw2x: remove duplicate MCPU #9921

Merged

boards/cpu: cleanup duplicate includes #9923

Merged

cladmi mentioned this issue Sep 20, 2018

ESP32 port #9426

Merged

miri64 added the Type: tracking The issue tracks and organizes the sub-tasks of a larger effort label Sep 27, 2018

cladmi changed the title ~~Dependencies processing issues~~ Dependencies processing order issues Oct 1, 2018

cladmi mentioned this issue Oct 1, 2018

boards: always include cpu features #10078

Merged

This was referenced Oct 8, 2018

bootloader: add riotboot minimal bootloader #10065

Merged

cpu/stm32_common: correctly pull periph_flash_common dependency #10146

Closed

cpu/stm32_common: fix source selection declared as module dependencies (broken) #10153

Merged

cladmi mentioned this issue Oct 17, 2018

dist/tools: add build system sanity check script #10179

Merged

cladmi mentioned this issue Nov 12, 2018

cpu/esp32: fixes compile problems #10362

Merged

aabadie mentioned this issue Dec 7, 2019

cpu: move cpu level dependencies in dedicated Makefile.dep files #12898

Merged

aabadie mentioned this issue Dec 7, 2019

make: add possibility to provide board specific application dependencies in a separate Makefile #12755

Merged

This was referenced Jan 21, 2020

cpu/efm32: cleanup #13174

Merged

boards/slwstk6000b-*: cleanup #13180

Merged

aabadie mentioned this issue Feb 5, 2020

boards: move remaining uses of USEMODULE from Makefile.include to Makefile.dep #13291

Merged

This was referenced Feb 13, 2020

cpu/cortexm-fpu: use it via a feature instead of DEFAULT_MODULE #13236

Closed

cpu/cortexm: move CPU_ARCH/FAM to Makefile.features #13392

Merged

fjmolinas mentioned this issue Mar 2, 2020

Makefile.dep: Fix duplicate modules from USEMODULE #13401

Merged

This was referenced Mar 23, 2020

cpu/esp[32, 8266, _common]: Move dependency resolutions to Makefile.dep #13685

Merged

cpu/cortexm: cleanup dependencies #13738

Merged

This was referenced May 20, 2020

cpu/native: Move dependencies to Makefile.dep #14109

Merged

cpu/kinetis: Move dependencies to Makefile.dep #14111

Merged

This was referenced Jun 24, 2020

cpu/mips32r2_common: include newlib_syscalls_mips_uhi in Makefile.dep #14344

Merged

sys/arduino: include arduino_sketches in Makefile.dep #14350

Merged

Makefile.include: resolve dependencies before Makefile.include #14351

Merged

fjmolinas mentioned this issue Feb 12, 2021

makefiles/feature_precheck: check libc availability prior deciding usage #15993

Open

kfessel mentioned this issue Feb 15, 2021

makefile/..picolibc: make missing picolibc fail louder #16008

Merged

MrKevinWeiss added this to the Release 2021.07 milestone Jun 22, 2021

MrKevinWeiss removed this from the Release 2021.07 milestone Jul 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build dependencies - processing order issues #9913

Build dependencies - processing order issues #9913

cladmi commented Sep 10, 2018 •

edited by leandrolanzieri

Loading

cladmi commented Sep 11, 2018

miri64 commented Sep 11, 2018

cladmi commented Sep 12, 2018

cladmi commented Sep 20, 2018

cladmi commented Oct 31, 2018

cladmi commented Nov 16, 2018

aabadie commented Dec 7, 2019

aabadie commented Dec 7, 2019

gschorcht commented Dec 7, 2019

gschorcht commented Dec 7, 2019

aabadie commented Dec 7, 2019

gschorcht commented Dec 7, 2019

aabadie commented Dec 7, 2019

aabadie commented Jun 23, 2020

fjmolinas commented Jun 23, 2020

fjmolinas commented Jun 23, 2020

Build dependencies - processing order issues #9913

Build dependencies - processing order issues #9913

Comments

cladmi commented Sep 10, 2018 • edited by leandrolanzieri Loading

Steps

Explanation

cladmi commented Sep 11, 2018

miri64 commented Sep 11, 2018

cladmi commented Sep 12, 2018

Main issue

Proposed solution

Constraint

Then on the somehow required:

cladmi commented Sep 20, 2018

cladmi commented Oct 31, 2018

cladmi commented Nov 16, 2018

aabadie commented Dec 7, 2019

aabadie commented Dec 7, 2019

gschorcht commented Dec 7, 2019

gschorcht commented Dec 7, 2019

aabadie commented Dec 7, 2019

gschorcht commented Dec 7, 2019

aabadie commented Dec 7, 2019

aabadie commented Jun 23, 2020

fjmolinas commented Jun 23, 2020

fjmolinas commented Jun 23, 2020

cladmi commented Sep 10, 2018 •

edited by leandrolanzieri

Loading