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

reflect-cpp: add option for using with msgpack #24262

Merged
merged 2 commits into from
Jun 13, 2024

Conversation

uyha
Copy link
Contributor

@uyha uyha commented Jun 8, 2024

Specify library name and version: lib/1.0


@conan-center-bot

This comment has been minimized.

@AbrilRBS AbrilRBS self-assigned this Jun 10, 2024
Copy link
Member

@AbrilRBS AbrilRBS left a comment

Choose a reason for hiding this comment

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

Hi @uyha thanks a lot for taking the time to create the PR.

This being a header-only library, asking for compilation logs would not be too useful - could you instead provide an example program using this library with msgpack enabled that makes use of the new available features thanks to the new dependency? Thanks!

@uyha
Copy link
Contributor Author

uyha commented Jun 10, 2024

Do you mean add a test that uses the msgpack feature?

@AbrilRBS
Copy link
Member

yes, that'd work :)
Let me know if you need any help adding it

@jcar87 jcar87 self-assigned this Jun 13, 2024
@@ -56,6 +58,8 @@ def requirements(self):
self.requires("flatbuffers/23.5.26", transitive_headers=True)
if self.options.with_yaml:
self.requires("yaml-cpp/0.8.0", transitive_headers=True)
if self.options.with_msgpack:
self.requires("msgpack-cxx/6.1.1", transitive_headers=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

reflect-cpp should require msgpack-c, not msgpack-cxx, as per readme here:

https://github.com/getml/reflect-cpp/blob/0342e5c9a224039146a9f4261ac3cb695457422a/README.md#L37-L38

@jcar87
Copy link
Contributor

jcar87 commented Jun 13, 2024

I've amended the PR to use the correct msgpack-c library, and amended the test so that this functionality is tested, as per the reflect-cpp readme.

@uyha, thanks for your contribution - please be mindful that for PRs that introduce features that are not tested by CI (such as an optional feature), we do require that it is tested locally first, and it would appear this was not the case. We'll try to do it ourselves where possible - but otherwise we run the risk of introducing broken options, and it can only delay the PR being merged if the changes are useful.

(venv) root@079fdee7cb8c:/reflect-cpp/all/test_package# conan test . reflect-cpp/0.10.0  -s compiler.cppstd=20 -o "*:with_msgpack=True"

======== Input profiles ========
Profile host:
[settings]
arch=armv8
build_type=Release
compiler=gcc
compiler.cppstd=20
compiler.libcxx=libstdc++11
compiler.version=13
os=Linux
[options]
*:with_msgpack=True

Profile build:
[settings]
arch=armv8
build_type=Release
compiler=gcc
compiler.cppstd=gnu17
compiler.libcxx=libstdc++11
compiler.version=13
os=Linux


======== Launching test_package ========

======== Computing dependency graph ========
Graph root
    reflect-cpp/0.10.0 (test package): /reflect-cpp/all/test_package/conanfile.py
Requirements
    msgpack-c/6.0.0#769a7b807a4b0f758f24755361069c68 - Cache
    reflect-cpp/0.10.0#dc114ad4bed2f24030d91807dc823210 - Cache

======== Computing necessary packages ========
Requirements
    msgpack-c/6.0.0#769a7b807a4b0f758f24755361069c68:258056170a5add18e564ac86c77027634e725af3#8b1fe2818466b69415b3668971c3575d - Cache
    reflect-cpp/0.10.0#dc114ad4bed2f24030d91807dc823210:da39a3ee5e6b4b0d3255bfef95601890afd80709#426c762de577fa544c490fdc098ca730 - Cache

======== Installing packages ========
msgpack-c/6.0.0: Already installed! (1 of 2)
reflect-cpp/0.10.0: Already installed! (2 of 2)
WARN: deprecated: Usage of deprecated Conan 1.X features that will be removed in Conan 2.X:
WARN: deprecated:     'cpp_info.names' used in: msgpack-c/6.0.0
WARN: deprecated:     'cpp_info.build_modules' used in: msgpack-c/6.0.0

======== Testing the package ========
Removing previously existing 'test_package' build folder: /reflect-cpp/all/test_package/build/gcc-13-armv8-20-release
reflect-cpp/0.10.0 (test package): Test package build: build/gcc-13-armv8-20-release
reflect-cpp/0.10.0 (test package): Test package build folder: /reflect-cpp/all/test_package/build/gcc-13-armv8-20-release
reflect-cpp/0.10.0 (test package): Writing generators to /reflect-cpp/all/test_package/build/gcc-13-armv8-20-release/generators
reflect-cpp/0.10.0 (test package): Generator 'CMakeDeps' calling 'generate()'
reflect-cpp/0.10.0 (test package): CMakeDeps necessary find_package() and targets for your CMakeLists.txt
    find_package(reflect-cpp)
    target_link_libraries(... reflect-cpp::reflect-cpp)
reflect-cpp/0.10.0 (test package): Generator 'VirtualRunEnv' calling 'generate()'
reflect-cpp/0.10.0 (test package): Calling generate()
reflect-cpp/0.10.0 (test package): Generators folder: /reflect-cpp/all/test_package/build/gcc-13-armv8-20-release/generators
reflect-cpp/0.10.0 (test package): CMakeToolchain generated: conan_toolchain.cmake
reflect-cpp/0.10.0 (test package): CMakeToolchain generated: /reflect-cpp/all/test_package/build/gcc-13-armv8-20-release/generators/CMakePresets.json
reflect-cpp/0.10.0 (test package): CMakeToolchain generated: /reflect-cpp/all/test_package/CMakeUserPresets.json
reflect-cpp/0.10.0 (test package): Generating aggregated env files
reflect-cpp/0.10.0 (test package): Generated aggregated env files: ['conanrun.sh', 'conanbuild.sh']

======== Testing the package: Building ========
reflect-cpp/0.10.0 (test package): Calling build()
reflect-cpp/0.10.0 (test package): Running CMake.configure()
reflect-cpp/0.10.0 (test package): RUN: cmake -G "Unix Makefiles" -DCMAKE_TOOLCHAIN_FILE="generators/conan_toolchain.cmake" -DCMAKE_INSTALL_PREFIX="/reflect-cpp/all/test_package" -DCONAN_TEST_WITH_MSGPACK="ON" -DCMAKE_POLICY_DEFAULT_CMP0091="NEW" -DCMAKE_BUILD_TYPE="Release" "/reflect-cpp/all/test_package"
-- Using Conan toolchain: /reflect-cpp/all/test_package/build/gcc-13-armv8-20-release/generators/conan_toolchain.cmake
-- Conan toolchain: C++ Standard 20 with extensions OFF
-- The CXX compiler identification is GNU 13.2.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Conan: Target declared 'reflect-cpp::reflect-cpp'
-- Conan: Target declared 'msgpack-c'
-- Configuring done (0.2s)
-- Generating done (0.0s)
-- Build files have been written to: /reflect-cpp/all/test_package/build/gcc-13-armv8-20-release

reflect-cpp/0.10.0 (test package): Running CMake.build()
reflect-cpp/0.10.0 (test package): RUN: cmake --build "/reflect-cpp/all/test_package/build/gcc-13-armv8-20-release" -- -j10
[ 50%] Building CXX object CMakeFiles/test_package.dir/test_package.cpp.o
[100%] Linking CXX executable test_package
[100%] Built target test_package


======== Testing the package: Executing test ========
reflect-cpp/0.10.0 (test package): Running test()
reflect-cpp/0.10.0 (test package): RUN: ./test_package
msgpack test: ��x�name�test_package
reflect-cpp test successful

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 2 (578b97e66cab01c405bbbdb33e5bb961be2fbce2):

  • reflect-cpp/0.6.0:
    All packages built successfully! (All logs)

  • reflect-cpp/0.10.0:
    All packages built successfully! (All logs)

  • reflect-cpp/0.11.0:
    All packages built successfully! (All logs)

@AbrilRBS
Copy link
Member

Just as a tip for next time: header-library packages already infer transitive_headers=True for all their dependencies, it's not needed to explicitly set them :)

@conan-center-bot conan-center-bot merged commit 5f5ee4d into conan-io:master Jun 13, 2024
23 checks passed
memsharded pushed a commit to memsharded/conan-center-index that referenced this pull request Jun 14, 2024
* reflect-cpp: add option for using with msgpack

* reflect-cpp: fix with_msgpack option

---------

Co-authored-by: Luis Caro Campos <[email protected]>
conan-center-bot added a commit that referenced this pull request Jun 17, 2024
* upgrading to Conan 2

* clean yaml

* removed from config.yml

* Cleanup

* use tool_requires

* wip

* (#24282) bump boost dependency

* (#23502) (#23474) simdjson/3.9.1: remove unnecessary constraints

* (#18680) kealib: migrate to Conan v2

* kealib: migrate to Conan v2

* kealib: transitive_libs=True

* kealib: simplify patch

* kealib: add v1.5.1

* kealib: bump hdf5

* kealib: add min cppstd check

* kealib: require GCC 6+

* kealib: drop v1.4.13

* kealib: bump to 1.5.2

* kealib: bump to 1.4.15

* kealib: fix MSVC lib name

* kealib: bump hdf5

* kealib: fix MSVC library names

* kealib: set HDF5_THREADSAFE based on hdf5 option

* kealib: restore GCC 5 support

Co-authored-by: Uilian Ries <[email protected]>

* kealib: fix a recipe bug

* kealib: add CMAKE_POLICY_DEFAULT_CMP0077

* kealib: use valid_min_cppstd()

* kealib: add cxx_std_11 to test_package

* Add missing system lib in windows

* Improve invalid config message.

---------

Co-authored-by: Uilian Ries <[email protected]>
Co-authored-by: Rubén Rincón Blanco <[email protected]>

* (#19228) msix: migrate to Conan v2

* msix: migrate to Conan v2

* msix: use OpenSSL v1

* msix: bump deps

* msix: fix Clang build issue

* msix: fix OpenSSL 3 support

* msix: add missing system libs

* msix: simplify xml_parser option

* msix: Clang 12 is not supported

* msix: yaml

* msix: test_package cxx_std_14

* (#24296) cpp-httplib: add version 0.16.0

* (#24297) glaze: add version 2.8.1

* (#24106) arrow: fix target names

* Use the same CMake target names as upstream

* Bugfix

* Bugfix

* Remove redundant calls to set_property("cmake_target_name")

* Revert "Remove redundant calls to set_property("cmake_target_name")"

This reverts commit fc113f0.

* Bugfix

* Bugfix

* (#24307) libftp: add version 0.5.1

* (#24298) [sentry-native] Add versions 0.7.5/0.7.6 and remove 0.7.1/0.7.2

* (#24309) glaze: add version 2.8.2

* Update PULL_REQUEST_TEMPLATE.md (#24311)

Update PR template:

- Mention list of PRs by recipe
- Simplify wording around testing locally
- request that PRs are motivated and detailed (what/why/how)

* (#24308) octo-logger-cpp: fix checksum for 1.12.0

* (#22456) proposal: update dependencies

* proposal: update dependencies

* Update recipes/proposal/all/conanfile.py

* Update recipes/proposal/all/conanfile.py

* Update recipes/proposal/all/conanfile.py

* proposal: build fewer versions

---------

Co-authored-by: Luis Caro Campos <[email protected]>

* (#18949) userspace-rcu: migrate to Conan v2

* userspace-rcu: migrate to Conan v2

* userspace-rcu: add v0.14.0

* userspace-rcu: no need for a custom bootstrap script

* userspace-rcu: fix test_v1_package

* userspace-rcu: disable apple-clang for v0.11.4

* (#18986) librhash: migrate to Conan v2

* librhash: migrate to Conan v2

* librhash: use OpenSSL v1

* librhash: bump deps

* librhash: fix installation

* librhash: downgrade OpenSSL

* librhash: switch to custom CMakeLists.txt

The build scripts in the project are quite non-standard and not too portable.

Based on rhash/RHash#103

* librhash: bump openssl

* librhash: add v1.4.4

* librhash: fix the list of sources used in CMakeLists.txt

* librhash: fix missing lib prefix for libs

* librhash: don't use PROJECT_NAME

* (#18950) pbc: migrate to Conan v2

* pbc: migrate to Conan v2

* pbc: bump gmp

* pbc: use winflexbison on Windows

* pbc: add transitive_headers=True to gmp

* pbc: fix build

* pbc: fix apple cross building bug

* pbc: transitive_libs=True

* pbc: bump deps

* pbc: fix configure issue

* pbc: fix to_apple_arch()

* pbc: add MSVC support

* pbc: add AutotoolsDeps workaround for MSVC

* pbc: add MSVC ./configure workarounds

* pbc: not compatible with MSVC due to use of GNU extensions

* pbc: fix_apple_shared_install_name()

* (#18948) hiredis/0.x.x: remove 0.x recipe

* hiredis/0.x.x: migrate to Conan v2

* hiredis/0.x.x: fix_apple_shared_install_name(self)

* Dropped support for very old 0.x.x versions

---------

Co-authored-by: PerseoGI <[email protected]>

* (#23997) mini: add recipe

* mini: add recipe

* Update recipes/mini/all/test_package/test_package.cpp

---------

Co-authored-by: Luis Caro Campos <[email protected]>

* (#24300) octo-logger-cpp: bumped fmt dependency

* (#20586) pcre2: fix invalid pcre2-config output for static libs

* pcre2: fix invalid pcre2-config output for static libs

* pcre2: ignore noise from self.run()

* pcre2: make pcre2-config test a bit more robust

* pcre2: handle debug postfix correctly

* pcre2: no need for win_bash = True

* pcre2: drop self.tool_requires(self.tested_reference_str)

* Update recipes/pcre2/all/test_package/conanfile.py

* Update recipes/pcre2/all/test_package/conanfile.py

* Update recipes/pcre2/all/test_package/conanfile.py

* Update recipes/pcre2/all/test_package/conanfile.py

* Update recipes/pcre2/all/test_package/conanfile.py

* Update recipes/pcre2/all/test_package/conanfile.py

* fix replace for 10.43

* fix fix replace

---------

Co-authored-by: Daniel <[email protected]>

* (#19239) pro-mdnsd: migrate to Conan v2

Co-authored-by: Carlos Zoido <[email protected]>

* (#24262) reflect-cpp: add option for using with msgpack

* reflect-cpp: add option for using with msgpack

* reflect-cpp: fix with_msgpack option

---------

Co-authored-by: Luis Caro Campos <[email protected]>

* (#24011) arrow: add version 16.1.0

* arrow: add version 16.1.0

* Format build method

---------

Co-authored-by: PerseoGI <[email protected]>

* (#24312) [bot] Update authorized users list (2024-06-13)

Co-authored-by: conan-center-bot <[email protected]>

* (#24273) [boost] Add -fcoroutines for GCC10 only

* Add -fcoroutines for GCC10 only

Signed-off-by: Uilian Ries <[email protected]>

* Use b2 ranged version

Signed-off-by: Uilian Ries <[email protected]>

---------

Signed-off-by: Uilian Ries <[email protected]>

* (#24318) glaze: add version 2.8.4

* (#24316) jsoncons: add version 0.176.0

* (#24306) libmaxminddb: add version 1.10.0

* libmaxmind: add version 1.10.0

* add with_binaries option

* Cleanups

* append PATH on with_binaries=True

---------

Co-authored-by: Rubén Rincón Blanco <[email protected]>

* (#24323) pcre2: add version 10.44

* test pr do not merge

* Revert "test pr do not merge"

This reverts commit f146b96.

* (#24328) change gn validate to validate_build

* package license

* update to set_property for cmake_build_modules

* add library type

---------

Signed-off-by: Uilian Ries <[email protected]>
Co-authored-by: Rubén Rincón Blanco <[email protected]>
Co-authored-by: Jean-Marco Alameddine <[email protected]>
Co-authored-by: Jie Cheng <[email protected]>
Co-authored-by: Martin Valgur <[email protected]>
Co-authored-by: Uilian Ries <[email protected]>
Co-authored-by: toge <[email protected]>
Co-authored-by: Roberto Rossini <[email protected]>
Co-authored-by: Martin Delille <[email protected]>
Co-authored-by: Luis Caro Campos <[email protected]>
Co-authored-by: PerseoGI <[email protected]>
Co-authored-by: Daniel <[email protected]>
Co-authored-by: Carlos Zoido <[email protected]>
Co-authored-by: Beartama <[email protected]>
Co-authored-by: Conan Center Index Bot <[email protected]>
Co-authored-by: conan-center-bot <[email protected]>
@uyha uyha deleted the reflect-cpp branch July 28, 2024 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants