Makefile.include: default to RIOTBOARD when BOARD not in BOARDSDIR by fjmolinas · Pull Request #12972 · RIOT-OS/RIOT

fjmolinas · 2019-12-17T08:06:14Z

Contribution description

#12183 defined BOARDSDIR as a new variable to allow setting external directories where to set a BOARD.

But when BOARDSDIR is set to an external directory it wont see BOARDS that are not in BORDSDIR, so no BOARD in RIOTBOARD.

BOARD=samr21-xpro make -C tests/external_board_native/
make: Entering directory '/home/francisco/workspace/RIOT-test/tests/external_board_native'
/home/francisco/workspace/RIOT-test/tests/external_board_native/../..//Makefile.include:269: *** The specified board samr21-xpro does not exist..  Stop.
make: Leaving directory '/home/francisco/workspace/RIOT-test/tests/external_board_native'

This PR wants to use RIOTBOARD as a fallback for BOARDSDIR when the BOARD is not found in the external directory. The handling is replicated in info-global.inc.mk so info-boards-supported would see BOARDS in BOARDSDIR and RIOTBOARD.

Testing procedure

Finds the samr21-xpro (this fails in master)

BOARDSDIR=/home/ BOARD=samr21-xpro make -C examples/hello-world/

make: Entering directory '/home/francisco/workspace/RIOT-test/examples/hello-world'
Building application "hello-world" for "samr21-xpro" with MCU "samd21".

"make" -C /home/francisco/workspace/RIOT-test/boards/samr21-xpro
"make" -C /home/francisco/workspace/RIOT-test/core
"make" -C /home/francisco/workspace/RIOT-test/cpu/samd21

BOARDSDIR=/home/ BOARD=samr21-xpro make -C examples/hello-world

BOARDSDIR=/home/ BOARD=samr21-xpro make -C examples/hello-world/ info-boards-supported 
make: Entering directory '/home/francisco/workspace/RIOT-test/examples/hello-world'
acd52832 airfy-beacon arduino-due arduino-duemilanove arduino-leonardo arduino-mega2560 arduino-mkr1000 arduino-mkrfox1200 arduino-mkrwan1300 arduino-mkrzero arduino-nano arduino-uno arduino-zero atmega1284p atmega256rfr2-xpro atmega328p avr-rss2 avsextrem b-l072z-lrwan1 b-l475e-iot01a blackpill blackpill-128kib bluepill bluepill-128kib calliope-mini cc1352-launchpad cc2538dk cc2650-launchpad cc2650stk chronos derfmega128 derfmega256 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 hamilton hifive1 hifive1b i-nucleo-lrwan1 ikea-tradfri iotlab-a8-m3 iotlab-m3 limifrog-v1 lobaro-lorabox lsn50 maple-mini mbed_lpc1768 mega-xplained microbit microduino-corerf msb-430 msb-430h msba2 msbiot mulle native nrf51dk nrf51dongle nrf52832-mdk nrf52840-mdk 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 nucleo-l4r5zi nz32-sc151 opencm904 openmote-b openmote-cc2538 p-l496g-cell02 particle-argon particle-boron particle-xenon pba-d-01-kw2x phynode-kw41z pic32-clicker pic32-wifire pyboard reel remote-pa remote-reva remote-revb ruuvitag samd21-xpro same54-xpro saml10-xpro saml11-xpro saml21-xpro samr21-xpro samr30-xpro samr34-xpro seeeduino_arch-pro sensebox_samd21 slstk3401a slstk3402a sltb001a slwstk6000b-slwrb4150a slwstk6000b-slwrb4162a slwstk6220a sodaq-autonomo sodaq-explorer sodaq-one sodaq-sara-aff spark-core stk3600 stk3700 stm32f030f4-demo stm32f0discovery stm32f3discovery stm32f429i-disc1 stm32f4discovery stm32f723e-disco stm32f769i-disco stm32l0538-disco stm32l476g-disco teensy31 telosb thingy52 ublox-c030-u201 udoo usb-kw41z waspmote-pro wsn430-v1_3b wsn430-v1_4 yunjia-nrf51822 z1
make: Leaving directory '/home/francisco/workspace/RIOT-test/examples/hello-world

Test with an external BOARD, easy way just copy the current external native to an external directory:

mkdir <your-dir>
cp -r tests/external_board_native/external_boards/native/ <your-dir>/native-2/
BOARDSDIR=/home/francisco/workspace/external-boards/ BOARD=native-2 make -C examples/hello-world

Issues/PRs references

fjmolinas · 2019-12-17T08:07:25Z

Makefile.include

+# e.g. when set by the environment fallback to searching in RIOTBOARD
+ifeq (,$(wildcard $(BOARDSDIR)/$(BOARD)))
+  ifneq ($(RIOTBOARD),$(BOARDSDIR))
+    # The specified board $(BOARD) was not found in $(BOARDSDIR) fallback to RIOTBOARD)


