fix: retry cache removal after modify directory permissions#1699
Conversation
kenrel overlay will generated a 000 permission work directory under workdir. Signed-off-by: reddevillg <reddevillg@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: dengbo11 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new utility function, makeDirectoryTreeRemovable, which recursively ensures that directories within a tree have owner permissions, allowing for successful deletion of cache directories even when permissions are restricted. This utility is integrated into PackageManager::removeCache as a fallback mechanism, and a corresponding unit test has been added. The review feedback correctly identifies a critical issue where the function will fail immediately if the root directory itself has restricted permissions, preventing the construction of the recursive directory iterator. A code suggestion is provided to resolve this by ensuring the root directory's permissions are fixed first.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| std::error_code ec; | ||
| fs::recursive_directory_iterator iter{ root, | ||
| fs::directory_options::skip_permission_denied, | ||
| ec }; | ||
| if (ec) { | ||
| return LINGLONG_ERR(fmt::format("failed to iterate directory: {}", root), ec); | ||
| } |
There was a problem hiding this comment.
If the root directory itself has restricted permissions (e.g., 000 or lacks owner read/write/execute permissions), constructing the std::filesystem::recursive_directory_iterator will fail immediately with a permission denied error. This prevents the function from fixing the permissions of any subdirectories or the root itself.
To make this robust, we should first check if the root directory exists, and if so, ensure that the root directory itself has owner permissions (fs::perms::owner_all) before attempting to construct the iterator.
std::error_code ec;
if (!fs::exists(root, ec)) {
if (ec) {
return LINGLONG_ERR(fmt::format("failed to check existence of root: {}", root), ec);
}
return LINGLONG_OK;
}
auto rootStatus = fs::status(root, ec);
if (ec) {
return LINGLONG_ERR(fmt::format("failed to get status of root directory: {}", root), ec);
}
if (rootStatus.type() == fs::file_type::directory) {
auto ret = ensureOwnerPermissionsIfNeeded(root, rootStatus.permissions(), fs::perms::owner_all);
if (!ret) {
return ret;
}
}
fs::recursive_directory_iterator iter{ root,
fs::directory_options::skip_permission_denied,
ec };
if (ec) {
return LINGLONG_ERR(fmt::format("failed to iterate directory: {}", root), ec);
}
dengbo11
merged commit e2a26de
into
OpenAtom-Linyaps:release/1.13
deepin pr auto review★ 总体评分：100分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 linglong::utils::error::Result<void>
makeDirectoryTreeRemovable(const std::filesystem::path &root) noexcept
{
LINGLONG_TRACE("make directory tree removable");
namespace fs = std::filesystem;
std::error_code ec;
auto rootStatus = fs::symlink_status(root, ec);
if (ec) {
return LINGLONG_ERR(fmt::format("failed to get root status: {}", root), ec);
}
if (rootStatus.type() != fs::file_type::directory) {
return LINGLONG_ERR(fmt::format("root path is not a directory: {}", root));
}
fs::recursive_directory_iterator iter{ root,
fs::directory_options::skip_permission_denied,
ec };
if (ec) {
return LINGLONG_ERR(fmt::format("failed to iterate directory: {}", root), ec);
}
const fs::recursive_directory_iterator end;
for (; iter != end; iter.increment(ec)) {
if (ec) {
return LINGLONG_ERR(fmt::format("failed to iterate directory: {}", root), ec);
}
auto status = iter->symlink_status(ec);
if (ec) {
return LINGLONG_ERR(fmt::format("failed to get entry status: {}", iter->path()), ec);
}
if (status.type() == fs::file_type::directory) {
auto ret = ensureOwnerPermissionsIfNeeded(iter->path(),
status.permissions(),
fs::perms::owner_all);
if (!ret) {
return ret;
}
}
}
return LINGLONG_OK;
} |
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
3 participants
kenrel overlay will generated a 000 permission work directory under workdir.