New recipe: reflectcpp; intended to replace recipe reflect-cpp, which is named incorrectly

[liuzicheng1987]

Summary

Adds a new recipe: reflectcpp/0.14.0. The recipe "reflect-cpp" should be deprecated or removed, because its name is wrong.

Motivation

As I explained to @AbrilRBS and @uilianries in #24819, the problem is the following:

I am the main author of reflectcpp, but I didn't upload any of the previous Conan packages for reflectcpp. Other people did that without informing me. I found out about its existence, because users were complaining to me about the Conan package.

In principle, I am happy about people uploading my library to Conan, however they made a mistake: The have called they package "reflect-cpp", but in our CMakeLists.txt, we have always referred to the package as "reflectcpp":

https://github.com/getml/reflect-cpp/blob/main/CMakeLists.txt

But a requirement of the Conan package is that the name of the installed package be the same as the name of the package in the CMakeLists.txt (which makes a lot of sense).

To be honest, I may have contributed to the confusion, because I often refer to the package as "reflect-cpp". However, it is the name in the CMakeLists.txt that matters for our purposes and that name is "reflectcpp".

This didn't matter as long as the library was treated as header-only. But this was a workaround from the get-go and in any case it won't work for any of the newer versions.

The name of the package on Conan is wrong, plain and simple. If you stick with the old name you won't be able to host any of the newer versions of reflectcpp and will be stuck with an outdated version for all eternity. It needs to be fixed.

Details

I have added a build pipeline. I do not use the bundled dependencies, but install them via Conan instead. JSON support is activated by default. Since you only want very simple tests, not comprehensive functional testing (https://docs.conan.io/2/tutorial/creating_packages/test_conan_packages.html), I have done just that.

---

• Read the contributing guidelines
• Checked that this PR is not a duplicate: list of PRs by recipe
• Tested locally with at least one configuration using a recent version of Conan

[liuzicheng1987]

Can someone give me some advice? This package only support C++20, but the pipeline keeps on trying to build it for earlier versions. What do I do here? This can't be the first package facing this problem.

[toge]

@liuzicheng1987
I have written several PR updating reflect-cpp recipes.
Now I see that reflectcpp is the official name, not reflect-cpp.
I had no doubts about the name of reflect-cpp because of your reddit post and the name of repository.
I apologize for the unnecessary hassle this has caused you.

You need to implement validate() to make CI work only for compilers that support C++20.
Could you please refer me to the existing reflect-cpp recipe or to the conan-center-index template?

https://github.com/conan-io/conan-center-index/blob/master/docs/package_templates/header_only/all/conanfile.py#L63-L71

[uilianries]

@liuzicheng1987 Please, keep the original test package from https://github.com/conan-io/conan-center-index/tree/master/recipes/reflect-cpp/all/test_package

The test package is used to validate those packaged artifacts, like headers and libraries, not including the headers, but also linking libraries. Plus, we do not expose rules of settings in test_package.

[liuzicheng1987]

@toge, absolutely no problem. Like I said, I certainly contributed to the confusion, because I often refer to it as "reflect-cpp" myself.

I am grateful there are so many people who are willing to contribute to the library.

[uilianries]

@liuzicheng1987 Thank you again for your effort on this PR. I opened a PR to suggest all changes requested by me, instead of doing directly in the PR. It would faster than asking with suggestions here. Please, take a look: liuzicheng1987#1

[liuzicheng1987]

@uilianries , sounds great. Thank you so much for your help. I will take a closer look later today.

[liuzicheng1987]

@uilianries , I think that I now have resolved all of the issues and this is ready to be merged.

If you agree, I will maintain the Conan package from now on and also add more comprehensive testing on my side to make sure that the Conan options we provide all work as expected.

The issue with the shared build was a bit more complicated than we both anticipated, but the good news is that I got it resolved.

Fixing it required some changes on our side. That is why the shared library version of reflectcpp-0.14.0 and reflectcpp-0.15.0 won't build. But I have included the necessary changes in the newest release, reflectcpp-0.16.0.

The other change necessary was to add this line to the CMakeLists.txt:

SET(CMAKE_EXE_LINKER_FLAGS  "${CMAKE_EXE_LINKER_FLAGS} -Wl,--copy-dt-needed-entries")

Passing this linker flag is necessary whenever, the following three statements hold true:

• You are linking a shared library
• The shared library links to another shared library that is written in a different language
• You also require direct access to the other library

Because YYJSON is written in C (as well as some of the other libraries we use), these three statements hold true when shared=True.

[uilianries]

@liuzicheng1987 Thank you for taking time and effort to fix this recipe!

If you agree, I will maintain the Conan package from now on and also add more comprehensive testing on my side to make sure that the Conan options we provide all work as expected.

All recipes in CCI, they don't a maintainers, anyone is able to request a new version or send a PR.

Fixing it required some changes on our side. That is why the shared library version of reflectcpp-0.14.0 and reflectcpp-0.15.0 won't build. But I have included the necessary changes in the newest release, reflectcpp-0.16.0.

Is this change incorporated into the upstream already? Is there an issue that we link in the recipe itself?

When consuming the package, users should not need to configure such flags. In fact, it's first that I see this flag being used in CCI. What error did it cause?

[liuzicheng1987]

@uilianries, yes, the changes are incorporated upstream.

The error message is the following:

DSO missing from command line

I would refer you to this website which at greater detail how this error occurs.

The DSO missing from command line message will be shown when the linker can’t find the required symbol with its normal search, but the symbol is available in one of the dependencies of a directly specified dynamic library.

In the past, the linker thought that symbols in languages that depended on other languages were available. But in a later version, that changed, and now the linker has a stricter view of what is available. So, the message is meant to help with this change.

https://linuxpip.org/how-to-fix-dso-missing-from-command-line/?utm_content=cmp-true

Again, it's only a problem for dynamic linking, not for static linking.

[uilianries]

@liuzicheng1987 You current error is because yyjson's method yyjson_mut_doc_new is exposed directly in write.hpp, and Conan 2.x does not export dependencies libraries:

https://github.com/getml/reflect-cpp/blob/main/include/rfl/json/write.hpp#L44

In order to fix that, you have to explicit tell to Conan to make yyjson available to the upstream.

[liuzicheng1987]

@uilianries, I have merged your suggestions and added a missing import. It now works without the ugly additional flag. Thank you so much for your suggestions.

[uilianries]

Hello @liuzicheng1987!

I’m glad to hear that your PR is working now! I’ve tested it on both Linux and Mac with all options enabled (including dependencies), and it looks promising. However, there is a small fix that still needs to be addressed.

I recently spoke with the Conan team regarding your current PR and the previous reflect-cpp recipe in Conan Center. It’s important to note that your project is named reflect-cpp (getml/reflect-cpp), and users will search for the project name rather than the CMake project name, which is less visible in the CMakeLists.txt file. Therefore, the Conan package can be named reflect-cpp while providing CMake targets as reflectcpp. This adjustment requires just one additional line in the package_info() function to customize.

Additionally, we would like to know if you plan to rename your GitHub repository to reflectcpp. This change is significant for users, as they typically search for the project name rather than library file names, based on our experience.

Regarding your current PR, please rest assured that your efforts are not in vain. We can incorporate all your changes into the existing reflect-cpp recipe, which will address some bugs we identified during our review and update the project version available in Conan Center.

I apologize for the delay in bringing this up, but since your PR is looking really good now, I wanted to clarify the best way to resolve the current package name conflict before merging.

Thank you for your hard work!

[uilianries]

Build logs using all options as True (dependencies), include the CMakeDeps fix for flatbuffers. It looks good in Linux and Mac, built locally:

• reflectcpp-0.16.0-linux-shared.log
• reflectcpp-0.16.0-macos-static.log

[liuzicheng1987]

Hi @uilianries , no I do not plan to rename anything.

If we can make it the "reflect-cpp" recipe work, I would greatly prefer that over calling it "reflectcpp". I always thought this was a limitation of Conan.

[liuzicheng1987]

@uilianries, if there is anything I can do to help, please let me know.

[uilianries]

@liuzicheng1987 Thank you for understanding! I'll do a git mv, using your PR, over the current reflect-cpp recipe in ConanCenterIndex. So your PR will be preserved, including all your commits, and the previous recipe in the master branch will be overridden.

Now we are really close to merge it!

[uilianries]

It requires MSVC version 193, Update 8. However, CCI does not use Update in settings. I'll check with the team.

[uilianries]

@liuzicheng1987 I did few changes in your PR:

• Moved your PR to folder reflect-cpp, so it's no longer a "new" entire recipe, but a change over the current reflect-cpp
• Added features related to Conan 2.x. The Conan 2.x is mandatory now in CCI: Conan Center will stop receiving updates for Conan 1.x packages soon #25461
• The CI is running MSVC 19.3 update 6, but your project requires update 8. The settings.yml supports the update number as part of the profile, but we do not use it, so we can not validate it in the recipe as well. The CI should be updated to use the latest update version available, but it takes time and we don't want to delay you even more, so the minimal MSVC required is 194 in the recipe, but should be changed as soon as we have the CI fixed.

Thank you again for your big effort over this recipe. I built locally again, using dependencies and it looks good to me now.

[uilianries]

LGTM

[liuzicheng1987]

@uilianries, thank you so much for your comments as well!