lemire / FastPFor

The FastPFOR C++ library: Fast integer compression
Apache License 2.0
873 stars 123 forks source link

Implemented packaging using CPack #82

Closed KOLANICH closed 1 year ago

lemire commented 3 years ago

This PR breaks the build under Visual Studio 2017, see CI tests...

LINK : fatal error LNK1104: cannot open file 'Debug\FastPFOR.lib' [C:\projects\fastpfor\build\FastPFOR_unittest.vcxproj]
lemire commented 3 years ago

@KOLANICH Please sync with the main branch. I have switched the CI tests to run with the Release version of the code. The tests should run much faster.

KOLANICH commented 3 years ago

Hmm, it is definitely not timeout, but something else.

lemire commented 3 years ago

@KOLANICH The error indicates that the DLL is not found.

lemire commented 3 years ago

With a static build (the default in CMake), the library is included in the binaries. With a dynamic build, a distinct file (e.g., a DLL) is created in addition to the binary executable. In a portable way, one must then ensure that when the binary run, the dynamic library is found. This requires extra care.

KOLANICH commented 2 years ago

I'm sorry, IDK what is wrong. For me (Windows 7, 32-bit, core2duo CPU (conroe, no avx, only sse3), MSYS2 MinGW-w64 + clang-12) everything builds (though there are issues with -lpthread not being added, I have tried to add it into PUBLIC dependencies of libfastpfor without much success (helped only for some libs), it helped to add it into all the exes, but I don't understand why I should do it and I don't think that that patch should go here).

Also: on my configuration the second test fails with reports of wrong answers.

lemire commented 2 years ago

@KOLANICH Please review our CI logs. We have two independent Windows CI tests and this PR fails both of them.

Note that building is not sufficient, we need to run the tests successfully.

lemire commented 1 year ago

I am going to close this for now. We would love to have support for cpack and support dynamic libraries, but it is crucially important that we be able to continue running out tests successfully on various platforms. We cannot merge a PR that would break our build.