Skip to content

Makefile.include: allow overwriting flash-recipe #11433

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 28, 2019

Conversation

cladmi
Copy link
Contributor

@cladmi cladmi commented Apr 23, 2019

Contribution description

This allows changing the flashing commands from the outside or in a BSP.

Implementation

It cannot be set as define flash-recipe ?= as the feature is only introduced only in gnumake 4.
Also this allows re-using default-flash-recipe if wanted.

Testing procedure

Flashing still works for a board that flashes. (CI will test this)

It can be overwritten:

make --no-print-directory -C examples/hello-world/ flash-only flash-recipe="echo changed recipe"
echo changed recipe
changed recipe

Issues/PRs references

Could be used for #11093 without introducing a one flasher feature for the moment.

I need to overwrite flash-recipe to handle flashing on another machine in my setup and currently need to rely on override for setting the variable.

This would allow refactoring the esp flashing command to use commands on different lines
https://github.com/RIOT-OS/RIOT/blob/master/cpu/esp32/Makefile.include

@cladmi cladmi added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation 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 CI: run tests If set, CI server will run tests on hardware for the labeled PR CI: disable test cache If set, CI will always run all tests regardless of whether they have been run successfully before labels Apr 23, 2019
@cladmi cladmi added this to the Release 2019.07 milestone Apr 23, 2019
@cladmi cladmi requested a review from jcarrano April 23, 2019 13:12
@cladmi
Copy link
Contributor Author

cladmi commented May 10, 2019

Updated I was missing the $( ) ><

@cladmi
Copy link
Contributor Author

cladmi commented May 13, 2019

@jcarrano if you want to have a look, I fixed the missing evaluation.

@jcarrano jcarrano removed CI: disable test cache If set, CI will always run all tests regardless of whether they have been run successfully before CI: run tests If set, CI server will run tests on hardware for the labeled PR labels May 13, 2019
Copy link
Contributor

@jcarrano jcarrano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Squash and we go.

@jcarrano
Copy link
Contributor

I removed "Disable cache" and "Run tests" since it has been already tested, and there's no point in wasting CI resources in a squash.

This allows changing the flashing commands from the outside or in a BSP.
@cladmi cladmi force-pushed the pr/make/flash_recipe/allow_overriding branch from 8b4071e to 13de591 Compare May 13, 2019 17:42
@cladmi
Copy link
Contributor Author

cladmi commented May 13, 2019

Squashed.

@cladmi
Copy link
Contributor Author

cladmi commented May 28, 2019

This one was pending and no changes have been done around that variable since then.

@cladmi cladmi merged commit 545a5f7 into RIOT-OS:master May 28, 2019
@cladmi cladmi deleted the pr/make/flash_recipe/allow_overriding branch May 28, 2019 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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 Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants