From c49059f3e9be7f379a33f48cdeaaa434997317aa Mon Sep 17 00:00:00 2001 From: mokapsing <39441028+mokapsing@users.noreply.github.com> Date: Mon, 4 Mar 2024 14:13:17 +0800 Subject: [PATCH 1/6] fix(glog): no read permission on the cwd on Android --- src/rime/lever/deployment_tasks.cc | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/rime/lever/deployment_tasks.cc b/src/rime/lever/deployment_tasks.cc index 375bfa1bd..b2346b334 100644 --- a/src/rime/lever/deployment_tasks.cc +++ b/src/rime/lever/deployment_tasks.cc @@ -632,10 +632,15 @@ bool CleanOldLogFiles::Run(Deployer* deployer) { // E.g. on Android, there's no write permission on the cwd. vector dirs; for (auto& dir : google::GetLoggingDirectories()) { - auto perms = fs::status(dir).permissions(); - if ((perms & (fs::perms::owner_write | fs::perms::group_write | - fs::perms::others_write)) != fs::perms::none) { - dirs.push_back(dir); + try { + auto perms = fs::status(dir).permissions(); + if ((perms & (fs::perms::owner_write | fs::perms::group_write | + fs::perms::others_write)) != fs::perms::none) { + dirs.push_back(dir); + } + } + catch (...) { + continue; } } DLOG(INFO) << "scanning " << dirs.size() << " temp directory for log files."; From 56fcc9c0aa5ea816252206857c19a24a232bffe0 Mon Sep 17 00:00:00 2001 From: mokapsing <39441028+mokapsing@users.noreply.github.com> Date: Mon, 4 Mar 2024 15:43:04 +0800 Subject: [PATCH 2/6] never check permission just catch it and skip --- src/rime/lever/deployment_tasks.cc | 57 +++++++++++++----------------- 1 file changed, 25 insertions(+), 32 deletions(-) diff --git a/src/rime/lever/deployment_tasks.cc b/src/rime/lever/deployment_tasks.cc index b2346b334..82159f4c8 100644 --- a/src/rime/lever/deployment_tasks.cc +++ b/src/rime/lever/deployment_tasks.cc @@ -630,46 +630,39 @@ bool CleanOldLogFiles::Run(Deployer* deployer) { // Make sure we have sufficient permissions on the scanned directories. // E.g. on Android, there's no write permission on the cwd. - vector dirs; - for (auto& dir : google::GetLoggingDirectories()) { - try { - auto perms = fs::status(dir).permissions(); - if ((perms & (fs::perms::owner_write | fs::perms::group_write | - fs::perms::others_write)) != fs::perms::none) { - dirs.push_back(dir); - } - } - catch (...) { - continue; - } - } + vector dirs(google::GetLoggingDirectories()); + DLOG(INFO) << "scanning " << dirs.size() << " temp directory for log files."; int removed = 0; for (const auto& dir : dirs) { vector files; DLOG(INFO) << "temp directory: " << dir; - // preparing files - for (const auto& entry : fs::directory_iterator(dir)) { - const string& file_name(entry.path().filename().string()); - if (entry.is_regular_file() && !entry.is_symlink() && - boost::starts_with(file_name, "rime.") && - !boost::contains(file_name, today)) { - files.push_back(entry.path()); + try { + // preparing files + for (const auto& entry : fs::directory_iterator(dir)) { + const string& file_name(entry.path().filename().string()); + if (entry.is_regular_file() && !entry.is_symlink() && + boost::starts_with(file_name, "rime.") && + !boost::contains(file_name, today)) { + files.push_back(entry.path()); + } } - } - // remove files - for (const auto& file : files) { - try { - DLOG(INFO) << "removing log file '" << file.filename() << "'."; - // ensure write permission - fs::permissions(file, fs::perms::owner_write); - fs::remove(file); - ++removed; - } catch (const fs::filesystem_error& ex) { - LOG(ERROR) << ex.what(); - success = false; + // remove files + for (const auto& file : files) { + try { + DLOG(INFO) << "removing log file '" << file.filename() << "'."; + // ensure write permission + fs::permissions(file, fs::perms::owner_write); + fs::remove(file); + ++removed; + } catch (const fs::filesystem_error& ex) { + LOG(ERROR) << ex.what(); + success = false; + } } + } catch (...) { + continue; } } if (removed != 0) { From f7bbdc8328c79b3747893208c349aabc9dc4071c Mon Sep 17 00:00:00 2001 From: mokapsing <39441028+mokapsing@users.noreply.github.com> Date: Mon, 4 Mar 2024 16:45:31 +0800 Subject: [PATCH 3/6] remove duplicate try catch --- src/rime/lever/deployment_tasks.cc | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/rime/lever/deployment_tasks.cc b/src/rime/lever/deployment_tasks.cc index 82159f4c8..8e9e7f0ef 100644 --- a/src/rime/lever/deployment_tasks.cc +++ b/src/rime/lever/deployment_tasks.cc @@ -650,18 +650,15 @@ bool CleanOldLogFiles::Run(Deployer* deployer) { } // remove files for (const auto& file : files) { - try { - DLOG(INFO) << "removing log file '" << file.filename() << "'."; - // ensure write permission - fs::permissions(file, fs::perms::owner_write); - fs::remove(file); - ++removed; - } catch (const fs::filesystem_error& ex) { - LOG(ERROR) << ex.what(); - success = false; - } + DLOG(INFO) << "removing log file '" << file.filename() << "'."; + // ensure write permission + fs::permissions(file, fs::perms::owner_write); + fs::remove(file); + ++removed; } - } catch (...) { + } catch (const fs::filesystem_error& ex) { + LOG(ERROR) << ex.what(); + success = false; continue; } } From 0e5f8bc806c5fc4ab9984a380f460227195cf42b Mon Sep 17 00:00:00 2001 From: mokapsing <39441028+mokapsing@users.noreply.github.com> Date: Mon, 4 Mar 2024 17:24:17 +0800 Subject: [PATCH 4/6] use the try catch separately --- src/rime/lever/deployment_tasks.cc | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/rime/lever/deployment_tasks.cc b/src/rime/lever/deployment_tasks.cc index 8e9e7f0ef..03f89d258 100644 --- a/src/rime/lever/deployment_tasks.cc +++ b/src/rime/lever/deployment_tasks.cc @@ -630,6 +630,8 @@ bool CleanOldLogFiles::Run(Deployer* deployer) { // Make sure we have sufficient permissions on the scanned directories. // E.g. on Android, there's no write permission on the cwd. + // update: Now it no longer actively detects directory permissions, + // leaving it up to the try catch below to avoid permissions exceptions. vector dirs(google::GetLoggingDirectories()); DLOG(INFO) << "scanning " << dirs.size() << " temp directory for log files."; @@ -648,18 +650,22 @@ bool CleanOldLogFiles::Run(Deployer* deployer) { files.push_back(entry.path()); } } - // remove files - for (const auto& file : files) { + } catch (const fs::filesystem_error& ex) { + LOG(ERROR) << ex.what(); + continue; + } + // remove files + for (const auto& file : files) { + try { DLOG(INFO) << "removing log file '" << file.filename() << "'."; // ensure write permission fs::permissions(file, fs::perms::owner_write); fs::remove(file); ++removed; + } catch (const fs::filesystem_error& ex) { + LOG(ERROR) << ex.what(); + success = false; } - } catch (const fs::filesystem_error& ex) { - LOG(ERROR) << ex.what(); - success = false; - continue; } } if (removed != 0) { From 4ae628dc62f5dad60aa7b1cb2cf38f00f8c49b47 Mon Sep 17 00:00:00 2001 From: mokapsing Date: Mon, 4 Mar 2024 19:29:07 +0800 Subject: [PATCH 5/6] update comments --- src/rime/lever/deployment_tasks.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/rime/lever/deployment_tasks.cc b/src/rime/lever/deployment_tasks.cc index 03f89d258..eb93fc2cf 100644 --- a/src/rime/lever/deployment_tasks.cc +++ b/src/rime/lever/deployment_tasks.cc @@ -628,10 +628,6 @@ bool CleanOldLogFiles::Run(Deployer* deployer) { string today(ymd); DLOG(INFO) << "today: " << today; - // Make sure we have sufficient permissions on the scanned directories. - // E.g. on Android, there's no write permission on the cwd. - // update: Now it no longer actively detects directory permissions, - // leaving it up to the try catch below to avoid permissions exceptions. vector dirs(google::GetLoggingDirectories()); DLOG(INFO) << "scanning " << dirs.size() << " temp directory for log files."; @@ -651,6 +647,8 @@ bool CleanOldLogFiles::Run(Deployer* deployer) { } } } catch (const fs::filesystem_error& ex) { + // Catch error to skip up when we have no sufficient permissions. + // E.g. on Android, there's no write permission on the cwd. LOG(ERROR) << ex.what(); continue; } From e0797e0e200d78da09d8fc548ddcd5a51f20b4c2 Mon Sep 17 00:00:00 2001 From: mokapsing <39441028+mokapsing@users.noreply.github.com> Date: Mon, 4 Mar 2024 20:29:55 +0800 Subject: [PATCH 6/6] Update log level and content Co-authored-by: ksqsf --- src/rime/lever/deployment_tasks.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rime/lever/deployment_tasks.cc b/src/rime/lever/deployment_tasks.cc index eb93fc2cf..17a51f4b5 100644 --- a/src/rime/lever/deployment_tasks.cc +++ b/src/rime/lever/deployment_tasks.cc @@ -648,8 +648,8 @@ bool CleanOldLogFiles::Run(Deployer* deployer) { } } catch (const fs::filesystem_error& ex) { // Catch error to skip up when we have no sufficient permissions. - // E.g. on Android, there's no write permission on the cwd. - LOG(ERROR) << ex.what(); + // E.g. on Android, there's no read permission on the cwd. + LOG(WARNING) << "couldn't list directory '" << dir << "': " << ex.what(); continue; } // remove files