Is there any predefined function which returns the file size limit for a given file system?

Maybe we could cache the value of KFileSystemType::fileSystemType() somewhere so we don't need to calculate it again for every file, since it's the same for every one.

Thanks, this is looking better. I will test it out later.

It's not a huge deal, but 4294967296 plus a comment saying /* 4Gb */ or something might be clearer and more readable than qPow(4*M_E, 9). Then you could avoid adding the <QtMath> import too.

Thank you for indulging me 😊

Tested this out and couldn't actually make it work the way I asked--with the FAT32 file size limit checked first. I think I see why: there are two total size > available space? checks that output ERR_DISK_FULL on error, and both of them execute first. The FAT32 file size limit check should be before the first one, probably near // Stat the next src url.

1ul << 32) -1 needs a comment explaining what it means, for people like me with puny brains. :)

There are two checks in this file that result in an ERR_DISK_FULL if there is not enough space. The FAT32 check is now before the second one, but not the first one, so the first one still will still look for adequate space before checking to see whether or not the copy is even possible due to FAT32 file size restrictions.

@ngraham No, please recheck, the check is before the first m_totalSize > m_freeSpace check

@bruns Apart from these, does the code seems sane to you?

Can I get a review?

Please update the diff.

"Format with destination drive to a filesystem type which" -> "Reformat the destination drive to use a filesystem that"

Uh-oh, I think the latest update to this diff isn't quite correct. I see unrelated changes that you didn't make being marked as additions.

You might try using arc in the future. It makes this process much simpler. See https://community.kde.org/Infrastructure/Phabricator#Using_Arcanist_to_post_patches

Uh-oh, now the patch doesn't apply for me anymore:

$ arc patch D16249
 INFO  Base commit is not in local repository; trying to fetch.
Created and checked out branch arcpatch-D16249.
Checking patch src/core/job_error.cpp...
error: while searching for:
    case KIO::ERR_CANNOT_CREATE_SLAVE:
        result = i18n("Unable to create io-slave. %1", errorText);
        break;
    default:
        result = i18n("Unknown error code %1\n%2\nPlease send a full bug report at https://bugs.kde.org.", errorCode, errorText);
        break;
    }


error: patch failed: src/core/job_error.cpp:245
Hunk #2 succeeded at 1068 (offset -4 lines).
Checking patch src/core/global.h...
Checking patch src/core/copyjob.cpp...
Applying patch src/core/job_error.cpp with 1 reject...
Rejected hunk #1.
Hunk #2 applied cleanly.
Applied patch src/core/global.h cleanly.
Applied patch src/core/copyjob.cpp cleanly.

 Patch Failed! 
Usage Exception: Unable to apply patch!

Might need to be re-based on the current state of master. But you really should look into using arc; it makes all of this so much easier. Setting it up takes all of 5 minutes. See https://community.kde.org/Infrastructure/Phabricator#Using_Arcanist

@bruns @ngraham what's your view on this? Did you tested it out?

@ngraham @bruns do you have any objections?

Sorry for the extended review time; I've found myself super busy with family stuff during this vacation. :)

Please address Elvis' inline comment and then this will look good to me.

@bruns is it good to go?

Waiting for green flags.

Also +1 from me.

However, I would strongly suggest waiting till after Saturday when frameworks is tagged.
Then you'll have a month for translators and for some testing.

I agree. Because this contains new strings and we're so close to the 5.53 release, let's land this for Frameworks 5.54 after tagging.

@shubham I'm afraid this means you'll now need to change all the instances of 5.53 to 5.54! 😛 I know it's taken a long time, but don't worry, it will be landed soon...

Awesome, can't wait to have this in!

This can land now!