Skip to content

Conversation

@Databean
Copy link
Member

The full target files zip file is not downloaded unless it is requested by other flags.

Bug: b/439877215

The full target files zip file is not downloaded unless it is requested
by other flags.

Bug: b/439877215
@Databean Databean requested a review from cjreynol August 19, 2025 19:22
@Databean Databean added kokoro:run Run e2e tests. kokoro:force-run Trigger a presubmit build unconditionally. labels Aug 19, 2025
@GoogleCuttlefishTesterBot GoogleCuttlefishTesterBot removed kokoro:run Run e2e tests. kokoro:force-run Trigger a presubmit build unconditionally. labels Aug 19, 2025
@Databean Databean added kokoro:force-run Trigger a presubmit build unconditionally. kokoro:run Run e2e tests. labels Aug 19, 2025
@GoogleCuttlefishTesterBot GoogleCuttlefishTesterBot removed kokoro:run Run e2e tests. kokoro:force-run Trigger a presubmit build unconditionally. labels Aug 19, 2025
@Databean Databean added kokoro:force-run Trigger a presubmit build unconditionally. kokoro:run Run e2e tests. labels Aug 20, 2025
@GoogleCuttlefishTesterBot GoogleCuttlefishTesterBot removed kokoro:run Run e2e tests. kokoro:force-run Trigger a presubmit build unconditionally. labels Aug 20, 2025

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

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.

Databean added a commit to Databean/android-cuttlefish that referenced this pull request Oct 23, 2025
This enables a new flow to download the super image components out of
the target_files zip, using a path that only downloads the fragments of
the zip necessary.

Downloading the super image fragments is comparable to downloading the
image zip, but these files can be used to build a super image ourselves
with any last-minute modifications we need.

Some performance and correctness improvements in the partial zip
download are included in this change: increasing the chunk size for
partial downloads, and working around libzip doing read-all behavior
from its read calls. The partial zip download takes twice the time of
the full image zip download, so hopefully more performance improvements
are possible afterward.

This change also supersedes
google#1558 .

Bug: b/439877215
@Databean Databean closed this Oct 23, 2025
Databean added a commit to Databean/android-cuttlefish that referenced this pull request Oct 23, 2025
This enables a new flow to download the super image components out of
the target_files zip, using a path that only downloads the fragments of
the zip necessary.

Downloading the super image fragments is comparable to downloading the
image zip, but these files can be used to build a super image ourselves
with any last-minute modifications we need.

Some performance and correctness improvements in the partial zip
download are included in this change: increasing the chunk size for
partial downloads, and working around libzip doing read-all behavior
from its read calls. The partial zip download takes twice the time of
the full image zip download, so hopefully more performance improvements
are possible afterward.

This change also supersedes
google#1558 .

Bug: b/439877215
Databean added a commit to Databean/android-cuttlefish that referenced this pull request Oct 23, 2025
This enables a new flow to download the super image components out of
the target_files zip, using a path that only downloads the fragments of
the zip necessary.

Downloading the super image fragments is comparable to downloading the
image zip, but these files can be used to build a super image ourselves
with any last-minute modifications we need.

Some performance and correctness improvements in the partial zip
download are included in this change: increasing the chunk size for
partial downloads, and working around libzip doing read-all behavior
from its read calls. The partial zip download takes twice the time of
the full image zip download, so hopefully more performance improvements
are possible afterward.

This change also supersedes
google#1558 .

Bug: b/439877215
Databean added a commit to Databean/android-cuttlefish that referenced this pull request Oct 23, 2025
This enables a new flow to download the super image components out of
the target_files zip, using a path that only downloads the fragments of
the zip necessary.

Downloading the super image fragments is comparable to downloading the
image zip, but these files can be used to build a super image ourselves
with any last-minute modifications we need.

Some performance and correctness improvements in the partial zip
download are included in this change: increasing the chunk size for
partial downloads, and working around libzip doing read-all behavior
from its read calls. The partial zip download takes twice the time of
the full image zip download, so hopefully more performance improvements
are possible afterward.

This change also supersedes
google#1558 .

Bug: b/439877215
github-merge-queue bot pushed a commit that referenced this pull request Oct 24, 2025
This enables a new flow to download the super image components out of
the target_files zip, using a path that only downloads the fragments of
the zip necessary.

Downloading the super image fragments is comparable to downloading the
image zip, but these files can be used to build a super image ourselves
with any last-minute modifications we need.

Some performance and correctness improvements in the partial zip
download are included in this change: increasing the chunk size for
partial downloads, and working around libzip doing read-all behavior
from its read calls. The partial zip download takes twice the time of
the full image zip download, so hopefully more performance improvements
are possible afterward.

This change also supersedes
#1558 .

Bug: b/439877215
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants