Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix compilation errors on Windows for the PQC and Crypto Refresh code #2135

Closed
wants to merge 1 commit into from

Conversation

TJ-91
Copy link
Contributor

@TJ-91 TJ-91 commented Oct 23, 2023

I've been building the experimental Crypto Refresh and PQC code on Windows (Visual Studio 2022) and got a few errors.

  • On Windows, the compiler complains about a deleted copy constructor so I provide one (via the already-implemented copy-assign constructor).
  • I also fixed a minor variable-length array issue.

@codecov
Copy link

codecov bot commented Oct 23, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6a15c33) 76.89% compared to head (40b53ab) 76.88%.

❗ Current head 40b53ab differs from pull request most recent head 96a1c63. Consider uploading reports for the commit 96a1c63 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2135      +/-   ##
==========================================
- Coverage   76.89%   76.88%   -0.01%     
==========================================
  Files         193      193              
  Lines       37029    37030       +1     
==========================================
- Hits        28472    28471       -1     
- Misses       8557     8559       +2     

see 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@maxirmx
Copy link
Member

maxirmx commented Oct 24, 2023

@ni4 @ronaldtse
vcpkg installs botan-3 by default now and windows workflows are configured to use botan-2

There is a piece of code in rnp/src/lib/CMakeLists.txt that shall handle this case

if (CRYPTO_BACKEND_BOTAN3)
  find_package(Botan 3.0.0 REQUIRED)
elseif (CRYPTO_BACKEND_BOTAN)
  find_package(Botan 2.14.0 REQUIRED)
  if(BOTAN_VERSION VERSION_GREATER_EQUAL 3.0.0)
    set(CRYPTO_BACKEND_BOTAN3 1)
  endif()
endif()

but build script fails earlier.

@TJ-91
Copy link
Contributor Author

TJ-91 commented Nov 8, 2023

This is now also addressed in #2142 but I will leave this PR open since it contains an open issue raised by @maxirmx

@maxirmx: Feel free to close this and make a separate issue

@maxirmx
Copy link
Member

maxirmx commented Nov 8, 2023

Windows issue with FindBotan has been fixed:
#2138

@maxirmx maxirmx closed this Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants