[logme] New port#49397
Conversation
|
@microsoft-github-policy-service agree |
There was a problem hiding this comment.
Looks like CI is failing due to tests executables being invoked in non-native triplets, if there's no option to avoid running them then we should patch out the library's buildsystem.
CMake Error at /vcpkg/downloads/tools/cmake-3.31.10-linux/cmake-3.31.10-linux-x86_64/share/cmake-3.31/Modules/GoogleTestAddTests.cmake:132 (message):
Error running test executable.
Path: '/mnt/vcpkg-ci/b/logme/arm-neon-android-dbg/LevelFilter'
Working directory: '/mnt/vcpkg-ci/b/logme/out/Tests/LevelFilter'
Result: 2
Output:
BillyONeal
left a comment
There was a problem hiding this comment.
This is mostly right but I have a few nitpicks.
- In general when the automatically generated usage is correct we don't supply extra usage to make different ports more uniform, and the automatic one works OK here.
- The version database is damaged; there should only be one version here.
I have submitted fixes for these as efmsoft#1
- The CMakeLists.txt here improperly reaches into "parent" directories which is likely to have debug and release builds "race" with each other. The "../" probably needs to be patched out of the
add_subdirectorycalls (for example https://github.com/efmsoft/logme/blob/82665205859db9b6a8246abb65cfa07e2adb805a/CMakeLists.txt#L100 ).
(vcpkg runs debug and release concurrently and expects that to be safe because we build them into different directories, but this ../ is 'escaping' that)
Delete usage and fix version database.
|
Removed manual usage, fixed versions db, and updated logme to v2.4.4 with the CMake add_subdirectory() binary-dir escaping fix. |
dg0yt
left a comment
There was a problem hiding this comment.
I would probably completely omit the examples feature, for a simpler portfile.
| if(VCPKG_LIBRARY_LINKAGE STREQUAL "static") | ||
| file(REMOVE_RECURSE "${CURRENT_PACKAGES_DIR}/bin" "${CURRENT_PACKAGES_DIR}/debug/bin") | ||
| endif() |
There was a problem hiding this comment.
I guess what is really needed is vcpkg_copy_tools.
| vcpkg_cmake_config_fixup( | ||
| PACKAGE_NAME logme | ||
| CONFIG_PATH lib/cmake/logme | ||
| ) |
There was a problem hiding this comment.
| vcpkg_cmake_config_fixup( | |
| PACKAGE_NAME logme | |
| CONFIG_PATH lib/cmake/logme | |
| ) | |
| vcpkg_cmake_config_fixup(CONFIG_PATH lib/cmake/logme) |
|
|
||
| if(VCPKG_TARGET_IS_UWP) | ||
| list(APPEND FEATURE_OPTIONS | ||
| -DLOGME_BUILD_EXAMPLES=OFF | ||
| -DLOGME_BUILD_TOOLS=OFF | ||
| ) | ||
| endif() |
There was a problem hiding this comment.
| if(VCPKG_TARGET_IS_UWP) | |
| list(APPEND FEATURE_OPTIONS | |
| -DLOGME_BUILD_EXAMPLES=OFF | |
| -DLOGME_BUILD_TOOLS=OFF | |
| ) | |
| endif() |
Handled by supports in the manifest.
There was a problem hiding this comment.
All suggested changes are now applied (examples removed, tools handled via vcpkg_copy_tools, config fixup simplified). Could you please re-review? Thanks!
There was a problem hiding this comment.
We removed the tools feature entirely. The CLI utility is not required for the library package, and attempting to package it caused repeated issues with vcpkg_copy_tools and feature testing (especially on single-config triplets). To avoid fragile install logic and CI failures, we decided to ship only the library.
Head branch was pushed to by a user without write access
|
Applied the suggestions: removed the examples feature, switched tool handling to vcpkg_copy_tools (logmectl), and simplified vcpkg_cmake_config_fixup to CONFIG_PATH only. |
Co-authored-by: Victor Romero <romerosanchezv@gmail.com>
Co-authored-by: Victor Romero <romerosanchezv@gmail.com>
Co-authored-by: Victor Romero <romerosanchezv@gmail.com>
|
Regenerated versions DB with x-add-version; pushed. |
|
@microsoft-github-policy-service rerun |
|
Thank you! |
This PR adds a new port for logme, a cross-platform C/C++ logging framework.
Port details
Notes
New port checklist
vcpkg.jsonmatches upstream.vcpkg.jsonmatches upstream.vcpkg x-add-version.