From c48641410b65baf242122567c206e9400634fa3f Mon Sep 17 00:00:00 2001 From: BleskoDev Date: Sun, 29 May 2016 17:23:09 +0200 Subject: [PATCH 1/2] Fixes issue #73 'qibuild make used with -J does not have correct exit code' --- python/qibuild/parallel_builder.py | 44 ++++++++++++++----- .../with_compile_error/CMakeLists.txt | 11 +++++ .../test/projects/with_compile_error/main.cpp | 10 +++++ .../projects/with_compile_error/qiproject.xml | 1 + python/qibuild/test/test_parallel_builder.py | 14 ++++++ 5 files changed, 70 insertions(+), 10 deletions(-) create mode 100644 python/qibuild/test/projects/with_compile_error/CMakeLists.txt create mode 100644 python/qibuild/test/projects/with_compile_error/main.cpp create mode 100644 python/qibuild/test/projects/with_compile_error/qiproject.xml diff --git a/python/qibuild/parallel_builder.py b/python/qibuild/parallel_builder.py index 93939d7a1..e3fe4a07b 100644 --- a/python/qibuild/parallel_builder.py +++ b/python/qibuild/parallel_builder.py @@ -108,13 +108,21 @@ def build(self, *args, **kwargs): self.pending_jobs.remove(job) # check if any worker failed - for worker in self._workers: - if not worker.result.ok: - all_ok = False - self.failed_project = worker.result.failed_project - break + all_ok = not self._has_any_worker_failed() # end (all done or error) + # Wait for the last job to finish processing if needed. + # running_jobs.empty() will return true when the last job + # is 'get' from the queue, so the loop will end too early + # to get the status of the last job. + + # all jobs except the last were ok? + if all_ok: + # wait for the end of the last job + self.running_jobs.join() + # check the last job's result + all_ok = not self._has_any_worker_failed() + # say to all workers to stop for worker in self._workers: worker.stop() @@ -151,6 +159,14 @@ def _find_job_by_name(self, name): return None + def _has_any_worker_failed(self): + for worker in self._workers: + with worker.result_lock: + if not worker.result.ok: + self.failed_project = worker.result.failed_project + return True + + return False class BuildResult(object): @@ -169,6 +185,7 @@ def __init__(self, queue, worker_index, *args, **kwargs): self.kwargs = kwargs self._should_stop = False self.result = BuildResult() + self.result_lock = threading.Lock() # should be locked when accessing self.result! def stop(self): """ Tell the worker it should stop trying to read items from the queue """ @@ -187,9 +204,16 @@ def run(self): try: ui.info(ui.green, "Worker #%i starts working on " % (self.index + 1), ui.reset, ui.bold, job.project.name) job.execute(*self.args, **self.kwargs) - self.queue.task_done() + except qibuild.build.BuildFailed as failed_build: + # not an exceptional condition -> no need to display backtrace + with self.result_lock: + self.result.ok = False + self.result.failed_project = failed_build.project except Exception, e: - self.result.ok = False - self.result.failed_project = job.project - ui.error(ui.red, - *ui.message_for_exception(e, "Python exception during build")) + with self.result_lock: + self.result.ok = False + self.result.failed_project = job.project + ui.error(ui.red, + *ui.message_for_exception(e, "Python exception during build")) + + self.queue.task_done() diff --git a/python/qibuild/test/projects/with_compile_error/CMakeLists.txt b/python/qibuild/test/projects/with_compile_error/CMakeLists.txt new file mode 100644 index 000000000..2521de82a --- /dev/null +++ b/python/qibuild/test/projects/with_compile_error/CMakeLists.txt @@ -0,0 +1,11 @@ +## Copyright (c) 2012-2016 Aldebaran Robotics. All rights reserved. +## Use of this source code is governed by a BSD-style license that can be +## found in the COPYING file. +cmake_minimum_required(VERSION 2.8) +project(with_compile_error) + +find_package(qibuild) +qi_sanitize_compile_flags() + +qi_create_bin(foo "main.cpp") + diff --git a/python/qibuild/test/projects/with_compile_error/main.cpp b/python/qibuild/test/projects/with_compile_error/main.cpp new file mode 100644 index 000000000..606b532b0 --- /dev/null +++ b/python/qibuild/test/projects/with_compile_error/main.cpp @@ -0,0 +1,10 @@ +/* + * Copyright (c) 2012-2016 Aldebaran Robotics. All rights reserved. + * Use of this source code is governed by a BSD-style license that can be + * found in the COPYING file. + */ + +int main() +{ + return toto; +} diff --git a/python/qibuild/test/projects/with_compile_error/qiproject.xml b/python/qibuild/test/projects/with_compile_error/qiproject.xml new file mode 100644 index 000000000..64239caef --- /dev/null +++ b/python/qibuild/test/projects/with_compile_error/qiproject.xml @@ -0,0 +1 @@ + diff --git a/python/qibuild/test/test_parallel_builder.py b/python/qibuild/test/test_parallel_builder.py index 26239f66d..db70cc9f6 100644 --- a/python/qibuild/test/test_parallel_builder.py +++ b/python/qibuild/test/test_parallel_builder.py @@ -1,4 +1,7 @@ import qibuild.parallel_builder +import qibuild.build + +import pytest class FakeProject(object): build_log = list() @@ -29,3 +32,14 @@ def test_simple(): build_log = FakeProject.build_log assert is_before(build_log, "a", "c") assert is_before(build_log, "b", "c") + +def test_running_build_with_compilation_errors_fails(qibuild_action): + # Running `qibuild make -J1` on a c++ project with compilation + # errors should fail + + qibuild_action.add_test_project("with_compile_error") + qibuild_action("configure", "with_compile_error") + + # pylint: disable-msg=E1101 + with pytest.raises(qibuild.build.BuildFailed): + qibuild_action("make", "-J1", "with_compile_error") From b0696b8c75eb7e5c65c6247c1c3f1b82d1ad5c7a Mon Sep 17 00:00:00 2001 From: BleskoDev Date: Mon, 30 May 2016 22:06:21 +0200 Subject: [PATCH 2/2] Simplify BuildResult in parallel builder to remove the need for job result locking. --- python/qibuild/parallel_builder.py | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/python/qibuild/parallel_builder.py b/python/qibuild/parallel_builder.py index e3fe4a07b..a9bb8dc86 100644 --- a/python/qibuild/parallel_builder.py +++ b/python/qibuild/parallel_builder.py @@ -161,17 +161,15 @@ def _find_job_by_name(self, name): def _has_any_worker_failed(self): for worker in self._workers: - with worker.result_lock: - if not worker.result.ok: - self.failed_project = worker.result.failed_project - return True + if worker.result.failed_project: + self.failed_project = worker.result.failed_project + return True return False class BuildResult(object): def __init__(self): - self.ok = True self.failed_project = None @@ -185,14 +183,13 @@ def __init__(self, queue, worker_index, *args, **kwargs): self.kwargs = kwargs self._should_stop = False self.result = BuildResult() - self.result_lock = threading.Lock() # should be locked when accessing self.result! def stop(self): """ Tell the worker it should stop trying to read items from the queue """ self._should_stop = True def run(self): - while not self._should_stop and self.result.ok: + while not self._should_stop and not self.result.failed_project: job = None try: job = self.queue.get(True, 1); @@ -206,14 +203,10 @@ def run(self): job.execute(*self.args, **self.kwargs) except qibuild.build.BuildFailed as failed_build: # not an exceptional condition -> no need to display backtrace - with self.result_lock: - self.result.ok = False - self.result.failed_project = failed_build.project + self.result.failed_project = failed_build.project except Exception, e: - with self.result_lock: - self.result.ok = False - self.result.failed_project = job.project - ui.error(ui.red, - *ui.message_for_exception(e, "Python exception during build")) + self.result.failed_project = job.project + ui.error(ui.red, + *ui.message_for_exception(e, "Python exception during build")) self.queue.task_done()