MINIFICPP-2346-P1: Integrated Conan2 for OpenSSL, CURL, ZLIB & Build MiNiFi

[james94]

Result Update (Oct 3, 2024): After addressing @szaszm , @lordgamez and @martinzink feedback. I verified that when I build MiNiFi with conan build, 218 CTESTs pass out of 219. Similarly, when I build MiNiFi with standalone CMake, then I run CTESTs, 218 out of 219 pass too. Similarly, when I create a MiNiFi conan package and then run CTESTs, 218 out of 219 pass. For now, ControllerTests fail since libarchive is disabled.

• Also I was able to revert the RocksDB patches (arm7.patch, dboptions_equality_operator.patch, cstdint.patch) back to how they were in MiNiFi main branch. With conan fuzzy patching, I could then create a custom RocksDB conan package even with patch code vs actual source code having some slight differences. RocksDB custom conan package is separate from standalone CMake's BundledRocksDB. So, when a user wants to build MiNiFi using conan build, MiNiFi will use the custom RocksDB conan package. When a user wants to build MiNiFi using standalone CMake, MiNiFi will use BundledRocksDB. Theres a GetRocksDB.cmake to help with choosing conan or standalone CMake RocksDB.

Result Update (Oct 2, 2024): Created RocksDB Conan Recipe to take patches like BundledRocksDB. Also when we create the RocksDB conan package, it not only builds with the patches like BundledRocksDB, it also incorporates ZLib and ZStd conan packages too. Therefore, now that we use a conan RocksDB for building MiNiFi with conan, when we build MiNiFi with standalone CMake, we use the original version of BundledRocksDB that was in 'main' branch before I modified it. I verified that when I build MiNiFi with conan, 218 CTESTs pass out of 219. Similarly, when I build MiNiFi with standalone CMake, then I run CTESTs, 218 out of 219 pass too.

Result Update (Aug 15, 2024): For the conan build and conan create approaches for MiNiFi C++, 217/219 TESTs PASS. For standalone CMake approach, 275/275 TESTs PASS. For the conan approach, we see 219 TESTs were run since I DISABLED some of the MiNiFi features (Libarchive, AWS, SQL, GCP, Lua). Also a heads up, I already have most of these MiNiFi settings ENABLED using the conan approach in my original MiNiFi PR-1775, but I wanted to keep this PR-1793 small, so left them DISABLED, so its easier to create the MiNiFi conan package. For the standalone CMake approach, we see all 275 TESTs were run since I went with the default settings when building MiNiFi.

Verified I can Install & Build MiNiFi w OpenSSL, CURL, ZLIB using conan, then I set the MINIFI_HOME environment variable & ran minifi binary executable.

Will add the steps to reproduce (later we can integrate conan into the python bootstrap, the goal was to introduce you guys to the conan build approach with minimal changes): @lordgamez @szaszm

# make sure to install conan2 for your environment
sudo pip install --force-reinstall -v "conan==2.0.17"

# create a "default" conan profile, so conan has it on record, before using your own custom profile. Gets created in ~/.conan2/
conan profile detect

# conanfile.py is in root dir of MiNiFi C++ project
cd $HOME/nifi-minifi-cpp

# create MINIFI_HOME env variable for binary executable minifi
export MINIFI_HOME=$(pwd)

# In case you need to delete all conan packages
# conan remove "*" -c

# Create custom RocksDB conan package with MiNiFi's patches applied
pushd thirdparty/rocksdb/all

conan create . --user=minifi --channel=develop --version=8.10.2 -pr=../../../etc/conan/profiles/release-linux

popd # Exit thirdparty/rocksdb/all

# install conan packages for MiNiFi C++ using conanfile.py invoking Conan
# since we created default profile earlier, we can override it with our own minifi profile
# make sure path is correct; Our conanfile.py also includes our custom RocksDB conan package
conan install . --build=missing --output-folder=build_conan -pr=$HOME/nifi-minifi-cpp/etc/conan/profiles/release-linux

# build MiNiFi C++ using conanfile.py invoking Conan & CMake
conan build . --output-folder=build_conan -pr=$HOME/nifi-minifi-cpp/etc/conan/profiles/release-linux

# verify we can run minifi binary executable
./build_conan/bin/minifi

Similar to make package that we would run using standalone CMake, with conan, we can create a MiNiFi C++ conan package, which allows us to more easily integrate MiNiFi C++ library and its assets into other C++ projects. Here is the command to create the MiNiFi C++ conan package:

cd $HOME/nifi-minifi-cpp

conan create . --user=minifi --channel=develop -pr=$HOME/nifi-minifi-cpp/etc/conan/profiles/release-linux

• NOTE: since I added conan create into this PR, my other PR can be closed: MINIFICPP-2346-P2: Added Support to Create MiNiFi Conan2 Package, Easier Integration into C++ RT Infra #1813

Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

For all changes:

• Is there a JIRA ticket associated with this PR? Is it referenced
  in the commit message?

• Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.

• Has your PR been rebased against the latest commit within the target branch (typically main)?

• Is your initial contribution a single, squashed commit?

For code changes:

• If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
• If applicable, have you updated the LICENSE file?
• If applicable, have you updated the NOTICE file?

For documentation related changes:

• Have you ensured that format looks appropriate for the output in which it is rendered?

Note:

Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.

[szaszm]

The macOS CI failure will be fixed in #1790, and the Docker integration test failure in #1795. The clang job fails because of a linter issue in the conanfile.

[james94]

@szaszm If I get access to a macOS, I can looking into fixing the linter issue in the conanfile. Currently, I am testing MiNiFi from Ubuntu 22.04. I can later test it in Windows too if needed. For now, I addressed @martinzink suggested updates for integrating conan into MiNiFi build infrastructure.

[szaszm]

@james94 You don't need a macOS to fix the python linter issue, it's not OS-specific. Just run this from a build directory:

cmake --build . --target flake8

And make sure flake8 is installed.

This is the output:

[1/2] cd /home/szaszm/nifi-minifi-cpp-3/build-clang-ninja && /home/szaszm/nifi-minifi-cpp-3/run_flake8.sh /home/szaszm/nifi-minifi-cpp-3
FAILED: CMakeFiles/flake8 /home/szaszm/nifi-minifi-cpp-3/build-clang-ninja/CMakeFiles/flake8 
cd /home/szaszm/nifi-minifi-cpp-3/build-clang-ninja && /home/szaszm/nifi-minifi-cpp-3/run_flake8.sh /home/szaszm/nifi-minifi-cpp-3
/home/szaszm/nifi-minifi-cpp-3/conanfile.py:2:1: F401 'conan.tools.env.VirtualRunEnv' imported but unused
/home/szaszm/nifi-minifi-cpp-3/conanfile.py:9:1: E302 expected 2 blank lines, found 1
/home/szaszm/nifi-minifi-cpp-3/conanfile.py:18:53: E231 missing whitespace after ','
ninja: build stopped: subcommand failed.

[szaszm]

I'm having trouble with conan, due to a missing profile. It seems like installation alone is not enough.

[james94]

