Don't set "unset" unless stricly necessary

"unset" is a special value cvd uses to let cvd_internal_start know a
value in a vectorized flag is not set. This special value is not
recognized by all versions of cvd_internal_start, so when the flag is
unset for all instances cvd simply doesn't pass the flag to
cvd_internal_start instead of passing something like "unset,unset".

This allows using cvd environment configs with older android branches,
albeit with some restrictions. For example, properties that could
generate an "unset" must be either set in all instances or none.

Author: jemoreira

base/cvd/cuttlefish/host/commands/cvd/cli/parser/instance/cf_disk_configs.cpp

@@ -15,6 +15,7 @@
  */
 #include "cuttlefish/host/commands/cvd/cli/parser/instance/cf_disk_configs.h"
 
+#include <algorithm>
 #include <string>
 #include <vector>
 
@@ -31,6 +32,12 @@ using cvd::config::Instance;
 std::vector<std::string> GenerateDiskFlags(
     const EnvironmentSpecification& config) {
   std::vector<std::string> data_image_mbs;
+  if (std::none_of(config.instances().cbegin(), config.instances().cend(),
+                   [](const auto& instance) {
+                     return instance.disk().has_blank_data_image_mb();
+                   })) {
+    return {};
+  }
   for (const auto& instance : config.instances()) {
     const auto& disk = instance.disk();
     if (disk.has_blank_data_image_mb()) {

base/cvd/cuttlefish/host/commands/cvd/cli/parser/instance/cf_graphics_configs.cpp

@@ -34,6 +34,7 @@
 #include "cuttlefish/host/commands/cvd/cli/parser/load_config.pb.h"
 
 namespace cuttlefish {
+namespace {
 
 using cvd::config::Display;
 using cvd::config::EnvironmentSpecification;
@@ -108,16 +109,33 @@ bool RecordScreen(const Instance& instance) {
   }
 }
 
-std::string GpuMode(const Instance& instance) {
-  if (instance.graphics().has_gpu_mode() &&
-      !instance.graphics().gpu_mode().empty()) {
-    return instance.graphics().gpu_mode();
-  } else {
-    // Use the instance default
-    // https://github.com/google/android-cuttlefish/blob/c4f1643479f98bdc7310d281e81751188595233b/base/cvd/cuttlefish/host/commands/assemble_cvd/flags.cc#L948
-    // See also b/406464352#comment7
-    return "unset";
+std::optional<std::string> GpuMode(const Instance& instance) {
+  if (!instance.graphics().has_gpu_mode() ||
+      instance.graphics().gpu_mode().empty()) {
+    return std::nullopt;
   }
+  return instance.graphics().gpu_mode();
+}
+
+std::optional<std::vector<std::string>> GpuModes(const EnvironmentSpecification& cfg) {
+  std::vector<std::optional<std::string>> opts;
+  for (const Instance& instance: cfg.instances()) {
+    opts.emplace_back(GpuMode(instance));
+  }
+  if (std::none_of(opts.begin(), opts.end(),
+                   [](const auto& opt) { return opt.has_value(); })) {
+    return std::nullopt;
+  }
+  std::vector<std::string> values;
+  for (const std::optional<std::string>& opt: opts) {
+  // Use the instance default
+  // https://github.com/google/android-cuttlefish/blob/c4f1643479f98bdc7310d281e81751188595233b/base/cvd/cuttlefish/host/commands/assemble_cvd/flags.cc#L948
+  // See also b/406464352#comment7
+    values.emplace_back(opt.value_or("unset"));
+  }
+  return values;
+}
+
 }
 
 Result<std::vector<std::string>> GenerateGraphicsFlags(
@@ -128,7 +146,10 @@ Result<std::vector<std::string>> GenerateGraphicsFlags(
     flags.push_back(std::move(display_flag.value()));
   }
   flags.push_back(GenerateInstanceFlag("record_screen", cfg, RecordScreen));
-  flags.push_back(GenerateInstanceFlag("gpu_mode", cfg, GpuMode));
+  std::optional<std::vector<std::string>> gpu_modes = GpuModes(cfg);
+  if (gpu_modes.has_value()) {
+    flags.push_back(GenerateVecFlag("gpu_mode", gpu_modes.value()));
+  }
   return flags;
 }
 

base/cvd/cuttlefish/host/commands/cvd/cli/parser/instance/cf_vm_configs.cpp

@@ -16,6 +16,7 @@
 #include "cuttlefish/host/commands/cvd/cli/parser/instance/cf_vm_configs.h"
 
 #include <cstdint>
+#include <optional>
 #include <string>
 #include <utility>
 #include <vector>
@@ -112,9 +113,10 @@ static std::string V4l2Proxy(const Instance& instance) {
   return crosvm.has_v4l2_proxy() ? crosvm.v4l2_proxy() : default_val;
 }
 
-static Result<std::string> CustomConfigsFlagValue(const Instance& instance) {
+static Result<std::optional<std::string>> CustomConfigsFlagValue(
+    const Instance& instance) {
   if (instance.vm().custom_actions().empty()) {
-    return "unset";
+    return std::nullopt;
   }
   std::vector<std::string> json_entries;
   for (const auto& action : instance.vm().custom_actions()) {
@@ -132,8 +134,11 @@ static Result<std::vector<std::string>> CustomConfigsFlags(
     const EnvironmentSpecification& cfg) {
   std::vector<std::string> ret;
   for (const auto& instance : cfg.instances()) {
-    auto value = CF_EXPECT(CustomConfigsFlagValue(instance));
-    ret.emplace_back(fmt::format("--custom_actions={}", value));
+    std::optional<std::string> opt =
+        CF_EXPECT(CustomConfigsFlagValue(instance));
+    if (opt.has_value()) {
+      ret.emplace_back(fmt::format("--custom_actions={}", opt.value()));
+    }
   }
   return ret;
 }

base/cvd/cuttlefish/host/commands/cvd/cli/parser/instance/disk_configs_test.cc

@@ -42,9 +42,9 @@ TEST(BootFlagsParserTest, ParseTwoInstancesBlankDataImageEmptyJson) {
       << "Invalid Json string";
   auto serialized_data = LaunchCvdParserTester(json_configs);
   EXPECT_TRUE(serialized_data.ok()) << serialized_data.error().Trace();
-  EXPECT_TRUE(
-      FindConfig(*serialized_data, R"(--blank_data_image_mb=unset,unset)"))
-      << "blank_data_image_mb flag is missing or wrongly formatted";
+  EXPECT_FALSE(
+      FindConfig(*serialized_data, R"(--blank_data_image_mb=)"))
+      << "blank_data_image_mb flag is set";
 }
 
 TEST(BootFlagsParserTest, ParseTwoInstancesBlankDataImagePartialJson) {

base/cvd/cuttlefish/host/commands/cvd/cli/parser/instance/vm_configs_test.cc

@@ -942,8 +942,8 @@ TEST(VmFlagsParserTest, ParseTwoInstancesCustomActionsFlagEmptyJson) {
       << "Invalid Json string";
   auto serialized_data = LaunchCvdParserTester(json_configs);
   EXPECT_TRUE(serialized_data.ok()) << serialized_data.error().Trace();
-  EXPECT_TRUE(FindConfig(*serialized_data, R"(--custom_actions=unset)"))
-      << "custom_actions flag is missing or wrongly formatted";
+  EXPECT_FALSE(FindConfig(*serialized_data, R"(--custom_actions=)"))
+      << "custom_actions flag is set";
 }
 
 TEST(VmFlagsParserTest, ParseTwoInstancesCustomActionsFlagPartialJson) {
@@ -986,10 +986,7 @@ TEST(VmFlagsParserTest, ParseTwoInstancesCustomActionsFlagPartialJson) {
       custom_actions.emplace_back(flag.substr(sizeof(kPrefix) - 1));
     }
   }
-  std::sort(custom_actions.begin(), custom_actions.end());
-
-  EXPECT_EQ(custom_actions.size(), 2);
-  EXPECT_EQ(custom_actions[1], "unset");
+  EXPECT_EQ(custom_actions.size(), 1);
 
   Json::Value expected_actions;
   expected_actions[0]["device_states"][0]["lid_switch_open"] = false;