Skip to content

Commit 92e042a

Browse files
committed
[BH-2065] Fix race condition between CMake targets
* Fix for the issue that resulted in race condition between target creating and populating databases and the one copying created databases to required directories. The issue was caused by lack of dependency between those targets, what allowed CMake engine to parallelize them. As a result, databases were copied before their creation has been finished, what resulted in weird discrepancies in databases content and occasional rsync errors. * Removed unused multicomp_install function. * Refactored DB tests readContent helper.
1 parent 80da585 commit 92e042a

File tree

9 files changed

+21
-45
lines changed

9 files changed

+21
-45
lines changed

CMakeLists.txt

+1-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ endif()
5454

5555
if (${ENABLE_TESTS})
5656
enable_testing()
57-
add_custom_target(check ${CMAKE_CTEST_COMMAND} -V DEPENDS create_test_product_databases)
57+
add_custom_target(check ${CMAKE_CTEST_COMMAND} -V)
5858
add_subdirectory(test)
5959
include(PureCoverage)
6060
endif ()

cmake/modules/Assets.cmake

+2-2
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ function(add_assets_target)
2828
${_ASSETS_SYSTEM_DEST_DIR}
2929

3030
# Create 'golden copy' of DBs
31-
COMMAND mkdir -p ${_ASSETS_SYSTEM_DEST_DIR}/db/factory
32-
COMMAND rsync -qlptgoDu
31+
# 'v' flag intentionally left for debugging purposes, can be removed if you're sure it's no longer needed
32+
COMMAND rsync -vlptgoDu
3333
${_ASSETS_SYSTEM_DEST_DIR}/db/*
3434
${_ASSETS_SYSTEM_DEST_DIR}/db/factory
3535

cmake/modules/DownloadAsset.cmake

-2
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ function(download_asset_release asset_name_in asset_name_out asset_repo asset_ve
1717
)
1818

1919
add_custom_target(${asset_name_out}-target DEPENDS ${asset_repo})
20-
21-
# multicomp_install(PROGRAMS ${CMAKE_BINARY_DIR}/${asset_repo} DESTINATION "./" COMPONENTS Standalone Update)
2220
endfunction()
2321

2422
function(download_asset_release_json

cmake/modules/Utils.cmake

-14
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,3 @@
1-
# An equivalent of install() which allows to declare multiple components using
2-
# a custom 'COMPONENTS' clause. This clause must be the last on the
3-
# argument list. The original 'COMPONENT' from install() clause must not appear
4-
# on the argument list.
5-
function(multicomp_install)
6-
list(FIND ARGN "COMPONENTS" CLAUSE_INDEX)
7-
list(SUBLIST ARGN 0 ${CLAUSE_INDEX} INSTALL_ARGN)
8-
math(EXPR COMPS_INDEX "${CLAUSE_INDEX}+1")
9-
list(SUBLIST ARGN ${COMPS_INDEX} ${ARGC} COMPONENTS)
10-
foreach(COMP IN LISTS COMPONENTS)
11-
install(${INSTALL_ARGN} COMPONENT ${COMP})
12-
endforeach()
13-
endfunction()
14-
151
macro(print_var VARIABLE)
162
if(${VARIABLE})
173
message(STATUS "${Green}${VARIABLE}: '${Orange}${${VARIABLE}}${ColourReset}'")

image/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ set(SYSTEM_DEST_DIR ${SYSROOT_PATH}/system_a)
55
set(USER_DEST_DIR ${SYSROOT_PATH}/user)
66

77
set(ASSETS_DEPENDENCIES "json-common-target")
8+
list(APPEND ASSETS_DEPENDENCIES "create_product_databases")
89

910
if (${ASSETS_TYPE} STREQUAL "Proprietary")
1011
list(APPEND ASSETS_DEPENDENCIES "json-proprietary-target")

module-db/tests/Helpers.cpp

+16-21
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) 2017-2023, Mudita Sp. z.o.o. All rights reserved.
1+
// Copyright (c) 2017-2024, Mudita Sp. z.o.o. All rights reserved.
22
// For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md
33

44
#include "Helpers.hpp"
@@ -8,39 +8,34 @@
88
#include <set>
99
#include <algorithm>
1010
#include <utility>
11+
#include <fstream>
1112

1213
namespace
1314
{
14-
std::string readContent(const std::string &filename) noexcept
15+
std::string readContent(const std::string &filename)
1516
{
16-
auto getFileSize = [](FILE *fd) -> std::size_t {
17-
std::fseek(fd, 0, SEEK_END);
18-
const auto size = std::ftell(fd);
19-
std::rewind(fd);
20-
return size;
21-
};
22-
23-
std::vector<char> fileContent;
24-
if (const auto fp = std::fopen(filename.c_str(), "r")) {
25-
const auto fileSize = getFileSize(fp);
26-
27-
fileContent.reserve(fileSize + 1);
28-
std::fread(fileContent.data(), 1, fileSize, fp);
29-
std::fclose(fp);
30-
fileContent[fileSize] = '\0';
31-
return fileContent.data();
17+
std::ifstream file{filename};
18+
if (!file.is_open()) {
19+
return {};
3220
}
33-
return {};
21+
22+
const auto fileSize = std::filesystem::file_size(filename);
23+
auto fileContent = std::make_unique<char[]>(fileSize + 1);
24+
25+
file.read(fileContent.get(), fileSize);
26+
fileContent[fileSize] = '\0';
27+
28+
return std::string{fileContent.get()};
3429
}
3530

3631
std::vector<std::string> readCommands(const std::filesystem::path &filePath)
3732
{
38-
const auto fileContent = readContent(filePath.c_str());
33+
const auto &fileContent = readContent(filePath.c_str());
3934
std::string currentStatement{};
4035
std::vector<std::string> statements{};
4136

4237
std::string line{};
43-
for (const auto &c : fileContent) {
38+
for (const auto c : fileContent) {
4439
if (c != '\n') {
4540
line += c;
4641
}

products/BellHybrid/CMakeLists.txt

-2
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,6 @@ if (${PROJECT_TARGET} STREQUAL "TARGET_RT1051")
105105
SYSROOT sysroot
106106
DEPENDS
107107
install_scripts
108-
create_product_databases
109108
create_user_directories
110109
assets
111110
recovery.bin-target
@@ -121,7 +120,6 @@ else()
121120
LUTS ""
122121
DEPENDS
123122
install_scripts
124-
create_product_databases
125123
remove_var_directory
126124
create_user_directories
127125
assets

products/PurePhone/CMakeLists.txt

-2
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,6 @@ if (${PROJECT_TARGET} STREQUAL "TARGET_RT1051")
121121
SYSROOT sysroot
122122
DEPENDS
123123
install_scripts
124-
create_product_databases
125124
create_user_directories
126125
assets
127126
recovery.bin-target
@@ -136,7 +135,6 @@ else()
136135
SYSROOT sysroot
137136
DEPENDS
138137
install_scripts
139-
create_product_databases
140138
remove_var_directory
141139
create_user_directories
142140
assets

test/CMakeLists.txt

+1-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ download_asset_json(json-test-proprietary-target
4646
${MUDITA_CACHE_DIR}
4747
)
4848

49-
set(ASSETS_DEPENDENCIES "")
49+
set(ASSETS_DEPENDENCIES "create_test_product_databases")
5050

5151
if (${ASSETS_TYPE} STREQUAL "Proprietary")
5252
list(APPEND ASSETS_DEPENDENCIES "json-test-proprietary-target")

0 commit comments

Comments
 (0)