Build success on Windows (with cmake and mingw)

dhthwy commented 1 year ago

At first I tried bazel, but it seems its broken with the VS toolchain ( at least in versions 2017+ ).

So cmake it is. For a saner build environment, I use msys2.

msys2 includes a package manager 'pacman' for installing the dev tools and libraries required for building. Ensure you select the mingw variants of golang, gcc, and cmake, or you'll encounter build errors.

You can get a list of packages with: pacman -Ss

For the most part, the build went well. It is missing unistd.h's 'pread' , and cc/util/util_test.cc makes use of file permissions not supported by Windows (specifically 'group' and 'other'). Those were the only two issues blocking compilation for me.

I was able to get the aead_cli example working. Here is the CMakeLists.txt I used to build my exe.

cmake_minimum_required(VERSION 3.5)
project(aead_cli CXX)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_STANDARD 14)

add_subdirectory(third_party/tink)
add_subdirectory(util)

add_executable(aead_cli aead_cli.cc)
set(CMAKE_EXE_LINKER_FLAGS "-static-libgcc -static-libstdc++ -static")
target_link_libraries(aead_cli tink::static absl::flags_parse util)

There is an issue with it trying to link in librt on my setup, which doesn't fly for Windows. For the time being, I just delete the -lrt in build.ninja file after running cmake -DCMAKE_GENERATOR=ninja

I created a pull request #660 in case there's any interest by the tink devs.

morambro commented 1 year ago

Hi @dhthwy thanks for looking into this! Have you tried building Tink with -DTINK_BUILD_TESTS=ON and running the unit tests with ctest to check if you patch works for the util file classes?

dhthwy commented 1 year ago

Nope. But I'll try doing that and provide an update.

dhthwy commented 1 year ago

Great news. The tests are built. It spit out a bunch of individual binaries for the tests.

Running unit tests now...

$ ./tink_test_core_restricted_data_test.exe
Running main() from C:/msys64/home/da/projects/app/third_party/tink/__third_party/com_google_googletest/src/googletest
/src/gtest_main.cc
[==========] Running 9 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 9 tests from RestrictedDataTest
[ RUN      ] RestrictedDataTest.CreateAndGetSecret
[       OK ] RestrictedDataTest.CreateAndGetSecret (2 ms)
[ RUN      ] RestrictedDataTest.GenerateRandomAndSize
[       OK ] RestrictedDataTest.GenerateRandomAndSize (0 ms)
[ RUN      ] RestrictedDataTest.GenerateRandomNegative
Running main() from C:/msys64/home/da/projects/app/third_party/tink/__third_party/com_google_googletest/src/googletest
/src/gtest_main.cc
[       OK ] RestrictedDataTest.GenerateRandomNegative (331 ms)
[ RUN      ] RestrictedDataTest.Equals
[       OK ] RestrictedDataTest.Equals (0 ms)
[ RUN      ] RestrictedDataTest.NotEquals
[       OK ] RestrictedDataTest.NotEquals (0 ms)
[ RUN      ] RestrictedDataTest.CopyConstructor
[       OK ] RestrictedDataTest.CopyConstructor (0 ms)
[ RUN      ] RestrictedDataTest.CopyAssignment
[       OK ] RestrictedDataTest.CopyAssignment (0 ms)
[ RUN      ] RestrictedDataTest.MoveConstructor
[       OK ] RestrictedDataTest.MoveConstructor (0 ms)
[ RUN      ] RestrictedDataTest.MoveAssignment
[       OK ] RestrictedDataTest.MoveAssignment (0 ms)
[----------] 9 tests from RestrictedDataTest (625 ms total)

[----------] Global test environment tear-down
[==========] 9 tests from 1 test suite ran. (749 ms total)
[  PASSED  ] 9 tests.

morambro commented 1 year ago

You can use ctest --rerun-failed --output-on-failure to run them all.

dhthwy commented 1 year ago

Yeah, sorry, I missed that part. Seems to get hung on test 42, so I force skipped that one for now. Looks like there might be issues with calls to pread. I think that's the reason some tests are failing. I'll look into it further.

 94% tests passed, 14 tests failed out of 222

Total Test time (real) = 774.14 sec

