-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Add wineditline/2.206 #8553
Add wineditline/2.206 #8553
Conversation
This comment has been minimized.
This comment has been minimized.
fa78e4f
to
effd338
Compare
I was missing |
This comment has been minimized.
This comment has been minimized.
The archive that contains the source also comes with prebuilt binaries. I deleted these thinking it'd be a waste to leave in the source.
All green in build 3 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
info.set_property("cmake_file_name", name) | ||
info.set_property("cmake_target_name", f"{name}::{name}") | ||
|
||
info.names.update({ | ||
"cmake_find_package": name, | ||
"cmake_find_package_multi": name, | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't recommend to explicitly set those values when upstream doesn't provide CMake config file, it's confusing for other maintainers.
And side comment: obfuscating self.cpp_info
may not be a good idea
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have sent an email with a patch file to @ptosco to add a CMake package upstream, so let's hope that goes through.
I will remove this in a followup PR.
cmake_minimum_required(VERSION 3.1) | ||
|
||
project(wineditline_wrapper C) | ||
|
||
include("${CMAKE_BINARY_DIR}/conanbuildinfo.cmake") | ||
conan_basic_setup() | ||
|
||
set(src source_subfolder/src) | ||
|
||
add_library(edit "${src}/editline.c" "${src}/fn_complete.c" "${src}/history.c") | ||
if(BUILD_SHARED_LIBS) | ||
target_sources(edit PRIVATE "${src}/libedit.def") | ||
endif() | ||
|
||
target_include_directories(edit PRIVATE "${src}") | ||
|
||
include(GNUInstallDirs) | ||
|
||
install( | ||
TARGETS edit | ||
RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}" | ||
ARCHIVE DESTINATION "${CMAKE_INSTALL_LIBDIR}" | ||
) | ||
|
||
install(DIRECTORY "${src}/editline" DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are CMakeLists in archive, maybe not super neat but it can be patched. It's better to rely on upstream CMakeLists if available, and eventually submit PR to improve it.
* Add wineditline/2.206 * Use `conan_basic_setup(TARGETS)` in test_package * Leave the fetched source untouched The archive that contains the source also comes with prebuilt binaries. I deleted these thinking it'd be a waste to leave in the source. Co-authored-by: friendlyanon <friendlyanon@users.noreply.github.com>
Specify library name and version: wineditline/2.206
Fixes #8552
conan-center hook activated.