makefiles/scan-build: separate scan-build-analyze and make it produce an error #10391
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is required to have reference builds context using the right RIOT_VERSION_OVERRIDE=buildtest.
Update the SCANBUILD_OUTPUTDIR to be per board. Also remove un-necessary export and 'CURDIR'.
This correctly defines a `scan-build-analyze` target that does not display the result webpage. `scan-build-view` has now been moved to a private target as should not be used directly. The handling of displaying the page on the host system and implementing 'scan-build-analyze' are now explicitely done with separate targets and not double implemented target when in docker or on the host that were executed twice with different implementations.
This makes doing 'scan-build-analyze' produce an error at execution if WERROR=1. When used from `scan-build` it will not procude an error to display the result webpage.
cladmi
added
Type: enhancement
miri64
reviewed
Nov 14, 2018
cladmi
added
the
CI: ready for build
jcarrano
approved these changes
Nov 21, 2018
cladmi
added
CI: ready for build
|
Re-triggering murdock, got lost in a reboot I think. |
miri64
added
CI: ready for build
|
Thank for the review. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Contribution description
This refactors the scan-build integration to have a separate
scan-build-analyzethat does not displays the webpage and make it produce an error if it find warnings whenWERROR=1It would help using it in automated testing.
There are also other refactoring inlined that I can split on request or this PR gets stuck too long:
Testing procedure
I am using
gnrc_networkingexample that, when writing the PR, still has warnings, but may change before getting merged.scan-build-analyzeWhen executing
scan-build-analyzeno webpage is opened at the end with this PR.It was opened in
masteras it was just basically a internal implementation function.Executing the following commands fails as warnings are found
Also same behavior with
dockerNo error is produced when using
WERROR=0In master it did not return an error and always opened the webpage.
scan-builddid not changeThis executes
scan-buildand opens a webpage with the result. Even when usingdockerfor both this PR andmasterNote: there is a "Showing most recent report in your web browser... and xdg-open" that shows it is opening the result, and it should open in your browser
Can also be done with docker
Issues/PRs references
Was found while trying to run
scan-buildduring the release RIOT-OS/Release-Specs#81 (comment)This is also required to use
scan-build-analyzewith RIOT-OS/Release-Specs#87 as compilation command. Or just any automated testing that would rely on it. (CI nightly?).