@szaszm yeah I was hoping that the conan installation would have automatically created a default conan profile. First lets create a default profile for conan first (then later we'll override it with our minifi conan profile):

# create a "default" conan profile, so conan has it on record, before using your own custom profile. Gets created in ~/.conan2/
conan profile detect

Once you have the default conan profile, then you can try overriding it again with our minifi conan profile:

# install conan packages for MiNiFi C++ using conanfile.py invoking Conan
# since we created default profile earlier, we can override it with our own minifi profile
# make sure path is correct
conan install . --build=missing --output-folder=build_conan -pr=$HOME/nifi-minifi-cpp/etc/build/conan/profiles/release-linux

NOTE: The nice thing about docker environment is we would have already created the default conan profile at the beginning and then later used our specific minifi conan profile.

Does the extra conan profile detect command help?

[szaszm]

@james94 what docker environment do you mean? I was trying on arch linux, no containers.

[james94]

@szaszm I meant we could leverage a docker environment like the one that is mentioned in the README and extend it with conan2 support: https://github.com/apache/nifi-minifi-cpp?tab=readme-ov-file#building-a-docker-image

I usually just launch my own docker container with needed build dependencies for MiNiFi C++, so I can build it within my docker container.

You could also try it directly on linux too.

[james94]

@szaszm @martinzink @lordgamez just checking in, anything I can do to help speed up testing toward this PR?

[martinzink]

Sorry for the late reply, I was on holiday.
It would be nice if the minifi options were configurable through the conan options so we could set the extensions without editing conanfile.py, but overall I like the direction (and that could be implemented in followup PRs) 👍

[james94]

Yes I agree with you @martinzink
I do plan to add being able to configure minifi options mapping to conan options through the bootstrap py script, but that will be in a later PR, so we keep this initial PR light as @szaszm suggested.

[szaszm]

I ran the following commands:

conan profile detect
export MINIFI_HOME=$(pwd)  # I was in the source tree root
conan install . --build=missing --output-folder=build_conan
conan build . --output-folder=build_conan

conan install ran suspiciously quickly, I don't think it had to build anything from source, but it did install zlib/1.3.1, openssl/3.2.1 and libcurl/8.6.0.

Unfortunately OpenSSL and Zlib were still built from source using ExternalProject during the CMake build. I think when calling CMake through Conan, they should be using the Conan packages.

[james94]

@szaszm @martinzink with respect to commit "49de56c": I verified I can build MiNiFi C++ using conan with the specific conan profile etc/conan/profiles/release-linux and with the sources for MINIFI_LIBCURL_SOURCE, MINIFI_OPENSSL_SOURCE
and MINIFI_ZLIB_SOURCE all set to CONAN successfully. There were 2 ENABLED options that I needed to set to OFF, which were ENABLE_LIBARCHIVE and ENABLE_AWS since the build would fail, but later I can get those working again since on my bigger PR-1775 "#1775", I was able to keep those options ENABLED when building MiNiFi C++ with conan. I used the following command to build MiNiFi C++ with conan:

conan build . --build=missing --output-folder=build_conan -pr=etc/conan/profiles/release-linux

I did double check I can still build MiNiFi C++ using standalone CMake approach with all the default ENABLED options set to their original values as specified in MiNiFiOptions.cmake too. I used these commands to build MiNiFi C++ with cmake:

mkdir build_cmake
cd build_cmake

cmake ..
make -j $(nproc)

Please try the conan build MiNiFi C++ approach again when you get a chance

[lordgamez]

@james94 I checked and it seems to build successfully for me. Could you please rebase the PR to the latest main brach to be able to run the CI on it?

[james94]

@lordgamez thanks for verifying you can build MiNiFi C++ with conan on your side. I appreciate it. I will rebase on latest MiNiFi main branch.

[james94]

@lordgamez @szaszm I merged latest upstream main branch into my forked main branch and then merged my forked main branch into my MINIFICPP-2346-P1 branch.

• Also I chose not to rebase and instead went for the merge approach since rebasing in the passed caused me to lose my updates on previous branches.

After merging upstream main, I verified I can build MiNiFi using conan and then standalone CMake.

[lordgamez]

Hi @james94, thanks for the update! Unfortunately the lua.org page where we retrieved the lua sources went down, that's the reason the CI jobs are failing. We merged a commit to move from lua.org to the github page to retrieve the source files with the following commit: b0b2cf6 Could you rebase it again to be able to run the CI correctly?

[szaszm]

@james94 I could rebase your branch easily without data loss. The existing merge complicates things a bit, but not too much. Here's what I did:

git checkout main
git pull upstream main  # update local main branch from apache/nifi-minifi-cpp
git checkout MINIFICPP-2346-P1
git reset --hard HEAD~1  # remove merge commit from the top
git rebase -i main  # interactive rebase
# then edited the file, removing Gabor's commits, which were the first 6 IIRC, like here on the "commits" tab. "Making OpenSSL and curl mandatory", + 5x "review update" commits. At the end, exited the editor and git did the rebase.
# at the end, there is a successful rebase to the latest main, with no conflicts, and only your commits.

Thanks for sharing the conan profile at etc/conan/profiles/release-linux. I have concerns about the hardcoded compiler version and "system package manager" settings, but I don't know if this causes any real issues.

Tried compiling on a half-upgraded Ubuntu 22.04 to 24.04, the system packages should already be the 24.04 versions. I'll try again later with another system. I encountered these issues:

CMake Error at <line in CMakeLists.txt> (target_link_libraries):                                                                                                                                                                                                      
  Target "<target>" links to:                                                                                                                                                                                                                                     
                                                                                                                                                                                                                                                                 
    zstd::zstd                                                                                                                                                                                                                                                   
                                                                                                                                                                                                                                                                 
  but the target was not found.  Possible reasons include:                                                                                                                                                                                                       
                                                                                                                                                                                                                                                                 
    * There is a typo in the target name.                                                                                                                                                                                                                        
    * A find_package call is missing for an IMPORTED target.                                                                                                                                                                                                     
    * An ALIAS target is missing.                                                                                                                                                                                                                                

Affected targets are rocksdb, db_bench, db_sanity_test, write_stress, db_repl_stress, rocksdb_dump, rocksdb_undump, db_stress. It looks like all of them are RocksDB-related.

Also, I would appreciate if you could add the necessary changes to keep the AWS and libarchive extensions enabled and working, so that we can eventually merge this PR standalone without breaking important functionality.

[james94]

Thanks @szaszm for the steps to run a git rebase without causing data loss. I will give it a try. I think the reason we are seeing the zstd::zstd target not found issue is because we move from Ubuntu 22.04 to Ubuntu 24.04 and maybe conancenter doesn't provide the prebuilt conan package out of box. I think we just need to run these two conan commands:

conan install . --build=missing --output-folder=build_conan
conan build . --output-folder=build_conan

I will look into it today after work. I will follow your rebase steps for Ubuntu 22.04. Thanks again for sharing your steps. Later I will try Ubuntu 24.04. I believe there shouldn't be much of a difference.

[james94]

@szaszm @lordgamez for this latest git push, I followed marton's steps to do an interactive rebase and also removed the top merge commit that I had to clean up the history and leave only my commits for the git history. Now PR-1793's branch MINIFICPP-2346-P1 should be on top of the latest MiNiFi main branch. I also verified I could build MiNiFi using conan with conan's cmake wrapper and then I followed up by verifying I could still build MiNiFi using standalone CMake. Also it looked like the TESTS were on there way to passing too. When you guys get a chance, please review.

[szaszm]

thanks for your hard work and patience