[BUG] Unexpected behavior moving directories to external drives

General JUCE discussion
1

I’ve encountered what I believe to be a bug when attempting to move a non-empty directory to an external SSD. The operation fails, and digging into the source code I’ve found the problem:

bool File::moveInternal (const File& dest) const
{
    if (rename (fullPath.toUTF8(), dest.getFullPathName().toUTF8()) == 0)
        return true;

    if (hasWriteAccess() && copyInternal (dest))
    {
        if (deleteFile())
            return true;

        dest.deleteFile();
    }

    return false;
}

Assuming that dest is a location on an external drive and *this is a File on the main disk, the rename syscall fails with errno == EXDEV, because renames are not allowed across different filesystems (my drive is ExFAT for Windows cross-compatibility).

After the rename fails, the copyInternal actually succeeds without a hitch. The issue is that the attempt to deleteFile() on the origin directory fails, because of this code:

        if (isDirectory())
            return rmdir (fullPath.toUTF8()) == 0;

        dest.deleteFile();

rmdir doesn’t succeed here because it only operates on empty directories (fails with errno == ENOTEMPTY).

To me, it seems like the intention of this copy-and-delete operation would be for the entire directory to be copied over, and the source directory to be deleted alongside its contents, hence my sense that this is a bug. Furthermore, the dest.deleteFile() line fails for the same reason, meaning the copied destination directory still exists after the “failure”. I’m going to write a workaround in my own code, but wanted to bring this to the team’s attention.

File::moveToTrash() does not work if file is not on internal drive
2

You’re right that in your case using File::moveFileTo() will fail and leave unwanted file system changes behind.

A change was released on develop that will fail the operation before making any file system changes.

The File::moveFileTo() is generally not appropriate to use with directories, because the API lacks the necessary options to specify what to do in various cases when the target directory already exists and contains some files. A word about this has been added to the documentation.

In some cases it will work correctly, but when it fails, the copyDirectoryTo() and deleteRecursively() functions can be used to prescribe the required filesystem operations unambiguously.

3

Thanks for your response (and sorry for getting back to you so late). The behavior + documentation in the commit you linked are perfect, thank you for getting that in there. I was able to workaround using the copy/delete idiom you described, glad to hear that’s actually the prescription.