[stable-3.16] Bugfix/delete errors

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()) {

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);
     }
 }
 

src/gui/folder.cpp

@@ -1736,11 +1736,9 @@ void Folder::slotNeedToRemoveRemnantsReadOnlyFolders(const QList<SyncFileItemPtr
 
     setSyncPaused(true);
     for(const auto &oneFolder : folders) {
-#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= 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 {

src/libsync/discovery.cpp

@@ -1870,11 +1870,8 @@ 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 +2075,8 @@ 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;

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<void(const QString &path, bool isDir)> &onDeleted, QStringList *errors)
 {
+    FileSystem::setFolderPermissions(path, FileSystem::FolderPermissions::ReadWrite);
+
     bool allRemoved = true;
     QDirIterator di(path, QDir::AllEntries | QDir::Hidden | QDir::System | QDir::NoDotAndDotDot);
 
@@ -274,11 +276,9 @@ bool FileSystem::removeRecursively(const QString &path, const std::function<void
         } else {
             QString removeError;
 
-#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= 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 +295,10 @@ bool FileSystem::removeRecursively(const QString &path, const std::function<void
             allRemoved = false;
     }
     if (allRemoved) {
-#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= 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 +324,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
 {
@@ -540,8 +537,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)
     {
@@ -564,6 +561,4 @@ FileSystem::FilePermissionsRestore::~FilePermissionsRestore()
     }
 }
 
-#endif
-
 } // namespace OCC

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
 }
 
 /** @} */

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();

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);
 

src/libsync/propagatedownload.h

@@ -23,9 +23,7 @@
 #include <QBuffer>
 #include <QFile>
 
-#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15
 #include <filesystem>
-#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;
 };
 }

src/libsync/propagatorjobs.cpp

@@ -32,9 +32,7 @@
 #include <qstack.h>
 #include <QCoreApplication>
 
-#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15
 #include <filesystem>
-#endif
 #include <ctime>
 
 
@@ -60,21 +58,15 @@ QByteArray localFileIdFromFullId(const QByteArray &id)
 bool PropagateLocalRemove::removeRecursively(const QString &path)
 {
     QString absolute = propagator()->fullLocalPath(_item->_file + path);
-    QStringList errors;
     QList<QPair<QString, bool>> 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) {
-            // 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.
@@ -92,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;
 }
@@ -121,26 +111,24 @@ 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 {
             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);
+                    done(SyncFileItem::NormalError, tr("Temporary error when removing local item removed from server."), ErrorCategory::GenericError);
                     return;
                 }
             }
@@ -200,7 +188,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 +212,6 @@ void PropagateLocalMkdir::startLocalMkdir()
     {
         qCWarning(lcPropagateLocalMkdir) << "exception when checking parent folder access rights";
     }
-#endif
 
     emit propagator()->touchedFile(newDirStr);
     QDir localDir(propagator()->localPath());
@@ -234,7 +220,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 +264,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 +344,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 +414,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;

src/libsync/propagatorjobs.h

@@ -44,7 +44,7 @@ class PropagateLocalRemove : public PropagateItemJob
 
 private:
     bool removeRecursively(const QString &path);
-    QString _error;
+
     bool _moveToTrash = false;
 };
 

test/syncenginetestutils.cpp

@@ -17,9 +17,7 @@
 #include <QJsonValue>
 
 #include <memory>
-#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15
 #include <filesystem>
-#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
     }
 }