From 65f4d1912b9e31cfc393cfaa0fafd28bf3d9cab6 Mon Sep 17 00:00:00 2001 From: Shiv Tyagi Date: Sat, 25 Jul 2026 13:50:53 +0530 Subject: [PATCH 1/7] build_manager: shorten build ids and name archives vehicle-board-id Co-authored-by: Cursor --- build_manager/manager.py | 15 +++++++++------ build_manager/progress_updater.py | 4 +++- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/build_manager/manager.py b/build_manager/manager.py index 60bbc2e..befc8e7 100644 --- a/build_manager/manager.py +++ b/build_manager/manager.py @@ -200,13 +200,12 @@ def __generate_build_id(self, build_info: BuildInfo) -> str: build_info (BuildInfo): The build information object. Returns: - str: The generated build ID (64 characters). + str: The generated build ID (8 characters). """ h = hashlib.md5( f"{build_info}-{time.time_ns()}".encode() - ).hexdigest() - bid = f"{build_info.vehicle_id}-{build_info.board}-{h}" - return bid + ).hexdigest()[:8] + return h def submit_build(self, build_info: BuildInfo) -> str: @@ -460,19 +459,23 @@ def get_build_log_path(self, build_id: str) -> str: 'build.log' ) - def get_build_archive_path(self, build_id: str) -> str: + def get_build_archive_path( + self, build_id: str, vehicle_id: str, board: str + ) -> str: """ Return the path to the build archive. Parameters: build_id (str): The ID of the build. + vehicle_id (str): The vehicle identifier. + board (str): The board identifier. Returns: str: The path to the build archive. """ return os.path.join( self.get_build_artifacts_dir_path(build_id), - f"{build_id}.tar.gz" + f"{vehicle_id}-{board}-{build_id}.tar.gz" ) @staticmethod diff --git a/build_manager/progress_updater.py b/build_manager/progress_updater.py index ee15504..efda989 100644 --- a/build_manager/progress_updater.py +++ b/build_manager/progress_updater.py @@ -186,7 +186,9 @@ def __refresh_running_build_state(self, build_id: str) -> BuildState: # Builder ships the archive post completion # This is irrespective of SUCCESS or FAILURE if not os.path.exists( - bm.get_singleton().get_build_archive_path(build_id) + bm.get_singleton().get_build_archive_path( + build_id, build_info.vehicle_id, build_info.board + ) ): return BuildState.RUNNING From d89285c55e046d77521b8538fef062c75ee4ae73 Mon Sep 17 00:00:00 2001 From: Shiv Tyagi Date: Sat, 25 Jul 2026 13:50:53 +0530 Subject: [PATCH 2/7] builder: use renamed archive path and match tar inner folder Co-authored-by: Cursor --- builder/builder.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/builder/builder.py b/builder/builder.py index 7a16cb1..c687e9a 100644 --- a/builder/builder.py +++ b/builder/builder.py @@ -226,7 +226,9 @@ def __generate_archive(self, build_id: str) -> None: build_id (str): Unique identifier for the build. """ build_info = bm.get_singleton().get_build_info(build_id) - archive_path = bm.get_singleton().get_build_archive_path(build_id) + archive_path = bm.get_singleton().get_build_archive_path( + build_id, build_info.vehicle_id, build_info.board + ) files_to_include = [] @@ -261,10 +263,11 @@ def __generate_archive(self, build_id: str) -> None: ) files_to_include.append(extra_hwdef_path_abs) - # create archive + # create archive (inner folder matches download basename) + folder_name = Path(archive_path).name.removesuffix(".tar.gz") with tarfile.open(archive_path, "w:gz") as tar: for file in files_to_include: - arcname = f"{build_id}/{os.path.basename(file)}" + arcname = f"{folder_name}/{os.path.basename(file)}" self.logger.debug(f"Added {file} as {arcname}") tar.add(file, arcname=arcname) self.logger.info(f"Generated {archive_path}.") From d922c9a8341bed44ae81adfcd3cce9e4f1e32038 Mon Sep 17 00:00:00 2001 From: Shiv Tyagi Date: Sat, 25 Jul 2026 13:50:53 +0530 Subject: [PATCH 3/7] web: serve artifact download using archive basename Co-authored-by: Cursor --- web/api/v1/builds.py | 3 ++- web/services/builds.py | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/web/api/v1/builds.py b/web/api/v1/builds.py index 2c68c6a..9f33077 100644 --- a/web/api/v1/builds.py +++ b/web/api/v1/builds.py @@ -1,3 +1,4 @@ +import os from typing import List, Optional from fastapi import ( APIRouter, @@ -219,5 +220,5 @@ async def download_artifact( return FileResponse( path=artifact_path, media_type='application/gzip', - filename=f"{build_id}.tar.gz" + filename=os.path.basename(artifact_path) ) diff --git a/web/services/builds.py b/web/services/builds.py index 9cc6b67..64af64d 100644 --- a/web/services/builds.py +++ b/web/services/builds.py @@ -282,7 +282,9 @@ def get_artifact_path(self, build_id: str) -> Optional[str]: ]: return None - artifact_path = self.manager.get_build_archive_path(build_id) + artifact_path = self.manager.get_build_archive_path( + build_id, build_info.vehicle_id, build_info.board + ) if os.path.exists(artifact_path): return artifact_path From 4a77ba3cfa67f7c6b2e44317a3c876741933edc5 Mon Sep 17 00:00:00 2001 From: Shiv Tyagi Date: Sat, 25 Jul 2026 14:53:42 +0530 Subject: [PATCH 4/7] build_manager: store selected features as API labels Co-authored-by: Cursor --- build_manager/manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build_manager/manager.py b/build_manager/manager.py index befc8e7..9a623dd 100644 --- a/build_manager/manager.py +++ b/build_manager/manager.py @@ -61,7 +61,7 @@ def __init__(self, source commit to build on. git_hash (str): The git commit hash to build on. board (str): Board to build for. - selected_features (set): Set of features selected for the build. + selected_features (set): Set of feature API labels/IDs for the build. """ self.vehicle_id = vehicle_id self.version_id = version_id From 5d18e3f97531681a789464713945676fce83427f Mon Sep 17 00:00:00 2001 From: Shiv Tyagi Date: Sat, 25 Jul 2026 14:53:42 +0530 Subject: [PATCH 5/7] builder: resolve feature labels to defines for extra_hwdef Co-authored-by: Cursor --- builder/builder.py | 41 ++++++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/builder/builder.py b/builder/builder.py index c687e9a..dced6d2 100644 --- a/builder/builder.py +++ b/builder/builder.py @@ -17,6 +17,27 @@ CBS_BUILD_TIMEOUT_SEC = int(os.getenv('CBS_BUILD_TIMEOUT_SEC', 900)) +def resolve_feature_defines(selected_labels, all_features): + """ + Map API feature labels to preprocessor defines for extra_hwdef. + + Returns: + tuple: (enabled_defines, disabled_defines, all_defines, unknown_labels) + """ + label_to_define = { + feature.label: feature.define for feature in all_features + } + all_defines = set(label_to_define.values()) + selected = set(selected_labels) + known = set(label_to_define) + unknown_labels = selected.difference(known) + enabled_defines = { + label_to_define[label] for label in known.intersection(selected) + } + disabled_defines = all_defines.difference(enabled_defines) + return enabled_defines, disabled_defines, all_defines, unknown_labels + + class Builder: """ Processes build requests, perform builds and ship build artifacts @@ -102,22 +123,24 @@ def __generate_extrahwdef(self, build_id: str) -> None: ) build_info = bm.get_singleton().get_build_info(build_id) - selected_features = build_info.selected_features + selected_labels = build_info.selected_features self.logger.debug( - f"Selected features for {build_id}: {selected_features}" + f"Selected feature labels for {build_id}: {selected_labels}" ) all_features = apfetch.get_singleton().get_build_options_at_commit( remote=build_info.remote_info.name, commit_ref=build_info.git_hash, ) - all_defines = { - feature.define - for feature in all_features - } - enabled_defines = selected_features.intersection(all_defines) - disabled_defines = all_defines.difference(enabled_defines) + enabled_defines, disabled_defines, all_defines, unknown_labels = ( + resolve_feature_defines(selected_labels, all_features) + ) + if unknown_labels: + self.logger.warning( + f"Unknown feature labels not found in build options; " + f"skipping: {sorted(unknown_labels)}" + ) self.logger.info(f"Enabled defines for {build_id}: {enabled_defines}") - self.logger.info(f"Disabled defines for {build_id}: {enabled_defines}") + self.logger.info(f"Disabled defines for {build_id}: {disabled_defines}") with open(self.__get_path_to_extra_hwdef(build_id), "w") as f: # Undefine all defines at the beginning From b7daf38462d303708b9c900c611865324894fb64 Mon Sep 17 00:00:00 2001 From: Shiv Tyagi Date: Sat, 25 Jul 2026 14:53:42 +0530 Subject: [PATCH 6/7] web: submit and return feature labels without define mapping Co-authored-by: Cursor --- web/services/builds.py | 79 ++---------------------------------------- 1 file changed, 2 insertions(+), 77 deletions(-) diff --git a/web/services/builds.py b/web/services/builds.py index 64af64d..53bdfa1 100644 --- a/web/services/builds.py +++ b/web/services/builds.py @@ -103,38 +103,6 @@ def create_build( commit_ref=commit_ref ) - # Map feature labels (IDs from API) to defines - # (required by build manager) - selected_feature_defines = set() - if build_request.selected_features: - # Get build options to map labels to defines - with self.repo.get_checkout_lock(): - options = ( - self.ap_src_metadata_fetcher - .get_build_options_at_commit( - remote=remote_name, - commit_ref=commit_ref - ) - ) - - # Create label to define mapping - label_to_define = { - option.label: option.define for option in options - } - - # Map each selected feature label to its define - for feature_label in build_request.selected_features: - if feature_label in label_to_define: - selected_feature_defines.add( - label_to_define[feature_label] - ) - else: - logger.warning( - f"Feature label '{feature_label}' not found in " - f"build options for {vehicle_id} {remote_name} " - f"{commit_ref}" - ) - # Create build info build_info = build_manager.BuildInfo( vehicle_id=vehicle_id, @@ -142,7 +110,7 @@ def create_build( remote_info=remote_info, git_hash=git_hash, board=board_name, - selected_features=selected_feature_defines + selected_features=set(build_request.selected_features), ) # Submit build @@ -317,49 +285,6 @@ def _build_info_to_output( url=build_info.remote_info.url ) - # Map feature defines back to labels for API response - selected_feature_labels = [] - if build_info.selected_features: - try: - # Get build options to map defines back to labels - with self.repo.get_checkout_lock(): - options = ( - self.ap_src_metadata_fetcher - .get_build_options_at_commit( - remote=build_info.remote_info.name, - commit_ref=build_info.git_hash - ) - ) - - # Create define to label mapping - define_to_label = { - option.define: option.label for option in options - } - - # Map each selected feature define to its label - for feature_define in build_info.selected_features: - if feature_define in define_to_label: - selected_feature_labels.append( - define_to_label[feature_define] - ) - else: - # Fallback: use define if label not found - logger.warning( - f"Feature define '{feature_define}' not " - f"found in build options for build " - f"{build_id}" - ) - selected_feature_labels.append(feature_define) - except Exception as e: - logger.error( - f"Error mapping feature defines to labels for " - f"build {build_id}: {e}" - ) - # Fallback: use defines as-is - selected_feature_labels = list( - build_info.selected_features - ) - vehicle = self.vehicles_manager.get_vehicle_by_id( build_info.vehicle_id ) @@ -379,7 +304,7 @@ def _build_info_to_output( remote_info=remote_info, git_hash=build_info.git_hash ), - selected_features=selected_feature_labels, + selected_features=list(build_info.selected_features), progress=progress, time_created=build_info.time_created, ) From be7f66e00698dd0763a1516296f4abe5762de7e7 Mon Sep 17 00:00:00 2001 From: Shiv Tyagi Date: Sat, 25 Jul 2026 14:53:42 +0530 Subject: [PATCH 7/7] tests: cover label storage and resolve_feature_defines Co-authored-by: Cursor --- tests/builder/test_resolve_feature_defines.py | 56 +++++++++++++++ tests/web/test_builds_service.py | 72 ++++--------------- 2 files changed, 68 insertions(+), 60 deletions(-) create mode 100644 tests/builder/test_resolve_feature_defines.py diff --git a/tests/builder/test_resolve_feature_defines.py b/tests/builder/test_resolve_feature_defines.py new file mode 100644 index 0000000..c9bca81 --- /dev/null +++ b/tests/builder/test_resolve_feature_defines.py @@ -0,0 +1,56 @@ +"""Tests for builder label to define resolution.""" +from unittest.mock import Mock + +from builder.builder import resolve_feature_defines + + +def _option(label, define): + opt = Mock() + opt.label = label + opt.define = define + return opt + + +def test_resolve_feature_defines_maps_labels(): + features = [ + _option("HAL_LOGGING_ENABLED", "HAL_LOGGING_ENABLED_DEFINE"), + _option("HAL_WITH_EKF3", "HAL_WITH_EKF3_DEFINE"), + ] + + enabled, disabled, all_defines, unknown = resolve_feature_defines( + ["HAL_LOGGING_ENABLED"], + features, + ) + + assert enabled == {"HAL_LOGGING_ENABLED_DEFINE"} + assert disabled == {"HAL_WITH_EKF3_DEFINE"} + assert all_defines == { + "HAL_LOGGING_ENABLED_DEFINE", + "HAL_WITH_EKF3_DEFINE", + } + assert unknown == set() + + +def test_resolve_feature_defines_returns_unknown_labels(): + features = [_option("HAL_LOGGING_ENABLED", "HAL_LOGGING_ENABLED_DEFINE")] + + enabled, disabled, all_defines, unknown = resolve_feature_defines( + ["COMPLETELY_UNKNOWN_FEATURE"], + features, + ) + + assert enabled == set() + assert disabled == all_defines == {"HAL_LOGGING_ENABLED_DEFINE"} + assert unknown == {"COMPLETELY_UNKNOWN_FEATURE"} + + +def test_resolve_feature_defines_empty_selection_disables_all(): + features = [_option("HAL_LOGGING_ENABLED", "HAL_LOGGING_ENABLED_DEFINE")] + + enabled, disabled, all_defines, unknown = resolve_feature_defines( + [], features + ) + + assert enabled == set() + assert disabled == all_defines == {"HAL_LOGGING_ENABLED_DEFINE"} + assert unknown == set() diff --git a/tests/web/test_builds_service.py b/tests/web/test_builds_service.py index 8270ec2..bbe6aa4 100644 --- a/tests/web/test_builds_service.py +++ b/tests/web/test_builds_service.py @@ -79,7 +79,7 @@ def make_build_info( remote_info=ManagerRemoteInfo(name=remote_name, url=remote_url), git_hash=git_hash, board=board, - selected_features=selected_features or set(), + selected_features=set(selected_features) if selected_features is not None else set(), ) info.progress = bm.BuildProgress(state=state, percent=percent) return info @@ -242,17 +242,12 @@ def test_create_build_raises_value_error_when_board_not_in_version( with pytest.raises(ValueError, match="Invalid board for this version"): service.create_build(request) - def test_create_build_maps_feature_labels_to_defines( + def test_create_build_stores_feature_labels( self, service, - mock_ap_src_metadata_fetcher, mock_build_manager, ): - """Selected feature labels are translated to defines before build submission.""" - opt = Mock() - opt.label = "HAL_LOGGING_ENABLED" - opt.define = "HAL_LOGGING_ENABLED_DEFINE" - mock_ap_src_metadata_fetcher.get_build_options_at_commit.return_value = [opt] + """Selected feature labels are stored on BuildInfo as-is.""" request = BuildRequest( vehicle_id="copter", board_id="MatekH743", @@ -263,37 +258,14 @@ def test_create_build_maps_feature_labels_to_defines( service.create_build(request) submitted: bm.BuildInfo = mock_build_manager.submit_build.call_args[1]["build_info"] - assert "HAL_LOGGING_ENABLED_DEFINE" in submitted.selected_features - - def test_create_build_ignores_unknown_feature_labels( - self, - service, - mock_ap_src_metadata_fetcher, - mock_build_manager, - ): - """Unknown feature labels are silently skipped (not added to defines set).""" - opt = Mock() - opt.label = "HAL_LOGGING_ENABLED" - opt.define = "HAL_LOGGING_ENABLED_DEFINE" - mock_ap_src_metadata_fetcher.get_build_options_at_commit.return_value = [opt] - request = BuildRequest( - vehicle_id="copter", - board_id="MatekH743", - version_id="copter-4.5.0-stable", - selected_features=["COMPLETELY_UNKNOWN_FEATURE"], - ) - - service.create_build(request) - - submitted: bm.BuildInfo = mock_build_manager.submit_build.call_args[1]["build_info"] - assert len(submitted.selected_features) == 0 + assert submitted.selected_features == {"HAL_LOGGING_ENABLED"} - def test_create_build_no_features_submits_empty_set( + def test_create_build_no_features_submits_empty_list( self, service, mock_build_manager, ): - """When selected_features is empty, build is submitted with an empty set.""" + """When selected_features is empty, build is submitted with an empty list.""" request = BuildRequest( vehicle_id="copter", board_id="MatekH743", @@ -304,7 +276,7 @@ def test_create_build_no_features_submits_empty_set( service.create_build(request) submitted: bm.BuildInfo = mock_build_manager.submit_build.call_args[1]["build_info"] - assert len(submitted.selected_features) == 0 + assert submitted.selected_features == set() # Tests for list_builds @@ -546,40 +518,20 @@ def test_get_build_output_has_correct_vehicle_and_board( assert result.vehicle.id == "plane" assert result.board.id == "CubeOrange" - def test_get_build_maps_feature_defines_to_labels( + def test_get_build_uses_stored_feature_labels( self, service, mock_build_manager, - mock_ap_src_metadata_fetcher, ): - """Feature defines in BuildInfo are mapped back to labels in the output.""" + """API output uses feature labels stored on BuildInfo at submit time.""" mock_build_manager.build_exists.return_value = True mock_build_manager.get_build_info.return_value = make_build_info( - selected_features={"HAL_LOGGING_ENABLED_DEFINE"} - ) - opt = Mock() - opt.define = "HAL_LOGGING_ENABLED_DEFINE" - opt.label = "HAL_LOGGING_ENABLED" - mock_ap_src_metadata_fetcher.get_build_options_at_commit.return_value = [opt] - - result = service.get_build("build-abc123") - - assert "HAL_LOGGING_ENABLED" in result.selected_features - - def test_get_build_falls_back_to_define_when_label_not_found( - self, - service, - mock_build_manager, - ): - """When a define has no matching label, the define itself is used as fallback.""" - mock_build_manager.build_exists.return_value = True - mock_build_manager.get_build_info.return_value = make_build_info( - selected_features={"ORPHANED_DEFINE"} + selected_features=["HAL_LOGGING_ENABLED"], ) result = service.get_build("build-abc123") - assert "ORPHANED_DEFINE" in result.selected_features + assert result.selected_features == ["HAL_LOGGING_ENABLED"] def test_get_build_no_selected_features_returns_empty_list( self, @@ -589,7 +541,7 @@ def test_get_build_no_selected_features_returns_empty_list( """When a build has no selected features, the output list is empty.""" mock_build_manager.build_exists.return_value = True mock_build_manager.get_build_info.return_value = make_build_info( - selected_features=set() + selected_features=[], ) result = service.get_build("build-abc123")