update paho-mqtt-c to 1.3.7

[a4z]

Specify library name and version: paho-mqtt-c/1.3.7

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

if #3833 gets in soon, we should maybe wait and update the openssl requirement (what has still to be absolute value in cci and is therefore broken problematic)

Needed to update the src/CMakeLists.txt patch, since there is a new file involved, I wish this src/CMakeLists.txt patch would not be required.
I will therefore ask for an review from @icraggs , since he created the patch my change is based on with the previous version

[ghost]

I detected other pull requests that are modifying paho-mqtt-c/all recipe:

• add paho-mqtt-c/1.3.8 #4063

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

[a4z]

#3382 has been closed now

[icraggs]

I was going to wait and add 1.3.8 to conan rather than rush with 1.3.7 as there are a few useful changes in 1.3.8 and it shouldn't be long.

This update adds a source file change which is a fix which will be in 1.3.8. I think this is bad practice. How many other fixes are going to be thrown in as patches? Better to get the changes added into the base project rather than confuse behaviour between libraries with the same version number but different packaging. This also just increases the maintenance workload, having fixes in two different places.

Why update the OpenSSL version requirement? It's fine if there's a good reason, I'd just like to know what it is. Adding changes without knowing why makes it difficult to know what's ok to change.

I'd like to not have any patches, but the original author didn't leave any explanation as to why they were necessary, so I've treaded carefully in reducing them. Maybe I'll learn a bit more in packaging the next version.

[a4z]

The patch fixes an existing bug in 1.3.7, so it is required
And you have no maintenance since I added it ;-)
I hope you do not seriously think adding a patch that fixes a problem is bad practice, ...

The original patch author left a comment in the code,
see also https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_alpn_protos.html

It is fatal to the connection if this callback returns a value other than SSL_TLSEXT_ERR_OK
SSL_CTX_set_alpn_protos() and SSL_set_alpn_protos() return 0 on success, and non-0 on failure.

There is also this extra not in the openssl doc

WARNING: these functions reverse the return value convention.

so the patch does the right thing, going into the error branch if the return value is != 0

I hope you do no prefer to distribute a buggy version over a fixed one, and I hope you understand that there are project that need a correct behaviour for this call and that can not wait until 1.3.8 is released and packaged.

The openssl topic is a conan thing, if you use several packages that have dependencies to openssl, you do not want to end up with several openssl versions as indirect dependencies. Openssl is a pain to build, and I build 11 compiler/os combinations, 22 if I build release and debug. So having just 1 openssl version as project dependency is absolute required.
see also #1737

[icraggs]

You said yourself you didn't like patch files. Now you're cherry picking a fix and adding a patch for it because you want it in a hurry. If everyone did that there would be patches all over the place. It doesn't help the base project to throw in fixes as patches in a packaging mechanism. There are always fixes that are needed for various reasons - other people have fixes that are important to them too. Who is to say what is the most important? So yes, I think you're subverting whatever process there is.

OpenSSL versions. Who defines what the 'correct' level is to use and where?

[Croydon]

OpenSSL is security relevant, hence we always use the latest patch version (h, soon i) available from the latest version series supported by the upstream project (1.1.1). Since OpenSSL patch versions are API and ABI compatible and only fix security issues and critical bugs we try to keep them up to date for all recipes.

Regarding patches, it is desired to try to upstream patches so we can get rid of them in upcoming versions. So maybe some of you could have a look at the fix-MinGW-and-OSX-builds and fix-cmake-install patches to see what would be general improvements for upstream. After all, we have those patches around since version 1.3.0

I'm not sure if I completely understood you two regarding the new 0005_SSL_CTX_set_alpn_protos-for-1-3-7.patch. This fix is already in upstream and will be part of 1.3.8, is this correct? Since it appears to be not a build error but rather a runtime error, we don't have a strict policy on this.

Personally, I would say, since this patch is pretty small and can be dropped in the next version, it would be okay to keep it.

