Makefile.include: flash: do not peek into MAKECMDGOALS.

[jcarrano]

Contribution description

When flash-only was introduced (in #8373), the flash rule was made conditionally dependent on all by looking for flash-only in MAKECMDGOALS. This was done to avoid code duplication.

There's a cleaner way, by using canned recipes.

Testing procedure

Go to any example and type make flash-only and make flash. You don't even need to have the board connected.

In the first case it should not attempt to make all and in the second it should.

Issues/PRs references

Refactor of #8373.

[cladmi]

I globally agree with the change. When introduced there were not many usages of define or usage of canned recipe so was not going in this direction as it would have asked to repeat the check and the FLASHER lines.

My only remark, is that I would prefer to use an active verb.

The check_cmd does check the command exists.
In the documentation they use run-yacc or frobnicate, where here flash-commands it is more a descriptive name or its content than of what it does.

[jcarrano]

What about "do-flash"?

[cladmi]

Sent too early…

So maybe run-flash, or flash-firmware, or if you have any idea.

The build will also need to be re-triggered before merging as it was not done with CI: run tests but it can wait until updated.

[cladmi]

Also, some infos I found about the define auie = that could fail with make 3.81 which is the version from osx if using automatic variables:

https://stackoverflow.com/questions/5032935/why-gnu-make-canned-recipe-doesnt-work
https://stackoverflow.com/questions/5033181/gnu-make-differences-in-multiline-variable-declarations

Here there should not be any issue for the moment as it is not using the name but maybe it is better to not put the = as it would be copied to other cases ?

[cladmi]

Which also means, we need an OSX test of the current version at least.

[jcarrano]

That sucks! It means we cannot ?= a canned recipe. OSX sucks so bad it spreads its suckyness everywhere.

For reference, OSX's version of make is 12 years old.

[cladmi]

It was also in debian wheezy, but its LTS support has been ended since May 2018 https://www.debian.org/releases/wheezy/

[cladmi]

Shown irl, I found this line by checking something else:

RIOT/boards/pba-d-01-kw2x/Makefile.include

Line 14 in b6abbad

flash: $(RIOTCPU)/$(CPU)/dist/wdog-disable.bin

There might be others and they must be updated to FLASHDEPS then.

[cladmi]

For upcoming steps, as you are also working on cleaning it.
This flash-recipe can also be used in mcuboot.mk and in riotboot PR also.

[cladmi]

I tested that the wdog-disable.o wdog-disable.bin is indeed created when doing make flash and also when doing make all; make flash-only for BOARD=mulle.

For me you can squash. Please put the kinetis commit first as it is required to have a working history.

I would like a test on mac before merging this too @smlng

[jcarrano]

@smlng can you take look at this? or someone else with a fruit computer?

[smlng]

tested on macOS with pba-d-01-kw2x aka PhyNode, does work with flash and flash-only targets as expected.

[jcarrano]

@smlng Thanks!

[smlng]

I give the ACK, but would prefer to have @cladmi have the final word and honour to hit the green button.

[cladmi]

Just a last nitpick, can you please put in the commit description canned recipe definition that we do no use define name = or even define name ?= because of make 3.81 limitations. It would simplify understanding for future changes. You can directly amend the commit.

Note for future PRs, this can also be propagated later to mcuboot and riotboot but will need adaptation on their side so should come in followup PRs.

[cladmi]

BTW github still shows the kinetis commit after the other one, but the order is correct when looked locally.

[jcarrano]

I updated the commit message and added a comment too.

[cladmi]

ACK. Changes reviewed and also work with mac.
Commit messages and order has been reviewed.

I locally re-tested this branch merged with riot/master.

I used flash and flash-only for iotlab-m3.
And also checked the wdog-disable.bin file was created when doing flash and flash-only for mulle, pba-d-01-kw2x and with USE_OLD_OPENOCD=1 BOARD=frdm-k22f. I cleaned the file between each command to verify it was created for both flash and flash-only.