Skip to content

Download META/ab_partitions.txt from the target files zip. #1558

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Download META/ab_partitions.txt from the target files zip. #1558

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions base/cvd/cuttlefish/host/commands/cvd/fetch/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,13 @@ cf_cc_library(
"//cuttlefish/host/libs/web:android_build",
"//cuttlefish/host/libs/web:android_build_api",
"//cuttlefish/host/libs/web:android_build_string",
"//cuttlefish/host/libs/web:build_api_zip",
"//cuttlefish/host/libs/web:build_zip_name",
"//cuttlefish/host/libs/web:chrome_os_build_string",
"//cuttlefish/host/libs/web:luci_build_api",
"//cuttlefish/host/libs/web/http_client:curl_global_init",
"//cuttlefish/host/libs/zip:zip_cc",
"//cuttlefish/host/libs/zip:zip_file",
"//libbase",
],
)
Expand Down
20 changes: 18 additions & 2 deletions base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_cvd.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,13 @@
#include "cuttlefish/host/libs/web/android_build.h"
#include "cuttlefish/host/libs/web/android_build_api.h"
#include "cuttlefish/host/libs/web/android_build_string.h"
#include "cuttlefish/host/libs/web/build_api_zip.h"
#include "cuttlefish/host/libs/web/build_zip_name.h"
#include "cuttlefish/host/libs/web/chrome_os_build_string.h"
#include "cuttlefish/host/libs/web/http_client/curl_global_init.h"
#include "cuttlefish/host/libs/web/luci_build_api.h"
#include "cuttlefish/host/libs/zip/zip_cc.h"
#include "cuttlefish/host/libs/zip/zip_file.h"

namespace cuttlefish {
namespace {
Expand Down Expand Up @@ -248,10 +251,11 @@ Result<void> FetchDefaultTarget(BuildApi& build_api, const Builds& builds,
trace.CompletePhase("Desparse image files");
}

std::string target_files_name =
GetBuildZipName(*builds.default_build, "target_files");
ReadableZip target_files_zip = ReadableZip::Invalid();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than adding ReadableZip::Invalid to allow the target_files_zip to be the cross-condition variable, could we extract the ab_partitions.txt logic to be a helper we call in both blocks. Then we could have the resulting filepath be the shared variable. I think intentionally adding an invalid state is adding mental overhead to future updates, and in this case I think avoiding it does not make things worse.

Roughly thinking:

std::string ExtractAbPartitions(const std::string& target_directory, ReadableZip target_files) {
  //...
}

then the conditional becomes:

std::string ab_partitions_filepath;
if (...) {
  //...
  ReadableZip target_files = ...;
  ab_partitions_filepath = ExtractAbPartitions(target_directories.default_target_files, target_files);
} else {
  ReadableZip target_files = ...;
  ab_partitions_filepath = ExtractAbPartitions(...); 
}
CF_EXPECT(config.AddFilesToConfig(...));

Copy link
Member Author

@Databean Databean Oct 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the call out.

I forgot that I had already made this PR earlier and rewrote this code in #1738 . It doesn't include ReadableZip::Invalid and instead uses a ? : operator to initialize the ReadableZip value.

if (builds.system || flags.download_target_files_zip) {
LOG(INFO) << "Downloading target files zip for " << *builds.default_build;
std::string target_files_name =
GetBuildZipName(*builds.default_build, "target_files");
std::string target_files = CF_EXPECT(build_api.DownloadFile(
*builds.default_build, target_directories.default_target_files,
target_files_name));
Expand All @@ -260,7 +264,19 @@ Result<void> FetchDefaultTarget(BuildApi& build_api, const Builds& builds,
CF_EXPECT(config.AddFilesToConfig(FileSource::DEFAULT_BUILD,
default_build_id, default_build_target,
{target_files}, target_directories.root));
target_files_zip = CF_EXPECT(ZipOpenRead(target_files));
} else {
target_files_zip =
CF_EXPECT(OpenZip(build_api, *builds.default_build, target_files_name));
}
std::string ab_partitions = "ab_partitions.txt";
std::string ab_partitions_txt_out =
target_directories.default_target_files + "/" + ab_partitions;
CF_EXPECT(ExtractFile(target_files_zip, "META/" + ab_partitions,
ab_partitions_txt_out));
CF_EXPECT(config.AddFilesToConfig(
FileSource::DEFAULT_BUILD, default_build_id, default_build_target,
{ab_partitions_txt_out}, target_directories.root));
return {};
}

Expand Down
4 changes: 4 additions & 0 deletions base/cvd/cuttlefish/host/libs/zip/zip_cc.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,10 @@ Result<ReadableZip> ReadableZip::FromSource(SeekableZipSource source) {
std::move(fake_writable_source)));
}

ReadableZip ReadableZip::Invalid() {
return ReadableZip(std::unique_ptr<Impl>());
}

ReadableZip::ReadableZip(ReadableZip&&) = default;
ReadableZip::~ReadableZip() = default;
ReadableZip& ReadableZip::operator=(ReadableZip&&) = default;
Expand Down
1 change: 1 addition & 0 deletions base/cvd/cuttlefish/host/libs/zip/zip_cc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ class ReadableZip {
friend class WritableZipSource;

static Result<ReadableZip> FromSource(SeekableZipSource);
static ReadableZip Invalid();

ReadableZip(ReadableZip&&);
virtual ~ReadableZip();
Expand Down
Loading