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

[ports/ed25519/CMakeLists.txt] Install all headers #35143

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
+2 −2
Draft

[ports/ed25519/CMakeLists.txt] Install all headers #35143

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
74028e8
[ports/ed25519/CMakeLists.txt] Install all headers
SamuelMarks Nov 16, 2023
d42fce6
[ports/ed25519/CMakeLists.txt] Install four headers
SamuelMarks Nov 16, 2023
12450b0
[versions/e-/ed25519.json] Fixed `git-tree` value for previous version
SamuelMarks Nov 17, 2023
089c63a
Merge branch 'master' into ed25519-all-headers
SamuelMarks Feb 28, 2024
5c0cfa2
Merge branch 'master' into ed25519-all-headers
SamuelMarks Oct 18, 2024
d43eae8
[versions/e-/ed25519.json] `./vcpkg x-add-version --all --overwrite-v…
SamuelMarks Oct 18, 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
2 changes: 1 addition & 1 deletion ports/ed25519/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ write_basic_package_version_file(
COMPATIBILITY SameMajorVersion
)
install(FILES "${VERSION_FILE_PATH}" DESTINATION "share/unofficial-${PROJECT_NAME}")
install(FILES "src/ed25519.h" DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}")
install(FILES "src/ed25519.h" "src/fe.h" "src/fixedint.h" "src/ge.h" DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
install(FILES "src/ed25519.h" "src/fe.h" "src/fixedint.h" "src/ge.h" DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}")
install(FILES "src/ed25519.h" DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vicroms That's what I had before; but the library I am integrated vcpkg into uses these other headers directly and I don't want to edit their codebase [source or header files] in order to support the platforms vcpkg supports.

Copy link
Contributor

Choose a reason for hiding this comment

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

There is still the suggestion to put the private headers into a unique subdir. This is one change for that project (to use an additional include dir), instead of having the private headers in everybody's search path.
(And for specific single-user requirements, their are overlay ports and other customization points.)

Copy link
Member

Choose a reason for hiding this comment

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

The vcpkg team agrees with @dg0yt's points:

  • Moving the private headers into a subdirectory (e.g.: orlp-ed25519/*.h) would be acceptable.
  • Forcing other users to deal with the consequences of a project deciding to take a dependency on private headers not meant for consumption is not acceptable.
  • Users can override the behavior by using overlay ports with the changes you have made here.

Please let me know if this resolution is acceptable to you.


install(
EXPORT "unofficial-${PROJECT_NAME}Config"
Expand Down
2 changes: 1 addition & 1 deletion versions/e-/ed25519.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"versions": [
{
"git-tree": "ce2c9dc71e927201adc881b0cfb43341520161ae",
"git-tree": "1b13a0fb0821eb9bc7dbb2a3a8c43f146e1979f5",
"version-date": "2017-02-10",
"port-version": 1
},
Expand Down