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

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

Open
wants to merge 26 commits into
base: main
Choose a base branch
from

Commits on Oct 9, 2024

  1. Configuration menu
    Copy the full SHA
    fcdee15 View commit details
    Browse the repository at this point in the history
  2. Added USE_CONAN_PACKAGER & USE_STANDALONE_CMAKE to MiNiFiOptions

    Verified I can also build MiNiFi using standalone CMake still
    james94 committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    74b8dc7 View commit details
    Browse the repository at this point in the history
  3. Review update: Added Get LibCURL, OpenSSL, ZLIB cmake & built MiNiFi

    Addressed martinzink's feedback adding GetLibCURL, GetOpenSSL and
    GetZLIB cmake files that use prebuilt conan package when
    MINIFI_USE_CONAN_PACKAGER is set, otherwise it uses bundled cmake
    files from standalone cmake approach. After updating the files,
    I verified I could build MiNIFi using conan build and then verified
    I could build MiNiFi using standalone CMake approach. Furthermore,
    I also verified I could run minifi binary
    james94 committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    fe770df View commit details
    Browse the repository at this point in the history
  4. Removed USE_STANDALONE_CMAKE conan cmake tc variable, only need USE_C…

    …ONAN_PACKAGER
    
    tc variables can be used to change minifi options
    james94 committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    22d216c View commit details
    Browse the repository at this point in the history
  5. Updated conanfile.py tc.variables to MINIFI_USE_CONAN_PACKAGER

    After addressing some feedback and working with Marton on some build issues he was facing
    with conan on his side, I realized that conan's OpenSSL, ZLib and LibCURL were being ignored
    because I didn't update conanfile.py file's USE_CONAN_PACKAGER to be MINIFI_USE_CONAN_PACKAGER
    to match with MiNiFiOptions.cmake one. Once I made that I update then when building MiNiFi C++,
    it shows in the build output for OpenSSL, 'Using Conan Packager to manage installing prebuilt OpenSSL external lib'
    and similar messages for zlib and libcurl. Thus, OpenSSL, ZLib and LibCURL arent build from source
    
    since we use the conan prebuilt ones.
    
    Also I verified that I can build MiNiFi C++ using default conan v2 profile using the similar steps
    Marton tried. The main reason I suggested we use a particular conan profile in MiNiFi C++ is so each
    environment where MiNiFi C++ is built has a consistent conan profile since each environment default generated
    conan profile could be unique to that environment, which could cause issues in downloading prebuilt conan
    packages.
    james94 committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    950e9b9 View commit details
    Browse the repository at this point in the history
  6. Replaced MINIFI_USE_CONAN_PACKAGER w MINIFI_{ZLIB,OPENSSL,LIBCURL}_SO…

    …URCE vars
    
    Was able to build MiNiFi C++ with the ENABLE_{External_Lib} variables set to OFF initially.
    However, after a suggestion from Marton to leave the ENABLE external lib variables set to
    their default values from MiNiFiOptions.cmake, I removed the tc.variables['ENABLE_{...}']
    that overrode MiNiFiOptions.cmake ones and now am trying to build MiNiFi C++ using conan
    where we use conan's OpenSSL, ZLib and LibCURL prebuilt conan packages alternative to building
    them from source. I am debugging issues that come up from building MiNiFi using conan where
    partly we use conan's prebuilt conan packages and then the remaining external libs are the ones
    built from source. For instance, one finding related to needing to use the predefined conan profile
    located in etc/conan/profiles/release-linux because of the gnu20 was required to build rocksdb
    successfully. I found that for my environment, conan's generated default profile used gnu17,
    which caused rocksdb to fail building, but setting conan to use gnu20 allowed it to succeed in
    building rocksdb.
    james94 committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    ae61051 View commit details
    Browse the repository at this point in the history
  7. Able to build minifi w most default ENABLED options except 2 of them

    After I added new MiNiFi options for MINIFI_LIBCURL_SOURCE, MINIFI_OPENSSL_SOURCE
    and MINIFI_ZLIB_SOURCE all set to CONAN, I tried building MiNiFi with all the default
    ENABLED options, but there were two I needed to switch to OFF. I had to switch
    ENABLE_LIBARCHIVE and ENABLE_AWS to OFF because they failed to build.
    
    I do want to note that on my much larger PR where I built majority of MiNiFi
    using conan to install most of the external libs, I was able to build MiNiFi
    with openssl, libcurl, zlib and libarchive with no issues. I was also able to keep
    AWS enabled.
    
    I can do a follow PR where I bring back ENABLE_LIBARCHIVE and ENABLE_AWS to where
    we can set them to ON and be able to successfully build MiNiFi using conan
    james94 committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    bc9b0c2 View commit details
    Browse the repository at this point in the history
  8. Rebased on main, verified can build MiNiFi w conan & cmake

    Followed Marton's suggested steps to remove my merge commit, then do an interactive rebase to only include
    my commits and remove Gabor's commits to keep a clean PR-1793 for my MINIFICPP-2346-P1 branch. For building
    MiNiFi using conan with conan's cmake wrapper, I disabled building TESTS and disabled expression language
    since they were causing compile issues. I believe I resolved them in my follow up PR-1813, which will only
    be ready for further review once my PR-1793 is ready for merge and merged. After disabling these two components
    of the build, I was able to build MiNiFi successfully. I verified I could still build MiNiFi using standalone cmake
    and also verified I could still build it with TESTS enabled and expression language enabled and the other necessary
    features that needed to be enabled to build MiNIFi with TESTS enabled. For the MiNiFi standalone cmake build,
    I disabled features that werent needed like extra extensions for building faster.
    james94 committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    c69d90c View commit details
    Browse the repository at this point in the history
  9. Update cmake/MiNiFiOptions.cmake's add_minifi_multi_option w Comments

    Update cmake/MiNiFiOptions.cmake for add_multi_option with Comments portraying the Meaning of Each Option.
    
    Co-authored-by: Márton Szász <[email protected]>
    james94 and szaszm committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    d34d7fd View commit details
    Browse the repository at this point in the history
  10. Configuration menu
    Copy the full SHA
    7e50ac4 View commit details
    Browse the repository at this point in the history
  11. Updated conan version support to be at least 2.0

    Thus, other people using Nix to install conan can now install conan 2.0.17 or
    alternative supported version. I was able to build MiNiFi c++ using conan 2.0.17.
    In process of testing creating the conan package too
    james94 committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    f76f76e View commit details
    Browse the repository at this point in the history
  12. Verified Can Create MiNiFi Conan Package after removing MINIFI_BUILD_…

    …CONAN_PACKAGE
    
    One of the concerns was that we wanted to make sure that README.md, LICENSE
    , NOTICE and even the binary ones were included in the MiNiFi C++ conan package.
    
    I removed the MINIFI_BUILD_CONAN_PACKAGE condition, so CPACK_PACKAGE_DESCRIPTION_FILE
    and CPACK_RESOURCE_FILE_LICENSE are not ignored when creating the MiNiFi conan package.
    This is for README.md and LICENSE.txt and NOTICE files including the binary ones.
    
    As an alternative to using CPACK with CMake to make sure these files are included
    in the MiNiFi conan package, through conanfile.py we assign our shared_sources variable
    to export_sources and in our py file's package() method, we run cmake.install() to
    ensure that the source files and binary files assets are copied into the MiNiFi conan package.
    
    Also for the MiNiFi conan package, we copy over the .h, .i, .a and .so files into their
    appropriate include/ folder and lib/ folder.
    james94 committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    4119768 View commit details
    Browse the repository at this point in the history
  13. Refactored conan cmake include packages to find_package()

    Verified after switching cmake include(...) conan package libs
    to find_package(...) of that conan package lib name that I could
    run conan build and conan create successfully
    james94 committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    5de1793 View commit details
    Browse the repository at this point in the history
  14. Removed OPENWSMAN check; Forgot to Remove During Rebase

    Verified can still create MiNiFi C++ conan package successfully
    james94 committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    d331f3e View commit details
    Browse the repository at this point in the history
  15. Removed OPENWSMAN tc.variable; Forgot to Remove During Rebase

    Verified can still create MiNiFi C++ conan package successfully. This OPENWSMAN tc.variable was related to the CMake
    check that is now removed.
    james94 committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    831745c View commit details
    Browse the repository at this point in the history
  16. Configuration menu
    Copy the full SHA
    4d64fc8 View commit details
    Browse the repository at this point in the history
  17. Fixed Standalone CMake CTEST Issues, So All 275 TESTs PASS for Ubuntu…

    …22.04
    
    Verified that I can build MiNiFi C++ in standalone CMake mode and that all 275 CTESTs PASS.
    Verified I can run conan to build MiNIFi C++ with some features DISABLED, so its easier to
    create the MiNiFi C++ conan package. In the case of building MiNiFi C++ with conan, when
    we run CTEST, 219 TESTs run and 217 TESTs PASS. We dont include in conanfile.py 'cmake.ctest()',
    so that we can manually run ctest after conan finishes building MiNiFi C++.
    
    Also removed the test_package/ folder and C++ MiNiFi C++ test for AbstractProcessorTest since I
    plan to later do a follow up PR that runs a new TEST that will be closer to how I plan
    to use MiNiFi C++ in other C++ projects. For example, in one of the follow up PRs, I plan
    to programmatically create an edge data pipeline that processes medical images as an example.
    
    I think these updates will enable the CI/CD to pass, but also need to double check
    james94 committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    27bf34c View commit details
    Browse the repository at this point in the history
  18. Added Conan Build MiNiFi & Create MiNiFi Conan Package Section to README

    The Build MiNiFi & Create MiNiFi Conan Package section in the README points to a CONAN.md
    file that goes step by step on using the conan commands to compile MiNiFi and create
    a MiNiFi conan package.
    james94 committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    cc72571 View commit details
    Browse the repository at this point in the history
  19. Fixed the LoggerTests fmt formatting issue

    For conan, we install fmt 10.2.1 prebuilt conan package to not conflict with spdlog 1.14.0 prebuilt conan package.
    However, when we switched to a newer version of fmt, that caused the LoggerTest to fail, which didn't happen
    when we were using fmt 10.1.0 that was version installed when we used the standalone CMake approach. So, we updated
    our TEST_CASE for 'fmt formatting works with the logger' to explicity create the chrono minutes duration,
    followed by fmt format and then we pass our formatted minutes duration to our logger->log_critical, so we get
    the expected fmt formatting to be '1m'. We didn't have to do this explicitly when we used fmt 10.1.0 since even
    if we passed our logger->log_critical '1min', it would auto format to '1m'. For fmt 10.2.1 to have backward compatibility
    with fmt 10.1.0 and the result we expected from this TEST_CASE in LoggerTests, we made this update.
    
    I checked that when I build MiNiFi using conan and then build MiNiFi using standalone CMake, their LoggerTest PASSES.
    james94 committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    2c9f5e9 View commit details
    Browse the repository at this point in the history
  20. Added zlib 1.2.11 conan package to resolve missing headers issue

    Now when we build MiNiFi using conan and run ctests, we shouldn't see that zlib missing headers issue
    james94 committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    467c43b View commit details
    Browse the repository at this point in the history
  21. Updated conan to install zstd/1.5.2 & zlib/1.2.11 to successfully bui…

    …ld BundledRocksDB
    
     On a fresh dev Ubuntu 22.04 docker environment that mainly comes with the apt packages needed
    to build MiNiFi, I realized that there were problems with compilation coming from BundledRocksDB,
    
    so once I updated the BundledRocksDB to temporarily account for conan packages zstd & zlib,
    
      BundledRocksDB built rocksdb system library. When building MiNiFi with conan, I later plan
    to switch from installing system library rocksdb to installing the conan package rocksdb with
    
     the needed patches. We were able to successfully install rocksdb sysem library when building
    
      MiNiFi with conan build and MiNiFi then built successfully using conan build.
    
        However, two ctests failed '34 - ProvenanceTests (Subprocess aborted)' and
    '219 - ControllerTests (Failed)'.
    
      I didn't see the compilation failure for MiNiFi conan build related to
    
    'librdkafka/kafka-external-prefix/src/kafka-external/src/rdcrc32.h:57:10' for
    
    'fatal error: zlib.h: No such file or directory'.
    james94 committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    90cb219 View commit details
    Browse the repository at this point in the history
  22. 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, then I run CTESTs, 218 out of 219 pass too.
    james94 committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    ee1957c View commit details
    Browse the repository at this point in the history
  23. Updated conan profile to ISO CPP & Get<lib>.cmake conan msg to be con…

    …cise
    
    Addressed Marton's feedback for conan linux profile to use ISO C++ & Then updated the Get<lib>.cmake
    files messages related to conan to be more concise.
    
    Rebuilt MiNiFi with conan and then standalone cmake. Also created MiNiFi conan package. In these 3
    cases, I also checked the CTESTs to see that 218 out of 219 TEST CASES passed.
    '219 - ControllerTests (Failed)' and thats expected for now since LIBARCHIVE is DISABLED.
    james94 committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    13cd0e7 View commit details
    Browse the repository at this point in the history
  24. Addressed Martons Feedback

    Reverted rocksdb patches: cstdint.patch and doptions_equality_operator.patch by copying over
    the ones from MiNiFi main branch.
    
    Kept the brief update I made to arm7.patch since when I tried the original version
    of arm7.patch from main branch, conan complained when trying to apply the patch.
    
    Added the warning message in CONAN.md about conan integration with MiNiFi being experimental.
    
    Updated the messages for GetLibXml2.cmake and GetSpdlog.cmake.
    
    Verified I can build MiNiFi with standalone CMake, conan build and conan create
    james94 committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    97a80f3 View commit details
    Browse the repository at this point in the history
  25. Reverted RocksDB arm7.patch based on main branch one & created conan …

    …package
    
    After reverting RocksDB arm7.patch back to the one based on MiNiFi main branch, verified
    I could still create custom RocksDB conan package using Fuzzy patching. Fuzzy patching
    allows us to proceed creating a conan package like RocksDB where even if there is a minor
    difference between patch code and actual source code, we can still apply the patch. This
    fuzzy patching was key in us being able to successfully apply arm7.patch when creating
    a custom RocksDB conan package. Previously we used apply_conandata_patches() when creating
    the custom RocksDB conan package and it was too strict, so we'll stick with the fuzzy
    patching approach.
    
    Then I verified I could build MiNiFi using the conan build approach and then the standalone
    CMake approach. Afterward, I verified I could create the MiNiFi conan package.
    
    Finally, I double checked that 218 out of 219 CTEST cases passed for building MiNiFi
    using conan build and then standalone CMake approaches. 218 out of 219 CTEST cases passed
    after creating the MiNiFi conan package.
    james94 committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    b620b53 View commit details
    Browse the repository at this point in the history
  26. Updated Conan Package to Just 'Conan' for Get<lib>.cmake messages

    Verified I can build MiNiFi with conan build and standalone cmake.
    Verified I can create MiNIFi conan package with conan create.
    james94 committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    b451224 View commit details
    Browse the repository at this point in the history