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

[librdata] Add new recipe library #26024

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions recipes/librdata/all/conandata.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
sources:
"0.1":
url: "https://github.com/WizardMac/librdata/archive/refs/heads/master.zip"
sha256: "0f3eb4a386b9dbf9bdebba424061f7e34a0e1ec27fa4e631257c3be6ca450c57"
Comment on lines +2 to +4
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
"0.1":
url: "https://github.com/WizardMac/librdata/archive/refs/heads/master.zip"
sha256: "0f3eb4a386b9dbf9bdebba424061f7e34a0e1ec27fa4e631257c3be6ca450c57"
"0.0.0.cci.20231003":
url: "https://github.com/WizardMac/librdata/archive/33bd276ecb0bbcd8997ccc71a544149b3da0d940.tar.gz"
sha256: "fc3e0d655c21d4b5144a0ad73f79a249903b5dde4dc8f7091cd0393d214294a2"

Please, read https://github.com/conan-io/conan-center-index/blob/master/docs/faqs.md#what-version-should-packages-use-for-libraries-without-official-releases for extra info

88 changes: 88 additions & 0 deletions recipes/librdata/all/conanfile.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import os

from conan import ConanFile
from conan.tools.gnu import Autotools, AutotoolsDeps, AutotoolsToolchain
from conan.tools.files import copy, rm, rmdir, get
from conan.tools.apple import fix_apple_shared_install_name
from conan.tools.microsoft import is_msvc
from conan.tools.scm import Git
from conan.tools.layout import basic_layout

required_conan_version = ">=2.0.0"

class Libreadstat(ConanFile):
name = "librdata"
version = "0.1"
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
version = "0.1"

Version should not be listed in the conan recipe

description = "librdata is a library for read and write R data frames from C"
license = "MIT"
url = "https://github.com/conan-io/conan-center-index"
homepage = "https://github.com/WizardMac/librdata"
topics = ("r", "rdata", "rds", "data-frames")
package_type = "library"
settings = "os", "arch", "compiler", "build_type"
options = {
"shared": [True, False],
"fPIC": [True, False],
"with_zip": [True, False],
}
default_options = {
"shared": False,
"fPIC": True,
"with_zip": False,
Comment on lines +26 to +31
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
"with_zip": [True, False],
}
default_options = {
"shared": False,
"fPIC": True,
"with_zip": False,
}
default_options = {
"shared": False,
"fPIC": True,

I would suggest not adding an option for dependencies. The upstream does not expose an option as well when running configure script. I would suggest including all dependencies by default.

}

def config_options(self):
if self.settings.os == "Windows":
del self.options.fPIC

def configure(self):
if self.options.shared:
self.options.rm_safe("fPIC")

def layout(self):
basic_layout(self, src_folder="src")

def requirements(self):
if self.options.with_zip:
self.requires("bzip2/1.0.8")
self.requires("zlib/1.2.13")
self.requires("xz_utils/5.6.3")
Comment on lines +48 to +49
Copy link
Contributor

Choose a reason for hiding this comment

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

Should use version ranges for these - see the CCI docs for details.

Comment on lines +46 to +49
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
if self.options.with_zip:
self.requires("bzip2/1.0.8")
self.requires("zlib/1.2.13")
self.requires("xz_utils/5.6.3")
self.requires("bzip2/1.0.8")
self.requires("zlib/[>=1.2.11 <2]")
self.requires("xz_utils/[>=5.4.5 <6]")

Please, read https://github.com/conan-io/conan-center-index/blob/master/docs/adding_packages/dependencies.md#version-ranges for further information about allowed range versions in CCI.

Copy link
Author

Choose a reason for hiding this comment

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

Im sure it should only required when with_zip.


def source(self):
# git = Git(self)
# git.clone(url="https://github.com/WizardMac/librdata.git", target=".")
Comment on lines +52 to +53
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
# git = Git(self)
# git.clone(url="https://github.com/WizardMac/librdata.git", target=".")

Git feature is not used in CCI. Instead, download specific tar/zip file from the upstream.

get(self, **self.conan_data["sources"][self.version], strip_root=True)


def generate(self):
tc = AutotoolsToolchain(self)
tc.generate()
dep = AutotoolsDeps(self)
dep.generate()

def build(self):
autotools = Autotools(self)
autotools.autoreconf()
autotools.configure()
autotools.make()

def package(self):
copy(self, pattern="LICENSE*", dst=os.path.join(self.package_folder, "licenses"), src=self.source_folder)
if is_msvc(self):
Copy link
Member

Choose a reason for hiding this comment

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

Have you really tested on Windows? Usually, it requires more when talking about autotools. In case you did not and has no intention, the recipe still can be accepted in CCI, just need more information.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for your comment!

I really just want to build it for MacOS and Windows instead of Linux, but even so far I haven't found a suitable way to successfully build on Windows. Always fails at the config stage.

Copy link
Member

Choose a reason for hiding this comment

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

I could take a look during the week for windows support. Right now, I don't have access to windows machine, but I'll have by tomorrow. You can take a look in gettext recipe as example: https://github.com/conan-io/conan-center-index/blob/master/recipes/libgettext/all/conanfile.py

copy(self, "rdata.h", src=os.path.join(self.source_folder, "headers"), dst=os.path.join(self.package_folder, "include"))
copy(self, "*.a", src=self.build_folder, dst=os.path.join(self.package_folder, "lib"), keep_path=False)
copy(self, "*.so", src=self.build_folder, dst=os.path.join(self.package_folder, "lib"), keep_path=False)
copy(self, "*.dll", src=self.source_folder, dst=os.path.join(self.package_folder, "bin"), keep_path=False)
else:
autotools = Autotools(self)
autotools.install()
rm(self, "*.la", os.path.join(self.package_folder, "lib"))
rmdir(self, os.path.join(self.package_folder, "lib", "pkgconfig"))
rmdir(self, os.path.join(self.package_folder, "share"))
fix_apple_shared_install_name(self)

def package_info(self):
suffix = "_i" if is_msvc(self) and self.options.shared else ""
self.cpp_info.libs = [f"librdata{suffix}"]
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
self.cpp_info.libs = [f"librdata{suffix}"]
self.cpp_info.libs = [f"rdata{suffix}"]

The prefix lib should not part of the library name, as it's a standard based on the platform, not the library names itself.

Copy link
Author

Choose a reason for hiding this comment

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

But the library does named as librdata...

if self.settings.os in ("FreeBSD", "Linux"):
self.cpp_info.system_libs.append("m")
7 changes: 7 additions & 0 deletions recipes/librdata/all/test_package/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
cmake_minimum_required(VERSION 3.15)
project(test_package LANGUAGES C)

find_package(librdata REQUIRED CONFIG)

add_executable(${PROJECT_NAME} test_package.c)
target_link_libraries(${PROJECT_NAME} PRIVATE librdata::librdata)
25 changes: 25 additions & 0 deletions recipes/librdata/all/test_package/conanfile.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
from conan import ConanFile
from conan.tools.build import can_run
from conan.tools.cmake import cmake_layout, CMake
import os


class TestPackageConan(ConanFile):
settings = "os", "arch", "compiler", "build_type"
generators = "CMakeDeps", "CMakeToolchain"

def requirements(self):
self.requires(self.tested_reference_str)

def layout(self):
cmake_layout(self)

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

def test(self):
if can_run(self):
bin_path = os.path.join(self.cpp.build.bindirs[0], "test_package")
self.run(bin_path, env="conanrun")
16 changes: 16 additions & 0 deletions recipes/librdata/all/test_package/test_package.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#include "rdata.h"


int main(int argc, char const *argv[])
{
rdata_parser_t *parser = rdata_parser_init();
if (parser == NULL) {
fprintf(stderr, "Failed to initialize rdata_parser.\n");
return 1;
}

rdata_parser_free(parser);
printf("librdata is successfully included, linked, and used.\n");
Comment on lines +4 to +13
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
int main(int argc, char const *argv[])
{
rdata_parser_t *parser = rdata_parser_init();
if (parser == NULL) {
fprintf(stderr, "Failed to initialize rdata_parser.\n");
return 1;
}
rdata_parser_free(parser);
printf("librdata is successfully included, linked, and used.\n");
int main()
{
rdata_parser_t *parser = rdata_parser_init();
rdata_parser_free(parser);

It does not need to check the method. The idea is checking only linkage and headers.


return 0;
}
3 changes: 3 additions & 0 deletions recipes/librdata/config.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
versions:
"0.1":
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
"0.1":
"0.0.0.cci.20231003":

folder: all