If you decide to keep it, it needs to get added to the conandata.yml file however, right now the patch does not get applied.

[prince-chrismc]

To add the "where" answer to the stellar remark above...

➡️ OpenSSL change log very clearly states the motivation for each new release.

Most of them are due to CVEs which are published and registered with organizations dedicated to managing there impact.

For instance the new i patch pending has this vulnerability #3833 (review) that is fixed.

Given the sensitive nature of data being passed in many of today's applications, I would argue there's a very definitive answer to "correctness" of OpenSSL versioning.

If we have not satisfied the original question:

Why update the OpenSSL version requirement? It's fine if there's a good reason, I'd just like to know what it is. Adding changes without knowing why makes it difficult to know what's ok to change.

I have contact information (as do others) on my profile and I will gladly take more time to prepare and share resources!

---

As to the other topic I would like to politely ask you to remember the code of conduct 😸

we don't have a strict policy on this.

If the CCI team sees anything needs to be done with the added details about the patch, they have the right to request change.

That being said thank you everyone for putting this together and reviewing it! 🚂 it's a delight getting to work with passionate individuals from around the world 🗺️

[a4z]

Ians behaviour is kind of hostile. Luckily I am not that sensible :-)

0005_SSL_CTX_set_alpn_protos-for-1-3-7.patch is required since it fixes a real bug that causes mqtt-c not working under some conditions. The link to the documentation has been posted, it takes only 5 minutes to verify and you do not need to be an expert in anything to check. Especially respecting the info I already gave here.

Back-porting such fixes is super normal in packaging. Look at existing package of your Linux distro. This discussion is here is so not productive, the only proper reaction would have been a thank you for doing the work and focusing on the actual changes.

Thanks a lot @Croydon for pointing out that I missed adding the patch to coandata.yaml ! That was a great catch!!

Any idea why the pipeline seems to hang?

[Croydon]

This fix is already in upstream and will be part of 1.3.8, is this correct?

You did no answer this question. Please provide a direct source for this patch. Or did you write it yourself?

Back-porting such fixes is super normal in packaging.

There is no universal truth for what is the right way to do packaging. The general discussion should however be spin out in another issue.

[a4z]

@Croydon , the source for the patch is here eclipse-paho/paho.mqtt.c#1005
all information is in the change, I explicit asked about it, and it got the 1.3.8 label.
Look at the code and the help page to openssl that I linked, and the warning there I quoted. You will see that this is absolute required if you use functionality that calls that function.

I am slightly surprised that this topic exploded like this, but maybe it is not about technical questions, at least that this is my impression.

[icraggs]

OpenSSL is security relevant, hence we always use the latest patch version (h, soon i) available from the latest version series supported by the upstream project (1.1.1). Since OpenSSL patch versions are API and ABI compatible and only fix security issues and critical bugs we try to keep them up to date for all recipes.

Thanks for the explanation @Croydon

[a4z]

because the problem this patch fixes is a show stopper when we talk to (cloud provider 1, can not tell) mqtt.
We do in fact not have any problem without the patch when talking with (cloud provider, can not tell) cloud. So this problem is new to me.
It has something to do how cloud providers want their Auth different, even if its both done via openssl (from our site)

If any of the other things is a show stopper like that one, I can not say, if it would be I would add it here.

[icraggs]

All sort of issues can be show stoppers for different consumers of the library. That's what I manage all the time in the base project. There are issues which will be included in 1.3.8 which are 'show stoppers' for the Paho C++ client next release. Which is why there is an incentive to get 1.3.8 out soon.

I mean go ahead with this if you all want to. I've just been trying to do as suggested by @Croydon (and explained above in my last comment):

Regarding patches, it is desired to try to upstream patches so we can get rid of them in upcoming versions. So maybe some of you could have a look at the fix-MinGW-and-OSX-builds and fix-cmake-install patches to see what would be general improvements for upstream. After all, we have those patches around since version 1.3.0

And so I've been reducing the content of patches and incorporating those changes into the base. But not all in one go.

[a4z]

if some user finds a show stopper within the functionality of this build, and spare others hours of debugging, I would be happy if these would be back ported to this build.

about looking at the CMake patches, this is actually why I triggered the conversation at eclipse-paho/paho.mqtt.c#1017

Maybe we can together find out what is why required, so we get the changes with according documentation
(this should become a rule here at cci anyway, no patches without comment what they do and why they are required, either in the code ore somewhere else)

Still don't know why the build stuck sice yesterday ...

[conan-center-bot]

An unexpected error happened and has been reported. Help is on its way! 🏇

[icraggs]

I don't want to hold this up any longer (if I was) - I'm working on getting 1.3.8 out, and then I'll see where we've got to.

I'll just make the observation that adding patches for issues doesn't scale and bypasses the CI tests.

[a4z]

and an other day without a review, passed, thanks to all the Apple Banana comparisons and fog grenade that had been thrown for reasons of .... who knows. (Hope the creator of this chaos is now proud of the outcome)
meanwhile we know that what I did follows exactly the policies of every linux distribution, taking a patch from upstream that does not change API/ABI and add it to fix a clear bug in the current version, but why bother with facts and tech when it is more convenient to spent time on how to create bureaucracy #3903, not that I do not find that useful, but how this works is and the discussion goes is... not the best way and shows dramatic lack of praxis relevant handling of such situations.

And meanwhile, cci ships for an other day a package with a known bug without caring about the users, wow!

[ericLemanissier]

You cannot blame conan or anyone contributing here for any bug present in a project outside of conan scope. Sure, there are bugs in ALL the libraries packaged with conan, and there will continue to be, because all computer projects have bugs. If you need bug fixes so critically that you cannot wait for few days, then you should really consider maintaining a fork of the recipes you need to patch.
I don't know anything at all about paho_mqtt, so I have no competence to review this patch, and that's most probably the case for other reviewers.

[jgsogo]

We need to reach a consensus about patches. We really appreciate all the work everyone is doing contributing recipes to these repositories and all the authors writing the libraries. Sometimes it is hard for us to keep pace, sometimes there are issues related to the infrastructure and sometimes we need to define a policy before taking an action.

I'm blocking this PR until we define that policy. Sorry for the inconvenience.

[SSE4]

side note: the PR is failed, that's one more reason why it wasn't reviewed yet.
I personally use the following filter: is:pr is:open -reviewed-by:@me status:success.
I believe most of the reviewers use similar filters to find out PRs to review among others.
I think it's already well-known, that pushing changes will invalidate existing reviews, that's why it's usually pointless to submit reviews against failed PR, as new changes will be required to fix the build.

[a4z]

nobody cared about the unexpected Unexpected Error, but created bureaucracy for no brainers like that patch issue

since I do not expect any soon decision for that doc issue, since obviously the involved persons miss basic knoledge on the subject, I count this as major failur and bug in and of the cci team

Thanks for nothing, lesson learned, in future I will schedule my priorities different

[a4z]

let's see if a simple close an re open fixes the error of the build

[a4z]

I'm blocking this PR until we define that policy. Sorry for the inconvenience.

This is not inconvenience, this is just incopetent, to prioritise deliver know buggy software,
You just made cci basically unusable for serious usage

[conan-center-bot]

All green in build 3 (d7f03c179bfbb1de090e9538c5314ec7343a4b05)! 😊

• paho-mqtt-c/1.3.0: Generated 76 packages. All logs here
• paho-mqtt-c/1.3.1: Generated 76 packages. All logs here
• paho-mqtt-c/1.3.4: Generated 88 packages. All logs here
• paho-mqtt-c/1.3.5: Generated 88 packages. All logs here
• paho-mqtt-c/1.3.6: Generated 88 packages. All logs here
• paho-mqtt-c/1.3.7: Generated 88 packages. All logs here