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

add microsoft/bond 9.0.0 #2379

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 5 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
9 changes: 9 additions & 0 deletions recipes/bond/all/FindBond.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# find_path(BOND_INCLUDE_DIR NAMES bond PATHS ${CONAN_INCLUDE_DIRS_BOND})
Copy link
Member

Choose a reason for hiding this comment

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

Please, remove this file. Conan should be able to generate the file by https://docs.conan.io/en/latest/integrations/build_system/cmake/cmake_find_package_generator.html

find_library(BOND_LIBRARY NAMES ${CONAN_LIBS_BOND} PATHS ${CONAN_LIB_DIRS_BOND})
find_file(BOND_GBC NAMES gbc.exe gbc PATHS ${CONAN_BIN_DIRS_BOND})

set(BOND_FOUND TRUE)
set(BOND_INCLUDE_DIRS ${CONAN_INCLUDE_DIRS_BOND})
set(BOND_LIBRARIES ${BOND_LIBRARY})
set(BOND_GBC ${BOND_GBC})
mark_as_advanced(BOND_LIBRARY BOND_INCLUDE_DIR)
57 changes: 57 additions & 0 deletions recipes/bond/all/conanfile.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
from conans import ConanFile, CMake, tools
import platform, os


class BondConan(ConanFile):
name = "bond"
version = "9.0.0"
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 = "9.0.0"

The version should be consumed from conandata.yml: https://docs.conan.io/en/latest/reference/config_files/conandata.yml.html

license = "MIT License"
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
license = "MIT License"
license = "MIT"

Use the short-name form https://spdx.org/licenses/MIT

homepage = "https://github.com/microsoft/bond.git"
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
homepage = "https://github.com/microsoft/bond.git"
homepage = "https://github.com/microsoft/bond"

url = "https://github.com/conan-io/conan-center-index"
description = "Bond is a cross-platform framework for working with schematized data. It supports cross-language de/serialization and powerful generic mechanisms for efficiently manipulating data. Bond is broadly used at Microsoft in high scale services."
topics = ("bond", "microsoft")
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
topics = ("bond", "microsoft")
topics = ("bond", "microsoft", "serialization", "cross-language", "schematized-data")

settings = "os", "compiler", "build_type", "arch"
options = {
"shared": [False, True],
"fPIC": [True, False]}
default_options = {
"shared": False,
"fPIC": True}
build_requires = "boost/1.71.0@conan/stable"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
build_requires = "boost/1.71.0@conan/stable"
requires = "boost/1.73..0"

A require is really necessary.
If boost is shared, then the library must be installed too.

