Skip to content

Makefile.include: refactor command present check#8839

Merged
kaspar030 merged 1 commit intoRIOT-OS:masterfrom
cladmi:pr/make/command_check
Mar 28, 2018
Merged

Makefile.include: refactor command present check#8839
kaspar030 merged 1 commit intoRIOT-OS:masterfrom
cladmi:pr/make/command_check

Conversation

@cladmi
Copy link
Contributor

@cladmi cladmi commented Mar 27, 2018

Contribution description

Refactor the code checking if command is present.
Message is taken from the compiler check as it was more precise than the others.

Testing

By overwriting the checked variables I correctly get the error

make -C examples/hello-world/ CC=hello-maintainer
make: Entering directory '/home/cladmi/git/work/RIOT/examples/hello-world'
Compiler hello-maintainer is required but not found in PATH.  Aborting.
make: *** [/home/cladmi/git/work/RIOT/examples/hello-world/../../Makefile.include:350: ..compiler-check] Error 1
make: Leaving directory '/home/cladmi/git/work/RIOT/examples/hello-world'

make -C examples/hello-world/ flash-only FLASHER=pleasemergeme
make: Entering directory '/home/cladmi/git/work/RIOT/examples/hello-world'
Flash program pleasemergeme is required but not found in PATH.  Aborting.
make: *** [/home/cladmi/git/work/RIOT/examples/hello-world/../../Makefile.include:409: flash] Error 1
make: Leaving directory '/home/cladmi/git/work/RIOT/examples/hello-world'

Issues/PRs references

Is part of #8838 PR

Refactor the code checking if command is present.
Message is taken from the compiler check as it was more precise than the others.
@cladmi cladmi added Area: build system Area: Build system Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 27, 2018
@cladmi cladmi requested a review from aabadie March 27, 2018 14:01
endif
endif # BUILD_IN_DOCKER

# Check given command is available in the path
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to move it in a separate .mk file in makefiles directory so it can be easily imported by other makefiles ? I think it could also be usefull for #8573.

Copy link
Contributor

Choose a reason for hiding this comment

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

Folo!

@aabadie
Copy link
Contributor

aabadie commented Mar 27, 2018

Very useful PR. Thanks @cladmi

Copy link
Contributor

@kaspar030 kaspar030 left a comment

Choose a reason for hiding this comment

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

Nice one! And looong overdue! ;) ACK.

@kaspar030 kaspar030 merged commit 217c6a7 into RIOT-OS:master Mar 28, 2018
@cladmi cladmi deleted the pr/make/command_check branch March 28, 2018 12:40
@cladmi cladmi added this to the Release 2018.04 milestone Nov 7, 2018
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 Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants