Change builds ls command to default to all parts (#1019)

For backwards compatibility, `builds ls` retains past behavior of listing all parts, but allow both `builds download` and `builds prime-cache` to use the new standard of only operating on the "default" part.

3 files changed, 29 insertions, 27 deletions

diff --git a/src/zen/cmds/builds_cmd.cpp b/src/zen/cmds/builds_cmd.cpp
index 775c3d283..22e292f9c 100644
--- a/src/zen/cmds/builds_cmd.cpp
+++ b/src/zen/cmds/builds_cmd.cpp
@@ -1565,7 +1565,7 @@ BuildsDownloadSubCmd::BuildsDownloadSubCmd(BuildsConfiguration& Config)
 					"",
 					"build-part-name",
 					"Build part names list separated by ','. If neither --build-part-id nor --build-part-name is given, "
-					"the part named 'default' is selected. Use '*' (alone) to select all parts.",
+					"only the 'default' part is downloaded. Use '*' (alone) to download all parts.",
 					cxxopts::value(m_BuildPartNames),
 					"<name>");
 	Opts.add_option("",
@@ -1645,7 +1645,11 @@ BuildsDownloadSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/)
 
 	std::vector<Oid>		 BuildPartIds	= ParseBuildPartIds(m_BuildPartIds, Opts);
 	std::vector<std::string> BuildPartNames = ParseBuildPartNames(m_BuildPartNames, Opts);
-	NormalizePartSelection(BuildPartIds, BuildPartNames, Opts.help());
+	if (BuildPartIds.empty() && BuildPartNames.empty())
+	{
+		BuildPartNames = {"default"};
+	}
+	ValidatePartSelection(BuildPartIds, BuildPartNames, Opts.help());
 
 	EPartialBlockRequestMode PartialBlockRequestMode = ParseAllowPartialBlockRequests(Opts);
 
@@ -1718,7 +1722,7 @@ BuildsLsSubCmd::BuildsLsSubCmd(BuildsConfiguration& Config) : BuildsSubCmdBase(C
 					"",
 					"build-part-name",
 					"Build part names list separated by ','. If neither --build-part-id nor --build-part-name is given, "
-					"the part named 'default' is selected. Use '*' (alone) to select all parts.",
+					"all parts are listed. Use '*' (alone) to explicitly list all parts.",
 					cxxopts::value(m_BuildPartNames),
 					"<name>");
 
@@ -1767,7 +1771,7 @@ BuildsLsSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/)
 
 	std::vector<Oid>		 BuildPartIds	= ParseBuildPartIds(m_BuildPartIds, Opts);
 	std::vector<std::string> BuildPartNames = ParseBuildPartNames(m_BuildPartNames, Opts);
-	NormalizePartSelection(BuildPartIds, BuildPartNames, Opts.help());
+	ValidatePartSelection(BuildPartIds, BuildPartNames, Opts.help());
 
 	std::unique_ptr<CbObjectWriter> StructuredOutput;
 	if (!m_ResultPath.empty())
@@ -1936,7 +1940,7 @@ BuildsPrimeCacheSubCmd::BuildsPrimeCacheSubCmd(BuildsConfiguration& Config)
 					"",
 					"build-part-name",
 					"Build part names list separated by ','. If neither --build-part-id nor --build-part-name is given, "
-					"the part named 'default' is selected. Use '*' (alone) to select all parts.",
+					"only the 'default' part is primed. Use '*' (alone) to prime all parts.",
 					cxxopts::value(m_BuildPartNames),
 					"<name>");
 	Opts.add_option("",
@@ -1972,7 +1976,11 @@ BuildsPrimeCacheSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/)
 
 	std::vector<Oid>		 BuildPartIds	= ParseBuildPartIds(m_BuildPartIds, Opts);
 	std::vector<std::string> BuildPartNames = ParseBuildPartNames(m_BuildPartNames, Opts);
-	NormalizePartSelection(BuildPartIds, BuildPartNames, Opts.help());
+	if (BuildPartIds.empty() && BuildPartNames.empty())
+	{
+		BuildPartNames = {"default"};
+	}
+	ValidatePartSelection(BuildPartIds, BuildPartNames, Opts.help());
 
 	std::uint64_t PreferredMultipartChunkSize = 32u * 1024u * 1024u;
 
diff --git a/src/zenremotestore/builds/buildstorageutil.cpp b/src/zenremotestore/builds/buildstorageutil.cpp
index dc8f79a47..7636c6d60 100644
--- a/src/zenremotestore/builds/buildstorageutil.cpp
+++ b/src/zenremotestore/builds/buildstorageutil.cpp
@@ -556,7 +556,7 @@ ResolveBuildPartNames(CbObjectView				   BuildObject,
 }
 
 void
-NormalizePartSelection(std::vector<Oid>& BuildPartIds, std::vector<std::string>& BuildPartNames, std::string_view HelpText)
+ValidatePartSelection(std::vector<Oid>& BuildPartIds, std::vector<std::string>& BuildPartNames, std::string_view HelpText)
 {
 	const bool HasWildcard = std::find(BuildPartNames.begin(), BuildPartNames.end(), "*") != BuildPartNames.end();
 	if (HasWildcard)
@@ -568,11 +568,6 @@ NormalizePartSelection(std::vector<Oid>& BuildPartIds, std::vector<std::string>&
 		BuildPartNames.clear();
 		return;
 	}
-
-	if (BuildPartIds.empty() && BuildPartNames.empty())
-	{
-		BuildPartNames.push_back("default");
-	}
 }
 
 ChunkedFolderContent
@@ -1220,55 +1215,54 @@ namespace buildstorageoperations_testutils {
 
 TEST_SUITE_BEGIN("remotestore.buildstorageutil");
 
-TEST_CASE("normalizepartselection.empty_defaults_to_default")
+TEST_CASE("validatepartselection.empty_unchanged")
 {
 	std::vector<Oid>		 Ids;
 	std::vector<std::string> Names;
-	NormalizePartSelection(Ids, Names, {});
+	ValidatePartSelection(Ids, Names, {});
 	CHECK(Ids.empty());
-	REQUIRE_EQ(Names.size(), 1u);
-	CHECK_EQ(Names[0], "default");
+	CHECK(Names.empty());
 }
 
-TEST_CASE("normalizepartselection.wildcard_alone_clears_names")
+TEST_CASE("validatepartselection.wildcard_alone_clears_names")
 {
 	std::vector<Oid>		 Ids;
 	std::vector<std::string> Names = {"*"};
-	NormalizePartSelection(Ids, Names, {});
+	ValidatePartSelection(Ids, Names, {});
 	CHECK(Ids.empty());
 	CHECK(Names.empty());
 }
 
-TEST_CASE("normalizepartselection.wildcard_with_other_name_throws")
+TEST_CASE("validatepartselection.wildcard_with_other_name_throws")
 {
 	std::vector<Oid>		 Ids;
 	std::vector<std::string> Names = {"*", "foo"};
-	CHECK_THROWS_AS(NormalizePartSelection(Ids, Names, {}), OptionParseException);
+	CHECK_THROWS_AS(ValidatePartSelection(Ids, Names, {}), OptionParseException);
 }
 
-TEST_CASE("normalizepartselection.wildcard_with_ids_throws")
+TEST_CASE("validatepartselection.wildcard_with_ids_throws")
 {
 	std::vector<Oid>		 Ids   = {Oid::NewOid()};
 	std::vector<std::string> Names = {"*"};
-	CHECK_THROWS_AS(NormalizePartSelection(Ids, Names, {}), OptionParseException);
+	CHECK_THROWS_AS(ValidatePartSelection(Ids, Names, {}), OptionParseException);
 }
 
-TEST_CASE("normalizepartselection.explicit_name_unchanged")
+TEST_CASE("validatepartselection.explicit_name_unchanged")
 {
 	std::vector<Oid>		 Ids;
 	std::vector<std::string> Names = {"foo"};
-	NormalizePartSelection(Ids, Names, {});
+	ValidatePartSelection(Ids, Names, {});
 	CHECK(Ids.empty());
 	REQUIRE_EQ(Names.size(), 1u);
 	CHECK_EQ(Names[0], "foo");
 }
 
-TEST_CASE("normalizepartselection.ids_only_unchanged")
+TEST_CASE("validatepartselection.ids_only_unchanged")
 {
 	const Oid				 Id	 = Oid::NewOid();
 	std::vector<Oid>		 Ids = {Id};
 	std::vector<std::string> Names;
-	NormalizePartSelection(Ids, Names, {});
+	ValidatePartSelection(Ids, Names, {});
 	REQUIRE_EQ(Ids.size(), 1u);
 	CHECK_EQ(Ids[0], Id);
 	CHECK(Names.empty());
diff --git a/src/zenremotestore/include/zenremotestore/builds/buildstorageutil.h b/src/zenremotestore/include/zenremotestore/builds/buildstorageutil.h
index df35f65be..ae5e733ed 100644
--- a/src/zenremotestore/include/zenremotestore/builds/buildstorageutil.h
+++ b/src/zenremotestore/include/zenremotestore/builds/buildstorageutil.h
@@ -101,7 +101,7 @@ std::vector<std::pair<Oid, std::string>> ResolveBuildPartNames(CbObjectView
 															   std::span<const std::string> BuildPartNames,
 															   std::uint64_t&				OutPreferredMultipartChunkSize);
 
-void NormalizePartSelection(std::vector<Oid>& BuildPartIds, std::vector<std::string>& BuildPartNames, std::string_view HelpText);
+void ValidatePartSelection(std::vector<Oid>& BuildPartIds, std::vector<std::string>& BuildPartNames, std::string_view HelpText);
 
 ChunkedFolderContent GetRemoteContent(LoggerRef										  InLog,
 									  StorageInstance&								  Storage,