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

[utf8.h] Add library and packages #4858

Merged
merged 3 commits into from Mar 13, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions recipes/utf8.h/all/conandata.yml
@@ -0,0 +1,4 @@
sources:
"cci.20210310":
url: "https://github.com/sheredom/utf8.h/archive/ee5a7d4beb7755da13e4d4ec3eccfb65a0530456.tar.gz"
sha256: "d7c9ad480b640c76292a1c4b9735497fd635cb8828e95beabb7db91ea4cecf55"
32 changes: 32 additions & 0 deletions recipes/utf8.h/all/conanfile.py
@@ -0,0 +1,32 @@
import os
import re
jgsogo marked this conversation as resolved.
Show resolved Hide resolved
from conans import ConanFile, tools

required_conan_version = ">=1.28.0"
Copy link
Member

Choose a reason for hiding this comment

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

Is this needed? I believe it was added for components names or similar

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's more or less informational only. CCI policy is to require latest client anyway.

Copy link
Member

Choose a reason for hiding this comment

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

Indeed, it can be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not needed, sorry. It was a leftover (copy/paste) from another recipe with validate method.


class Utf8HConan(ConanFile):
name = "utf8.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

is the . acceptable for the naming convention?
Is this too generic? https://github.com/search?l=C%2B%2B&p=2&q=utf8&type=Repositories

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see valid reason why dot shouldn't be allowed in the reference. I'd actually allow any characters, maybe except / and @ which are used as separators.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We already have a dot in approvaltests.cpp package and utf8.h is the name of the library. I also feel a bit strange, but IMHO it is a valid character for a reference and it's been the authors' choice, I think it is better to use it than to rename the library.

url = "https://github.com/conan-io/conan-center-index"
homepage = "https://github.com/sheredom/utf8.h"
description = "Single header utf8 string functions for C and C++"
topics = ("utf8", "unicode", "text")
license = "Unlicense"
no_copy_source = True

@property
def _source_subfolder(self):
return "source_subfolder"

def source(self):
tools.get(**self.conan_data["sources"][self.version])
url = self.conan_data["sources"][self.version]['url']
m = re.match(r'.*/([0-9a-f]+)\.tar.gz', url)
extracted_dir = self.name + "-" + m[1]
jgsogo marked this conversation as resolved.
Show resolved Hide resolved
os.rename(extracted_dir, self._source_subfolder)

def package(self):
self.copy("utf8.h", dst="include", src=self._source_subfolder)
self.copy("LICENSE", dst="licenses", src=self._source_subfolder)

def package_id(self):
self.info.header_only()
10 changes: 10 additions & 0 deletions recipes/utf8.h/all/test_package/CMakeLists.txt
@@ -0,0 +1,10 @@
cmake_minimum_required(VERSION 3.1)
project(test_package CXX)

include(${CMAKE_BINARY_DIR}/conanbuildinfo.cmake)
conan_basic_setup()

find_package(utf8.h REQUIRED CONFIG)

add_executable(${PROJECT_NAME} test_package.cpp)
target_link_libraries(${PROJECT_NAME} utf8.h::utf8.h)
Comment on lines +7 to +10
Copy link
Contributor

@SpaceIm SpaceIm Mar 12, 2021

Choose a reason for hiding this comment

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

As you wish, but I would not recommend to show non-official CMake config files or imported targets in test_package.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, here we have a problem. We want to push existing packages and consumers to start using targets (right now, moving to cmake_find_package[_multi] generator), what to do with these libraries that don't have official or unofficial targets?

I would really like to know what you think cc/ @uilianries @SSE4 , maybe we are wrong trying to force using targets and/or config files?

Copy link
Contributor

@SpaceIm SpaceIm Mar 12, 2021

Choose a reason for hiding this comment

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

I see. It's fine for me, I just try to use cmake_find_package[_multi] in test_package when we properly mimic an official find or config module file, but there is no "rule".
In fact, a usage file in each recipe would be nice, to properly document if a given recipe provides transparent integration through cmake_find_package, cmake_find_package_multi or pkg_config generator, how to call find_package, which components are defined, and which imported targets etc.
Because it's might not be obvious for consumers if a recipe properly emulates official config files or not (several recipes have very complex and non-intuitive package_info()). But that's a different topic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can run conan install xxxx --generator markdown and it will show some information, among that information it is how to consume the library using common generators.

Copy link
Contributor

@SpaceIm SpaceIm Mar 12, 2021

Choose a reason for hiding this comment

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

I know, but it doesn't say if this is a proper emulation of official CMake config or module files. Looking at this markdown, consumer can't really know if he can rely on these informations for transparent integration or not. He still have to lost time to dig into original project (which is not always obvious if there is a lack of documention) to figure out if we properly emulate those informations or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Understood, but the only alternative would be to read the actual source from test_package/CMakeLists.txt with its comments (assuming we are commenting if the integration is using official targets or not).. and that's not possible.

Copy link
Contributor

Choose a reason for hiding this comment

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

Typically for these cases we just opt for the ${CONAN_LIBS}

there's nothing in the recipe adding something special to the cmake find generators ... why test the Conan implementation here? I think it's easier to test the recipe rather than how it might be consumed.

Copy link
Contributor

Choose a reason for hiding this comment

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

the projects which don't originate from the CMake (e.g. use another build system) usually don't have official module/target names. still, CMake documentation states that these projects could be imported to the CMake:
https://cmake.org/cmake/help/v3.20/manual/cmake-developer.7.html#find-modules
https://cmake.org/cmake/help/v3.20/manual/cmake-packages.7.html#creating-packages
so, if foreign projects follow the sane CMake naming conventions, they could still be used in CMake, even without an official target.
as soon as official target added later, conan package could be updated to provide an alias with build modules.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If some generators like cmake and cmake_find_package[_multi] get deprecated in the future in favor of CMakeDeps (conan-io/conan#8568) and the CMakeToolchain, we need recipes to be prepared for that migration: the targets will be available in CMakeDeps, but ${CONAN_LIBS} won't...

17 changes: 17 additions & 0 deletions recipes/utf8.h/all/test_package/conanfile.py
@@ -0,0 +1,17 @@
import os
from conans import ConanFile, CMake, tools


class TestPackageConan(ConanFile):
settings = "os", "compiler", "build_type", "arch"
generators = "cmake", "cmake_find_package_multi"

def build(self):
cmake = CMake(self)
cmake.configure()
cmake.build()

def test(self):
if not tools.cross_building(self.settings):
bin_path = os.path.join("bin", "test_package")
self.run(bin_path, run_environment=True)
13 changes: 13 additions & 0 deletions recipes/utf8.h/all/test_package/test_package.cpp
@@ -0,0 +1,13 @@
#include "utf8.h"

int main()
{
const char ref[] = {'\xcf', '\xb4', '\xce', '\x98', '\xce',
'\x98', '\xce', '\x98', '\0'};
char str[] = {'\xcf', '\xb4', '\xce', '\xb8', '\xce',
'\x98', '\xcf', '\x91', '\0'};

int r = utf8ncasecmp(ref, str, 8);

return 0;
}
3 changes: 3 additions & 0 deletions recipes/utf8.h/config.yml
@@ -0,0 +1,3 @@
versions:
"cci.20210310":
folder: all