make: allow alias for board names #7976

haukepetersen · 2017-11-09T10:25:34Z

Some boards have unique, but quite ugly names (e.g. pba-d-01-kw2x) and we use different names in the day-to-day operation (e.g. phynode for the former). Event though this 'nicer' name ist not a 100% correct (as there are also phynodes based on other CPUs out there, just not in the RIOT universe...), it is sometimes much easier to remember and use. Also, there are some boards that have a specific, cryptic name (e.g. SLTB001A), but also a nice user friendly one (thunderboard-sense in that case). So I think it would be nice, to be able to use both (or even more?) aliases for a certain board.

This PR contains a quick draft, on how this could be implemented. It works, but is it nice enough?

BOARD=phynode make ...   -> will build `BOARD=pba-d-01-kw2x`
BOARD=linux make ... -> will build 'BOARD=native`, just for demo purposes...

Maybe we can even think of something that combines this approach with my idea about board revisions ( #7975). What do you think?

jnohlgard

See my comments inline.
It might make the file a bit more readable if we use the variable inside variable names look up table design pattern. I don't think it is possible to abuse the variable name lookup by setting malicious values for BOARD, because it will always have the prefix BOARD_ALIAS_ regardless of what BOARD is set to.

jnohlgard · 2017-11-09T12:41:07Z

makefiles/boards/alias.inc.mk

+ifneq (,$(filter phynode,$(BOARD)))
+  BOARD := pba-d-01-kw2x
+endif
+ifneq (,$(filter linux,$(BOARD)))


Ditch the filter call, BOARD is a single value, not a list.

Make this an else if

else ifneq (linux, $(BOARD))

yes, makes sense -> gotten so used to the filter thing that I don't even question it anymore as it seems :-)

jnohlgard · 2017-11-09T12:43:32Z

makefiles/boards/alias.inc.mk

@@ -0,0 +1,9 @@
+ALIAS = linux phynode
+
+# this basically implements a look-up table, is there a nicer way in make?


the only other way that I know of is using variables inside variable names, but it's ugly in a different way.

BOARD_ALIAS_linux = native BOARD_ALIAS_phynode = pba-d-01-kw2x ifneq (,$(BOARD_ALIAS_$(BOARD))) override BOARD = $(BOARD_ALIAS_$(BOARD)) endif

jnohlgard · 2017-11-09T12:44:59Z

makefiles/boards/alias.inc.mk

+
+# this basically implements a look-up table, is there a nicer way in make?
+ifneq (,$(filter phynode,$(BOARD)))
+  BOARD := pba-d-01-kw2x


This needs override in order to be able to replace the BOARD setting if given on the command line: make BOARD=linux, instead of BOARD=linux make or export BOARD=linux

it worked without the override, but maybe that war more a coincidence then? Anyway, makes much sense and I will fix this.

jnohlgard · 2017-11-09T12:45:36Z

makefiles/boards/alias.inc.mk

@@ -0,0 +1,9 @@
+ALIAS = linux phynode


this is unused as far as I can see?

yes, leftover from some earlier playing around... Seems like i forgot to delete it.

jnohlgard · 2017-11-09T12:48:36Z

I like the idea of having easier to remember names for some of the boards, especially the pba-d-01-kw2x was one that I kept having trouble remembering at first..

aabadie

I like the idea.

I propose another solution for the lookup table, see the comment below. What do you think ?

aabadie · 2017-11-09T11:46:15Z

makefiles/boards/alias.inc.mk

@@ -0,0 +1,9 @@
+ALIAS = linux phynode


What is this line for ?

aabadie · 2017-11-09T12:47:07Z

makefiles/boards/alias.inc.mk

@@ -0,0 +1,9 @@
+ALIAS = linux phynode
+
+# this basically implements a look-up table, is there a nicer way in make?


Maybe this SO thread could be the start of a solution.

What about:

# pba-d-01-kw2x Makefile: BOARD_ALIAS += phynode # native Makefile: BOARD_ALIAS += linux raspberrypi ubuntu # in alias.inc.mk: alias.phynode := pba-d-01-kw2x alias.linux := native ifneq (,$(filter $(BOARD), $(BOARD_ALIAS))) BOARD := ${alias.${BOARD}} endif

Not sure if it works.

This won't work per se, as the contents of the board makefiles are not yet known when parsing the alias.inc.mk file. In fact, the board can only be started to be parsed after the alias has been resolved...

But think I like the idea using variable names, will provide another draft of this

aabadie · 2017-11-09T12:52:41Z

@gebart, you were faster :)

haukepetersen · 2017-11-09T13:15:38Z

how about this?

haukepetersen · 2017-11-09T13:16:53Z

wait, now the non-aliased targets don't work anymore... Will provide a fix.

haukepetersen · 2017-11-09T13:28:29Z

done. Now what do you think about this solution?

aabadie · 2017-11-09T13:30:21Z

makefiles/boards/alias.inc.mk

+#TODO remove this line (as its just for show)
+ALIAS.linux     = native
+
+# this temporary variable is needed as we can not use BOARD when assigning BOARD


I don't see how this can work. How about filling a list of existing alias on a per board basis and if the BOARD name is found in this list, use the alias trick ? (as I suggested in one of my previous comment)

for one: it does :-)
second: this basically checks the list of defined aliases above, and if it finds a fitting entry the board variable is substituted. I think makes calls this concept Computed Variable Names

I don't understand why you think this doesn't work?
All of the alias definitions are on the lines above.
If BOARD is set to linux, then ALIAS_TARGET will be assigned the value $(ALIAS.linux), which evaluates to native.

Oh ok, now I have it.

jnohlgard · 2017-11-09T13:44:19Z

makefiles/boards/alias.inc.mk

+
+# this temporary variable is needed as we can not use BOARD when assigning BOARD
+ALIAS_TARGET := $(ALIAS.$(BOARD))
+ifdef ALIAS_TARGET


Better to check for non-empty value: ifneq (,$(ALIAS_TARGET))

actually, this did for some reason not work. Not sure if I had a typo or if something else went wrong. Could you maybe quickly verify if the non-empty value check works for you?

jnohlgard · 2017-11-09T13:51:14Z

makefiles/boards/alias.inc.mk

+#TODO remove this line (as its just for show)
+ALIAS.linux     = native
+
+# this temporary variable is needed as we can not use BOARD when assigning BOARD


I don't understand why you think this doesn't work?
All of the alias definitions are on the lines above.
If BOARD is set to linux, then ALIAS_TARGET will be assigned the value $(ALIAS.linux), which evaluates to native.

haukepetersen · 2017-11-09T14:24:10Z

@gebart never mind, seems like a had some kind of typo in my file... Works now like a charm

aabadie · 2017-11-09T17:02:05Z

I tried to build one of the example applications and it doesn't build. Changing the BOARD variable with an alias has some unwanted effect on the build:

$  make BOARD=linux -C examples/hello-world/ all term
make: Entering directory '/home/aabadie/RIOT/examples/hello-world'
Building application "hello-world" for "native" with MCU "native".

"make" -C /home/aabadie/RIOT/boards/linux
make[2]: *** /home/aabadie/boards/linux: No such file or directory.  Stop.
/home/aabadie/RIOT/Makefile.base:20: recipe for target 'ALL--/home/aabadie/RIOT/boards/linux' failed
make[1]: *** [ALL--/home/aabadie/RIOT/boards/linux] Error 2
/home/aabadie/RIOT/examples/hello-world/../../Makefile.include:309: recipe for target 'link' failed
make: *** [link] Error 2
make: Leaving directory '/home/aabadie/RIOT/examples/hello-world'

haukepetersen · 2017-11-09T18:21:36Z

arg... I'd seem to never understand the different handling of env variables dependent on how one sets them. In this case, the override BOARD... line seems to have not effect.

It seems to me, that when calling make like you do, the given BOARD value preceeds the one exported by vars.inc.mk. But why, I can't say, seems like my understanding on the way the processes are created and given their environment is limited here. Anyone?

haukepetersen · 2017-11-09T18:30:09Z

Hm, as I understand it, the override should take care of making sure that our version of the BOARD var instead of the one passed from the command line is used in all spawned make calls. Strange...

cladmi · 2017-11-09T18:41:23Z

If I understood correctly, override only overrides values from environment variables, for command line it should be done with MAKEOVERRIDES.
That's why in Makefile.include both are done.

cladmi · 2017-11-09T18:56:53Z

makefiles/boards/alias.inc.mk

+ALIAS.linux     = native
+
+# this temporary variable is needed as we can not use BOARD when assigning BOARD
+ALIAS_TARGET := $(ALIAS.$(BOARD))


ALIAS_TARGET is just used in here but is a generic name, I would like to see it namespaced, and so use BOARD_ALIAS_TARGET, or just BOARD_ALIAS.

kaspar030 · 2017-11-09T20:13:07Z

makefiles/boards/alias.inc.mk

+# this temporary variable is needed as we can not use BOARD when assigning BOARD
+ALIAS_TARGET := $(ALIAS.$(BOARD))
+ifneq (,$(ALIAS_TARGET))
+  override BOARD = $(ALIAS_TARGET)


Doesn't this have to be BOARD := $(ALIAS_TARGET)?

In general yes, and also tried that, but does not solve the problem above.

