From c21503d21bd84f827cb6c1b06f6857cbc9e56293 Mon Sep 17 00:00:00 2001 From: Lutz Justen Date: Wed, 7 Dec 2022 11:25:43 +0100 Subject: [PATCH] [cdc_stream] Fix issues found in tests (#40) * [cdc_stream] Fix issues found in tests Fixes a couple of issues found by integration testing: - Unicode command line args in cdc_stream show up as question marks. - Log is still named assets_stream_manager instead of cdc_stream. - An error message contains stadia_assets_stream_manager_v3.exe. - mount_dir was not the last arg as required by FUSE - Promoted cache cleanup logs to INFO level since they're important for the proper workings of the system. - Asset streaming cache dir is still %APPDATA%\GGP\asset_streaming. * Address comments --- cdc_stream/cdc_fuse_manager.cc | 9 +++++---- cdc_stream/main.cc | 23 ++++++++++++++++++++++- cdc_stream/multi_session.cc | 3 ++- cdc_stream/multi_session.h | 4 ++-- cdc_stream/multi_session_test.cc | 13 +++++++------ cdc_stream/start_service_command.cc | 2 +- data_store/data_provider.cc | 4 ++-- 7 files changed, 41 insertions(+), 17 deletions(-) diff --git a/cdc_stream/cdc_fuse_manager.cc b/cdc_stream/cdc_fuse_manager.cc index 163285c..28c4c0f 100644 --- a/cdc_stream/cdc_fuse_manager.cc +++ b/cdc_stream/cdc_fuse_manager.cc @@ -26,6 +26,7 @@ namespace cdc_ft { namespace { +constexpr char kExeFilename[] = "cdc_stream.exe"; constexpr char kFuseFilename[] = "cdc_fuse_fs"; constexpr char kLibFuseFilename[] = "libfuse.so"; constexpr char kFuseStdoutPrefix[] = "cdc_fuse_fs_stdout"; @@ -95,8 +96,8 @@ absl::Status CdcFuseManager::Start(const std::string& mount_dir, if (!status.ok()) { return absl::NotFoundError(absl::StrFormat( "Required gamelet component not found. Make sure the files %s and %s " - "reside in the same folder as stadia_assets_stream_manager_v3.exe.", - kFuseFilename, kLibFuseFilename)); + "reside in the same folder as %s.", + kFuseFilename, kLibFuseFilename, kExeFilename)); } std::string component_args = GameletComponent::ToCommandLineArgs(components); @@ -113,8 +114,8 @@ absl::Status CdcFuseManager::Start(const std::string& mount_dir, RemoteUtil::QuoteForSsh(instance_), RemoteUtil::QuoteForSsh(component_args), remote_port, kCacheDir, verbosity, cleanup_timeout_sec, access_idle_timeout_sec, enable_stats, - check, cache_capacity, RemoteUtil::QuoteForSsh(mount_dir), - debug ? " -d" : "", singlethreaded ? " -s" : ""); + check, cache_capacity, debug ? "-d " : "", singlethreaded ? "-s " : "", + RemoteUtil::QuoteForSsh(mount_dir)); bool needs_deploy = false; RETURN_IF_ERROR( diff --git a/cdc_stream/main.cc b/cdc_stream/main.cc index a167021..a6c5575 100644 --- a/cdc_stream/main.cc +++ b/cdc_stream/main.cc @@ -16,9 +16,30 @@ #include "cdc_stream/start_service_command.h" #include "cdc_stream/stop_command.h" #include "cdc_stream/stop_service_command.h" +#include "common/platform.h" #include "lyra/lyra.hpp" -int main(int argc, char* argv[]) { +#if PLATFORM_WINDOWS +int wmain(int argc, wchar_t* wargv[]) { + // Convert args from wide to UTF8 strings. + std::vector utf8_str_args; + utf8_str_args.reserve(argc); + for (int i = 0; i < argc; i++) { + utf8_str_args.push_back(cdc_ft::Util::WideToUtf8Str(wargv[i])); + } + + // Convert args from UTF8 strings to UTF8 c-strings. + std::vector utf8_args; + utf8_args.reserve(argc); + for (const auto& utf8_str_arg : utf8_str_args) { + utf8_args.push_back(utf8_str_arg.c_str()); + } + + const char** argv = utf8_args.data(); +#else +int main(int argc, char** argv) { +#endif + // Set up commands. auto cli = lyra::cli(); bool show_help = false; diff --git a/cdc_stream/multi_session.cc b/cdc_stream/multi_session.cc index 966cf63..95d1b7a 100644 --- a/cdc_stream/multi_session.cc +++ b/cdc_stream/multi_session.cc @@ -625,7 +625,8 @@ absl::StatusOr MultiSession::GetCachePath( path::Append(&appdata_path, ".cache"); #endif - std::string base_dir = path::Join(appdata_path, "GGP", "asset_streaming"); + std::string base_dir = + path::Join(appdata_path, "cdc-file-transfer", "chunks"); std::string cache_dir = GetCacheDir(src_dir); size_t total_size = base_dir.size() + 1 + cache_dir.size(); diff --git a/cdc_stream/multi_session.h b/cdc_stream/multi_session.h index 7c83739..189bc26 100644 --- a/cdc_stream/multi_session.h +++ b/cdc_stream/multi_session.h @@ -148,7 +148,7 @@ class MultiSession { // |process_factory| abstracts process creation. // |data_store| can be passed for tests to override the default store used. // By default, the class uses a DiskDataStore that writes to - // %APPDATA%\GGP\asset_streaming| on Windows. + // %APPDATA%\cdc-file-transfer\chunks\ on Windows. MultiSession(std::string src_dir, SessionConfig cfg, ProcessFactory* process_factory, MultiSessionMetricsRecorder const* metrics_recorder, @@ -220,7 +220,7 @@ class MultiSession { static std::string GetCacheDir(std::string dir); // Returns the directory where manifest chunks are cached, e.g. - // "%APPDATA%\GGP\asset_streaming\c__path_to_game_abcdef01" for + // "%APPDATA%\cdc-file-transfer\chunks\c__path_to_game_abcdef01" for // "C:\path\to\game". // The returned path is shortened to |max_len| by removing UTF8 code points // from the beginning of the actual cache directory (c__path...) if necessary. diff --git a/cdc_stream/multi_session_test.cc b/cdc_stream/multi_session_test.cc index 753f6b5..c8d6e33 100644 --- a/cdc_stream/multi_session_test.cc +++ b/cdc_stream/multi_session_test.cc @@ -241,7 +241,7 @@ TEST_F(MultiSessionTest, GetCachePath_ContainsExpectedParts) { ASSERT_OK(cache_path); EXPECT_TRUE(absl::EndsWith(*cache_path, kCacheDir)) << *cache_path; EXPECT_TRUE( - absl::StrContains(*cache_path, path::Join("GGP", "asset_streaming"))) + absl::StrContains(*cache_path, path::Join("cdc-file-transfer", "chunks"))) << *cache_path; } @@ -253,7 +253,7 @@ TEST_F(MultiSessionTest, GetCachePath_ShortensLongPaths) { ASSERT_OK(cache_path); EXPECT_EQ(cache_path->size(), MultiSession::kDefaultMaxCachePathLen); EXPECT_TRUE( - absl::StrContains(*cache_path, path::Join("GGP", "asset_streaming"))) + absl::StrContains(*cache_path, path::Join("cdc-file-transfer", "chunks"))) << *cache_path; // The hash in the end of the path is kept and not shortened. EXPECT_EQ(cache_dir.substr(cache_dir.size() - MultiSession::kDirHashLen), @@ -261,7 +261,8 @@ TEST_F(MultiSessionTest, GetCachePath_ShortensLongPaths) { } TEST_F(MultiSessionTest, GetCachePath_DoesNotSplitUtfCodePoints) { - // Find out the length of the %APPDATA%\GGP\asset_streaming\" + hash part. + // Find out the length of the %APPDATA%\cdc-file-transfer\chunks\" + hash + // part. absl::StatusOr cache_path = MultiSession::GetCachePath(""); ASSERT_OK(cache_path); size_t base_len = cache_path->size(); @@ -271,17 +272,17 @@ TEST_F(MultiSessionTest, GetCachePath_DoesNotSplitUtfCodePoints) { ASSERT_OK(cache_path); EXPECT_EQ(cache_path->size(), base_len); - // %APPDATA%\GGP\asset_streaming\abcdefg + // %APPDATA%\cdc-file-transfer\chunks\abcdefg cache_path = MultiSession::GetCachePath(u8"\u0200\u0200", base_len + 1); ASSERT_OK(cache_path); EXPECT_EQ(cache_path->size(), base_len); - // %APPDATA%\GGP\asset_streaming\\u0200abcdefg + // %APPDATA%\cdc-file-transfer\chunks\\u0200abcdefg cache_path = MultiSession::GetCachePath(u8"\u0200\u0200", base_len + 2); ASSERT_OK(cache_path); EXPECT_EQ(cache_path->size(), base_len + 2); - // %APPDATA%\GGP\asset_streaming\\u0200abcdefg + // %APPDATA%\cdc-file-transfer\chunks\\u0200abcdefg cache_path = MultiSession::GetCachePath(u8"\u0200\u0200", base_len + 3); ASSERT_OK(cache_path); EXPECT_EQ(cache_path->size(), base_len + 2); diff --git a/cdc_stream/start_service_command.cc b/cdc_stream/start_service_command.cc index 8611e78..4e915c4 100644 --- a/cdc_stream/start_service_command.cc +++ b/cdc_stream/start_service_command.cc @@ -116,7 +116,7 @@ absl::StatusOr> StartServiceCommand::GetLogger() { } return std::make_unique( - level, GetLogPath(log_dir_.c_str(), "assets_stream_manager").c_str()); + level, GetLogPath(log_dir_.c_str(), "cdc_stream").c_str()); } // Runs the session management service and returns when it finishes. diff --git a/data_store/data_provider.cc b/data_store/data_provider.cc index 76fbc1f..d6c4082 100644 --- a/data_store/data_provider.cc +++ b/data_store/data_provider.cc @@ -330,10 +330,10 @@ void DataProvider::CleanupThreadMain() { WriterMutexLockList locks; LockAllMutexes(&locks); chunks_updated_ = false; - LOG_DEBUG("Starting cache cleanup"); + LOG_INFO("Starting cache cleanup"); Stopwatch sw; absl::Status status = writer_->Cleanup(); - LOG_DEBUG("Finished cache cleanup in %0.3f seconds", sw.ElapsedSeconds()); + LOG_INFO("Finished cache cleanup in %0.3f seconds", sw.ElapsedSeconds()); next_cleanup_time = steady_clock_->Now() + std::chrono::seconds(cleanup_timeout_sec_); absl::MutexLock cleaned_lock(&cleaned_mutex_);