Failing unit tests on Windows

[wutzig]

@Lecrapouille - as to https://github.com/Lecrapouille/zipper/issues/4#issuecomment-2795154010 unit tests on Windows currently fail. When building the tests, e.g. with PWD="${CMAKE_SOURCE_DIR}/tests" some existence checks are successful while e.g. TestDir.fileName returns /foo/bar/file.txt instead of the expected file.txt.

[Lecrapouille]

Sorry mate I did not notice your 3 comments during my numerous rebases. Yes I know CI not longer work with cmake. I had not time to investigate. I'll read your comments tonight.

[Lecrapouille]

@wutzig now everything seems ok. I still need to complete the CI: give a try with linkage, run tests for Windows ...

[wutzig]

While the library and the tests are building correctly on all platforms, the unit tests are still failing on windows. Locally 21 out of the 33 tests are not passing. There seems to be something wrong on how paths are handled on Windows systems.

[Lecrapouille]

Yes, I'm fixing them. The issue is: my tests were made for Unix and the lib was using '/' as directory separator for Unix while now we are running them on Windows with '\' as directory separator.

[Lecrapouille]

Seems good to me. I'm tagging v2.3.0 and adding your name in credits.

[Lecrapouille]

~~@wutzig I reopened. I noticed that I added a temporary #if 0 that I forget to remove + I added a regression. I have to check back all stuffs and redo version. CI failed on MacOS looks like there is memory issues detected by Valgrind probably in outdated minizip lib. I gave up with this project~~

[Lecrapouille]

@wutzig I still have some Windows issues, It let you investigate because I have no more idea how to fix it. I fixed all others (valgrind, OSX ...) and added more tests. If we fix this, I'll be able to tag again the v2.3.0

[wutzig]

I saw your changes, looks great. I'll see if I can figure out what's still broken.

[wutzig]

There is one failing test ExtractFileWithNameOfDir. The path of the unzipped entry would exist already as a directory. This would have to be caught explicitly in Unzipper::extractToFile, for example by including in line 452

if(Path::isDir(filename)) {
    str << "Failed creating '" << Path::toNativeSeparators(filename)
        << "' file because Is a directory";
    
    m_error_code = make_error_code(unzipper_error::INTERNAL_ERROR,
        str.str()
    );
    return UNZ_ERRNO;
}

Another one is Issue83 where the archive is extracted to a non-existent location /does/not/exist/. However, Unzipper::extractToFile checks if the target folder isn't empty, which is true

int extractToFile(std::string const& filename, ZipEntry& info, bool const replace)
{
    // If zip entry is a directory then create it on disk
    std::string folder = Path::dirName(filename);
    if (!folder.empty())
    {
        ...

and then calls Path::createDir (line 395) recursively, This succeeds on Windows resulting in a path C:\does\not\exist. I am not sure if this is by design.

[Lecrapouille]

@wutzig I sent you an invitation to become a collaborator: like this you can commit directly in the git repo instead of doing pull requests. You may decline the invitation if you are not interested

[Lecrapouille]

@wutzig please do not commit yet on master (or do it in another branch) because I'm passing AI on the Zipper.cpp code, and of course, fixing old code (initially not mine) made the option SaveHierarchy for zipper.add() effective (it was not) ;( I dunno why the original author wanted to force people to add this flag. SaveHierarchy does not remove folders, while empty by default removes folders. Can we meet on Discord ? Lecrapouille#0106

[wutzig]

@Lecrapouille I sent you an invite on discord.

[Lecrapouille]

Done :)