Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit44b9708

Browse files
committed
Resolve review comments
1 parent4f338cf commit44b9708

File tree

6 files changed

+387
-586
lines changed

6 files changed

+387
-586
lines changed

‎base/cvd/cuttlefish/host/libs/web/cas/cas_downloader.cpp‎

Lines changed: 156 additions & 169 deletions
Original file line numberDiff line numberDiff line change
@@ -168,22 +168,22 @@ inline std::string ToSeconds(int timeout) {
168168

169169
Json::ValueConvertToConfigFlags(const CasDownloaderFlags& flags) {
170170
Json::Value config_flags;
171-
config_flags["cache-dir"] = flags.cache_dir.value;
172-
config_flags["cache-max-size"] = flags.cache_max_size.value;
173-
config_flags["cache-lock"] = flags.cache_lock.value;
174-
config_flags["use-hardlink"] = flags.use_hardlink.value;
175-
config_flags["cas-concurrency"] = flags.cas_concurrency.value;
176-
config_flags["memory-limit"] = flags.memory_limit.value;
177-
config_flags["rpc-timeout"] =ToSeconds(flags.rpc_timeout.value);
171+
config_flags["cache-dir"] = flags.cache_dir.value();
172+
config_flags["cache-max-size"] = flags.cache_max_size.value();
173+
config_flags["cache-lock"] = flags.cache_lock.value();
174+
config_flags["use-hardlink"] = flags.use_hardlink.value();
175+
config_flags["cas-concurrency"] = flags.cas_concurrency.value();
176+
config_flags["memory-limit"] = flags.memory_limit.value();
177+
config_flags["rpc-timeout"] =ToSeconds(flags.rpc_timeout.value());
178178
config_flags["get-capabilities-timeout"] =
179-
ToSeconds(flags.get_capabilities_timeout.value);
180-
config_flags["get-tree-timeout"] =ToSeconds(flags.get_tree_timeout.value);
179+
ToSeconds(flags.get_capabilities_timeout.value());
180+
config_flags["get-tree-timeout"] =ToSeconds(flags.get_tree_timeout.value());
181181
config_flags["batch-read-blobs-timeout"] =
182-
ToSeconds(flags.batch_read_blobs_timeout.value);
182+
ToSeconds(flags.batch_read_blobs_timeout.value());
183183
config_flags["batch-update-blobs-timeout"] =
184-
ToSeconds(flags.batch_update_blobs_timeout.value);
185-
config_flags["version"] = flags.version.value;
186-
config_flags["invocation-id"] = flags.invocation_id.value;
184+
ToSeconds(flags.batch_update_blobs_timeout.value());
185+
config_flags["version"] = flags.version.value();
186+
config_flags["invocation-id"] = flags.invocation_id.value();
187187
return config_flags;
188188
}
189189

@@ -208,209 +208,196 @@ Command GetCommand(const std::string downloader_path,
208208
return cmd;
209209
}
210210

211-
}// namespace
212-
213211
// Helper function to merge CLI values into config flags when CLI takes
214212
// precedence. This function implements the priority rule: if a flag was
215-
// specified on the CLI (user_specified=true), its value overrides any value
213+
// specified on the CLI (user_provided()=true), its value overrides any value
216214
// from the config file.
217-
namespace {
218215
voidMergeCliValuesIntoConfig(const CasDownloaderFlags& flags,
219216
Json::Value& config_flags) {
220217
// Merge CLI values (if provided) on top of config file values so CLI wins.
221218
// Use the same keys as ConvertToConfigFlags.
222219

223-
if (flags.cache_dir.user_specified) {
224-
config_flags["cache-dir"] = flags.cache_dir.value;
220+
if (flags.cache_dir.user_provided()) {
221+
config_flags["cache-dir"] = flags.cache_dir.value();
225222
}
226-
if (flags.invocation_id.user_specified) {
227-
config_flags["invocation-id"] = flags.invocation_id.value;
223+
if (flags.invocation_id.user_provided()) {
224+
config_flags["invocation-id"] = flags.invocation_id.value();
228225
}
229-
if (flags.cache_max_size.user_specified) {
230-
config_flags["cache-max-size"] = flags.cache_max_size.value;
226+
if (flags.cache_max_size.user_provided()) {
227+
config_flags["cache-max-size"] = flags.cache_max_size.value();
231228
}
232-
if (flags.cache_lock.user_specified) {
233-
config_flags["cache-lock"] = flags.cache_lock.value;
229+
if (flags.cache_lock.user_provided()) {
230+
config_flags["cache-lock"] = flags.cache_lock.value();
234231
}
235-
if (flags.use_hardlink.user_specified) {
236-
config_flags["use-hardlink"] = flags.use_hardlink.value;
232+
if (flags.use_hardlink.user_provided()) {
233+
config_flags["use-hardlink"] = flags.use_hardlink.value();
237234
}
238-
if (flags.cas_concurrency.user_specified) {
239-
config_flags["cas-concurrency"] = flags.cas_concurrency.value;
235+
if (flags.cas_concurrency.user_provided()) {
236+
config_flags["cas-concurrency"] = flags.cas_concurrency.value();
240237
}
241-
if (flags.memory_limit.user_specified) {
242-
config_flags["memory-limit"] = flags.memory_limit.value;
238+
if (flags.memory_limit.user_provided()) {
239+
config_flags["memory-limit"] = flags.memory_limit.value();
243240
}
244-
if (flags.rpc_timeout.user_specified) {
245-
config_flags["rpc-timeout"] =ToSeconds(flags.rpc_timeout.value);
241+
if (flags.rpc_timeout.user_provided()) {
242+
config_flags["rpc-timeout"] =ToSeconds(flags.rpc_timeout.value());
246243
}
247-
if (flags.get_capabilities_timeout.user_specified) {
244+
if (flags.get_capabilities_timeout.user_provided()) {
248245
config_flags["get-capabilities-timeout"] =
249-
ToSeconds(flags.get_capabilities_timeout.value);
246+
ToSeconds(flags.get_capabilities_timeout.value());
250247
}
251-
if (flags.get_tree_timeout.user_specified) {
252-
config_flags["get-tree-timeout"] =ToSeconds(flags.get_tree_timeout.value);
248+
if (flags.get_tree_timeout.user_provided()) {
249+
config_flags["get-tree-timeout"] =
250+
ToSeconds(flags.get_tree_timeout.value());
253251
}
254-
if (flags.batch_read_blobs_timeout.user_specified) {
252+
if (flags.batch_read_blobs_timeout.user_provided()) {
255253
config_flags["batch-read-blobs-timeout"] =
256-
ToSeconds(flags.batch_read_blobs_timeout.value);
254+
ToSeconds(flags.batch_read_blobs_timeout.value());
257255
}
258-
if (flags.batch_update_blobs_timeout.user_specified) {
256+
if (flags.batch_update_blobs_timeout.user_provided()) {
259257
config_flags["batch-update-blobs-timeout"] =
260-
ToSeconds(flags.batch_update_blobs_timeout.value);
258+
ToSeconds(flags.batch_update_blobs_timeout.value());
261259
}
262-
if (flags.version.user_specified) {
263-
config_flags["version"] = flags.version.value;
260+
if (flags.version.user_provided()) {
261+
config_flags["version"] = flags.version.value();
264262
}
265263
}
264+
265+
Result<bool>HasConfigFile(const FlagValue<std::string>& config_filepath_flag) {
266+
// Determine whether there's a config file to load. The
267+
// `cas_config_filepath` may contain a default path (empty if none). If the
268+
// user explicitly provided a config filepath and it does not exist, that's
269+
// an error. If a config file exists (either user-provided or default),
270+
// we'll load it and apply its values unless the corresponding CLI flag was
271+
// provided.
272+
const std::string& config_filepath = config_filepath_flag.value();
273+
if (config_filepath.empty()) {
274+
returnfalse;
275+
}
276+
if (FileExists(config_filepath)) {
277+
returntrue;
278+
}
279+
if (config_filepath_flag.user_provided()) {
280+
returnCF_ERRF("CAS Config file not found: {}", config_filepath);
281+
}
282+
returnfalse;// Path was a default, but file doesn't exist.
283+
}
284+
266285
}// namespace
267286

268287
Result<std::unique_ptr<CasDownloader>>CasDownloader::Create(
269288
const CasDownloaderFlags& cas_downloader_flags,
270289
const std::string& service_account_filepath) {
271-
// Forward to an internal implementation that uses CF_EXPECT for concise
272-
// error propagation. The public Create() wrapper will log the human-usable
273-
// error message (formatted for the environment) before returning the
274-
// failure, so callers get a clear explanation why CAS downloading is
275-
// disabled.
276-
auto CreateImpl = [&](const CasDownloaderFlags& cas_downloader_flags,
277-
const std::string& service_account_filepath)
278-
-> Result<std::unique_ptr<CasDownloader>> {
279-
// Start with values from the FlagValue wrappers (these contain defaults
280-
// and reflect any CLI-provided values via .user_specified).
281-
std::string downloader_path = cas_downloader_flags.downloader_path.value;
282-
bool prefer_uncompressed = cas_downloader_flags.prefer_uncompressed.value;
283-
std::vector<std::string> cas_flags;
284-
285-
// Determine whether there's a config file to load. The
286-
// `cas_config_filepath` may contain a default path (empty if none). If the
287-
// user explicitly provided a config filepath and it does not exist, that's
288-
// an error. If a config file exists (either user-provided or default),
289-
// we'll load it and apply its values unless the corresponding CLI flag was
290-
// provided.
291-
std::string config_filepath =
292-
cas_downloader_flags.cas_config_filepath.value;
293-
Json::Value config_flags;
294-
bool has_config_file =false;
295-
if (!config_filepath.empty() &&FileExists(config_filepath)) {
296-
has_config_file =true;
297-
}elseif (!config_filepath.empty() &&
298-
cas_downloader_flags.cas_config_filepath.user_specified) {
299-
// User requested a config file path that doesn't exist.
300-
returnCF_ERRF("CAS Config file not found: {}", config_filepath);
301-
}
290+
auto result =CreateImpl(cas_downloader_flags, service_account_filepath);
291+
if (result.ok()) {
292+
return result;
293+
}
294+
// Ensure callers and logs clearly indicate that CAS downloading is
295+
// disabled and why, using the same environment-aware formatting that
296+
// test helpers use.
297+
LOG(INFO) <<"CAS downloading disabled:" << result.error().FormatForEnv();
298+
returnandroid::base::unexpected(std::move(result).error());
299+
}
300+
301+
Result<std::unique_ptr<CasDownloader>>CasDownloader::CreateImpl(
302+
const CasDownloaderFlags& cas_downloader_flags,
303+
const std::string& service_account_filepath) {
304+
// Start with values from the FlagValue wrappers (these contain defaults
305+
// and reflect any CLI-provided values via .user_provided()).
306+
std::string downloader_path = cas_downloader_flags.downloader_path.value();
307+
bool prefer_uncompressed = cas_downloader_flags.prefer_uncompressed.value();
308+
std::vector<std::string> cas_flags;
302309

303-
if (!has_config_file) {
304-
// No config file available: use CLI values (or defaults if CLI didn't set
305-
// them). Convert current flag values into config_flags for downstream
306-
// processing.
307-
LOG(INFO) <<"Using CAS downloader flags from command line or defaults.";
308-
config_flags =ConvertToConfigFlags(cas_downloader_flags);
310+
Json::Value config_flags;
311+
bool has_config_file =
312+
CF_EXPECT(HasConfigFile(cas_downloader_flags.cas_config_filepath));
313+
if (has_config_file) {
314+
// A config file exists. Load it, then merge CLI values on top so
315+
// that CLI takes precedence.
316+
const std::string& config_filepath =
317+
cas_downloader_flags.cas_config_filepath.value();
318+
bool is_default_config = (config_filepath ==kDefaultCasConfigFilePath);
319+
if (is_default_config) {
320+
LOG(INFO) <<"Using default CAS config from:" << config_filepath;
309321
}else {
310-
// Load config file. We'll merge CLI values on top of the config file so
311-
// that CLI takes precedence.
312-
bool is_default_config = (config_filepath ==kDefaultCasConfigFilePath);
313-
if (is_default_config) {
314-
LOG(INFO) <<"Using default CAS config from:" << config_filepath;
315-
}else {
316-
LOG(INFO) <<"Using CAS config from:" << config_filepath;
317-
}
318-
std::string config_contents =
319-
CF_EXPECT(ReadFileContents(config_filepath));
320-
Json::Value config =CF_EXPECT(ParseJson(config_contents));
321-
322-
// Base config flags from file (may be empty object)
323-
config_flags = config[kKeyFlags];
324-
325-
// downloader-path and prefer-uncompressed are top-level in the config.
326-
// Apply them only if not provided on the CLI.
327-
if (!cas_downloader_flags.downloader_path.user_specified) {
328-
if (config.isMember(kKeyDownloaderPath)) {
329-
downloader_path = config[kKeyDownloaderPath].asString();
330-
}
331-
}
332-
if (!cas_downloader_flags.prefer_uncompressed.user_specified) {
333-
if (config.isMember("prefer-uncompressed")) {
334-
prefer_uncompressed = config["prefer-uncompressed"].asBool();
335-
}
336-
}
322+
LOG(INFO) <<"Using CAS config from:" << config_filepath;
323+
}
324+
std::string config_contents =CF_EXPECT(ReadFileContents(config_filepath));
325+
Json::Value config =CF_EXPECT(ParseJson(config_contents));
337326

338-
// For each supported flag key we merge CLI values (if provided) on top of
339-
// the config file values so CLI wins. Use the same keys as
340-
// ConvertToConfigFlags.
341-
MergeCliValuesIntoConfig(cas_downloader_flags, config_flags);
327+
// Base config flags from file (may be empty object)
328+
config_flags = config[kKeyFlags];
342329

343-
// If the config file didn't provide a downloader path and CLI didn't as
344-
// well, this will be caught below when we require a non-empty path.
330+
// downloader-path and prefer-uncompressed are top-level in the config.
331+
// Apply them only if not provided on the CLI.
332+
if (!cas_downloader_flags.downloader_path.user_provided()) {
333+
if (config.isMember(kKeyDownloaderPath)) {
334+
downloader_path = config[kKeyDownloaderPath].asString();
335+
}
345336
}
346-
347-
// Final sanity: ensure we have a downloader path and binary exists.
348-
CF_EXPECT(!downloader_path.empty(),
349-
"CAS downloader path not provided. Use --cas_downloader_path or"
350-
"set downloader-path in config file.");
351-
CF_EXPECT(FileExists(downloader_path),
352-
"CAS Downloader binary not found at:" << downloader_path);
353-
354-
// Create cas_flags from the merged config_flags (CLI-overrides applied
355-
// above)
356-
cas_flags =CreateCasFlags(downloader_path, config_flags);
357-
358-
if (!service_account_filepath.empty() &&
359-
FileExists(service_account_filepath)) {
360-
cas_flags.push_back("-" +std::string(kFlagServiceAccountJson) +"=" +
361-
std::string(service_account_filepath));
362-
}else {
363-
cas_flags.push_back("-" +std::string(kFlagUseAdc));
337+
if (!cas_downloader_flags.prefer_uncompressed.user_provided()) {
338+
if (config.isMember("prefer-uncompressed")) {
339+
prefer_uncompressed = config["prefer-uncompressed"].asBool();
340+
}
364341
}
365342

366-
return std::unique_ptr<CasDownloader>(
367-
new CasDownloader{downloader_path, cas_flags, prefer_uncompressed});
368-
};
343+
// For each supported flag key we merge CLI values (if provided) on top of
344+
// the config file values so CLI wins. Use the same keys as
345+
// ConvertToConfigFlags.
346+
MergeCliValuesIntoConfig(cas_downloader_flags, config_flags);
347+
348+
// If the config file didn't provide a downloader path and CLI didn't as
349+
// well, this will be caught below when we require a non-empty path.
350+
}else {
351+
// No config file available: use CLI values (or defaults if CLI didn't set
352+
// them). Convert current flag values into config_flags for downstream
353+
// processing.
354+
LOG(INFO) <<"Using CAS downloader flags from command line or defaults.";
355+
config_flags =ConvertToConfigFlags(cas_downloader_flags);
356+
}
369357

370-
auto result =CreateImpl(cas_downloader_flags, service_account_filepath);
371-
if (!result.ok()) {
372-
// Ensure callers and logs clearly indicate that CAS downloading is
373-
// disabled and why, using the same environment-aware formatting that
374-
// test helpers use.
375-
LOG(INFO) <<"CAS downloading disabled:" << result.error().FormatForEnv();
376-
returnandroid::base::unexpected(std::move(result).error());
377-
}
378-
return result;
358+
// Final sanity: ensure we have a downloader path and binary exists.
359+
CF_EXPECT(!downloader_path.empty(),
360+
"CAS downloader path not provided. Use --cas_downloader_path or"
361+
"set downloader-path in config file.");
362+
CF_EXPECT(FileExists(downloader_path),
363+
"CAS Downloader binary not found at:" << downloader_path);
364+
365+
// Create cas_flags from the merged config_flags (CLI-overrides applied
366+
// above)
367+
cas_flags =CreateCasFlags(downloader_path, config_flags);
368+
369+
if (!service_account_filepath.empty() &&
370+
FileExists(service_account_filepath)) {
371+
cas_flags.push_back("-" +std::string(kFlagServiceAccountJson) +"=" +
372+
std::string(service_account_filepath));
373+
}else {
374+
cas_flags.push_back("-" +std::string(kFlagUseAdc));
375+
}
376+
377+
return std::unique_ptr<CasDownloader>(
378+
new CasDownloader{downloader_path, cas_flags, prefer_uncompressed});
379379
}
380380

381381
voidAppendBuildInfoToInvocationId(const DeviceBuild& build,
382382
std::vector<std::string>& cas_flags) {
383383
// Append build info, including build id, branch, and flavor, to the
384384
// invocation-id flag. Do this only if the invocation-id flag is already
385385
// present and contains `caller` only.
386-
size_t found_index = std::string::npos;
387-
388-
for (size_t i =0; i < cas_flags.size(); ++i) {
389-
if (cas_flags[i].find("-invocation-id=caller=") ==0 &&
390-
cas_flags[i].find(",") == std::string::npos) {
391-
found_index = i;
392-
break;
386+
for (auto& flag : cas_flags) {
387+
if (flag.find("-invocation-id=caller=") ==0 &&
388+
flag.find(',') == std::string::npos) {
389+
if (!build.id.empty()) {
390+
flag +=",bid=" + build.id;
391+
}
392+
if (!build.branch.empty()) {
393+
flag +=",branch=" + build.branch;
394+
}
395+
if (!build.target.empty()) {
396+
flag +=",flavor=" + build.target;
397+
}
398+
return;
393399
}
394400
}
395-
396-
if (found_index == std::string::npos) {
397-
return;// Condition not met; nothing to update/replace.
398-
}
399-
400-
std::string invocation_id_flag = cas_flags[found_index];
401-
cas_flags.erase(cas_flags.begin() + found_index);
402-
403-
if (!build.id.empty()) {
404-
invocation_id_flag +=",bid=" + build.id;
405-
}
406-
if (!build.branch.empty()) {
407-
invocation_id_flag +=",branch=" + build.branch;
408-
}
409-
if (!build.target.empty()) {
410-
invocation_id_flag +=",flavor=" + build.target;
411-
}
412-
413-
cas_flags.push_back(invocation_id_flag);
414401
}
415402

416403
CasDownloader::CasDownloader(std::string downloader_path,

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp