[question] A correct way to update msgpack recipe

conan-io / conan-center-index

Recipes for the ConanCenter repository

https://conan.io/center

MIT License

953 stars 1.74k forks source link

[question] A correct way to update msgpack recipe #8247

Open kovdan01 opened 2 years ago

kovdan01 commented 2 years ago

Hello. I want to add support for msgpack 4.0.3 (C++) and msgpack 4.0.0 (C), but am not sure what way is most suitable for this. The problem is that starting from version 4.0.0, msgpack releases for C and C++ became separate (and the sources are actually completely independent). Moreover, now C++ version of msgpack requires boost, but this dependency is optional in current recipe. Given that, the recipe seems outdated. I see two possible options of adding support for new msgpack versions:

Stay with current recipe and rewrite it so if the version is 4.0.0 or above it will behave in one way, and if the version is lower - in another one. Looks not very good as for me.
Create two new recipes for C and C++ libraries, given that they are completely separate and the only common thing between them is the name. I personally prefer this option, and it seems that these new recipes will be shorter and simpler, and it will be much easier to maintain them.

What do you think about this question? Which option is more preferrable? Thanks!

toge commented 2 years ago

Hello, @kovdan01

I'm going to update the msgpack recipe too. I agree with the second option. In my personal opinion, I think it would be better to keep msgpack and make it for C++ so that people who use msgpack already will not be bothered.

I have already created a PR for msgpack-c. You can find it at https://github.com/conan-io/conan-center-index/pull/8183 Please let me know your opinion.

kovdan01 commented 2 years ago

Hello, @toge! Thanks!

Sorry for a long delay. PR #8183 looks good as for me, but I think that it might be cool to follow @prince-chrismc 's suggestion to split msgpack recipes to 3 folders (3.x, c and cpp) being still inside one msgpack package: https://github.com/conan-io/conan-center-index/pull/8183#issuecomment-986049920.

So config.yml file would look like

versions:
  "3.2.1":
    folder: "3.x"
  "3.3.0":
    folder: "3.x"
  "4.0.0-c":
    folder: "c"
  "4.0.3-cpp":
    folder: "cpp"

And you will have separate recipe files:

3.x/conanfile.py for old msgpack 3.x (c and cpp versions together)
c/conanfily.py for 4.x versions of msgpack-c
cpp/conanfile.py for 4.x versions of msgpack-cxxand new 4.x versions

It would be the best solution from my perspective. Feel free to ask me if you need any help.

toge commented 2 years ago

@kovdan01 @prince-chrismc Sorry for late. Your suggestion is very good for me. Before I noticed the two comments, msgpack-c was merged.

@prince-chrismc Do you know how I can cancel a package that has already been merged?

prince-chrismc commented 2 years ago

Nop! It's too late, we can not delete packages

toge commented 2 years ago

@kovdan01 @prince-chrismc Sorry for my slow response and not being able to use the good suggestion.

Now that msgpack-c has been registered, I'm going to take action to upgrade msgpack as C++ version. Or would it be better to make msgpack-cpp?

prince-chrismc commented 2 years ago

I'd vote for cpp