haukepetersen · 2017-11-10T08:38:51Z

adding BOARD to MAKEFILEOVERRIDES does also not solve the issue at hand:

ifneq (,$(ALIAS_TARGET))
  override BOARD := $(ALIAS_TARGET)
  MAKEOVERRIDES += BOARD
endif

Am I misunderstanding the usage of this var?

cladmi · 2017-11-10T10:12:50Z

MAKEOVERRIDES += BOARD=$(BOARD) It works with it.

The reason why we need these two, is because we are using recursive makefiles and want to modify the values.

haukepetersen · 2017-11-10T10:25:50Z

@cladmi thanks, that was it!

@aabadie: would you mind to try again? Should work now...

aabadie · 2017-11-10T10:30:00Z

Just tested again: works now.

Some side comments:

should we integrate this in the CI and if yes, how ?
it's a bit weird to define BOARD=linux and have message saying building for native etc
maybe the available aliases could be documented in the respective boards documentation ?

haukepetersen · 2017-11-13T10:49:07Z

but maybe from a different angle.

So meaning that you have an alternate/better approach to this in mind? And should be block this PR and evaluate your solution first? Or merge this PR and migrate to your solution if agreed upon?

smlng · 2017-11-13T10:56:27Z

So meaning that you have an alternate/better approach to this in mind?

not exactly, but my idea originated from that fact the we have lots of boards that have (nearly) identical configs only mainly differ by their name. The problem until now was that our build system requires a boards/<name> directory per board to build RIOT.

But see for instance arduino-uno, arduino-duemilanove and arduino-nano ( #7540 ), which are basically the same, my idea was to allow several aliases per actual board implementation and allow to adapt minimal board-specific config via #ifdef, for instance the arduino-uno has 6 ADC while the nano has 8 - such mini diffs do not require a fully copy of the directory.

However, to allow for that it would require to pass the board-name plus the name of the underlying board to the build system, the former to allow for usage of #ifdef (if needed) and the latter to find the correct subdirectory.

Besides the Arduinos this also applies for many Nucleos which share lots of stuff, in the end this would allow to reduce number of subdirectories in boards, ease changes to common config and thereby omit errors which result from bad copy-pasta.

Hope you get what I mean.

haukepetersen · 2017-11-13T11:48:14Z

I do, and I already proposed a solution to that :-) -> see #7975

smlng · 2017-11-13T12:17:11Z

I do, and I already proposed a solution to that

oops, I need to catch up with recent PRs - but great!

haukepetersen · 2017-11-13T12:22:49Z

No problem. Its work in progress with some open questions, so maybe you can add your ideas :-)

temmihoo · 2017-11-22T14:32:29Z

For the non-technical side, of what aliases to provide. I would like that you could find the boards by vendor, by the board name however obscure that is, by the default connectivity, by the MCU family and so on and so on.

Of the technical solution on how to make the aliases happen, I propose that instead of a .mk macro lookup, the aliases would be just symlinks. That way you could refer to them by their name starting from boards/ like so:
BOARD="nucleo64-f401"
BOARD="stm32/nucleo64-f401"
and so on and so on.

In this I'm not really proposing any actual hierarchy layout but just another technical solution on how the aliases are made to work. First I'd just like to see a few non-hirarchial simple alias names available.

I think it would be tremendously helpful if board names and their aliases showed up using the same tool ie. ls for us cli geeks and various file managers for gui folks instead of having to know there is a .mk macro file somewhere not in boards/ but under the makefiles/ tree.

