D29610: [kio_file] Handle renaming file 'A' to 'a' on FAT32 filesystems

[Ahmad Samir]

ahmadsamir added inline comments.

INLINE COMMENTS

> dfaure wrote in file_unix.cpp:1052
> I'm confused. We want to solve renaming 'a' to 'A' when 'A' does *not* exist.
> 
> QFile::rename will not overwrite an existing file, so it will do nothing if dest exists.

const QByteArray dest1 = "/mnt/fat32/A";
  const QByteArray dest2 = "/mnt/fat32/a";
  QT_STATBUF buff_dest;
  qDebug() << QT_LSTAT(dest1, &buff_dest);
  qDebug() << QT_LSTAT(dest2, &buff_dest);

IIUC, from FAT32 POV, 'A' exists if 'a' exists and vice versa.

> dfaure wrote in file_unix.cpp:1074
> Wouldn't it be enough to just call QFile::rename here?
> 
> The whole idea is: if QFile::rename is able to rename a file in all cases, including the a->A special case on FAT, then let's just delegate the renaming to QFile.
> 
> Then we don't need to have any special case in our code.
> 
> QFile::rename does not overwrite, though, so if the dest exists and the Overwrite flag is set, we might have to either delete the dest first (race condition, not sure it matters here), or in *that* case use ::rename() since we know it can't be a FAT32-case-change (FAT32 can't have both a and A).

Too many quirks if we don't use ::rename(). I tested with e.g. in file_unix.cpp:

  bool renamed =  QFile::rename(src, dest);
  if (!renamed) {
      if ((_flags & KIO::Overwrite) || isSrcSymLink) {
          renamed = ::rename(_src.data(), _dest.data()) == 0;
      }
  }

- QFile::rename doesn't overwrite as you said
- renaming symlinks depends on ::rename
- some unit tests are failing due to permissions

  FAIL!  : JobTest::statDetailsBasic() Compared values are not the same
     Actual   (kioItem.permissions()): 420
     Expected (438)                  : 438
     Loc: [/home/ahmad/dev/kio/autotests/jobtest.cpp(1464)]
  INFO   : JobTest::statDetailsBasicSetDetails() entering
  FAIL!  : JobTest::statDetailsBasicSetDetails() Compared values are not the same
     Actual   (kioItem.permissions()): 420
     Expected (438)                  : 438
     Loc: [/home/ahmad/dev/kio/autotests/jobtest.cpp(1503)]

That and ::rename() is used all over the place in file_unix.cpp, I am not that comfortable using QFile::rename except for the freaky FAT32 case :)

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D29610

To: ahmadsamir, #frameworks, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200515/ab35486e/attachment-0001.htm>