generators = "cmake"
exports_sources = ["FindBond.cmake"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this file (and export a conan wrapper CMakeLists.txt file instead).

As a consumer of this recipe, when using the cmake_find_package generator, conan will genere a FindBond.cmake script automatically.


def source(self):
self.run("git clone https://github.com/microsoft/bond.git")
self.run("git checkout fe6f582ce4beb65644d9338536066e07d80a0289", cwd='bond')
self.run("git submodule update --init --recursive", cwd='bond')
tools.replace_in_file("bond/CMakeLists.txt",
"project (bond)",
'''project (bond)
message("BOOST_ROOT=$ENV{BOOST_ROOT}")''')
Comment on lines +22 to +29
Copy link
Contributor

@madebr madebr Aug 3, 2020

Choose a reason for hiding this comment

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

Can you please package release 9.0.1: https://github.com/microsoft/bond/releases/tag/9.0.1
Then, you can use conandata.yml + export_source a CMakeLists.txt file, as all other recipes do.


def configure_cmake(self):
if platform.system() == "Linux":
Comment on lines +31 to +32
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
def configure_cmake(self):
if platform.system() == "Linux":
def _configure_cmake(self):
if tools.os_info.is_linux:

You can't add public methods, use protected instead.

self.run("curl -sSL https://get.haskellstack.org/ | sh", ignore_errors=True)
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need haskell? Downloading here is not allowed, but instead you can have a Conan package for haskell.

cmake = CMake(self)
cmake.definitions["BOND_ENABLE_GRPC"] = 'FALSE'
cmake.configure(source_folder="bond")
return cmake

def build(self):
cmake = self.configure_cmake()
cmake.build()

def package(self):
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
def package(self):
def package(self):
self.copy("LICENSE", dst="licenses", src="bond")
self.copy("Bond.cmake", dst=os.path.join("lib", "cmake", "bond"), src=os.path.join("bond", "cmake))

Don't forget the license
Bond.cmake seems be useful.

cmake = self.configure_cmake()
# CMAKE_INSTALL_PREFIX is set to self.package_folder
cmake.install()
if platform.system() == 'Linux':
self.run("chmod +rx *", cwd=f'{self.package_folder}/bin')
Comment on lines +47 to +48
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 platform.system() == 'Linux':
self.run("chmod +rx *", cwd=f'{self.package_folder}/bin')
if tools.os_info.is_linux:
self.run("chmod +rx *", cwd=f'{self.package_folder}/bin')

Use os.chmod instead. Is there an executable here?

self.copy("FindBond.cmake", ".", ".")
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.copy("FindBond.cmake", ".", ".")

Not allowed. Use cmake_find_package generator instead.


def package_info(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Apply this change only if bond creates BondConfig.cmake or FindBond.cmake files.

Suggested change
def package_info(self):
def package_info(self):
self.cpp_info.names["cmake_find_package"] = "Bond"
self.cpp_info.names["cmake_find_package_multi"] = "Bond"

self.cpp_info.includedirs = ['include']
self.cpp_info.libdirs = ['lib/bond']
Comment on lines +52 to +53
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't the library installed in the lib folder?

Suggested change
self.cpp_info.includedirs = ['include']
self.cpp_info.libdirs = ['lib/bond']
self.cpp_info.libdirs.append(os.path.join("lib", "bond"))

self.cpp_info.libs = ["bond"]
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 = ["bond"]
self.cpp_info.libs = ["bond"]
self.cpp_info.builddirs.append(os.path.join("lib", "cmake", "bond"))
self.cpp_info.build_modules.append(os.path.join("lib", "cmake", "bond", "Bond.cmake"))

The cmake file offered by author contains some macros. Conan can't re-create it, so the best approach is copying it to your package.

https://github.com/microsoft/bond/blob/master/cmake/Bond.cmake


14 changes: 14 additions & 0 deletions recipes/bond/all/test_package/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
cmake_minimum_required(VERSION 2.8.12)
project(PackageTest CXX)

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

add_executable(example example.cpp)
target_link_libraries(example ${CONAN_LIBS})

# CTest is a testing tool that can be used to test your project.
# enable_testing()
# add_test(NAME example
# WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/bin
# COMMAND example)
Comment on lines +8 to +14
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
target_link_libraries(example ${CONAN_LIBS})
# CTest is a testing tool that can be used to test your project.
# enable_testing()
# add_test(NAME example
# WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/bin
# COMMAND example)
target_link_libraries(example ${CONAN_LIBS})

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

from conans import ConanFile, CMake, tools


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

def build(self):
cmake = CMake(self)
# Current dir is "test_package/build/<build_id>" and CMakeLists.txt is
# in "test_package"
Comment on lines +12 to +13
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Current dir is "test_package/build/<build_id>" and CMakeLists.txt is
# in "test_package"

cmake.configure()
cmake.build()

def test(self):
if not tools.cross_building(self):
os.chdir("bin")
self.run(".%sexample" % os.sep)
Comment on lines +18 to +20
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if not tools.cross_building(self):
os.chdir("bin")
self.run(".%sexample" % os.sep)
if not tools.cross_building(self.settings):
bin_path = os.path.join("bin", "example")
self.run(bin_path, run_environment=True)

7 changes: 7 additions & 0 deletions recipes/bond/all/test_package/example.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#include <iostream>
#include <bond/core/bond_const_enum.h>

int main() {
std::cout << "bond is installed successfully" << std::endl;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please call at least one bond function (not using a define or header-only function).
That way we can be sure the library is tested properly.

return 0;
}
3 changes: 3 additions & 0 deletions recipes/bond/config.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
versions:
9.0.0:
folder: all