From 3c2ce6408cbd2f2852f201fac081069feb18eaa9 Mon Sep 17 00:00:00 2001 From: Brian Daniels Date: Tue, 27 Oct 2026 20:22:12 +0000 Subject: [PATCH 1/2] Allow DiskArtifact message to be used with other artifact fields When the DiskArtifact message was introduced, it was enforced that it wasn't used in combination with other artifact fields. This removes that check to allow different combinations of mixing to occur. Bug: 546186358 Test: bazel test ... --- .../cvd/cli/parser/cf_flags_validator.cpp | 19 +--- .../cvd/cli/parser/flags_parser_test.cc | 91 ------------------- 2 files changed, 1 insertion(+), 109 deletions(-) diff --git a/base/cvd/cuttlefish/host/commands/cvd/cli/parser/cf_flags_validator.cpp b/base/cvd/cuttlefish/host/commands/cvd/cli/parser/cf_flags_validator.cpp index 1e6d7ac9077..574a379accf 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/cli/parser/cf_flags_validator.cpp +++ b/base/cvd/cuttlefish/host/commands/cvd/cli/parser/cf_flags_validator.cpp @@ -28,30 +28,15 @@ namespace cuttlefish { namespace { -using cvd::config::Boot; -using cvd::config::Disk; using cvd::config::DiskArtifacts; using cvd::config::EnvironmentSpecification; using cvd::config::Instance; using google::protobuf::util::JsonStringToMessage; -bool HasLegacyArtifacts(const Instance& instance) { - const Disk& disk = instance.disk(); - const Boot& boot = instance.boot(); - return disk.has_super_partition() || boot.has_kernel() || boot.has_build() || - boot.has_bootloader() || boot.has_android_efi_loader(); -} - Result ValidateDiskArtifacts( const EnvironmentSpecification& launch_config) { - bool has_legacy_artifacts = false; - bool has_disk_artifacts = false; for (const Instance& instance : launch_config.instances()) { - if (HasLegacyArtifacts(instance)) { - has_legacy_artifacts = true; - } if (instance.disk().has_disk_artifacts()) { - has_disk_artifacts = true; const DiskArtifacts& disk_artifacts = instance.disk().disk_artifacts(); if (disk_artifacts.has_vendor_boot()) { CF_EXPECT(!disk_artifacts.vendor_boot().empty(), @@ -59,9 +44,7 @@ Result ValidateDiskArtifacts( } } } - CF_EXPECT(!(has_legacy_artifacts && has_disk_artifacts), - "Cannot use disk_artifacts together with legacy disk/boot " - "artifact options"); + return {}; } diff --git a/base/cvd/cuttlefish/host/commands/cvd/cli/parser/flags_parser_test.cc b/base/cvd/cuttlefish/host/commands/cvd/cli/parser/flags_parser_test.cc index aba32b4a8e6..bf0ccf2ab79 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/cli/parser/flags_parser_test.cc +++ b/base/cvd/cuttlefish/host/commands/cvd/cli/parser/flags_parser_test.cc @@ -449,97 +449,6 @@ TEST(FlagsParserTest, ParseMediaTwoInstancesOnlySecondHasDevice) { << "media flag is missing or wrongly formatted"; } -TEST(DiskArtifactsValidationTest, ConflictWithLegacyOverrideFails) { - const std::vector invalid_configs = { - R""""( -{ - "instances": [ - { - "disk": { - "super": { - "system": "@ab/git_main/cf_x86_64_auto" - }, - "disk_artifacts": {} - } - } - ] -} - )"""", - R""""( -{ - "instances": [ - { - "boot": { - "kernel": { - "build": "@ab/git_main/cf_x86_64_auto" - } - }, - "disk": { - "disk_artifacts": {} - } - } - ] -} - )"""", - R""""( -{ - "instances": [ - { - "boot": { - "build": "@ab/git_main/cf_x86_64_auto" - }, - "disk": { - "disk_artifacts": {} - } - } - ] -} - )"""", - R""""( -{ - "instances": [ - { - "boot": { - "bootloader": { - "build": "@ab/git_main/cf_x86_64_auto" - } - }, - "disk": { - "disk_artifacts": {} - } - } - ] -} - )"""", - R""""( -{ - "instances": [ - { - "boot": { - "android_efi_loader": { - "build": "@ab/git_main/cf_x86_64_auto" - } - }, - "disk": { - "disk_artifacts": {} - } - } - ] -} - )"""", - }; - - for (const std::string& test_string : invalid_configs) { - SCOPED_TRACE(test_string); - Json::Value json_configs; - std::string json_text(test_string); - - EXPECT_TRUE(ParseJsonString(json_text, json_configs)) - << "Invalid Json string"; - EXPECT_THAT(LaunchCvdParserTester(json_configs), IsError()); - } -} - TEST(DiskArtifactsValidationTest, EmptyVendorBootBuildFails) { const std::string test_string = R""""( { From bc0c70df9221e12aac0d2576fc1528d6f304431f Mon Sep 17 00:00:00 2001 From: Brian Daniels Date: Tue, 27 Oct 2026 20:24:05 +0000 Subject: [PATCH 2/2] Do not use a signed build if the image is being mixed. The current behavior will use a signed build for certain builds (e.g. `-user` builds). However, this prevents mixing with other build artifacts because the target files are used for these builds, and target files are unsigned. We detect if this mixing has been requested, and if so, we stick with unsigned builds. Bug: 546186358 Test: Confirm the following boots: cvd create --config_file config.json config.json contents: { "common": { "host_package": "@ab/git_26Q4-release/cf_x86_64_auto-user" }, "instances": [ { "@import": "auto", "disk": { "default_build": "@ab/git_26Q4-release/cf_x86_64_auto-user", "super": { "system": "@ab/git_26Q4-release/gsi_car_x86_64-user" } } } ] } --- .../host/commands/cvd/fetch/fetch_cvd.cc | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_cvd.cc b/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_cvd.cc index 1b16a07a916..6935a88b3f2 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_cvd.cc +++ b/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_cvd.cc @@ -244,15 +244,25 @@ Result FetchDefaultTarget(FetchBuildContext& context, if (!context.Artifact("misc_info.txt").Download().has_value()) { VLOG(0) << "Failed to download misc_info.txt, continuing"; } + const bool download_target_files = + has_system_build || flags.download_target_files_zip; + const bool use_target_files = + download_target_files || flags.dynamic_super_image; if (flags.download_img_zip) { LOG(INFO) << "Downloading image zip for " << context; const std::string img_zip_name = CF_EXPECT(context.GetBuildZipName(BuildZipKind::kImages)); std::string img_zip_artifact_name = img_zip_name; if (IsSignedBuild(context.Build())) { - img_zip_artifact_name = kSignedPrefix + img_zip_artifact_name; - LOG(INFO) << "Attempting to fetch SIGNED default image zip: " - << img_zip_artifact_name; + if (!use_target_files) { + img_zip_artifact_name = kSignedPrefix + img_zip_artifact_name; + LOG(INFO) << "Attempting to fetch SIGNED default image zip: " + << img_zip_artifact_name; + } else { + LOG(INFO) << "Signed build is available, but using the unsigned " + "build because other unsigned builds will be used " + "to mix a new super image."; + } } FetchArtifact img_zip = context.Artifact(img_zip_artifact_name); CF_EXPECT(img_zip.Download()); @@ -261,9 +271,7 @@ Result FetchDefaultTarget(FetchBuildContext& context, CF_EXPECT(img_zip.DeleteLocalFile()); } } - const bool download_target_files = - has_system_build || flags.download_target_files_zip; - if (download_target_files || flags.dynamic_super_image) { + if (use_target_files) { const std::string target_files_name = CF_EXPECT(context.GetBuildZipName(BuildZipKind::kTargetFiles)); FetchArtifact target_files = context.Artifact(target_files_name);