From 85e31029f74b56797e8a8b60d50c154a7bebb31d Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Fri, 4 Apr 2025 11:05:41 +0200 Subject: [PATCH 1/7] Qt 6.8 requires macSO 12 or higher: no need to compile against older clean up preprocessor directives for older macOS releases that are not supported by macOS minimum required release from the Qt 6.8 release we require Signed-off-by: Matthieu Gallien --- src/gui/folder.cpp | 2 -- src/libsync/discovery.cpp | 4 ---- src/libsync/filesystem.cpp | 7 ------- src/libsync/filesystem.h | 2 -- src/libsync/owncloudpropagator.cpp | 2 -- src/libsync/propagatedownload.cpp | 6 ------ src/libsync/propagatedownload.h | 4 ---- src/libsync/propagatorjobs.cpp | 16 ---------------- test/syncenginetestutils.cpp | 4 ---- test/testpermissions.cpp | 6 ------ 10 files changed, 53 deletions(-) diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index cc5a0f74a7691..f89dab9c58289 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -1736,11 +1736,9 @@ void Folder::slotNeedToRemoveRemnantsReadOnlyFolders(const QList= MAC_OS_X_VERSION_10_15 const auto fileInfo = QFileInfo{localPath + oneFolder->_file}; const auto parentFolderPath = fileInfo.dir().absolutePath(); const auto parentPermissionsHandler = FileSystem::FilePermissionsRestore{parentFolderPath, FileSystem::FolderPermissions::ReadWrite}; -#endif if (oneFolder->_type == ItemType::ItemTypeDirectory) { FileSystem::removeRecursively(localPath + oneFolder->_file); } else { diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index ce8ef8d306e0f..9d7cede827ba0 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -1870,11 +1870,9 @@ bool ProcessDirectoryJob::checkPermissions(const OCC::SyncFileItemPtr &item) item->_instruction = CSYNC_INSTRUCTION_ERROR; item->_errorString = tr("Not allowed because you don't have permission to add subfolders to that folder"); const auto localPath = QString{_discoveryData->_localDir + item->_file}; -#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 qCWarning(lcDisco) << "unexpected new folder in a read-only folder will be made read-write" << localPath; FileSystem::setFolderPermissions(localPath, FileSystem::FolderPermissions::ReadWrite); emit _discoveryData->remnantReadOnlyFolderDiscovered(item); -#endif return false; } else if (!item->isDirectory() && !perms.hasPermission(RemotePermissions::CanAddFile)) { qCWarning(lcDisco) << "checkForPermission: ERROR" << item->_file; @@ -2078,11 +2076,9 @@ int ProcessDirectoryJob::processSubJobs(int nbJobs) _dirItem->_instruction = CSYNC_INSTRUCTION_ERROR; _dirItem->_errorString = tr("Not allowed because you don't have permission to add subfolders to that folder"); const auto localPath = QString{_discoveryData->_localDir + _dirItem->_file}; -#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 qCWarning(lcDisco) << "unexpected new folder in a read-only folder will be made read-write" << localPath; FileSystem::setFolderPermissions(localPath, FileSystem::FolderPermissions::ReadWrite); emit _discoveryData->remnantReadOnlyFolderDiscovered(_dirItem); -#endif } _dirItem->_direction = _dirItem->_direction == SyncFileItem::Up ? SyncFileItem::Down : SyncFileItem::Up; diff --git a/src/libsync/filesystem.cpp b/src/libsync/filesystem.cpp index 8361d8af550ee..580562acc3d49 100644 --- a/src/libsync/filesystem.cpp +++ b/src/libsync/filesystem.cpp @@ -274,11 +274,9 @@ bool FileSystem::removeRecursively(const QString &path, const std::function= MAC_OS_X_VERSION_10_15 const auto fileInfo = QFileInfo{di.filePath()}; const auto parentFolderPath = fileInfo.dir().absolutePath(); const auto parentPermissionsHandler = FileSystem::FilePermissionsRestore{parentFolderPath, FileSystem::FolderPermissions::ReadWrite}; -#endif removeOk = FileSystem::remove(di.filePath(), &removeError); if (removeOk) { if (onDeleted) @@ -295,12 +293,10 @@ bool FileSystem::removeRecursively(const QString &path, const std::function= MAC_OS_X_VERSION_10_15 const auto fileInfo = QFileInfo{path}; const auto parentFolderPath = fileInfo.dir().absolutePath(); const auto parentPermissionsHandler = FileSystem::FilePermissionsRestore{parentFolderPath, FileSystem::FolderPermissions::ReadWrite}; FileSystem::setFolderPermissions(path, FileSystem::FolderPermissions::ReadWrite); -#endif allRemoved = QDir().rmdir(path); if (allRemoved) { if (onDeleted) @@ -326,7 +322,6 @@ bool FileSystem::getInode(const QString &filename, quint64 *inode) return false; } -#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 bool FileSystem::setFolderPermissions(const QString &path, FileSystem::FolderPermissions permissions) noexcept { @@ -564,6 +559,4 @@ FileSystem::FilePermissionsRestore::~FilePermissionsRestore() } } -#endif - } // namespace OCC diff --git a/src/libsync/filesystem.h b/src/libsync/filesystem.h index e14cf27681db0..e0439b8f52be4 100644 --- a/src/libsync/filesystem.h +++ b/src/libsync/filesystem.h @@ -128,9 +128,7 @@ namespace FileSystem { bool OWNCLOUDSYNC_EXPORT setFolderPermissions(const QString &path, FileSystem::FolderPermissions permissions) noexcept; -#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 bool OWNCLOUDSYNC_EXPORT isFolderReadOnly(const std::filesystem::path &path) noexcept; -#endif } /** @} */ diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index e239f05afbf04..beec5b96610a6 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -1465,7 +1465,6 @@ void PropagateDirectory::slotSubJobsFinished(SyncFileItem::Status status) || _item->_instruction == CSYNC_INSTRUCTION_NEW || _item->_instruction == CSYNC_INSTRUCTION_UPDATE_METADATA) { -#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 if (!_item->_remotePerm.isNull() && !_item->_remotePerm.hasPermission(RemotePermissions::CanAddFile) && !_item->_remotePerm.hasPermission(RemotePermissions::CanAddSubDirectories)) { @@ -1530,7 +1529,6 @@ void PropagateDirectory::slotSubJobsFinished(SyncFileItem::Status status) _item->_errorString = tr("The folder %1 cannot be made read-only: %2").arg("", tr("unknown exception")); } } -#endif if (!_item->_isAnyCaseClashChild && !_item->_isAnyInvalidCharChild) { if (_item->isEncrypted()) { _item->_e2eCertificateFingerprint = propagator()->account()->encryptionCertificateFingerprint(); diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index 9c6f1b579fe1a..2b05b51f97be3 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -769,19 +769,16 @@ void PropagateDownloadFile::setDeleteExistingFolder(bool enabled) void PropagateDownloadFile::done(const SyncFileItem::Status status, const QString &errorString, const ErrorCategory category) { -#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 if (_needParentFolderRestorePermissions) { FileSystem::setFolderPermissions(QString::fromStdWString(_parentPath.wstring()), FileSystem::FolderPermissions::ReadOnly); emit propagator()->touchedFile(QString::fromStdWString(_parentPath.wstring())); _needParentFolderRestorePermissions = false; } -#endif PropagateItemJob::done(status, errorString, category); } void PropagateDownloadFile::makeParentFolderModifiable(const QString &fileName) { -#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 try { const auto newDirPath = std::filesystem::path{fileName.toStdWString()}; Q_ASSERT(newDirPath.has_parent_path()); @@ -805,7 +802,6 @@ void PropagateDownloadFile::makeParentFolderModifiable(const QString &fileName) emit propagator()->touchedFile(QString::fromStdWString(_parentPath.wstring())); _needParentFolderRestorePermissions = true; } -#endif } const char owncloudCustomSoftErrorStringC[] = "owncloud-custom-soft-error-string"; @@ -1349,13 +1345,11 @@ void PropagateDownloadFile::downloadFinished() return; } -#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 if (_needParentFolderRestorePermissions) { FileSystem::setFolderPermissions(QString::fromStdWString(_parentPath.wstring()), FileSystem::FolderPermissions::ReadOnly); emit propagator()->touchedFile(QString::fromStdWString(_parentPath.wstring())); _needParentFolderRestorePermissions = false; } -#endif FileSystem::setFileHidden(filename, false); diff --git a/src/libsync/propagatedownload.h b/src/libsync/propagatedownload.h index 912cab03e9839..2200ee531b34e 100644 --- a/src/libsync/propagatedownload.h +++ b/src/libsync/propagatedownload.h @@ -23,9 +23,7 @@ #include #include -#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 #include -#endif namespace OCC { class PropagateDownloadEncrypted; @@ -270,9 +268,7 @@ private slots: PropagateDownloadEncrypted *_downloadEncryptedHelper = nullptr; -#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 std::filesystem::path _parentPath; -#endif bool _needParentFolderRestorePermissions = false; }; } diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp index 0759e6bd8ecaf..b1d85e198989d 100644 --- a/src/libsync/propagatorjobs.cpp +++ b/src/libsync/propagatorjobs.cpp @@ -32,9 +32,7 @@ #include #include -#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 #include -#endif #include @@ -62,12 +60,10 @@ bool PropagateLocalRemove::removeRecursively(const QString &path) QString absolute = propagator()->fullLocalPath(_item->_file + path); QStringList errors; QList> deleted; -#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 const auto fileInfo = QFileInfo{absolute}; const auto parentFolderPath = fileInfo.dir().absolutePath(); const auto parentPermissionsHandler = FileSystem::FilePermissionsRestore{parentFolderPath, FileSystem::FolderPermissions::ReadWrite}; FileSystem::setFolderPermissions(absolute, FileSystem::FolderPermissions::ReadWrite); -#endif bool success = FileSystem::removeRecursively( absolute, [&deleted](const QString &path, bool isDir) { @@ -132,12 +128,10 @@ void PropagateLocalRemove::start() } } else { if (FileSystem::fileExists(filename)) { -#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 const auto fileInfo = QFileInfo{filename}; const auto parentFolderPath = fileInfo.dir().absolutePath(); const auto parentPermissionsHandler = FileSystem::FilePermissionsRestore{parentFolderPath, FileSystem::FolderPermissions::ReadWrite}; -#endif if (!FileSystem::remove(filename, &removeError)) { done(SyncFileItem::NormalError, removeError, ErrorCategory::GenericError); @@ -200,7 +194,6 @@ void PropagateLocalMkdir::startLocalMkdir() return; } -#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 auto parentFolderPath = std::filesystem::path{}; auto parentNeedRollbackPermissions = false; try { @@ -225,7 +218,6 @@ void PropagateLocalMkdir::startLocalMkdir() { qCWarning(lcPropagateLocalMkdir) << "exception when checking parent folder access rights"; } -#endif emit propagator()->touchedFile(newDirStr); QDir localDir(propagator()->localPath()); @@ -234,7 +226,6 @@ void PropagateLocalMkdir::startLocalMkdir() return; } -#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 if (!_item->_remotePerm.isNull() && !_item->_remotePerm.hasPermission(RemotePermissions::CanAddFile) && !_item->_remotePerm.hasPermission(RemotePermissions::CanAddSubDirectories)) { @@ -279,7 +270,6 @@ void PropagateLocalMkdir::startLocalMkdir() { qCWarning(lcPropagateLocalMkdir) << "exception when checking parent folder access rights"; } -#endif // Insert the directory into the database. The correct etag will be set later, // once all contents have been propagated, because should_update_metadata is true. @@ -360,7 +350,6 @@ void PropagateLocalRename::start() return; } -#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 auto targetParentFolderPath = std::filesystem::path{}; auto targetParentFolderWasReadOnly = false; try { @@ -431,31 +420,26 @@ void PropagateLocalRename::start() }; const auto folderPermissionsHandler = FileSystem::FilePermissionsRestore{existingFile, FileSystem::FolderPermissions::ReadWrite}; -#endif emit propagator()->touchedFile(existingFile); emit propagator()->touchedFile(targetFile); if (QString renameError; !FileSystem::rename(existingFile, targetFile, &renameError)) { -#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 if (targetParentFolderWasReadOnly) { restoreTargetPermissions(targetParentFolderPath); } if (originParentFolderWasReadOnly) { restoreTargetPermissions(originParentFolderPath); } -#endif done(SyncFileItem::NormalError, renameError, ErrorCategory::GenericError); return; } -#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 if (targetParentFolderWasReadOnly) { restoreTargetPermissions(targetParentFolderPath); } if (originParentFolderWasReadOnly) { restoreTargetPermissions(originParentFolderPath); } -#endif } SyncJournalFileRecord oldRecord; diff --git a/test/syncenginetestutils.cpp b/test/syncenginetestutils.cpp index 637c9099c415c..8eac8a06ecba1 100644 --- a/test/syncenginetestutils.cpp +++ b/test/syncenginetestutils.cpp @@ -17,9 +17,7 @@ #include #include -#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 #include -#endif PathComponents::PathComponents(const char *path) : PathComponents { QString::fromUtf8(path) } @@ -52,11 +50,9 @@ void DiskFileModifier::remove(const QString &relativePath) if (fi.isFile()) { QVERIFY(_rootDir.remove(relativePath)); } else { -#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 const auto pathToDelete = fi.filePath().toStdWString(); std::filesystem::permissions(pathToDelete, std::filesystem::perms::owner_exec, std::filesystem::perm_options::add); QVERIFY(std::filesystem::remove_all(pathToDelete)); -#endif } } diff --git a/test/testpermissions.cpp b/test/testpermissions.cpp index a090d3b6fe141..5a1345741ee7f 100644 --- a/test/testpermissions.cpp +++ b/test/testpermissions.cpp @@ -11,9 +11,7 @@ #include -#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 #include -#endif #include using namespace OCC; @@ -91,7 +89,6 @@ private slots: QStandardPaths::setTestModeEnabled(true); } -#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 void t7pl() { FakeFolder fakeFolder{ FileInfo() }; @@ -428,7 +425,6 @@ private slots: QCOMPARE(count, 2); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); } -#endif static void setAllPerm(FileInfo *fi, OCC::RemotePermissions perm) { @@ -604,7 +600,6 @@ private slots: QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); } -#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 static void demo_perms(std::filesystem::perms p) { using std::filesystem::perms; @@ -952,7 +947,6 @@ private slots: QVERIFY(ensureReadOnlyItem("/readOnlyFolder/test/newFile.txt")); QVERIFY(ensureReadOnlyItem("/readOnlyFolder/newFolder")); } -#endif }; QTEST_GUILESS_MAIN(TestPermissions) From a0136117ca955e5618ba8af4a66d575ff347ed10 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Fri, 4 Apr 2025 11:06:59 +0200 Subject: [PATCH 2/7] always make the file we try to delete be read-write as required on windows you must only delete read-write files Signed-off-by: Matthieu Gallien --- src/common/filesystembase.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/common/filesystembase.cpp b/src/common/filesystembase.cpp index 90dea280ff6ad..ffcd4f36bfe0f 100644 --- a/src/common/filesystembase.cpp +++ b/src/common/filesystembase.cpp @@ -586,9 +586,7 @@ bool FileSystem::remove(const QString &fileName, QString *errorString) #ifdef Q_OS_WIN // You cannot delete a read-only file on windows, but we want to // allow that. - if (!isWritable(fileName)) { - setFileReadOnly(fileName, false); - } + setFileReadOnly(fileName, false); #endif QFile f(fileName); if (!f.remove()) { From 1b2db61ce47f7c800c4a1ca49ca3d9b6113d1324 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Fri, 4 Apr 2025 11:07:28 +0200 Subject: [PATCH 3/7] do not change permissions of invalid items on discovery if we are going to delete those items, we will make them have the proper permissions when we need Signed-off-by: Matthieu Gallien --- src/libsync/discovery.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 9d7cede827ba0..9939d44739021 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -1871,7 +1871,6 @@ bool ProcessDirectoryJob::checkPermissions(const OCC::SyncFileItemPtr &item) item->_errorString = tr("Not allowed because you don't have permission to add subfolders to that folder"); const auto localPath = QString{_discoveryData->_localDir + item->_file}; qCWarning(lcDisco) << "unexpected new folder in a read-only folder will be made read-write" << localPath; - FileSystem::setFolderPermissions(localPath, FileSystem::FolderPermissions::ReadWrite); emit _discoveryData->remnantReadOnlyFolderDiscovered(item); return false; } else if (!item->isDirectory() && !perms.hasPermission(RemotePermissions::CanAddFile)) { @@ -2077,7 +2076,6 @@ int ProcessDirectoryJob::processSubJobs(int nbJobs) _dirItem->_errorString = tr("Not allowed because you don't have permission to add subfolders to that folder"); const auto localPath = QString{_discoveryData->_localDir + _dirItem->_file}; qCWarning(lcDisco) << "unexpected new folder in a read-only folder will be made read-write" << localPath; - FileSystem::setFolderPermissions(localPath, FileSystem::FolderPermissions::ReadWrite); emit _discoveryData->remnantReadOnlyFolderDiscovered(_dirItem); } From 69599b010fd8ef3e1e5dc418d15961275f9c702e Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Fri, 4 Apr 2025 11:09:38 +0200 Subject: [PATCH 4/7] if we are going to need modified permissions: always change we might need to change the permissions, so do it instead of trying to be smart Signed-off-by: Matthieu Gallien --- src/libsync/filesystem.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsync/filesystem.cpp b/src/libsync/filesystem.cpp index 580562acc3d49..ecf950fe34e2c 100644 --- a/src/libsync/filesystem.cpp +++ b/src/libsync/filesystem.cpp @@ -535,8 +535,8 @@ FileSystem::FilePermissionsRestore::FilePermissionsRestore(const QString &path, _initialPermissions = FileSystem::isFolderReadOnly(stdStrPath) ? OCC::FileSystem::FolderPermissions::ReadOnly : OCC::FileSystem::FolderPermissions::ReadWrite; if (_initialPermissions != temporaryPermissions) { _rollbackNeeded = true; - FileSystem::setFolderPermissions(_path, temporaryPermissions); } + FileSystem::setFolderPermissions(_path, temporaryPermissions); } catch (const std::filesystem::filesystem_error &e) { From bf1a13913a5f378f70eed3063c35bf69da2477c4 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Fri, 4 Apr 2025 16:33:56 +0200 Subject: [PATCH 5/7] we must not use QFile::remove that may fail on windows read-only files Signed-off-by: Matthieu Gallien --- src/gui/conflictsolver.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gui/conflictsolver.cpp b/src/gui/conflictsolver.cpp index 780e4087b1dd4..5292d48e3280e 100644 --- a/src/gui/conflictsolver.cpp +++ b/src/gui/conflictsolver.cpp @@ -95,7 +95,7 @@ bool ConflictSolver::deleteLocalVersion() if (FileSystem::isDir(_localVersionFilename)) { return FileSystem::removeRecursively(_localVersionFilename); } else { - return QFile(_localVersionFilename).remove(); + return FileSystem::remove(_localVersionFilename); } } From b97f7eb84cb76f435f2b66d20a8c6a9fbae1f7d4 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Fri, 4 Apr 2025 16:37:11 +0200 Subject: [PATCH 6/7] when deleting a folder starts by making it read-write permissions Signed-off-by: Matthieu Gallien --- src/libsync/filesystem.cpp | 2 ++ src/libsync/propagatorjobs.cpp | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libsync/filesystem.cpp b/src/libsync/filesystem.cpp index ecf950fe34e2c..5b719bd8be645 100644 --- a/src/libsync/filesystem.cpp +++ b/src/libsync/filesystem.cpp @@ -259,6 +259,8 @@ qint64 FileSystem::getSize(const QString &filename) // Code inspired from Qt5's QDir::removeRecursively bool FileSystem::removeRecursively(const QString &path, const std::function &onDeleted, QStringList *errors) { + FileSystem::setFolderPermissions(path, FileSystem::FolderPermissions::ReadWrite); + bool allRemoved = true; QDirIterator di(path, QDir::AllEntries | QDir::Hidden | QDir::System | QDir::NoDotAndDotDot); diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp index b1d85e198989d..a443d84547988 100644 --- a/src/libsync/propagatorjobs.cpp +++ b/src/libsync/propagatorjobs.cpp @@ -63,7 +63,6 @@ bool PropagateLocalRemove::removeRecursively(const QString &path) const auto fileInfo = QFileInfo{absolute}; const auto parentFolderPath = fileInfo.dir().absolutePath(); const auto parentPermissionsHandler = FileSystem::FilePermissionsRestore{parentFolderPath, FileSystem::FolderPermissions::ReadWrite}; - FileSystem::setFolderPermissions(absolute, FileSystem::FolderPermissions::ReadWrite); bool success = FileSystem::removeRecursively( absolute, [&deleted](const QString &path, bool isDir) { From 31b10e519c0f00b5ea56e9fc567fcdfd1b8573cb Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Fri, 4 Apr 2025 16:37:47 +0200 Subject: [PATCH 7/7] better error message for user when deleting local items we might be displaying too technical errors that the user will have no way to understand for example on macOS, we might be getting: Unknown error: 513 Signed-off-by: Matthieu Gallien --- src/libsync/propagatorjobs.cpp | 21 ++++++++------------- src/libsync/propagatorjobs.h | 2 +- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp index a443d84547988..86086c3357e14 100644 --- a/src/libsync/propagatorjobs.cpp +++ b/src/libsync/propagatorjobs.cpp @@ -58,18 +58,15 @@ QByteArray localFileIdFromFullId(const QByteArray &id) bool PropagateLocalRemove::removeRecursively(const QString &path) { QString absolute = propagator()->fullLocalPath(_item->_file + path); - QStringList errors; QList> deleted; const auto fileInfo = QFileInfo{absolute}; const auto parentFolderPath = fileInfo.dir().absolutePath(); const auto parentPermissionsHandler = FileSystem::FilePermissionsRestore{parentFolderPath, FileSystem::FolderPermissions::ReadWrite}; - bool success = FileSystem::removeRecursively( - absolute, - [&deleted](const QString &path, bool isDir) { - // by prepending, a folder deletion may be followed by content deletions - deleted.prepend(qMakePair(path, isDir)); - }, - &errors); + const auto success = FileSystem::removeRecursively(absolute, + [&deleted](const QString &path, bool isDir) { + // by prepending, a folder deletion may be followed by content deletions + deleted.prepend(qMakePair(path, isDir)); + }); if (!success) { // We need to delete the entries from the database now from the deleted vector. @@ -87,8 +84,6 @@ bool PropagateLocalRemove::removeRecursively(const QString &path) qCWarning(lcPropagateLocalRemove) << "Failed to delete file record from local DB" << it.first.mid(propagator()->localPath().size()); } } - - _error = errors.join(", "); } return success; } @@ -116,13 +111,13 @@ void PropagateLocalRemove::start() if (_moveToTrash && propagator()->syncOptions()._vfs->mode() != OCC::Vfs::WindowsCfApi) { if ((QDir(filename).exists() || FileSystem::fileExists(filename)) && !FileSystem::moveToTrash(filename, &removeError)) { - done(SyncFileItem::NormalError, removeError, ErrorCategory::GenericError); + done(SyncFileItem::NormalError, tr("Temporary error when removing local item removed from server."), ErrorCategory::GenericError); return; } } else { if (_item->isDirectory()) { if (QDir(filename).exists() && !removeRecursively(QString())) { - done(SyncFileItem::NormalError, _error, ErrorCategory::GenericError); + done(SyncFileItem::NormalError, tr("Temporary error when removing local item removed from server."), ErrorCategory::GenericError); return; } } else { @@ -133,7 +128,7 @@ void PropagateLocalRemove::start() const auto parentPermissionsHandler = FileSystem::FilePermissionsRestore{parentFolderPath, FileSystem::FolderPermissions::ReadWrite}; if (!FileSystem::remove(filename, &removeError)) { - done(SyncFileItem::NormalError, removeError, ErrorCategory::GenericError); + done(SyncFileItem::NormalError, tr("Temporary error when removing local item removed from server."), ErrorCategory::GenericError); return; } } diff --git a/src/libsync/propagatorjobs.h b/src/libsync/propagatorjobs.h index d5e2148c15c42..c7436b00a565d 100644 --- a/src/libsync/propagatorjobs.h +++ b/src/libsync/propagatorjobs.h @@ -44,7 +44,7 @@ class PropagateLocalRemove : public PropagateItemJob private: bool removeRecursively(const QString &path); - QString _error; + bool _moveToTrash = false; };