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.

Sign up for GitHub

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

Jump to bottom

[cryptopp] Update port to support new pem-pack option #37360

Closed
wants to merge 2 commits into from
Closed

[cryptopp] Update port to support new pem-pack option #37360

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
75ae77f
Update port to support new pem-pack option
fwosar Mar 11, 2024
80d0069
Fix version
fwosar Mar 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 92 additions & 0 deletions ports/cryptopp/cryptopp-cmake.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
diff --git a/CMakeLists.txt b/CMakeLists.txt
index bb3bfbb..d533ed6 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -83,6 +83,11 @@ option(
ON
)

+option(
+ CRYPTOPP_USE_PEM_PACK
+ "Include the PEM-Pack in compiliation (https://github.com/noloader/cryptopp-pem)"
+)
+
if(CRYPTOPP_INCLUDE_PREFIX)
set(CRYPTOPP_INCLUDE_PREFIX
${CRYPTOPP_INCLUDE_PREFIX}
diff --git a/cmake/GetCryptoppSources.cmake b/cmake/GetCryptoppSources.cmake
index 809596a..3aaaab4 100644
--- a/cmake/GetCryptoppSources.cmake
+++ b/cmake/GetCryptoppSources.cmake
@@ -22,6 +22,11 @@ if(GIT_FOUND)
SOURCE_DIR
${CRYPTOPP_INCLUDE_PREFIX}
)
+ fetchcontent_declare(
+ cryptopp-pem
+ GIT_REPOSITORY "https://github.com/noloader/cryptopp-pem"
+ QUIET
+ )
Comment on lines +25 to +29
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be fetched in the port rather than being grabbed with FetchContent as the SHA needs to be locked, etc.

else()
message(STATUS "Downloading crypto++ from URL...")
cmake_policy(SET CMP0135 NEW)
@@ -39,5 +44,13 @@ else()
cryptopp
URL "${source_location}.zip" QUIET SOURCE_DIR ${CRYPTOPP_INCLUDE_PREFIX}
)
+ fetchcontent_declare(
+ cryptopp-pem
+ URL "https://github.com/noloader/cryptopp-pem/archive/refs/heads/master.zip"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fails 'no changes means no changes'. The same port needs to install the same bits if you build it again the next day; building whatever's in master is not OK.

+ QUIET
+ )
endif()
fetchcontent_populate(cryptopp)
+if(CRYPTOPP_USE_PEM_PACK)
+ fetchcontent_populate(cryptopp-pem)
+endif()
diff --git a/cryptopp/sources.cmake b/cryptopp/sources.cmake
index 7918f03..b0bf7db 100644
--- a/cryptopp/sources.cmake
+++ b/cryptopp/sources.cmake
@@ -182,6 +182,12 @@ set(cryptopp_SOURCES
zlib.cpp
)

