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

adopt src/CMakeLists.txt, since packaging the library (not just) for conan is astonishing difficult #1017

Closed
a4z opened this issue Dec 9, 2020 · 9 comments
Labels

Comments

@a4z
Copy link

a4z commented Dec 9, 2020

Is your feature request related to a problem? Please describe.
The conan package of this library requires a massiv patch

version 1.3.7 seems to contain a new file, so the existing patch does of course not work anymore.

This is a real impediment for people like me who would like to update this package.
See my PR for 1.3.7 at conan-center, conan-io/conan-center-index#3834

The addition of a new file required a new complex patch based on the existing patch for 1.3.6 , that just exists in the conan recipe. This is error prone and also maybe even intimidating for potential contributors that would like to maintain that recipe.

Describe the solution you'd like
My wish would be, for packaging this library, changing the version number and the sha sum is enough to produce a working
package ( + maybe apply some patche to source code since the version tag)

Basically, make this package good packageable to support all the package maintainer that help to distribute this wonderful software to various Linnux distributions and via package managers to users around the world.

Maybe it is possible to integrate the conan patch into this project? (if this is an option and would be accepted I could give it trial and open a PR)

Additional context
I am not sure about the problem that the CMake from conan differs so much form that one in the source,
I know about some of them, and they could be implemented via cmake options to not change existing behaviour.
But this should very possible be sorted out, otherwise updating the package will always add additional work for this project, or the package will not be updated at all

I am not sure how other package manager solved the problem, but I could imaging they would also benefit from those changes, at least vcpkg has also some patches, and has not updated the recipe for a long time, (1.3.5), maybe the complexity is one of the reasons.

@a4z a4z changed the title adopt src/CMakeLists.txt, since packaging the library for conan is astonishing difficult adopt src/CMakeLists.txt, since packaging the library (not just) for conan is astonishing difficult Dec 9, 2020
@icraggs
Copy link
Contributor

icraggs commented Dec 9, 2020

I was waiting for 1.3.8 to add a new version to conan, so you've jumped the gun a bit.

There are a few useful fixes in, or to be in 1.3.8, one of which you've added as a conan patch to 1.3.7, which I think is bad practice, both from a library behaviour point of view and a conan packaging maintainability point of view. You complained about patches then added a new one!

I didn't write the original conan packaging so I've been trying to get them closer over time. There was no explanation as to why they added patches to the CMake files, so I just had to guess. I shied away from making changes because I don't know why they were there in the first place.

@a4z
Copy link
Author

a4z commented Dec 9, 2020

the patch for the source code is totally different, since you can generate it from this repo
and it is needed for us so our communication works (otherwise the patch would not exist)
so it is maybe useful for others,

@a4z
Copy link
Author

a4z commented Dec 9, 2020

oh, and about the CMakeLists.txt, as mentioned, I can try to help with that, finding out the original reasons for conan doing the changes (some I know already ) and I might be able to find solutions.
I can test the build on osx, linux, android and ios, but for starting digging into that I would like to know if any help in this direction would be welcome

@icraggs
Copy link
Contributor

icraggs commented Dec 9, 2020

Sure, help is welcome. As long as things don't get overcomplicated, build-wise. Simpler, preferably.

And we have to focus on the needs of all the potential users of the library, not just a small section.

This seems more complicated.

And throwing in a slight in the title of the issue doesn't smooth things either.

the patch for the source code is totally different, since you can generate it from this repo
and it is needed for us so our communication works (otherwise the patch would not exist)
so it is maybe useful for others,

@icraggs
Copy link
Contributor

icraggs commented Dec 9, 2020

the patch for the source code is totally different, since you can generate it from this repo
and it is needed for us so our communication works (otherwise the patch would not exist)
so it is maybe useful for others,

All fixes are useful. If everyone did this for every fix they needed, it would be a mess.

@icraggs icraggs added the build label Dec 9, 2020
@a4z
Copy link
Author

a4z commented Dec 10, 2020

the patch for the source code is totally different, since you can generate it from this repo
and it is needed for us so our communication works (otherwise the patch would not exist)
so it is maybe useful for others,

All fixes are useful. If everyone did this for every fix they needed, it would be a mess.

this is discussed there, conan-io/conan-center-index#3834 , no need to bring in that unpleasant discussion in here.

I thought referencing the conan build is a good chance to make people aware of the topic and get help from people that have usually good experience with building software. And ask the authors of the original changes for their motivation and help.

I think, if it is possible to create a productive, welcoming open environment for this CMakeLists.txt update, it should be possible to get help to create the build in a way that is not overcomplicated and works for all. Hopefully that is possible.

@icraggs
Copy link
Contributor

icraggs commented Dec 10, 2020

I don't see why that's "unpleasant" - having lots of patch files around would be a mess, that's just a fact. And the conan project asked me to reduce the number of patch files.

And in reverse, perhaps the adoption of the CMake config should be the other way around - the conan packaging adopting this base project one? Why not that way around?

I've been working to merge the two CMake configs in the last few months while maintaining backward compatibility with both existing build methods. That process is ongoing.

@a4z
Copy link
Author

a4z commented Dec 10, 2020

And in reverse, perhaps the adoption of the CMake config should be the other way around - the conan packaging adopting this base project one? Why not that way around?

I absolutely agree!
Since there is no documentation about the why those patches exits, it might be the best idea start with a vanilla paho-mqtt-c conan build, see what breaks, and act accordingly but looking at the problem one by one.

@a4z
Copy link
Author

a4z commented Dec 18, 2020

Since 9 days your version 1.3.7 is shipped with known bugs that disable workflows with some of the big cloud provider. Because your users can wait, right? Well done!
Therefor you cause so much uncertainty at the cci maintainers that they flee into bureaucracy instead of doing their job.
And you managed to create big frustation and time wast for helpful people, like me. An other well done thing.
I do not think I have any more motivation to do anything for that issue.

@a4z a4z closed this as completed Dec 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants