[platform] Remove chmod-esque functions

include/multipass/file_ops.h

@@ -81,7 +81,6 @@ class FileOps : public Singleton<FileOps>
     virtual bool rename(QFile& file, const QString& newName) const;
     virtual bool resize(QFile& file, qint64 sz) const;
     virtual bool seek(QFile& file, qint64 pos) const;
-    virtual bool setPermissions(QFile& file, QFileDevice::Permissions permissions) const;
     virtual qint64 size(QFile& file) const;
     virtual qint64 write(QFile& file, const char* data, qint64 maxSize) const;
     virtual qint64 write(QFileDevice& file, const QByteArray& data) const;
@@ -112,7 +111,6 @@ class FileOps : public Singleton<FileOps>
     virtual bool remove(const fs::path& path, std::error_code& err) const;
     virtual void create_symlink(const fs::path& to, const fs::path& path, std::error_code& err) const;
     virtual fs::path read_symlink(const fs::path& path, std::error_code& err) const;
-    virtual void permissions(const fs::path& path, fs::perms perms, std::error_code& err) const;
     virtual fs::file_status status(const fs::path& path, std::error_code& err) const;
     virtual fs::file_status symlink_status(const fs::path& path, std::error_code& err) const;
     virtual std::unique_ptr<RecursiveDirIterator> recursive_dir_iterator(const fs::path& path,

include/multipass/path.h

@@ -20,10 +20,12 @@
 #ifndef MULTIPASS_PATH_H
 #define MULTIPASS_PATH_H
 
+#include <QFileDevice>
 #include <QString>
 
 namespace multipass
 {
 using Path = QString;
+using Perms = QFileDevice::Permissions;
 }
 #endif // MULTIPASS_PATH_H

include/multipass/platform.h

@@ -58,7 +58,7 @@ class Platform : public Singleton<Platform>
     virtual bool is_remote_supported(const std::string& remote) const;
     virtual bool is_backend_supported(const QString& backend) const; // temporary (?)
     virtual int chown(const char* path, unsigned int uid, unsigned int gid) const;
-    virtual bool set_permissions(const multipass::Path& path, const QFileDevice::Permissions permissions) const;
+    virtual bool set_permissions(const Path& path, const Perms permissions) const;
     virtual bool link(const char* target, const char* link) const;
     virtual bool symlink(const char* target, const char* link, bool is_dir) const;
     virtual int utime(const char* path, int atime, int mtime) const;

src/platform/platform_linux.cpp

@@ -317,7 +317,7 @@ void mp::platform::Platform::create_alias_script(const std::string& alias, const
     auto permissions =
         MP_FILEOPS.permissions(file) | QFileDevice::ExeOwner | QFileDevice::ExeGroup | QFileDevice::ExeOther;
 
-    if (!MP_FILEOPS.setPermissions(file, permissions))
+    if (!MP_PLATFORM.set_permissions(file.fileName(), permissions))
         throw std::runtime_error(fmt::format("cannot set permissions to alias script '{}'", file_path));
 }
 

src/platform/platform_unix.cpp

@@ -58,7 +58,7 @@ int mp::platform::Platform::chown(const char* path, unsigned int uid, unsigned i
     return ::lchown(path, uid, gid);
 }
 
-bool mp::platform::Platform::set_permissions(const mp::Path& path, const QFileDevice::Permissions permissions) const
+bool mp::platform::Platform::set_permissions(const Path& path, const Perms permissions) const
 {
     return QFile::setPermissions(path, permissions);
 }
@@ -126,9 +126,8 @@ void mp::platform::Platform::set_server_socket_restrictions(const std::string& s
     if (chown(socket_path.c_str(), 0, gid) == -1)
         throw std::runtime_error(fmt::format("Could not set ownership of the multipass socket: {}", strerror(errno)));
 
-    if (!set_permissions(socket_path.c_str(), mode))
-        throw std::runtime_error(
-            fmt::format("Could not set permissions for the multipass socket: {}", strerror(errno)));
+    if (!set_permissions(QString::fromStdString(socket_path), mode))
+        throw std::runtime_error(fmt::format("Could not set permissions for the multipass socket"));
 }
 
 QString mp::platform::Platform::multipass_storage_location() const

src/ssh/sftp_client.cpp

@@ -20,6 +20,7 @@
 #include "ssh_client_key_provider.h"
 #include <multipass/file_ops.h>
 #include <multipass/logging/log.h>
+#include <multipass/platform.h>
 #include <multipass/ssh/sftp_utils.h>
 #include <multipass/ssh/throw_on_error.h>
 #include <multipass/utils.h>
@@ -146,9 +147,8 @@ void SFTPClient::pull_file(const fs::path& source_path, const fs::path& target_p
     do_pull_file(source_path, *local_file);
 
     auto source_perms = mp_sftp_stat(sftp.get(), source_path.u8string().c_str())->permissions;
-    std::error_code err;
-    if (MP_FILEOPS.permissions(target_path, static_cast<fs::perms>(source_perms), err); err)
-        throw SFTPError{"cannot set permissions for local file {}: {}", target_path, err.message()};
+    if (!MP_PLATFORM.set_permissions(QString::fromStdWString(target_path.wstring()), static_cast<Perms>(source_perms)))
+        throw SFTPError{"cannot set permissions for local file {}", target_path};
 
     if (local_file->fail())
         throw SFTPError{"cannot write to local file {}: {}", target_path, strerror(errno)};
@@ -301,11 +301,11 @@ bool SFTPClient::pull_dir(const fs::path& source_path, const fs::path& target_pa
     for (auto it = subdirectory_perms.crbegin(); it != subdirectory_perms.crend(); ++it)
     {
         const auto& [path, perms] = *it;
-        MP_FILEOPS.permissions(path, static_cast<fs::perms>(perms), err);
-        if (err)
+        if (!MP_PLATFORM.set_permissions(QString::fromStdWString(path.wstring()), static_cast<Perms>(perms)))
         {
-            mpl::log(mpl::Level::error, log_category,
-                     fmt::format("cannot set permissions for local directory {}: {}", path, err.message()));
+            mpl::log(mpl::Level::error,
+                     log_category,
+                     fmt::format("cannot set permissions for local directory {}", path));
             success = false;
         }
     }

src/sshfs_mount/sftp_server.cpp

@@ -529,13 +529,11 @@ int mp::SftpServer::handle_mkdir(sftp_client_message msg)
         return reply_failure(msg);
     }
 
-    std::error_code err;
-    MP_FILEOPS.permissions(filename, static_cast<fs::perms>(msg->attr->permissions), err);
-    if (err)
+    if (!MP_PLATFORM.set_permissions(filename, static_cast<Perms>(msg->attr->permissions)))
     {
         mpl::log(mpl::Level::trace,
                  category,
-                 fmt::format("{}: set permissions failed for '{}': {}", __FUNCTION__, filename, err.message()));
+                 fmt::format("{}: set permissions failed for '{}'", __FUNCTION__, filename));
         return reply_failure(msg);
     }
 
@@ -1020,13 +1018,11 @@ int mp::SftpServer::handle_setstat(sftp_client_message msg)
 
     if (msg->attr->flags & SSH_FILEXFER_ATTR_PERMISSIONS)
     {
-        std::error_code err;
-        MP_FILEOPS.permissions(file.fileName().toStdString(), static_cast<fs::perms>(msg->attr->permissions), err);
-        if (err)
+        if (!MP_PLATFORM.set_permissions(file.fileName(), static_cast<Perms>(msg->attr->permissions)))
         {
             mpl::log(mpl::Level::trace,
                      category,
-                     fmt::format("{}: set permissions failed for '{}': {}", __FUNCTION__, filename, err.message()));
+                     fmt::format("{}: set permissions failed for '{}'", __FUNCTION__, filename));
             return reply_failure(msg);
         }
     }

src/utils/file_ops.cpp

@@ -145,11 +145,6 @@ bool mp::FileOps::seek(QFile& file, qint64 pos) const
     return file.seek(pos);
 }
 
-bool mp::FileOps::setPermissions(QFile& file, QFileDevice::Permissions permissions) const
-{
-    return file.setPermissions(permissions);
-}
-
 qint64 mp::FileOps::size(QFile& file) const
 {
     return file.size();
@@ -262,11 +257,6 @@ fs::path mp::FileOps::read_symlink(const fs::path& path, std::error_code& err) c
     return fs::read_symlink(path, err);
 }
 
-void mp::FileOps::permissions(const fs::path& path, fs::perms perms, std::error_code& err) const
-{
-    fs::permissions(path, perms, err);
-}
-
 fs::file_status mp::FileOps::status(const fs::path& path, std::error_code& err) const
 {
     return fs::status(path, err);

tests/linux/test_platform_linux.cpp

@@ -56,6 +56,7 @@
 #include <QString>
 
 #include <stdexcept>
+#include <tests/mock_platform.h>
 
 namespace mp = multipass;
 namespace mpt = multipass::test;
@@ -627,13 +628,16 @@ TEST_F(PlatformLinux, create_alias_script_overwrites)
 {
     auto [mock_utils, guard1] = mpt::MockUtils::inject();
     auto [mock_file_ops, guard2] = mpt::MockFileOps::inject();
+    auto [mock_platform, guard3] = mpt::MockPlatform::inject();
 
     EXPECT_CALL(*mock_utils, make_file_with_content(_, _, true)).Times(1);
     EXPECT_CALL(*mock_file_ops, permissions(_)).WillOnce(Return(QFileDevice::ReadOwner | QFileDevice::WriteOwner));
-    EXPECT_CALL(*mock_file_ops, setPermissions(_, _)).WillOnce(Return(true));
+    EXPECT_CALL(*mock_platform, set_permissions(_, _)).WillOnce(Return(true));
 
+    // Calls the platform function directly since MP_PLATFORM is mocked.
     EXPECT_NO_THROW(
-        MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "other_command", "map"}));
+        mock_platform->Platform::create_alias_script("alias_name",
+                                                     mp::AliasDefinition{"instance", "other_command", "map"}));
 }
 
 TEST_F(PlatformLinux, create_alias_script_throws_if_cannot_create_path)
@@ -664,14 +668,16 @@ TEST_F(PlatformLinux, create_alias_script_throws_if_cannot_set_permissions)
 {
     auto [mock_utils, guard1] = mpt::MockUtils::inject();
     auto [mock_file_ops, guard2] = mpt::MockFileOps::inject();
+    auto [mock_platform, guard3] = mpt::MockPlatform::inject();
 
     EXPECT_CALL(*mock_utils, make_file_with_content(_, _, true)).Times(1);
     EXPECT_CALL(*mock_file_ops, permissions(_)).WillOnce(Return(QFileDevice::ReadOwner | QFileDevice::WriteOwner));
-    EXPECT_CALL(*mock_file_ops, setPermissions(_, _)).WillOnce(Return(false));
+    EXPECT_CALL(*mock_platform, set_permissions(_, _)).WillOnce(Return(false));
 
     MP_EXPECT_THROW_THAT(
-        MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "command", "map"}),
-        std::runtime_error, mpt::match_what(HasSubstr("cannot set permissions to alias script '")));
+        mock_platform->Platform::create_alias_script("alias_name", mp::AliasDefinition{"instance", "command", "map"}),
+        std::runtime_error,
+        mpt::match_what(HasSubstr("cannot set permissions to alias script '")));
 }
 
 TEST_F(PlatformLinux, remove_alias_script_works)

tests/mock_file_ops.h

@@ -56,7 +56,6 @@ class MockFileOps : public FileOps
     MOCK_METHOD(bool, rename, (QFile&, const QString& newName), (const, override));
     MOCK_METHOD(bool, resize, (QFile&, qint64 sz), (const, override));
     MOCK_METHOD(bool, seek, (QFile&, qint64 pos), (const, override));
-    MOCK_METHOD(bool, setPermissions, (QFile&, QFileDevice::Permissions), (const, override));
     MOCK_METHOD(qint64, size, (QFile&), (const, override));
     MOCK_METHOD(qint64, write, (QFile&, const char*, qint64), (const, override));
     MOCK_METHOD(qint64, write, (QFileDevice&, const QByteArray&), (const, override));
@@ -87,7 +86,6 @@ class MockFileOps : public FileOps
     MOCK_METHOD(void, create_symlink, (const fs::path& to, const fs::path& path, std::error_code& err),
                 (override, const));
     MOCK_METHOD(fs::path, read_symlink, (const fs::path& path, std::error_code& err), (override, const));
-    MOCK_METHOD(void, permissions, (const fs::path& path, fs::perms perms, std::error_code& err), (override, const));
     MOCK_METHOD(fs::file_status, status, (const fs::path& path, std::error_code& err), (override, const));
     MOCK_METHOD(fs::file_status, symlink_status, (const fs::path& path, std::error_code& err), (override, const));
     MOCK_METHOD(std::unique_ptr<multipass::RecursiveDirIterator>, recursive_dir_iterator,

tests/test_file_ops.cpp

@@ -108,14 +108,6 @@ TEST_F(FileOps, symlink)
     EXPECT_FALSE(err);
 }
 
-TEST_F(FileOps, permissions)
-{
-    MP_FILEOPS.permissions(temp_file, fs::perms::all, err);
-    EXPECT_FALSE(err);
-    MP_FILEOPS.permissions(temp_dir / "nonexistent", fs::perms::all, err);
-    EXPECT_TRUE(err);
-}
-
 TEST_F(FileOps, status)
 {
     auto dir_status = MP_FILEOPS.status(temp_dir, err);