Skip to content
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

murdock: fail on broken application Makefile #7742

Merged
merged 1 commit into from
Oct 18, 2017

Conversation

kaspar030
Copy link
Contributor

A broken application Makefile would error when doing make -Capp_dir info-boards-supported, but due to the way Murdock called make, that error would just result in empty output.

This PR adds the necessary handling.

(includes a test commit to see if this actually works)

@kaspar030 kaspar030 added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: CI Area: Continuous Integration of RIOT components CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 17, 2017
@kaspar030 kaspar030 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 17, 2017
@kaspar030
Copy link
Contributor Author

Fixes #7741.

@kaspar030
Copy link
Contributor Author

Failed correctly: https://ci.riot-os.org/RIOT-OS/RIOT/7742/b87283bf07164cdbf7ab2788e4af6e6d462f0058/output.html

I'll remove the test commit.

@kaspar030 kaspar030 removed the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Oct 17, 2017
local boards="$(make --no-print-directory -C$appdir info-boards-supported 2>/dev/null || echo broken)"

if [ "$boards" = broken ]; then
echo "makefile_broken"
Copy link
Member

Choose a reason for hiding this comment

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

Why can't you put out the actual error message? E.g. by piping it to a temporary file or file descriptor instead of /dev/null above?

Copy link
Member

Choose a reason for hiding this comment

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

Ah I see that the output should go below, but regardless I think it is helpful to print the actual error message.

@miri64 miri64 added the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Oct 18, 2017
@miri64 miri64 self-assigned this Oct 18, 2017
Copy link
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

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

@kaspar030 and I discussed the proposed change offline and my change request does not seem to be that trivial. So let's merge it for now as is and review this at a later time.

@kaspar030 please provide a backport to the 2017.10 branch.

@miri64 miri64 merged commit 8934015 into RIOT-OS:master Oct 18, 2017
@kaspar030 kaspar030 removed the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Oct 18, 2017
@kaspar030
Copy link
Contributor Author

As discussed offline with the release manager, no backport needed.

@kaspar030 kaspar030 deleted the fix_murdock_get_jobs branch October 18, 2017 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: CI Area: Continuous Integration of RIOT components CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants