From 51944ddea62bc8976efb2c633c4ae267b301e649 Mon Sep 17 00:00:00 2001 From: Dan Engelbrecht Date: Tue, 21 Apr 2026 10:41:48 +0200 Subject: builds download "default" part if nothing is specified (#994) --- CHANGELOG.md | 1 + src/zen/cmds/builds_cmd.cpp | 36 ++++------- src/zenremotestore/builds/buildstorageutil.cpp | 75 ++++++++++++++++++++++ .../zenremotestore/builds/buildstorageutil.h | 2 + 4 files changed, 90 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b7e800a86..ddfc47a5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ - Improvement: Incremental hydration refreshes the modification time of existing S3 CAS entries during dehydrate so lifecycle-expiration policies do not evict objects still referenced by the module - Improvement: Sensitive values in command line arguments (credentials, tokens, passwords, connection strings) are scrubbed before being written to logs or sent to Sentry - Improvement: IMDS credential refresh log message no longer exposes the first 8 characters of the AWS AccessKeyId +- Improvement: `zen builds download`, `zen builds ls`, and `zen builds prime-cache` now default to the part named `default` when no `--build-part-id`/`--build-part-name` is given (previously all parts were selected). Pass `--build-part-name=*` to select all parts. - Bugfix: `builds download` partial-block fetch decisions now account for build storage host latency - Bugfix: Transfer rate displays in `builds` commands now smooth correctly - Bugfix: Structured cache PUT errors with a detail body no longer write the HTTP response twice diff --git a/src/zen/cmds/builds_cmd.cpp b/src/zen/cmds/builds_cmd.cpp index 01b21a31f..820ca9c45 100644 --- a/src/zen/cmds/builds_cmd.cpp +++ b/src/zen/cmds/builds_cmd.cpp @@ -1536,17 +1536,12 @@ BuildsDownloadSubCmd::BuildsDownloadSubCmd(BuildsConfiguration& Config) Opts.add_option("", "l", "local-path", "Root file system folder for build", cxxopts::value(m_Path), ""); Opts.add_option("", "", "build-id", "Build Id", cxxopts::value(m_BuildId), ""); - Opts.add_option("", - "", - "build-part-id", - "Build part Ids list separated by ',', if no build-part-ids or build-part-names are given all parts will be downloaded", - cxxopts::value(m_BuildPartIds), - ""); + Opts.add_option("", "", "build-part-id", "Build part Ids list separated by ','.", cxxopts::value(m_BuildPartIds), ""); Opts.add_option("", "", "build-part-name", - "Name of the build parts list separated by ',', if no build-part-ids or build-part-names are given " - "all parts will be downloaded", + "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.", cxxopts::value(m_BuildPartNames), ""); Opts.add_option("", @@ -1624,6 +1619,7 @@ BuildsDownloadSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) std::vector BuildPartIds = ParseBuildPartIds(m_BuildPartIds, Opts); std::vector BuildPartNames = ParseBuildPartNames(m_BuildPartNames, Opts); + NormalizePartSelection(BuildPartIds, BuildPartNames, Opts.help()); EPartialBlockRequestMode PartialBlockRequestMode = ParseAllowPartialBlockRequests(Opts); @@ -1691,17 +1687,12 @@ BuildsLsSubCmd::BuildsLsSubCmd(BuildsConfiguration& Config) : BuildsSubCmdBase(C Config.AddWildcardOptions(Opts); Opts.add_option("", "", "build-id", "Build Id", cxxopts::value(m_BuildId), ""); - Opts.add_option("", - "", - "build-part-id", - "Build part Ids list separated by ',', if no build-part-ids or build-part-names are given all parts will be downloaded", - cxxopts::value(m_BuildPartIds), - ""); + Opts.add_option("", "", "build-part-id", "Build part Ids list separated by ','.", cxxopts::value(m_BuildPartIds), ""); Opts.add_option("", "", "build-part-name", - "Name of the build parts list separated by ',', if no build-part-ids or build-part-names are given " - "all parts will be downloaded", + "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.", cxxopts::value(m_BuildPartNames), ""); @@ -1751,6 +1742,7 @@ BuildsLsSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) std::vector BuildPartIds = ParseBuildPartIds(m_BuildPartIds, Opts); std::vector BuildPartNames = ParseBuildPartNames(m_BuildPartNames, Opts); + NormalizePartSelection(BuildPartIds, BuildPartNames, Opts.help()); std::unique_ptr StructuredOutput; if (!m_ResultPath.empty()) @@ -1913,17 +1905,12 @@ BuildsPrimeCacheSubCmd::BuildsPrimeCacheSubCmd(BuildsConfiguration& Config) Config.AddWorkerOptions(Opts); Config.AddZenFolderOptions(Opts); Opts.add_option("", "", "build-id", "Build Id", cxxopts::value(m_BuildId), ""); - Opts.add_option("", - "", - "build-part-id", - "Build part Ids list separated by ',', if no build-part-ids or build-part-names are given all parts will be downloaded", - cxxopts::value(m_BuildPartIds), - ""); + Opts.add_option("", "", "build-part-id", "Build part Ids list separated by ','.", cxxopts::value(m_BuildPartIds), ""); Opts.add_option("", "", "build-part-name", - "Name of the build parts list separated by ',', if no build-part-ids or build-part-names are given " - "all parts will be downloaded", + "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.", cxxopts::value(m_BuildPartNames), ""); Opts.add_option("", @@ -1962,6 +1949,7 @@ BuildsPrimeCacheSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) std::vector BuildPartIds = ParseBuildPartIds(m_BuildPartIds, Opts); std::vector BuildPartNames = ParseBuildPartNames(m_BuildPartNames, Opts); + NormalizePartSelection(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 662f3f7e6..58cdfdeef 100644 --- a/src/zenremotestore/builds/buildstorageutil.cpp +++ b/src/zenremotestore/builds/buildstorageutil.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -554,6 +555,26 @@ ResolveBuildPartNames(CbObjectView BuildObject, return Result; } +void +NormalizePartSelection(std::vector& BuildPartIds, std::vector& BuildPartNames, std::string_view HelpText) +{ + const bool HasWildcard = std::find(BuildPartNames.begin(), BuildPartNames.end(), "*") != BuildPartNames.end(); + if (HasWildcard) + { + if (BuildPartNames.size() != 1 || !BuildPartIds.empty()) + { + throw OptionParseException("'*' cannot be combined with other part names or ids", std::string(HelpText)); + } + BuildPartNames.clear(); + return; + } + + if (BuildPartIds.empty() && BuildPartNames.empty()) + { + BuildPartNames.push_back("default"); + } +} + ChunkedFolderContent GetRemoteContent(LoggerRef InLog, StorageInstance& Storage, @@ -1199,6 +1220,60 @@ namespace buildstorageoperations_testutils { TEST_SUITE_BEGIN("remotestore.buildstorageutil"); +TEST_CASE("normalizepartselection.empty_defaults_to_default") +{ + std::vector Ids; + std::vector Names; + NormalizePartSelection(Ids, Names, {}); + CHECK(Ids.empty()); + REQUIRE_EQ(Names.size(), 1u); + CHECK_EQ(Names[0], "default"); +} + +TEST_CASE("normalizepartselection.wildcard_alone_clears_names") +{ + std::vector Ids; + std::vector Names = {"*"}; + NormalizePartSelection(Ids, Names, {}); + CHECK(Ids.empty()); + CHECK(Names.empty()); +} + +TEST_CASE("normalizepartselection.wildcard_with_other_name_throws") +{ + std::vector Ids; + std::vector Names = {"*", "foo"}; + CHECK_THROWS_AS(NormalizePartSelection(Ids, Names, {}), OptionParseException); +} + +TEST_CASE("normalizepartselection.wildcard_with_ids_throws") +{ + std::vector Ids = {Oid::NewOid()}; + std::vector Names = {"*"}; + CHECK_THROWS_AS(NormalizePartSelection(Ids, Names, {}), OptionParseException); +} + +TEST_CASE("normalizepartselection.explicit_name_unchanged") +{ + std::vector Ids; + std::vector Names = {"foo"}; + NormalizePartSelection(Ids, Names, {}); + CHECK(Ids.empty()); + REQUIRE_EQ(Names.size(), 1u); + CHECK_EQ(Names[0], "foo"); +} + +TEST_CASE("normalizepartselection.ids_only_unchanged") +{ + const Oid Id = Oid::NewOid(); + std::vector Ids = {Id}; + std::vector Names; + NormalizePartSelection(Ids, Names, {}); + REQUIRE_EQ(Ids.size(), 1u); + CHECK_EQ(Ids[0], Id); + CHECK(Names.empty()); +} + TEST_CASE("buildstorageoperations.upload.folder") { using namespace buildstorageoperations_testutils; diff --git a/src/zenremotestore/include/zenremotestore/builds/buildstorageutil.h b/src/zenremotestore/include/zenremotestore/builds/buildstorageutil.h index d8b8b320d..df35f65be 100644 --- a/src/zenremotestore/include/zenremotestore/builds/buildstorageutil.h +++ b/src/zenremotestore/include/zenremotestore/builds/buildstorageutil.h @@ -101,6 +101,8 @@ std::vector> ResolveBuildPartNames(CbObjectView std::span BuildPartNames, std::uint64_t& OutPreferredMultipartChunkSize); +void NormalizePartSelection(std::vector& BuildPartIds, std::vector& BuildPartNames, std::string_view HelpText); + ChunkedFolderContent GetRemoteContent(LoggerRef InLog, StorageInstance& Storage, const Oid& BuildId, -- cgit v1.2.3