+set(cryptopp_SOURCES_PEM
+ "${cryptopp-pem_SOURCE_DIR}/pem-com.cpp"
+ "${cryptopp-pem_SOURCE_DIR}/pem-rd.cpp"
+ "${cryptopp-pem_SOURCE_DIR}/pem-wr.cpp"
+)
+
# ***** Library headers *****
set(cryptopp_HEADERS
3way.h
@@ -374,6 +382,11 @@ set(cryptopp_HEADERS
zlib.h
)

+set(cryptopp_HEADERS_PEM
+ "${cryptopp-pem_SOURCE_DIR}/pem.h"
+ "${cryptopp-pem_SOURCE_DIR}/pem-com.h"
+)
+
# ***** Test sources *****
set(cryptopp_SOURCES_TEST
# adhoc.cpp
@@ -448,3 +462,16 @@ if(ANDROID)
${ANDROID_NDK}/sources/android/cpufeatures/cpu-features.c
)
endif()
+
+if(CRYPTOPP_USE_PEM_PACK)
+ list(
+ APPEND
+ cryptopp_SOURCES
+ ${cryptopp_SOURCES_PEM}
+ )
+ list(
+ APPEND
+ cryptopp_HEADERS
+ ${cryptopp_HEADERS_PEM}
+ )
+endif()
13 changes: 7 additions & 6 deletions ports/cryptopp/portfile.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ vcpkg_from_github(
REF "CRYPTOPP_${CRYPTOPP_VERSION}"
SHA512 3ec33b107ab627a514e1ebbc4b6522ee8552525f36730d9b5feb85e61ba7fc24fd36eb6050e328c6789ff60d47796beaa8eebf7dead787a34395294fae9bb733
HEAD_REF master
PATCHES cryptopp-cmake.patch
)

vcpkg_from_github(
Expand All @@ -24,6 +25,8 @@ file(COPY "${CMAKE_SOURCE_PATH}/test" DESTINATION "${SOURCE_PATH}")
file(COPY "${CMAKE_SOURCE_PATH}/cryptopp/cryptoppConfig.cmake" DESTINATION "${SOURCE_PATH}")
file(COPY "${CMAKE_SOURCE_PATH}/CMakeLists.txt" DESTINATION "${SOURCE_PATH}")

set(CRYPTOPP_USE_PEM_PACK "OFF")

if("pem-pack" IN_LIST FEATURES)
vcpkg_from_github(
OUT_SOURCE_PATH PEM_PACK_SOURCE_PATH
Expand All @@ -33,11 +36,7 @@ if("pem-pack" IN_LIST FEATURES)
HEAD_REF master
)

file(GLOB PEM_PACK_FILES
${PEM_PACK_SOURCE_PATH}/*.h
${PEM_PACK_SOURCE_PATH}/*.cpp
)
file(INSTALL ${PEM_PACK_FILES} DESTINATION "${CURRENT_PACKAGES_DIR}/include/${PORT}")
fwosar marked this conversation as resolved.
Show resolved Hide resolved
set(CRYPTOPP_USE_PEM_PACK "ON")
endif()

# disable assembly on ARM Windows to fix broken build
Expand All @@ -61,9 +60,11 @@ vcpkg_cmake_configure(
-DBUILD_STATIC=ON
-DCRYPTOPP_BUILD_TESTING=OFF
-DCRYPTOPP_BUILD_DOCUMENTATION=OFF
-DDISABLE_ASM=${CRYPTOPP_DISABLE_ASM}
-DCRYPTOPP_DISABLE_ASM=${CRYPTOPP_DISABLE_ASM}
-DUSE_INTERMEDIATE_OBJECTS_TARGET=OFF # Not required when we build static only
-DCMAKE_POLICY_DEFAULT_CMP0063=NEW # Honor "<LANG>_VISIBILITY_PRESET" properties
-DCRYPTOPP_USE_PEM_PACK=${CRYPTOPP_USE_PEM_PACK}
-Dcryptopp-pem_SOURCE_DIR=${PEM_PACK_SOURCE_PATH} # Usually this would be set when cryptopp-cmake downloads the source, but since we don't, we need to provide it instead
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused by this... you're adding a patch that tries to download from master but this says it doesn't try to do that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The patch is a diff created from the official cryptopp-cmake project that adds pem-pack support as was requested earlier instead of using their master branch. By default, cryptopp-cmake will download the source for both the pem-pack and Crypto++ themselves, but this behaviour is not used by vcpkg and disabled as those fetch_content directives will only execute when CRYPTOPP_SOURCES is not defined.

I can remove those parts from the patch, but it would mean that the resulting project is no longer a proper replica of the cryptopp-cmake with pem-pack changes cherrypicked.

In that case, all the pem-pack feature in vcpkg would do would be manipulating the source list after copying the required files to the right locations, essentially making the pem-pack support and its related options within cryptopp-cmake redundant.

MAYBE_UNUSED_VARIABLES
BUILD_STATIC
USE_INTERMEDIATE_OBJECTS_TARGET
Expand Down
1 change: 1 addition & 0 deletions ports/cryptopp/vcpkg.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"name": "cryptopp",
"version": "8.9.0",
"port-version": 1,
"description": "Crypto++ is a free C++ class library of cryptographic schemes.",
"homepage": "https://github.com/weidai11/cryptopp",
"license": "BSL-1.0",
Expand Down
2 changes: 1 addition & 1 deletion versions/baseline.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1998,7 +1998,7 @@
},
"cryptopp": {
"baseline": "8.9.0",
"port-version": 0
"port-version": 1
},
"cserialport": {
"baseline": "4.3.0",
Expand Down
5 changes: 5 additions & 0 deletions versions/c-/cryptopp.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
{
"versions": [
{
"git-tree": "c4b9ac31756b00115948f55c0d67e4114d7ffc00",
"version": "8.9.0",
"port-version": 1
},
{
"git-tree": "48788514ae1e84dea9055e603527c91f92c124fb",
"version": "8.9.0",
Expand Down