fix(data-transfer): fallback to copy when moveFile crosses filesystems - #786
Merged
deepin-bot[bot] merged 1 commit intoSep 8, 2026
Conversation
Reviewer's GuideUpdates moveFile to handle cross-filesystem transfers by attempting native renames first and falling back to copy-then-delete, with recursive directory support, destination parent creation, and corrected collision-path handling. Sequence diagram for cross-filesystem moveFile fallbacksequenceDiagram
participant Caller
participant SettingHelper
participant Filesystem
Caller->>SettingHelper: moveFile(src, dst)
SettingHelper->>Filesystem: mkpath(absolutePath(dst))
alt src is directory
SettingHelper->>Filesystem: QDir::rename(src, dst)
alt rename succeeds
Filesystem-->>SettingHelper: true
else rename fails
SettingHelper->>SettingHelper: copyDirectory(src, dst)
SettingHelper->>Filesystem: QDir::removeRecursively(src)
Filesystem-->>SettingHelper: true
end
else src is file
SettingHelper->>Filesystem: QFile::rename(src, dst)
alt rename succeeds
Filesystem-->>SettingHelper: true
else rename fails
SettingHelper->>Filesystem: QFile::copy(src, dst)
SettingHelper->>Filesystem: QFile::remove(src)
Filesystem-->>SettingHelper: true
end
end
SettingHelper-->>Caller: move result
Flow diagram for collision-safe destination handlingflowchart TD
A["moveFile(src, dst)"] --> B{Destination exists?}
B -- No --> D[mkpath destination parent]
B -- Yes --> C[Extract trailing filename and suffix]
C --> E[Build numbered destination until unused]
E --> D
D --> F{Source is directory?}
F -- Yes --> G[QDir::rename]
F -- No --> H[QFile::rename]
G --> I{Rename succeeds?}
H --> J{Rename succeeds?}
I -- Yes --> K[Return true]
J -- Yes --> K
I -- No --> L[copyDirectory]
J -- No --> M[QFile::copy]
L --> N[QDir::removeRecursively]
M --> O[QFile::remove]
N --> K
O --> K
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/plugins/data-transfer/core/utils/settinghepler.cpp" line_range="353-367" />
<code_context>
+ if (dir.rename(src, dst))
+ return true;
+ if (copyDirectory(src, dst)) {
+ QDir(src).removeRecursively(); // 数据已就位, 源清理失败不影响结果
+ return true;
+ }
+
+ WLOG << "moveFile error: rename dir failed: " << src.toStdString() << " -> " << dst.toStdString();
+ return false;
+ }
+ // ---------- 文件处理 ----------
+ // 优先 rename 移动; 跨设备时退化为复制后删除源
QFile f(src);
LOG << "moveFile dst: " << src.toStdString() << " " << dst.toStdString();
if (f.rename(dst))
return true;
+ if (QFile::copy(src, dst)) { // 跨设备时退化为复制后删除源
+ QFile::remove(src);
+ return true;
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** The copy-then-delete fallback returns true without checking whether source cleanup succeeded. If `QDir(src).removeRecursively()` or `QFile::remove(src)` fails, both source and destination remain while callers report “Transfer completed”, so a later retry can duplicate the data or leave the migration incomplete.
**Triggers:** When the source cannot be removed after a successful copy, such as due to permissions, read-only storage, or a file being in use.
**Suggested fix:** Return false when source removal fails, or explicitly surface the cleanup failure and prevent the operation from being reported as complete.
</issue_to_address>
### Comment 2
<location path="src/plugins/data-transfer/core/utils/settinghepler.cpp" line_range="355-358" />
<code_context>
+ for (const auto &entry : entries) {
+ const QString target = dst + QLatin1Char('/') + entry.fileName();
+ if (entry.isDir()) {
+ if (!copyDirectory(entry.absoluteFilePath(), target))
+ return false;
+ } else if (!QFile::copy(entry.absoluteFilePath(), target)) {
+ WLOG << "copy file failed: " << entry.absoluteFilePath().toStdString()
+ << " -> " << target.toStdString();
</code_context>
<issue_to_address>
**issue (bug_risk):** A failed recursive copy leaves the already-created destination tree in place. `moveFile` then returns false, but a retry sees that partial destination as an existing collision and redirects the remaining source to a new `(1)` path instead of replacing or resuming the incomplete tree.
**Triggers:** When copying any child entry fails after earlier entries have already been copied.
**Suggested fix:** Remove the partial destination tree before returning failure, or use a temporary destination and atomically rename it into place only after the full copy succeeds.
```suggestion
}
QDir(dst).removeRecursively();
WLOG << "moveFile error: rename dir failed: " << src.toStdString() << " -> " << dst.toStdString();
return false;
```
</issue_to_address>
### Comment 3
<location path="src/plugins/data-transfer/core/utils/settinghepler.cpp" line_range="308-313" />
<code_context>
+ if (!QDir().mkpath(dst))
+ return false;
+
+ const auto entries = srcDir.entryInfoList(
+ QDir::Files | QDir::Dirs | QDir::NoDotAndDotDot | QDir::Hidden | QDir::System);
+ for (const auto &entry : entries) {
+ const QString target = dst + QLatin1Char('/') + entry.fileName();
+ if (entry.isDir()) {
+ if (!copyDirectory(entry.absoluteFilePath(), target))
+ return false;
+ } else if (!QFile::copy(entry.absoluteFilePath(), target)) {
+ WLOG << "copy file failed: " << entry.absoluteFilePath().toStdString()
+ << " -> " << target.toStdString();
</code_context>
<issue_to_address>
**issue (bug_risk):** The recursive fallback follows directory symlinks because it classifies entries with `entry.isDir()` and does not exclude symbolic links. A symlink to an external directory is copied recursively outside the source tree, while a self-referential or cyclic symlink causes unbounded recursion until the copy fails, leaving an incomplete destination.
**Triggers:** When the transferred directory contains a symbolic link to a directory.
**Suggested fix:** Detect and handle symlinks explicitly, preserving the link or rejecting it, and exclude them from recursive directory traversal.
```suggestion
for (const auto &entry : entries) {
const QString target = dst + QLatin1Char('/') + entry.fileName();
if (entry.isSymLink())
return false;
if (entry.isDir()) {
if (!copyDirectory(entry.absoluteFilePath(), target))
return false;
} else if (!QFile::copy(entry.absoluteFilePath(), target)) {
```
</issue_to_address>
Comment on lines
+353
to
+367
| QDir(src).removeRecursively(); // 数据已就位, 源清理失败不影响结果 | ||
| return true; | ||
| } | ||
|
|
||
| WLOG << "moveFile error: rename dir failed: " << src.toStdString() << " -> " << dst.toStdString(); | ||
| return false; | ||
| } | ||
| // ---------- 文件处理 ---------- | ||
| // 优先 rename 移动; 跨设备时退化为复制后删除源 | ||
| QFile f(src); | ||
| LOG << "moveFile dst: " << src.toStdString() << " " << dst.toStdString(); | ||
| if (f.rename(dst)) | ||
| return true; | ||
| if (QFile::copy(src, dst)) { // 跨设备时退化为复制后删除源 | ||
| QFile::remove(src); |
There was a problem hiding this comment.
issue (bug_risk): The copy-then-delete fallback returns true without checking whether source cleanup succeeded. If QDir(src).removeRecursively() or QFile::remove(src) fails, both source and destination remain while callers report “Transfer completed”, so a later retry can duplicate the data or leave the migration incomplete.
Triggers: When the source cannot be removed after a successful copy, such as due to permissions, read-only storage, or a file being in use.
Suggested fix: Return false when source removal fails, or explicitly surface the cleanup failure and prevent the operation from being reported as complete.
Comment on lines
+355
to
+358
| } | ||
|
|
||
| WLOG << "moveFile error: rename dir failed: " << src.toStdString() << " -> " << dst.toStdString(); | ||
| return false; |
There was a problem hiding this comment.
issue (bug_risk): A failed recursive copy leaves the already-created destination tree in place. moveFile then returns false, but a retry sees that partial destination as an existing collision and redirects the remaining source to a new (1) path instead of replacing or resuming the incomplete tree.
Triggers: When copying any child entry fails after earlier entries have already been copied.
Suggested fix: Remove the partial destination tree before returning failure, or use a temporary destination and atomically rename it into place only after the full copy succeeds.
Suggested change
| } | |
| WLOG << "moveFile error: rename dir failed: " << src.toStdString() << " -> " << dst.toStdString(); | |
| return false; | |
| } | |
| QDir(dst).removeRecursively(); | |
| WLOG << "moveFile error: rename dir failed: " << src.toStdString() << " -> " << dst.toStdString(); | |
| return false; |
Comment on lines
+308
to
+313
| for (const auto &entry : entries) { | ||
| const QString target = dst + QLatin1Char('/') + entry.fileName(); | ||
| if (entry.isDir()) { | ||
| if (!copyDirectory(entry.absoluteFilePath(), target)) | ||
| return false; | ||
| } else if (!QFile::copy(entry.absoluteFilePath(), target)) { |
There was a problem hiding this comment.
issue (bug_risk): The recursive fallback follows directory symlinks because it classifies entries with entry.isDir() and does not exclude symbolic links. A symlink to an external directory is copied recursively outside the source tree, while a self-referential or cyclic symlink causes unbounded recursion until the copy fails, leaving an incomplete destination.
Triggers: When the transferred directory contains a symbolic link to a directory.
Suggested fix: Detect and handle symlinks explicitly, preserving the link or rejecting it, and exclude them from recursive directory traversal.
Suggested change
| for (const auto &entry : entries) { | |
| const QString target = dst + QLatin1Char('/') + entry.fileName(); | |
| if (entry.isDir()) { | |
| if (!copyDirectory(entry.absoluteFilePath(), target)) | |
| return false; | |
| } else if (!QFile::copy(entry.absoluteFilePath(), target)) { | |
| for (const auto &entry : entries) { | |
| const QString target = dst + QLatin1Char('/') + entry.fileName(); | |
| if (entry.isSymLink()) | |
| return false; | |
| if (entry.isDir()) { | |
| if (!copyDirectory(entry.absoluteFilePath(), target)) | |
| return false; | |
| } else if (!QFile::copy(entry.absoluteFilePath(), target)) { |
…n different filesystems - Move directories with QDir::rename instead of QFile::rename, which fails with EISDIR on directories - Fall back to recursive copy plus source removal when rename fails with EXDEV across filesystems: copyDirectory() for directories and QFile::copy for single files - Fix rename-collision handling: dst.remove(fileName) deleted every occurrence of the name and corrupted paths with repeated segments; only the trailing segment is cut now - Ensure the destination parent directory exists before moving, for targets such as ~/D:/ derived from Windows drive letters 修复(数据迁移): 修复源和目标位于不同文件系统时 moveFile 搬运失败的问题 - 目录改用 QDir::rename 移动，避免 QFile::rename 移动目录时报 EISDIR 错误 - rename 跨文件系统失败(EXDEV)时退化为复制后删除源：目录走 copyDirectory() 递归复制，单文件走 QFile::copy - 修复重名避让逻辑: dst.remove(fileName) 会误删路径中所有同名段导致路径破坏，改为只截取结尾段 - 移动前确保目标父目录存在，兼容 ~/D:/ 这类源自 Windows 盘符的目标路径 Log: 修复数据迁移搬运阶段接收目录与主目录不在同一文件系统时 rename 跨设备失败(EXDEV)导致搬运失败的问题 Bug: https://pms.uniontech.com/bug-view-375881.html
pppanghu77
force-pushed
the
fix/movefile-cross-device-v20
branch
from
September 7, 2026 09:19
59995da to
740cffb
Compare
Contributor
Author
|
/retrigger |
Contributor
Author
|
/retest |
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价： 良好 ✅ 通过 潜在问题：
建议： 建议在 copyDirectory 失败时清理已复制的部分文件，或在 moveFile 的目录处理分支中，当 copyDirectory 失败后清理目标目录中的残留文件 2. 代码质量 ✅评价： 优秀 ✅ 通过 潜在问题：
建议： 建议为 copyDirectory 函数添加详细的文档注释，说明参数含义、返回值以及跨设备复制时的行为 3. 代码性能 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 性能良好，rename 优先策略在同等文件系统下为 O(1)，跨设备时退化为复制是必要的 4. 代码安全 🔒评价： 优秀 ✅ 通过
安全漏洞详情： 建议： 存在0个安全漏洞，代码安全合规，路径操作使用 Qt 安全 API，无注入风险 💡 改进建议代码示例// 递归复制目录, 用于跨设备(EXDEV) rename 失败时的退化搬运
// 参数: src - 源目录路径, dst - 目标目录路径
// 返回: true 表示复制成功, false 表示复制失败(部分复制的内容会被清理)
static bool copyDirectory(const QString &src, const QString &dst)
{
QDir srcDir(src);
if (!srcDir.exists())
return false;
if (!QDir().mkpath(dst))
return false;
const auto entries = srcDir.entryInfoList(
QDir::Files | QDir::Dirs | QDir::NoDotAndDotDot | QDir::Hidden | QDir::System);
for (const auto &entry : entries) {
const QString target = dst + QLatin1Char('/') + entry.fileName();
if (entry.isDir()) {
if (!copyDirectory(entry.absoluteFilePath(), target)) {
QDir(dst).removeRecursively(); // 清理已复制的部分文件
return false;
}
} else if (!QFile::copy(entry.absoluteFilePath(), target)) {
WLOG << "copy file failed: " << entry.absoluteFilePath().toStdString()
<< " -> " << target.toStdString();
QDir(dst).removeRecursively(); // 清理已复制的部分文件
return false;
}
}
return true;
}本报告由 AI 代码审查工具自动生成 |
re2zero
approved these changes
Sep 8, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: pppanghu77, re2zero 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 |
Contributor
Author
|
/forcemerge |
|
This pr force merged! (status: unstable) |
deepin-bot
Bot
merged commit Sep 8, 2026
3792405
into
linuxdeepin:release/v20
13 of 16 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
问题
数据迁移搬运阶段失败，日志报：
根因：接收目录（~/Downloads）与主目录可能不在同一文件系统（例如接收目录是 fuse 挂载、主目录是 ext4）。
rename(2)不允许跨文件系统，返回 EXDEV（Errno 18），搬运必然失败；且移动目录时QFile::rename会直接报 EISDIR。修复内容（moveFile）
QDir::rename移动：QFile::rename移动目录报 EISDIR（"打开的文件是个目录"）copyDirectory()递归复制（含隐藏文件）→removeRecursively()删源QFile::copy→QFile::removedst.remove(fileName)会删除路径中所有出现的同名段（QString::remove是全局删除），路径含重复段时被截断成错误路径；改为只截结尾段~/D:这类源自 Windows 盘符的目标目录，mkpath 确保存在验证
stat -c %d设备号不同）三场景实测通过：~/D:路径（含子目录、隐藏文件、无后缀文件）os.rename实测复现 Errno 18 EXDEV 确认根因Bug: https://pms.uniontech.com//bug-view-375881.html