espruino/Espruino

provision.sh will throw an error for board files which contain dashes

Closed this issue · 6 comments

Hi, I am not sure how relevant this is, but when using the recommend way of building Espruino (https://github.com/espruino/Espruino/blob/master/README_Building.md) the provision.sh bash script will throw an error for board files which contain dashes. e.g. for a (custom) board file named B5-SDK12.py:

me@mypc:~/code/Espruino$ source scripts/provision.sh B5-SDK12
bash: export: "PROVISION_B5-SDK12=1": not a valid identifier
Provision BOARDNAME = B5-SDK12
Provision FAMILY = NRF52
===== NRF52
===== ARM
arm-none-eabi-gcc installed
me@mypc:~/code/Espruino$ 

Expected result: no bash export error.
To reproduce: create a board file containing a dash e.g. B5-SDK12 and run provision.sh. Above error should occur.
Workaround: simply removing the dash from the filename did the trick for me.

I can't tell if this error actually breaks something, or what the convention for these files is but wanted to mention it in case it is useful information. Maybe it could help to simply mention it on https://github.com/espruino/Espruino/blob/master/README_Building.md and /or https://www.espruino.com/Building+Custom+Firmware .

Thanks - yes, you might actually hit other issues with dashes in board names too. I think it tries to define a preprocessor variable BOARDNAME=1 and BOARD-NAME=1` wouldn't be valid anyway.

I've just added a note to https://www.espruino.com/Building+Custom+Firmware - I couldn't see anywhere obvious to mention it in README_Building.md but if you can see anywhere, a PR would be great.

Dashes work for me as long as you override boardname like https://github.com/espruino/Espruino/blob/master/boards/PUCKJS_MINIMAL.py#L23 then the board filename does not matter,
provision.sh may be actually one of last remaining pieces to no check for this. Fixing this may be useful even for boards like LINUX or RASPBERRY that are hardcoded by name in provision.sh and you could still have multiple board files with different configurations.

Like this line https://github.com/espruino/Espruino/blob/master/scripts/provision.sh#L46 the script should read also 'board.info["boardname"]' and override BOARDNAME variable with it if present. Then it would work. I'll try it and send PR.

the script should read also 'board.info["boardname"]' and override BOARDNAME variable with it if present. Then it would work.

If you do a PR, please can you make sure you document this? Otherwise it's basically useless. @arglurgl would still have hit the issue.

But IMO this really isn't a big deal. It's not going to kill someone modifying Espruino to just use an underscore instead.

Just spotted you mentioned overriding BOARDNAME in:

FAMILY=scripts/get_board_info.py $BOARDNAME 'board.chip["family"]'

IMO it totally shouldn't do that. What if you want to do a PUCKJS board build, but for a different processor family?

I mean that's the place where reading board.info["boardname"] in a similar way and overriding BOARDNAME value is missing. That FAMILY line should stay there as it is. something like

  FAMILY=`scripts/get_board_info.py $BOARDNAME 'board.chip["family"]'`
  if [ "$FAMILY" = "" ]; then
    echo "UNKNOWN BOARD ($BOARDNAME)"
    return 1
  fi
  BOARDOVERRIDE=`scripts/get_board_info.py $BOARDNAME 'board.info["boardname"]'`
  if [ "$BOARDOVERRIDE" != "" ]; then
    BOARDNAME=$BOARDOVERRIDE
  fi

But IMO this really isn't a big deal. It's not going to kill someone modifying Espruino to just use an underscore instead.

It is not about dash or underscore, it it about determining the board name correctly. Now if you would copy boards/RASPBERRY.py to e.g. boards/RASPBERRY_MINIMAL.py to have some other build settings, the provision.sh would not work correctly on that.

I think this board override is already documented and if it would work in provision.sh too (as it works in other place already) @arglurgl would not hit this issue. Well at least with that B5-SDK12.py he got from my repo.