diff --git a/base/cvd/cuttlefish/host/commands/cvd/fetch/BUILD.bazel b/base/cvd/cuttlefish/host/commands/cvd/fetch/BUILD.bazel index b32d774b19e..6d65b1517de 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/fetch/BUILD.bazel +++ b/base/cvd/cuttlefish/host/commands/cvd/fetch/BUILD.bazel @@ -165,7 +165,6 @@ cf_cc_library( "//cuttlefish/host/commands/cvd/fetch:fetch_context", "//cuttlefish/host/commands/cvd/fetch:fetch_cvd_parser", "//cuttlefish/host/commands/cvd/fetch:fetch_tracer", - "//cuttlefish/host/commands/cvd/fetch:host_package", "//cuttlefish/host/commands/cvd/fetch:host_tools_target", "//cuttlefish/host/commands/cvd/fetch:substitute", "//cuttlefish/host/commands/cvd/fetch:target_directories", @@ -222,23 +221,6 @@ cf_cc_library( hdrs = ["get_optional.h"], ) -cf_cc_library( - name = "host_package", - srcs = ["host_package.cc"], - hdrs = ["host_package.h"], - deps = [ - "//cuttlefish/common/libs/utils:archive", - "//cuttlefish/common/libs/utils:files", - "//cuttlefish/common/libs/utils:result", - "//cuttlefish/host/commands/cvd/fetch:fetch_tracer", - "//cuttlefish/host/commands/cvd/fetch:substitute", - "//cuttlefish/host/libs/web:android_build", - "//cuttlefish/host/libs/web:android_build_api", - "//cuttlefish/host/libs/web:build_api", - "//libbase", - ], -) - proto_library( name = "host_pkg_migration_proto", srcs = ["host_pkg_migration.proto"], diff --git a/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_context.cc b/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_context.cc index b7661bc6aac..1f958fffcf1 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_context.cc +++ b/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_context.cc @@ -30,6 +30,7 @@ #include "absl/strings/match.h" #include "absl/strings/strip.h" +#include "cuttlefish/common/libs/utils/archive.h" #include "cuttlefish/common/libs/utils/files.h" #include "cuttlefish/common/libs/utils/result.h" #include "cuttlefish/host/commands/cvd/fetch/builds.h" @@ -54,6 +55,10 @@ FetchArtifact::FetchArtifact(FetchBuildContext& context, std::string artifact_name) : fetch_build_context_(context), artifact_name_(artifact_name) {} +bool FetchArtifact::IsZip() const { + return absl::EndsWith(artifact_name_, ".zip"); +} + Result FetchArtifact::Download() { CF_EXPECT(DownloadTo(artifact_name_)); return {}; @@ -77,7 +82,7 @@ Result FetchArtifact::DownloadTo(std::string local_path) { CF_EXPECT(RenameFile(downloaded, new_path)); downloaded_path_ = new_path; - if (absl::EndsWith(downloaded_path_, ".zip")) { + if (IsZip()) { zip_ = CF_EXPECT(ZipOpenRead(downloaded_path_)); } } else { @@ -91,6 +96,7 @@ Result FetchArtifact::DownloadTo(std::string local_path) { Result FetchArtifact::AsZip() { if (!zip_) { + CF_EXPECT(IsZip(), "File name doesn't end in .zip"); zip_ = CF_EXPECT( ::cuttlefish::OpenZip(fetch_build_context_.fetch_context_.build_api_, fetch_build_context_.build_, artifact_name_)); @@ -104,15 +110,38 @@ Result FetchArtifact::ExtractAll() { } Result FetchArtifact::ExtractAll(const std::string& local_path) { - ReadableZip* zip = CF_EXPECT(AsZip()); - size_t entries = CF_EXPECT(zip->NumEntries()); - for (uint64_t i = 0; i < entries; i++) { - std::string member_name = CF_EXPECT(zip->EntryName(i)); - CF_EXPECT(!absl::StartsWith(member_name, ".")); - CF_EXPECT(!absl::StartsWith(member_name, "/")); - CF_EXPECT(!absl::StrContains(member_name, "/../")); - std::string extract_path = fmt::format("{}/{}", local_path, member_name); - CF_EXPECT(ExtractOneTo(member_name, extract_path)); + if (IsZip()) { + ReadableZip* zip = CF_EXPECT(AsZip()); + size_t entries = CF_EXPECT(zip->NumEntries()); + for (uint64_t i = 0; i < entries; i++) { + std::string member_name = CF_EXPECT(zip->EntryName(i)); + CF_EXPECT(!absl::StartsWith(member_name, ".")); + CF_EXPECT(!absl::StartsWith(member_name, "/")); + CF_EXPECT(!absl::StrContains(member_name, "/../")); + std::string extract_path = fmt::format("{}/{}", local_path, member_name); + CF_EXPECT(ExtractOneTo(member_name, extract_path)); + } + } else { + if (downloaded_path_.empty()) { + CF_EXPECT(Download()); + } + std::string extract_path = fmt::format( + "{}/{}", fetch_build_context_.target_directory_, local_path); + std::vector extracted = + CF_EXPECT(ExtractArchiveContents(downloaded_path_, extract_path, true)); + + std::string extract_phase = + fmt::format("Extracted '{}'", fmt::join(extracted, ",")); + fetch_build_context_.trace_.CompletePhase(std::move(extract_phase)); + + fetch_build_context_.DesparseFiles(extracted); + for (const std::string& file : extracted) { + // Hack: avoid assemble_cvd incorrect match against `ti50-emulator-kernel` + // file when it looks for files ending in `kernel`. b/461569369#comment2 + if (!absl::EndsWith(file, "kernel")) { + CF_EXPECT(fetch_build_context_.AddFileToConfig(file)); + } + } } return {}; } @@ -124,6 +153,8 @@ Result FetchArtifact::ExtractOne(const std::string& member_name) { Result FetchArtifact::ExtractOneTo(const std::string& member_name, const std::string& local_path) { + CF_EXPECT(IsZip(), "Extracting individual members requires a zip archive."); + ReadableZip* zip = CF_EXPECT(AsZip()); std::string extract_path = fmt::format("{}/{}", fetch_build_context_.target_directory_, local_path); @@ -226,11 +257,15 @@ std::ostream& operator<<(std::ostream& out, const FetchBuildContext& context) { FetchContext::FetchContext(BuildApi& build_api, const TargetDirectories& target_directories, - const Builds& builds, FetcherConfig& fetcher_config, - FetchTracer& tracer) + const std::string& host_package_directory, + const Builds& builds, + const Build& host_package_build, + FetcherConfig& fetcher_config, FetchTracer& tracer) : build_api_(build_api), target_directories_(target_directories), + host_package_directory_(host_package_directory), builds_(builds), + host_package_build_(host_package_build), fetcher_config_(fetcher_config), tracer_(tracer) {} @@ -304,4 +339,10 @@ std::optional FetchContext::OtaToolsBuild() { } } +FetchBuildContext FetchContext::HostPackageBuild() { + return FetchBuildContext(*this, host_package_build_, host_package_directory_, + FileSource::HOST_PACKAGE_BUILD, + tracer_.NewTrace("Host package")); +} + } // namespace cuttlefish diff --git a/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_context.h b/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_context.h index 9c794279fcd..29aefe088fc 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_context.h +++ b/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_context.h @@ -54,6 +54,8 @@ class FetchArtifact { FetchArtifact(class FetchBuildContext&, std::string artifact_name); + bool IsZip() const; + FetchBuildContext& fetch_build_context_; std::string artifact_name_; std::string downloaded_path_; @@ -112,8 +114,9 @@ std::ostream& operator<<(std::ostream&, const FetchBuildContext&); */ class FetchContext { public: - FetchContext(BuildApi&, const TargetDirectories&, const Builds&, - FetcherConfig&, FetchTracer&); + FetchContext(BuildApi&, const TargetDirectories&, + const std::string& host_package_directory, const Builds&, + const Build& host_package_build, FetcherConfig&, FetchTracer&); std::optional DefaultBuild(); std::optional SystemBuild(); @@ -122,6 +125,7 @@ class FetchContext { std::optional BootloaderBuild(); std::optional AndroidEfiLoaderBuild(); std::optional OtaToolsBuild(); + FetchBuildContext HostPackageBuild(); private: friend class FetchArtifact; @@ -129,7 +133,9 @@ class FetchContext { BuildApi& build_api_; const TargetDirectories& target_directories_; + const std::string& host_package_directory_; const Builds& builds_; + const Build& host_package_build_; FetcherConfig& fetcher_config_; FetchTracer& tracer_; }; diff --git a/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_cvd.cc b/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_cvd.cc index c2a0d0f5a03..2ee7fcb1a8f 100644 --- a/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_cvd.cc +++ b/base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_cvd.cc @@ -42,8 +42,8 @@ #include "cuttlefish/host/commands/cvd/fetch/fetch_context.h" #include "cuttlefish/host/commands/cvd/fetch/fetch_cvd_parser.h" #include "cuttlefish/host/commands/cvd/fetch/fetch_tracer.h" -#include "cuttlefish/host/commands/cvd/fetch/host_package.h" #include "cuttlefish/host/commands/cvd/fetch/host_tools_target.h" +#include "cuttlefish/host/commands/cvd/fetch/substitute.h" #include "cuttlefish/host/commands/cvd/fetch/target_directories.h" #include "cuttlefish/host/libs/config/fetcher_config.h" #include "cuttlefish/host/libs/config/file_source.h" @@ -349,6 +349,26 @@ Result FetchOtaToolsTarget(FetchBuildContext& context, return {}; } +Result FetchHostPackage(FetchBuildContext context, + const bool keep_archives) { + LOG(INFO) << "Preparing host package for " << context; + // This function is called asynchronously, so it may take a while to start. + // Complete a phase here to ensure that delay is not counted in the download + // time. + // The download time will still include time spent waiting for the mutex in + // the build_api though. + std::string host_tools_name = + context.GetFilepath().value_or("cvd-host_package.tar.gz"); + FetchArtifact artifact = context.Artifact(host_tools_name); + CF_EXPECT(artifact.Download()); + CF_EXPECT(artifact.ExtractAll()); + if (!keep_archives) { + CF_EXPECT(artifact.DeleteLocalFile()); + } + + return {}; +} + Result FetchChromeOsTarget( LuciBuildApi& luci_build_api, const ChromeOsBuildString& chrome_os_build_string, @@ -439,19 +459,23 @@ Result> Fetch(const FetchFlags& flags, downloaders.AndroidBuild(), host_target, fallback_host_build)); prefetch_trace.CompletePhase("GetBuilds"); - auto host_package_future = std::async( - std::launch::async, FetchHostPackage, - std::ref(downloaders.AndroidBuild()), std::cref(host_target_build), - std::cref(host_target.host_tools_directory), - std::cref(flags.keep_downloaded_archives), - std::cref(flags.host_substitutions), tracer.NewTrace("Host Package")); size_t count = 1; std::vector fetch_results; for (const auto& target : targets) { + std::future> host_package_future; FetcherConfig config; FetchContext fetch_context(downloaders.AndroidBuild(), target.directories, - target.builds, config, tracer); + host_target.host_tools_directory, target.builds, + host_target_build, config, tracer); LOG(INFO) << "Starting fetch to \"" << target.directories.root << "\""; + + if (count == 1) { + FetchBuildContext host_context = fetch_context.HostPackageBuild(); + host_package_future = + std::async(std::launch::async, FetchHostPackage, host_context, + flags.keep_downloaded_archives); + } + CF_EXPECT(FetchTarget(fetch_context, target.download_flags, flags.keep_downloaded_archives)); @@ -461,6 +485,11 @@ Result> Fetch(const FetchFlags& flags, flags.keep_downloaded_archives, config, tracer.NewTrace("ChromeOS"))); } + if (count == 1) { + CF_EXPECT(host_package_future.valid()); + CF_EXPECT(host_package_future.get()); + } + const std::string config_path = CF_EXPECT(SaveConfig(config, target.directories.root)); count++; @@ -472,7 +501,8 @@ Result> Fetch(const FetchFlags& flags, << "' (" << count << " out of " << targets.size() << ")"; } LOG(DEBUG) << "Waiting for host package fetch"; - CF_EXPECT(host_package_future.get()); + CF_EXPECT(HostPackageSubstitution(host_target.host_tools_directory, + flags.host_substitutions)); LOG(DEBUG) << "Performance stats:\n" << tracer.ToStyledString(); LOG(INFO) << "Completed all fetches"; diff --git a/base/cvd/cuttlefish/host/commands/cvd/fetch/host_package.cc b/base/cvd/cuttlefish/host/commands/cvd/fetch/host_package.cc deleted file mode 100644 index d66c56b736c..00000000000 --- a/base/cvd/cuttlefish/host/commands/cvd/fetch/host_package.cc +++ /dev/null @@ -1,65 +0,0 @@ -// -// Copyright (C) 2019 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "cuttlefish/host/commands/cvd/fetch/host_package.h" - -#include -#include - -#include -#include -#include - -#include - -#include "cuttlefish/common/libs/utils/archive.h" -#include "cuttlefish/common/libs/utils/files.h" -#include "cuttlefish/common/libs/utils/result.h" -#include "cuttlefish/host/commands/cvd/fetch/fetch_tracer.h" -#include "cuttlefish/host/commands/cvd/fetch/substitute.h" -#include "cuttlefish/host/libs/web/android_build.h" -#include "cuttlefish/host/libs/web/android_build_api.h" -#include "cuttlefish/host/libs/web/build_api.h" - -namespace cuttlefish { - -Result FetchHostPackage( - BuildApi& build_api, const Build& build, const std::string& target_dir, - const bool keep_archives, - const std::vector& host_substitutions, - FetchTracer::Trace trace) { - LOG(INFO) << "Preparing host package for " << build; - // This function is called asynchronously, so it may take a while to start. - // Complete a phase here to ensure that delay is not counted in the download - // time. - // The download time will still include time spent waiting for the mutex in - // the build_api though. - trace.CompletePhase("Async start delay"); - auto host_tools_name = GetFilepath(build).value_or("cvd-host_package.tar.gz"); - std::string host_tools_filepath = - CF_EXPECT(build_api.DownloadFile(build, target_dir, host_tools_name)); - trace.CompletePhase("Download", FileSize(host_tools_filepath)); - - CF_EXPECT( - ExtractArchiveContents(host_tools_filepath, target_dir, keep_archives)); - trace.CompletePhase("Extract"); - - CF_EXPECT(HostPackageSubstitution(target_dir, host_substitutions)); - - trace.CompletePhase("Substitute"); - return {}; -} - -} // namespace cuttlefish diff --git a/base/cvd/cuttlefish/host/libs/config/fetcher_config.cpp b/base/cvd/cuttlefish/host/libs/config/fetcher_config.cpp index 9c985755ea0..64395ad11d6 100644 --- a/base/cvd/cuttlefish/host/libs/config/fetcher_config.cpp +++ b/base/cvd/cuttlefish/host/libs/config/fetcher_config.cpp @@ -36,6 +36,7 @@ #include #include "absl/strings/match.h" #include "absl/strings/str_replace.h" +#include "absl/strings/strip.h" #include "cuttlefish/common/libs/utils/files.h" #include "cuttlefish/common/libs/utils/result.h" @@ -152,14 +153,44 @@ Json::Value CvdFileToJson(const CvdFile& cvd_file) { return json; } -Result NormalizePath(std::string path) { - CF_EXPECT(!absl::StrContains(path, "..")); +std::string NormalizePath(std::string path) { + std::string original_path = path; + while (absl::StrContains(path, "/./")) { + absl::StrReplaceAll({{"/./", "/"}}, &path); + } while (absl::StrContains(path, "//")) { absl::StrReplaceAll({{"//", "/"}}, &path); } + if (absl::StartsWith(path, "./")) { + std::string_view path_view = path; + while (absl::ConsumePrefix(&path_view, "./")) { + } + path = path_view; + } return path; } +std::string FindRelativePath(std::string_view path, std::string relative_dir) { + if (!absl::EndsWith(relative_dir, "/")) { + relative_dir += "/"; + } + int num_parents = 0; + std::string_view common_parent = relative_dir; + while (!absl::StartsWith(path, common_parent)) { + num_parents++; + size_t last_slash = common_parent.find_last_of("/"); + CHECK(last_slash != std::string::npos); + common_parent = common_parent.substr(0, last_slash); + CHECK(!common_parent.empty()); + } + std::stringstream out; + for (int i = 0; i < num_parents; i++) { + out << "../"; + } + out << absl::StripPrefix(path, common_parent); + return out.str(); +} + } // namespace bool FetcherConfig::add_cvd_file(const CvdFile& file, bool override_entry) { @@ -214,7 +245,7 @@ Result FetcherConfig::RemoveFileFromConfig(const std::string& path) { if (!dictionary_.isMember(kCvdFiles)) { return {}; } - std::string normalized = CF_EXPECT(NormalizePath(std::string(path))); + std::string normalized = NormalizePath(std::string(path)); auto& json_files = dictionary_[kCvdFiles]; CF_EXPECTF(json_files.isMember(normalized), "Unknown file '{}'", normalized); json_files.removeMember(normalized); @@ -225,19 +256,12 @@ Result BuildFetcherConfigMember(FileSource purpose, const std::string& build_id, const std::string& build_target, const std::string& path, - const std::string& directory_prefix) { - std::string_view local_path(path); - if (!android::base::ConsumePrefix(&local_path, directory_prefix)) { - LOG(ERROR) << "Failed to remove prefix " << directory_prefix << " from " - << local_path; - return {}; - } - while (android::base::StartsWith(local_path, "/")) { - android::base::ConsumePrefix(&local_path, "/"); - } - std::string normalized = CF_EXPECT(NormalizePath(std::string(local_path))); + const std::string& relative_dir) { + std::string normalized = NormalizePath(path); + std::string normalized_dir = NormalizePath(relative_dir); + std::string relative_path = FindRelativePath(normalized, normalized_dir); // TODO(schuffelen): Do better for local builds here. - return CvdFile(purpose, build_id, build_target, normalized); + return CvdFile(purpose, build_id, build_target, relative_path); } FetcherConfigs FetcherConfigs::Create(std::vector configs) { diff --git a/base/cvd/cuttlefish/host/libs/config/fetcher_config.h b/base/cvd/cuttlefish/host/libs/config/fetcher_config.h index bc101ef0eac..95f095d7747 100644 --- a/base/cvd/cuttlefish/host/libs/config/fetcher_config.h +++ b/base/cvd/cuttlefish/host/libs/config/fetcher_config.h @@ -86,7 +86,7 @@ Result BuildFetcherConfigMember(FileSource purpose, const std::string& build_id, const std::string& build_target, const std::string& path, - const std::string& directory_prefix); + const std::string& relative_dir); class FetcherConfigs { public: