james94 commented on code in PR #1793:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1793#discussion_r1737196067
##########
cmake/GetZLIB.cmake:
##########
Review Comment:
**UPDATED (Sept 2, 2024)**: @szaszm
I just double checked the **conanfile.py**, it appears zlibs is not in conan
package **shared_requires** tuple. I realized some other conan packages
depended on **zlib**, so conan automatically downloaded **zlib 1.3.1** prebuilt
conan package. I will add it explicitly, but with **zlib 1.2.11** for that
conan package version to match the version that standalone CMake would install
for MiNiFi. Here will be the updated **shared_requires** tuple
~~~bash
shared_requires = ("openssl/3.2.1", "libcurl/8.6.0", "civetweb/1.16",
"libxml2/2.12.6", "fmt/10.2.1", "spdlog/1.14.0", "catch2/3.5.4", "zlib/1.2.11",
"zstd/1.5.2")
~~~
I didn't see that missing header from zlib during compilation **librdkafka**
anymore when building MiNiFi with conan build. I also added in **zstd/1.5.2**
conan package to match standalone CMake's zstd version. I added this zstd conan
package because BundledRocksDB was complaining about not being able to find
**zstd::zstd**. So, for the conan build of MiNiFi, I added both zlib and zstd
to build MiNiFi successfully.
I will double check by manually building MiNiFi with conan and get back to
you, but for now, I updated the **conanfile.py** with **zlib** added to the
**shared_requires** tuple and its on the repo now @szaszm
##########
cmake/GetZLIB.cmake:
##########
Review Comment:
@szaszm When building MiNiFi with conan build, **217/219 ctests PASS**.
34 - ProvenanceTests (Subprocess aborted)
219 - ControllerTests (Failed)
I think **ProvenanceTests** from RocksDB fails because of
double free or corruption (!prev)
Test #34: ProvenanceTests ........ Subprocess aborted***Exception:
0.19 sec
In regards to this **ProvenanceTests**, the main difference between MiNiFi
conan build and standalone CMake build is that in conan build we use **zstd**
and **zlib** conan packages to try to link into CMake's system library
installed **RocksDB** while with standalone CMake approach, we use CMake to
**build and install zlib & zstd external system libs** while installing system
library **RocksDB**
@szaszm any thoughts on what could cause **ProvenanceTest** to have this
**double free or corruption (!prev)** error when building MiNiFi using conan?
This doesn't happen when I build MiNiFi with standalone CMake. Also in my much
larger PR where I added support to install **patched RocksDB conan package**
based on **standalone CMake's patched RocksDB**, I dont think I saw this ctest
error: https://github.com/apache/nifi-minifi-cpp/pull/1775
When building MiNiFi with standalone CMake, **all 275 ctests PASS.**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]