Makefile.include: default to RIOTBOARD when BOARD not in BOARDSDIR #12972
Conversation
Makefile.include
Outdated
| # 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) |
There was a problem hiding this comment.
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).
|
@cladmi might have good advice on this one. |
Makefile.include
Outdated
| @@ -95,6 +95,20 @@ __DIRECTORY_VARIABLES += \ | |||
|
|
|||
| BOARDSDIR := $(abspath $(BOARDSDIR)) | |||
|
|
|||
| # 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))) | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Addressed and fixed.
|
I tested this PR and it works as intended. With this PR: I'll let build system experts discuss the syntax introduces by this PR ;) |
Maybe @kaspar030 or @smlng might take a look at that? |
f70a7da to
2102184
Compare
| ifneq ($(RIOTBOARD),$(BOARDSDIR)) | ||
| # The specified board $(BOARD) was not found in $(BOARDSDIR) fallback to RIOTBOARD) | ||
| BOARDSDIR = $(RIOTBOARD) | ||
| endif |
There was a problem hiding this comment.
| endif | |
| else | |
| ifeq (,$(wildcard $(BOARDSDIR)/$(BOARD)/.)) | |
| $(error The specified board $(BOARD) does not exist.) | |
| endif |
There was a problem hiding this comment.
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
f809f92 to
33dbf9d
Compare
|
@aabadie I guess we are good to go here? |
Contribution description
#12183 defined
BOARDSDIRas a new variable to allow setting external directories where to set aBOARD.But when
BOARDSDIRis set to an external directory it wont seeBOARDSthat are not inBORDSDIR, so noBOARDinRIOTBOARD.This PR wants to use
RIOTBOARDas a fallback forBOARDSDIRwhen theBOARDis not found in the external directory. The handling is replicated ininfo-global.inc.mkso info-boards-supported would seeBOARDSinBOARDSDIRandRIOTBOARD.Testing procedure
samr21-xpro(this fails in master)BOARDSDIR=/home/ BOARD=samr21-xpro make -C examples/hello-world/
BOARDSDIR=/home/ BOARD=samr21-xpro make -C examples/hello-world
Issues/PRs references