The following tests FAILED:
          1 - tink_test_core_version_test (Failed)
         42 - tink_test_aead__internal_ssl_aead_large_inputs_test (Not Run)
        157 - tink_test_streamingaead_streaming_aead_wrapper_test (Failed)
        158 - tink_test_streamingaead_aes_gcm_hkdf_streaming_key_manager_test (Failed)
        159 - tink_test_streamingaead_aes_ctr_hmac_streaming_key_manager_test (Failed)
        164 - tink_test_streamingaead_decrypting_random_access_stream_test (Failed)
        166 - tink_test_streamingaead_shared_random_access_stream_test (Failed)
        175 - tink_test_subtle_aes_gcm_hkdf_streaming_test (Failed)
        176 - tink_test_subtle_aes_ctr_hmac_streaming_test (Failed)
        199 - tink_test_subtle_decrypting_random_access_stream_test (Failed)
        210 - tink_test_util_file_input_stream_test (Failed)
        211 - tink_test_util_file_output_stream_test (Failed)
        212 - tink_test_util_file_random_access_stream_test (Failed)
        214 - tink_test_util_ostream_output_stream_test (Failed)
Errors while running CTest

dhthwy commented 1 year ago

Error from pread:

C:/msys64/home/da/projects/app/third_party/tink/cc/streamingaead/aes_ctr_hmac_streaming_key_manager_test.cc:134: Failu re Value of: EncryptThenDecrypt(streaming_aead_from_manager_result.value().get(), streaming_aead_direct_result.value().ge t(), subtle::Random::GetRandomBytes(10000), "some associated data", params.ciphertext_offset) Expected: is a Status with an OK value Actual: INTERNAL: Random access decryption failed at position=0 with count=1 and status: INTERNAL: PRead failed with status: INVALID_ARGUMENT: verification failed (of type crypto::tink::util::Status), INTERNAL: Random access decryptio n failed at position=0 with count=1 and status: INTERNAL: PRead failed with status: INVALID_ARGUMENT: verification fai led

Test 1 errors:

C:/msys64/home/da/projects/app/third_party/tink/third_party/com_google_googletest/src/googletest/src/gtest-port.cc:8 65: Failure Failed Syntax error at index 0 in simple regular expression "[0-9]+[.][0-9]+[.][0-9]+(-[A-Za-z0-9]+)?": '[' is unsupported. C:/msys64/home/da/projects/app/third_party/tink/third_party/com_google_googletest/src/googletest/src/gtest-port.cc:8 65: Failure Failed Syntax error at index 4 in simple regular expression "[0-9]+[.][0-9]+[.][0-9]+(-[A-Za-z0-9]+)?": ']' is unsupported. C:/msys64/home/da/projects/app/third_party/tink/third_party/com_google_googletest/src/googletest/src/gtest-port.cc:8 65: Failure Failed Syntax error at index 6 in simple regular expression "[0-9]+[.][0-9]+[.][0-9]+(-[A-Za-z0-9]+)?": '[' is unsupported. C:/msys64/home/da/projects/app/third_party/tink/third_party/com_google_googletest/src/googletest/src/gtest-port.cc:8 65: Failure Failed Syntax error at index 8 in simple regular expression "[0-9]+[.][0-9]+[.][0-9]+(-[A-Za-z0-9]+)?": ']' is unsupported. C:/msys64/home/da/projects/app/third_party/tink/third_party/com_google_googletest/src/googletest/src/gtest-port.cc:8 65: Failure Failed Syntax error at index 9 in simple regular expression "[0-9]+[.][0-9]+[.][0-9]+(-[A-Za-z0-9]+)?": '[' is unsupported. C:/msys64/home/da/projects/app/third_party/tink/third_party/com_google_googletest/src/googletest/src/gtest-port.cc:8 65: Failure Failed Syntax error at index 13 in simple regular expression "[0-9]+[.][0-9]+[.][0-9]+(-[A-Za-z0-9]+)?": ']' is unsupported. C:/msys64/home/da/projects/app/third_party/tink/third_party/com_google_googletest/src/googletest/src/gtest-port.cc:8 65: Failure Failed Syntax error at index 15 in simple regular expression "[0-9]+[.][0-9]+[.][0-9]+(-[A-Za-z0-9]+)?": '[' is unsupported. C:/msys64/home/da/projects/app/third_party/tink/third_party/com_google_googletest/src/googletest/src/gtest-port.cc:8 65: Failure Failed Syntax error at index 17 in simple regular expression "[0-9]+[.][0-9]+[.][0-9]+(-[A-Za-z0-9]+)?": ']' is unsupported. C:/msys64/home/da/projects/app/third_party/tink/third_party/com_google_googletest/src/googletest/src/gtest-port.cc:8 65: Failure Failed Syntax error at index 18 in simple regular expression "[0-9]+[.][0-9]+[.][0-9]+(-[A-Za-z0-9]+)?": '[' is unsupported. C:/msys64/home/da/projects/app/third_party/tink/third_party/com_google_googletest/src/googletest/src/gtest-port.cc:8 65: Failure Failed Syntax error at index 22 in simple regular expression "[0-9]+[.][0-9]+[.][0-9]+(-[A-Za-z0-9]+)?": ']' is unsupported. C:/msys64/home/da/projects/app/third_party/tink/third_party/com_google_googletest/src/googletest/src/gtest-port.cc:8 65: Failure Failed Syntax error at index 24 in simple regular expression "[0-9]+[.][0-9]+[.][0-9]+(-[A-Za-z0-9]+)?": '(' is unsupported. C:/msys64/home/da/projects/app/third_party/tink/third_party/com_google_googletest/src/googletest/src/gtest-port.cc:8 65: Failure Failed Syntax error at index 26 in simple regular expression "[0-9]+[.][0-9]+[.][0-9]+(-[A-Za-z0-9]+)?": '[' is unsupported. C:/msys64/home/da/projects/app/third_party/tink/third_party/com_google_googletest/src/googletest/src/gtest-port.cc:8 65: Failure Failed Syntax error at index 36 in simple regular expression "[0-9]+[.][0-9]+[.][0-9]+(-[A-Za-z0-9]+)?": ']' is unsupported. C:/msys64/home/da/projects/app/third_party/tink/third_party/com_google_googletest/src/googletest/src/gtest-port.cc:8 65: Failure Failed Syntax error at index 38 in simple regular expression "[0-9]+[.][0-9]+[.][0-9]+(-[A-Za-z0-9]+)?": ')' is unsupported. C:/msys64/home/da/projects/app/third_party/tink/cc/core/version_test.cc:37: Failure Value of: Version::kTinkVersion Expected: matches regular expression "[0-9]+[.][0-9]+[.][0-9]+(-[A-Za-z0-9]+)?" Actual: "1.7.0" (of type char [6]) [ FAILED ] VersionTest.testVersionFormat (1393 ms) [----------] 1 test from VersionTest (1402 ms total)

[----------] Global test environment tear-down [==========] 1 test from 1 test suite ran. (1433 ms total) [ PASSED ] 0 tests. [ FAILED ] 1 test, listed below: [ FAILED ] VersionTest.testVersionFormat

morambro commented 1 year ago

I was able to make some quick tests on a Windows VM and I found that one of the issues is with the fact that open should open files for read/write as O_BINARY; otherwise the \x1A byte is considered EOF and the read stops. Adding this to all the open calls in cc/util/test_util.cc should make some of these failures go away.

dhthwy commented 1 year ago

Thanks. You're right.

Now we're left with:

The following tests FAILED: 1 - tink_test_core_version_test (Failed) 42 - tink_test_aead__internal_ssl_aead_large_inputs_test (Not Run) 212 - tink_test_util_file_random_access_stream_test (Failed)

42 takes roughly ~30 secs on my mac m1. I waited > 30 mins on my Windows box with no updates. So I skipped that one.

212 seemingly fails at random. It is also randomly failing on my mac. I just re-ran it again on Windows and it passed.

I haven't looked into '1' yet.

I pushed the changes. I'm assuming it's okay just to NOOP O_BINARY for other systems where it isn't applicable. It should get defined by sys/stat.h on Windows.

morambro commented 1 year ago

Some comments:

For 212 consider pulling from master. https://github.com/google/tink/commit/72208aa4b53d1347af42ea91884d8d3d81d85baf removed the issue in my local test environment.
For 42, in my test environment it completes successfully in ~40s.
For 1, this seems to fail because gtest doesn't support [ and ] in regexps (didn't investigate further though).
Finally, there is still the -lrt issue with msys2. This seems to be included because this is found here CMake finds it (in msys64's usr/lib), and thus adds the -lrt flag here. However, within msys2 the linker cannot locate the library for some reason.

dhthwy commented 1 year ago

212 is still randomly failing for me on Windows and OSX. I pulled a fresh copy. @ FileRandomAccessStreamTest.ConcurrentReads

$ time ./tink_test_aead__internal_ssl_aead_large_inputs_test.exe
Running main() from C:/msys64/home/da/projects/app/third_party/tink/__third_party/com_google_googletest/src/googletest
/src/gtest_main.cc
[==========] Running 5 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 5 tests from SslOneShotAeadLargeInputsTests/SslOneShotAeadLargeInputsTest
[ RUN      ] SslOneShotAeadLargeInputsTests/SslOneShotAeadLargeInputsTest.EncryptDecryptLargeInput/AesGcm256
[       OK ] SslOneShotAeadLargeInputsTests/SslOneShotAeadLargeInputsTest.EncryptDecryptLargeInput/AesGcm256 (173347 m
s)
[ RUN      ] SslOneShotAeadLargeInputsTests/SslOneShotAeadLargeInputsTest.EncryptDecryptLargeInput/AesGcm128
[       OK ] SslOneShotAeadLargeInputsTests/SslOneShotAeadLargeInputsTest.EncryptDecryptLargeInput/AesGcm128 (145109 m
s)
[ RUN      ] SslOneShotAeadLargeInputsTests/SslOneShotAeadLargeInputsTest.EncryptDecryptLargeInput/AesGcmSiv256
[       OK ] SslOneShotAeadLargeInputsTests/SslOneShotAeadLargeInputsTest.EncryptDecryptLargeInput/AesGcmSiv256 (10256
65 ms)
[ RUN      ] SslOneShotAeadLargeInputsTests/SslOneShotAeadLargeInputsTest.EncryptDecryptLargeInput/AesGcmSiv128
[       OK ] SslOneShotAeadLargeInputsTests/SslOneShotAeadLargeInputsTest.EncryptDecryptLargeInput/AesGcmSiv128 (75775
0 ms)
[ RUN      ] SslOneShotAeadLargeInputsTests/SslOneShotAeadLargeInputsTest.EncryptDecryptLargeInput/Xchacha20Poly1305
[       OK ] SslOneShotAeadLargeInputsTests/SslOneShotAeadLargeInputsTest.EncryptDecryptLargeInput/Xchacha20Poly1305 (
51371 ms)
[----------] 5 tests from SslOneShotAeadLargeInputsTests/SslOneShotAeadLargeInputsTest (2153469 ms total)

[----------] Global test environment tear-down
[==========] 5 tests from 1 test suite ran. (2153547 ms total)
[  PASSED  ] 5 tests.

real    35m53.943s
user    0m0.015s
sys     0m0.094s

Old CPU. Processor: AMD Phenom(tm) II X4 965 Processor, 3400 Mhz, 4 Core(s), 4 Logical Processor(s)

dhthwy commented 1 year ago

It's looking for librt in /mingw64/lib, but cmake finds it in /usr/lib

https://github.com/YosysHQ/nextpnr/issues/666

dhthwy commented 1 year ago

212 is still randomly failing for me on Windows and OSX. I pulled a fresh copy. @ FileRandomAccessStreamTest.ConcurrentReads

Scratch that. It was using the pread() variant in my pull request for some reason (on OSX), which isn't thread safe, causing the random fails for me. My bad, sorry!

Just added an absl::Mutex to the pread() function I stole from Google's glog, which seems to play nice now.

dhthwy commented 1 year ago

For gtest regex fails. It appears when POSIX regex.h isn't available gtest falls back to a less capable regex processor that doesn't support brackets, etc.

There is a library for mingw called 'libsystre' (pacman -Ss libsystre will pull it up) that supposedly provides a compatible POSIX regex to solve this problem.

morambro commented 1 year ago

Hi @dhthwy, we discussed internally and we decided not to merge https://github.com/google/tink/pull/660. We opted for the simpler solution of doing some refactoring to limit POSIX APIs to util/file.*stream.* classes, and simply excluding them from the list of build targets when WIN32. This removes the need for us to port these APIs to Windows and support different build environments. Related to that, we decided not to support msys2 and only focus on MSVC to align with other Google OSS C++ projects (see here). Though I think the changes above would work on msys2 as well.

Thanks a lot for your contribution, it helped me better understand what the delta was to make Tink C++ build on Windows.

dhthwy commented 1 year ago

Sure. Happy to be of some assistance. Thank you for the update!

tink-crypto / tink

Build success on Windows (with cmake and mingw) #661