Note, I didn't use a warning or info here since a lot of the code using info-boards-supported expects a clean output, only the boardslist (e.g. compile_and_test_for_board).

fjmolinas · 2019-12-17T08:35:41Z

@cladmi might have good advice on this one.

cladmi · 2019-12-17T09:12:24Z

Makefile.include


+# Include Board and CPU configuration, if BOARD is not found in BOARDSDIR
+# e.g. when set by the environment fallback to searching in RIOTBOARD
+ifeq (,$(wildcard $(BOARDSDIR)/$(BOARD)))


For the wildcard,this does not check for a directory, see what was done with ALLBOARDS should be $(wildcard $(BOARDSDIR)/$(BOARD)/.) I think.
However it's better to use wildcard than the old $(shell test -d).

This was moved before the global goals separate handling, so the handling does not seem correct for global targets.
A test with an external board (different name) should show this.

By this do you mean that it could ignore the external-boards? This is true, if calling for example info-board with and an external BOARDSDIR and BOARDS set to an internal board, then all BOARDS in the external BOARDSDIR would be ignored, is this what you meant?

Yes as it is done too early, it would overwrite BOARDSDIR for the multiple boards case. And so make all the handling done in makefiles/info-global.inc.mk and boards.inc.mk useless.

Addressed and fixed.

dylad

Quick overview, looks good. I'll try to test it asap.

makefiles/info-global.inc.mk

makefiles/boards.inc.mk

dylad · 2019-12-19T09:00:18Z

I tested this PR and it works as intended.
Current master:

$ BOARDSDIR=/home/dylan/software/mesotic/boards make BOARD=saml21-xpro -C tests/leds clean all
make : on entre dans le répertoire « /home/dylan/software/RIOT/tests/leds »
/home/dylan/software/RIOT/tests/leds/../../Makefile.include:267: *** The specified board saml21-xpro does not exist.. Arrêt.
make : on quitte le répertoire « /home/dylan/software/RIOT/tests/leds »

With this PR:

BOARDSDIR=/home/dylan/software/mesotic/boards make BOARD=saml21-xpro -C tests/leds clean all
make : on entre dans le répertoire « /home/dylan/software/RIOT/tests/leds »
Building application "tests_leds" for "saml21-xpro" with MCU "saml21".

"make" -C /home/dylan/software/RIOT/boards/saml21-xpro
"make" -C /home/dylan/software/RIOT/core
"make" -C /home/dylan/software/RIOT/cpu/saml21
"make" -C /home/dylan/software/RIOT/cpu/cortexm_common
"make" -C /home/dylan/software/RIOT/cpu/cortexm_common/periph
"make" -C /home/dylan/software/RIOT/cpu/sam0_common
"make" -C /home/dylan/software/RIOT/cpu/sam0_common/periph
"make" -C /home/dylan/software/RIOT/cpu/saml21/periph
"make" -C /home/dylan/software/RIOT/drivers
"make" -C /home/dylan/software/RIOT/drivers/periph_common
"make" -C /home/dylan/software/RIOT/sys
"make" -C /home/dylan/software/RIOT/sys/auto_init
"make" -C /home/dylan/software/RIOT/sys/isrpipe
"make" -C /home/dylan/software/RIOT/sys/newlib_syscalls_default
"make" -C /home/dylan/software/RIOT/sys/pm_layered
"make" -C /home/dylan/software/RIOT/sys/stdio_uart
"make" -C /home/dylan/software/RIOT/sys/test_utils/interactive_sync
"make" -C /home/dylan/software/RIOT/sys/tsrb
   text	   data	    bss	    dec	    hex	filename
   9388	    140	   2612	  12140	   2f6c	/home/dylan/software/RIOT/tests/leds/bin/saml21-xpro/tests_leds.elf
make : on quitte le répertoire « /home/dylan/software/RIOT/tests/leds »

I'll let build system experts discuss the syntax introduces by this PR ;)

fjmolinas · 2019-12-19T09:48:53Z

I'll let build system experts discuss the syntax introduces by this PR ;)

Maybe @kaspar030 or @smlng might take a look at that?

aabadie

Looks good in general. I'll test in the coming days.

aabadie · 2019-12-27T08:59:30Z

Makefile.include

+  ifneq ($(RIOTBOARD),$(BOARDSDIR))
+    # The specified board $(BOARD) was not found in $(BOARDSDIR) fallback to RIOTBOARD)
+    BOARDSDIR = $(RIOTBOARD)
+  endif


Suggested change

endif

else

ifeq (,$(wildcard $(BOARDSDIR)/$(BOARD)/.))

$(error The specified board $(BOARD) does not exist.)

endif

This was on purpose. If the first statement is fulfilled the second one will never be evaluated if else is used, but I want it to be evaluated in any case since BOARDSDIR value will have changed to RIOTBOARD, I can think of other ways of evaluating the conditional but it always ends in evaluating something twice. I could change it to:

ifeq (,$(wildcard $(BOARDSDIR)/$(BOARD)/.)) ifneq ($(RIOTBOARD),$(BOARDSDIR)) ifeq (,$(wildcard $(RIOTBOARD)/$(BOARD)/.)) $(error The specified board $(BOARD) is not in RIOTBOARD or BOARDSDIR) else # The specified board $(BOARD) was not found in $(BOARDSDIR) but in $(RIOTBOARD) BOARDSDIR = $(RIOTBOARD) endif else $(error The specified board $(BOARD) is not in RIOTBOARD) endif endif

or something like that if you prefer

hmm, ok I see.

makefiles/info-global.inc.mk

tests/external_board_native/Makefile

fjmolinas · 2019-12-30T09:21:04Z

@aabadie I updates the testing procedure to use an external BOARD, you will need #12999 for this.

aabadie

My comments were addressed and the code looks good now. It was tested by @dylad.

ACK, please squash.

fjmolinas · 2019-12-31T09:12:41Z

@aabadie I guess we are good to go here?

fjmolinas · 2019-12-31T10:05:16Z

@aabadie @dylad @cladmi thanks for the review and testing! :)

fjmolinas added Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Dec 17, 2019

fjmolinas commented Dec 17, 2019

View reviewed changes

fjmolinas mentioned this pull request Dec 17, 2019

Makefile.features: add driver_% provided features and FEATURES_REQUIRED_DRIVERS #12952

Closed

cladmi reviewed Dec 17, 2019

View reviewed changes

kaspar030 changed the title ~~Makefile.include: defualt to RIOTBOARD when BOARD not in BOARDSDIR~~ Makefile.include: default to RIOTBOARD when BOARD not in BOARDSDIR Dec 17, 2019

dylad reviewed Dec 18, 2019

View reviewed changes

makefiles/info-global.inc.mk Outdated Show resolved Hide resolved

makefiles/boards.inc.mk Outdated Show resolved Hide resolved

dylad added the Reviewed: 3-testing The PR was tested according to the maintainer guidelines label Dec 19, 2019

dylad added this to the Release 2020.01 milestone Dec 19, 2019

dylad added the Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation label Dec 19, 2019

fjmolinas requested review from aabadie and kaspar030 December 19, 2019 09:48

fjmolinas force-pushed the pr_boardsdir_riotboard branch from f70a7da to 2102184 Compare December 20, 2019 17:35

fjmolinas mentioned this pull request Dec 20, 2019

boards: use explicit paths for file inclusion #12999

Merged

aabadie mentioned this pull request Dec 27, 2019

tests/external_board_native: only whitelist native board #13002

Closed

aabadie requested changes Dec 27, 2019

View reviewed changes

fjmolinas removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 30, 2019

aabadie approved these changes Dec 30, 2019

View reviewed changes

fjmolinas added 4 commits December 30, 2019 17:12

Makefile.include: RIOTBOARD as BOARDSDIR fallback

1f66c0f

makefiles/boards.inc.mk: include BOARDSDIR and RIOTBOARD

18444ac

makefiles/info-global.inc.mk: RIOTBOAR as BOARDSDIR fallback

cacdd56

tests/external_board_native: require arch_native

33dbf9d

fjmolinas force-pushed the pr_boardsdir_riotboard branch from f809f92 to 33dbf9d Compare December 30, 2019 16:15

fjmolinas added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 30, 2019

aabadie merged commit 68210fe into RIOT-OS:master Dec 31, 2019

fjmolinas deleted the pr_boardsdir_riotboard branch December 31, 2019 10:05

This was referenced Jan 24, 2020

makefiles/boards.inc.mk: set RIOTBOARD #13198

Merged

makefiles/boards.inc.mk: set RIOTBOARD [backport 2020.01] #13206

Merged

-  endif
+  else
+    ifeq (,$(wildcard $(BOARDSDIR)/$(BOARD)/.))
+      $(error The specified board $(BOARD) does not exist.)
+    endif

Conversation

fjmolinas commented Dec 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fjmolinas commented Dec 17, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dylad left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dylad commented Dec 19, 2019

Uh oh!

fjmolinas commented Dec 19, 2019

Uh oh!

aabadie left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fjmolinas commented Dec 30, 2019

Uh oh!

aabadie left a comment

Choose a reason for hiding this comment

Uh oh!

fjmolinas commented Dec 31, 2019

Uh oh!

fjmolinas commented Dec 31, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fjmolinas commented Dec 17, 2019 •

edited

Loading