eclipse / paho.mqtt.cpp

Other
1.01k stars 432 forks source link

Fix Paho C build #471

Open laitingsheng opened 9 months ago

laitingsheng commented 9 months ago

This PR is mainly for fixing the CMake error when PAHO_WITH_MQTT_C is enabled.

I also changed the Git submodule URL to a relative path so that it can follow the same protocol used to clone the main repository.

Fixes #469.

fpagliughi commented 9 months ago

I was waiting to see if this PR would land in the Paho C repo before making any further changes to the build here. https://github.com/eclipse/paho.mqtt.c/pull/1428

Some things in the master/develop branches assume it will land.

laitingsheng commented 8 months ago

OK sure.

Zelif commented 8 months ago

@laitingsheng @fpagliughi This still seems to fail for the first clean cmake build with the same error in the issue.

Can repeat it in VS code by issuing CMake: Delete cache and Reconfigure

Ends up causing issues inside github actions as it is a clean build every time. Will try to figure out why the C lib is not found on the alias and install step for the first time.

laitingsheng commented 8 months ago

It works fine on Ubuntu (with or without OpenSSL). What error output did you get?

FYI, I am using this command to generate the build folder:

cmake --fresh -G Ninja -B build -S . -DPAHO_WITH_SSL:BOOL=TRUE -DPAHO_WITH_MQTT_C:BOOL=TRUE
Zelif commented 8 months ago

@laitingsheng Sorry for my derp, this seems to be a result of how I was setting the SSL variable in the cmake Having the top level cmake set this:

SET(PAHO_WITH_SSL                   ON)

Worked for the mqtt Cpp cmake but the C lib cmake seemed to keep seeing it as false, having the cache worked.

SET(PAHO_WITH_SSL                   ON CACHE BOOL "Enable SSL build")
laitingsheng commented 8 months ago

@Zelif

Maybe you can have a look at this policy: https://cmake.org/cmake/help/latest/policy/CMP0077.html

And you can probably set this policy variable prior to the PAHO_WITH_SSL: https://cmake.org/cmake/help/latest/variable/CMAKE_POLICY_DEFAULT_CMPNNNN.html

fpagliughi commented 3 months ago

@laitingsheng , thanks so much for this PR. I gave up on waiting for the upstream library release, and started hacking on the existing build, forgetting that this was already here. I think I covered most of this in my changes, but sometimes in slightly different ways, like I meant to push externals/ up to the top-level directory, and I definitely want to keep the submodule referring to GitHub, etc.

My one question is... what's the purpose of adding EXCLUDE_FROM_ALL to the add_subdirectory() call for the C lib code?

And if you notice that I missed anything, please let me know.

laitingsheng commented 3 months ago

My one question is... what's the purpose of adding EXCLUDE_FROM_ALL to the add_subdirectory() call for the C lib code?

This can avoid any unwanted targets in the subdirectory to be included in the implicit all target.

fpagliughi commented 3 months ago

Thanks, @laitingsheng ! I will read up on all of this.