beyond-all-reason / pr-downloader

console downloader for spring maps/games written in c++
GNU General Public License v2.0
0 stars 8 forks source link

CFileSystem::getMBsFree "still" seems to fail when the path contains non-ASCII characters #16

Closed rafidka closed 2 years ago

rafidka commented 2 years ago

I reported this issue back in April on spring's pr-downloader https://github.com/spring/pr-downloader/issues/145 and it is supposed to have been fixed https://github.com/spring/pr-downloader/issues/145 in there and ported to BAR's pr-downloader as well https://github.com/beyond-all-reason/pr-downloader/commit/34cf14ad65c1ac79b2ed3886ff9e778df595c98d. However, a user reported the same issue today so it doesn't seem to have been actually fixed.

p2004a commented 2 years ago

Thank you for opening it, I will try to reproduce the issue over the weekend and look at the fix.

p2004a commented 2 years ago

Notes from my analysis so far:

Reproduced this on Windows with simple call but also with

spawnSync('C:/Users/Marek/pr-downloader.exe', ['--filesystem-writepath', 'ąćźż汉语'], {stdio: 'inherit'});

in node.js shell.

Overall the issue is that argv on windows is being flatten to the default codepage that isn't unicode.

To get access to the full argv, it's possible to use the wmain variant, or use CommandLineToArgvW to get argumnts in regular main. ffmpeg does something like this: https://github.com/FFmpeg/FFmpeg/blob/6195a0ee191728b4e638cf3b620e68acde8854b0/fftools/cmdutils.c#L209

There is also stuff like https://github.com/boostorg/nowide that also provides the other side: allows to use utf8 with fopen etc. pr-downloader already has FileSystem that handles that properly, that could be rewritten with C++17's std::filesystem.

p2004a commented 2 years ago

Ok, I have a draft https://github.com/beyond-all-reason/pr-downloader/pull/23 ready, need to do a bit more testing.