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

add microsoft/bond 9.0.0 #2379

wants to merge 6 commits into from

Conversation

yelu
Copy link

@yelu yelu commented Aug 3, 2020

Specify library name and version: lib/1.0

  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the
    conan-center hook activated.

@CLAassistant
Copy link

CLAassistant commented Aug 3, 2020

CLA assistant check
All committers have signed the CLA.

@conan-center-bot
Copy link
Collaborator

Failure in build 2 (a46e36bf4e8d50e3d758a0dc3dd2d5e20982737e):
bond/9.0.0:

  • Error processing recipe: Linux x86_64, Release, gcc 5, libstdc++ . Options: bond:shared-True
    Unexpected error happened:
ERROR: bond/9.0.0: 'True' is not a valid 'options.shared' value.
Possible values are ['False']

@conan-center-bot
Copy link
Collaborator

Some configurations of 'bond/9.0.0' failed in build 4 (e535502cd1aa23d7e5ef09e7e0e7df8aa4fddf17):

  • Linux x86_64, Release, gcc 5, libstdc++ . Options: bond:shared-True
    • Hooks errors detected:
      • [HOOK - conan-center.py] pre_export(): ERROR: [RECIPE METADATA (KB-H003)] Conanfile doesn't have 'homepage' attribute. (https://github.com/conan-io/conan-center-index/wiki/Error-Knowledge-Base#KB-H003)
      • [HOOK - conan-center.py] pre_export(): ERROR: [CONAN CENTER INDEX URL (KB-H027)] The attribute 'url' should point to: https://github.com/conan-io/conan-center-index (https://github.com/conan-io/conan-center-index/wiki/Error-Knowledge-Base#KB-H027)
      • [HOOK - conan-center.py] pre_export(): ERROR: [TEST PACKAGE - NO IMPORTS() (KB-H034)] The method importsis not allowed in test_package/conanfile.py (https://github.com/conan-io/conan-center-index/wiki/Error-Knowledge-Base#KB-H034)
      • [HOOK - conan-center.py] pre_export(): ERROR: [NO AUTHOR (KB-H037)] Conanfile should not contain author. Remove 'author = "Lu Ye [email protected]"' (https://github.com/conan-io/conan-center-index/wiki/Error-Knowledge-Base#KB-H037)
    • Log download
    • Log error Download
    • Profile
  • Access to all the logs

@conan-center-bot
Copy link
Collaborator

Some configurations of 'bond/9.0.0' failed in build 5 (01f578c194e117b4d3e8138384c997a83d29367a):

  • Linux x86_64, Release, gcc 5, libstdc++ . Options: bond:shared-True
    You are depending on 'boost/1.71.0@conan/stable' but it is not in the repository

Comment on lines +24 to +31
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}")''')
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.

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.

Comment on lines +54 to +55
self.cpp_info.includedirs = ['include']
self.cpp_info.libdirs = ['lib/bond']
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"))

Comment on lines +8 to +14
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)
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})

Comment on lines +12 to +13
# Current dir is "test_package/build/<build_id>" and CMakeLists.txt is
# in "test_package"
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"

Comment on lines +18 to +20
if not tools.cross_building(self):
os.chdir("bin")
self.run(".%sexample" % os.sep)
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)

#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.

"fPIC": True}
build_requires = "boost/1.71.0@conan/stable"
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.

self.run("chmod +rx *", cwd=f'{self.package_folder}/bin')
self.copy("FindBond.cmake", ".", ".")

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"

@conan-center-bot
Copy link
Collaborator

Some configurations of 'bond/9.0.0' failed in build 6 (8866aeeb76b00e0ecafe512f3101de29442a7679):

  • Linux x86_64, Release, gcc 5, libstdc++
    • Hooks errors detected:
      • [HOOK - conan-center.py] pre_source(): ERROR: [IMMUTABLE SOURCES (KB-H010)] Create a file 'conandata.yml' file with the sources to be downloaded. (https://github.com/conan-io/conan-center-index/wiki/Error-Knowledge-Base#KB-H010)
    • Log download
    • Log error Download
    • Profile
  • Access to all the logs

@@ -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


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

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

name = "bond"
version = "9.0.0"
license = "MIT License"
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"

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


def configure_cmake(self):
if platform.system() == "Linux":
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.install()
if platform.system() == 'Linux':
self.run("chmod +rx *", cwd=f'{self.package_folder}/bin')
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.

Comment on lines +47 to +48
if platform.system() == 'Linux':
self.run("chmod +rx *", cwd=f'{self.package_folder}/bin')
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?

def package_info(self):
self.cpp_info.includedirs = ['include']
self.cpp_info.libdirs = ['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

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.

@stale
Copy link

stale bot commented Dec 1, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Dec 1, 2020
@stale
Copy link

stale bot commented Dec 31, 2020

This pull request has been automatically closed because it has not had recent activity. Thank you for your contributions.

@stale stale bot closed this Dec 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants