unzipping to a partition other than `C:/` fails with OSError

[maradotwebp]

On Windows, unzipping to a partition other than C:/ fails.

When unzipping, you creating a temporary folder & are writing archive contents to that temporary folder, then moving contents to the final folder. However, Nim currently has a bug where moveDir operations across partitions fail.

I would suggest either:

• waiting until the issue is fixed
• replacing moveDir(tempDir, dest) with a copyDir and removeDir operation
• write directly to the destination folder instead of using a temporary dir (is there a reason for that?)

[timotheecour]

@froehlichA what commands reproduce this issue? (showing the nim git hash always helps)

now that your PR that fixes https://github.com/nim-lang/Nim/issues/18216 got merged, this is a performance rather than correctness issue IIUC.

if a temporary dir is indeed needed (why?), that temporary dir should be on the same drive as destination

[guzba]

Currently, when extracting, a temporary directory is created and all of the files are written there first. Then this directory is moved to the final directory path.

This is done so that if something fails, there isn't a random half finished export polluting the directory. It will either fully work, or fully fail.

[timotheecour]

how about this:

import std/tempfiles
let tmpDir = createTempDir("zippy", ".tmp", dir = destDir)
try:
  unzip to tmpDir
except OsError as e:
  removeDir(tmpDir)
  raise e
moveDir(tmpDir, destDir)

then it avoids the expensive moveDir across drives

[maradotwebp]

isn't std/tempfiles experimental? And not available in the latest stable Nim release yet?

But the approach is correct, you could just create a ./tmp/ folder or a similar folder of any other name in the destination folder - there are guaranteed to be no conflicts with existing directories since extractAll fails in case of an existing destination folder anyway.

If you want to, I could whip up a PR to solve the issue.

[timotheecour]

isn't std/tempfiles experimental? And not available in the latest stable Nim release yet?

the API may be subject to change but it's not going to go away/be removed, in any case you can use:

when (NimMajor, NimMinor, NimPatch) >= (1,5,1): ...

so at least i'll be fixed for 1.5.1 onwards. better than rolling your own IMO, to avoid all the caveats that this involves