If any of RIOT tools actually traverses the boards/* files by file system traversal, my approach needs making that tool aware of symlinks, which by easiest is to skip all symlinks alltogether.

cladmi · 2018-01-12T15:34:24Z

Actually, I found the "phynode -> pba-d-01-kw2x" nicer to read then "pba-d-01-kw2x (phynode)", but this is very subjective I guess. Don't have a strong opinion here.

This would make default example break as the alias would not be listed in BOARD_PROVIDES_NETIF.

haukepetersen · 2018-01-23T11:19:27Z

This would make default example break as the alias would not be listed in BOARD_PROVIDES_NETIF.

Yapp, and this is because the default example is broken :-) Having all those 'provides netif' thingies was meant to be a quick-fix once, and it seemed to have survived quite a bit... In an ideal case, the Makefile in the default example should do something like if FEATURES_PROVIDED contains netif, then. But this does of course not work in the application makefile.

As a (quick) fix: could we simply include makefiles/boards/alias.inc.mk right after BOARD ?= xx in the application Makefile for all the applications that actually do something with the board name in their application makefile? This would also remove the resetriction on test apps only being buildable with the original board name. Any thoughts on this?

cladmi · 2018-01-23T14:02:11Z

I would agree for the quick fix with a comment.

A real fix should not use 'if' but declare a configuration that will be evaluated later. The problem of the current state and the 'if' is that it is not deferred but evaluated immediately.

These kind of configurations will need to be taken into account when updating the build system.
Optional dependencies and CFLAGS triggered by an available feature.

haukepetersen · 2018-03-02T09:04:04Z

included the (quick) fix for using aliases in example applications that use the $(BOARD) variable in their makefiles.

haukepetersen · 2018-06-07T14:04:50Z

rebased.

@cladmi @kaspar030 @aabadie @gebart Is this PR acceptable in the current form? I don't really want to spend time on this (as the Makesystem is not my primary concern...), so either we take it or I will close it and somebody else will need to continue this thought. What do you think?

haukepetersen · 2018-09-11T10:13:04Z

no progress here and I will not push this any further -> closing it as memo.

chrysn · 2022-09-14T08:09:52Z

This looks good to me at a first glance, with many (if not all) concerns addressed. @haukepetersen, can I revive this PR (and keep pushing those you listed in #7976 (comment) to clear their concerns), or would you rather have this handled in a new PR where I'd essentially squash-and-resubmit your commits?

haukepetersen assigned cladmi, jnohlgard, miri64, aabadie and kaspar030 Nov 9, 2017

jnohlgard requested changes Nov 9, 2017

View reviewed changes

aabadie reviewed Nov 9, 2017

View reviewed changes

jnohlgard reviewed Nov 9, 2017

View reviewed changes

cladmi reviewed Nov 9, 2017

View reviewed changes

kaspar030 requested changes Nov 9, 2017

View reviewed changes

haukepetersen mentioned this pull request Nov 10, 2017

boards: concept for defining board revisions #7975

Closed

smlng mentioned this pull request Nov 17, 2017

boards: move shared code to boards/common/xx #8058

Merged

bergzand mentioned this pull request Nov 22, 2017

boards: nucleo boards not having their marketing names #8096

Closed

haukepetersen force-pushed the eval_board_alias branch from a4e81ca to 4fd11b5 Compare March 2, 2018 09:03

bergzand mentioned this pull request Apr 16, 2018

boards: exact same configuration but different board names? #4673

Closed

haukepetersen added 8 commits June 7, 2018 15:58

make: allow alias for board names

85eaa15

fixup! another, cleaner, nicer try

fc1ad15

fixup! now working with org and alias names

5903a0c

fixup! used non-empty-check

7483e48

fixup! now overriding make vars should also work

de87baf

fixup! used BOARD:=x

6b437f6

RFC: adapt build message to use of alias

83dbde9

examples: allow board alias when using $(BOARD)

1f3b9da

haukepetersen force-pushed the eval_board_alias branch from 4fd11b5 to 1f3b9da Compare June 7, 2018 14:02

haukepetersen added the State: archived State: The PR has been archived for possible future re-adaptation label Sep 11, 2018

haukepetersen closed this Sep 11, 2018

aabadie mentioned this pull request Oct 20, 2018

Make support for the stm32f429i-disco more obvious #10116

Closed

aabadie mentioned this pull request Sep 13, 2022

RDM: Naming boards #18589

Merged

		@@ -0,0 +1,9 @@
		ALIAS = linux phynode

		# this basically implements a look-up table, is there a nicer way in make?

make: allow alias for board names #7976

make: allow alias for board names #7976

Conversation

haukepetersen commented Nov 9, 2017

jnohlgard left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jnohlgard commented Nov 9, 2017

aabadie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aabadie commented Nov 9, 2017

haukepetersen commented Nov 9, 2017

haukepetersen commented Nov 9, 2017

haukepetersen commented Nov 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

haukepetersen commented Nov 9, 2017

aabadie commented Nov 9, 2017 • edited Loading

haukepetersen commented Nov 9, 2017

haukepetersen commented Nov 9, 2017

cladmi commented Nov 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

haukepetersen commented Nov 10, 2017

cladmi commented Nov 10, 2017

haukepetersen commented Nov 10, 2017

aabadie commented Nov 10, 2017

haukepetersen commented Nov 13, 2017

smlng commented Nov 13, 2017

haukepetersen commented Nov 13, 2017

smlng commented Nov 13, 2017

haukepetersen commented Nov 13, 2017

temmihoo commented Nov 22, 2017

cladmi commented Jan 12, 2018

haukepetersen commented Jan 23, 2018

cladmi commented Jan 23, 2018

haukepetersen commented Mar 2, 2018

haukepetersen commented Jun 7, 2018

haukepetersen commented Sep 11, 2018

chrysn commented Sep 14, 2022

aabadie commented Nov 9, 2017 •

edited

Loading