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

Conan migration #516

Draft
wants to merge 112 commits into
base: development
Choose a base branch
from

Conversation

jusito
Copy link
Contributor

@jusito jusito commented Aug 24, 2022

Phasar meets conan

Our main idea was to reuse our just-simple.cmake, which is like phasar_macros.cmake but more generic, and apply it with all its features to phasar. It took us around a year to mature it, always in mind with good cmake practice. This simplifies the cmake maintenance a lot.

  • just-simple.cmake
    • The main idea is to keep it generic, because you can drop it in any c++ project so e.g. we can use it on internal projects and phasar gets the updates/features/fixes without any additional modification.
    • Clear folder structure enforced include, src, resource, test
    • target names = "project-folder" e.g. ./llvm/ = phasar-llvm and if test folder is present with tests phasar-llvm-test
    • all tests are added to ctest, including label = target under test
    • files in resource folder are automatically present for test invocation in working directory with same folder structure.
    • coverage can be added and dependencies are excluded, just run ninja ccov-all for debug preset and check *build*/ccov/all-merged/index.html
    • doxygen with incremental build added, also migrated your configuration *build*/html/index.html
    • configurable ninja job pool calculation for linking / compiling according to your current available ram / cpu count. Very helpful in shared ci environments.
  • Every file is moved with git mv so file history is kept and I double checked for *.h, *.cpp, *.hpp files
  • llvm_test_code generation can use clang / clang++ / opt from conan, because it has a dependency on it. Disabled for phasar in tree build, but so we can as developer upgrade our compiler without issues in ir generation.
  • fixed some testcases, e.g. openssl tests only worked for a prerelease
  • additional tooling added: ./utils/run_debugger.sh OpenSSL uses ctest to resolves binary according to test filter, executes binary with lldb/gdb in correct wdir and generates gtest filter, prints and executes it E.g. lldb -- phasar-llvm-test --gtest_filter=*IDETSAnalysisOpenSSL*
  • Important Compromise:
    • target names changed, to project-folder[-test|-tool]
    • This pr also means that phasar can't be build without conan. This is because just-simple.cmake is using the generator cmake instead of cmake_find_package, with cmake_find_package conan would be an option but not required. Still there are differences between the recipes in conan and the usage of system installation, so there would have to be a special handling for conan/non conan Example. In my opinion the very low maintenance costs on developer side is worth this trade. And yes this can still be mixed with find_package usage.
    • PhasarLLVM/targets are merged to one phasar-llvm because there were a lot of circular dependencies (A uses include from B but B depends on A). This can be reverted but I didn't want to touch a lot of code in this pr just cmake and fix paths. Suggesting a follow up after this.

(only for internal) Prepare conan to use prebuild llvm packages

  1. If you have access to our internal gitlab you can reuse my compiled llvm, else you need to compile it 4 times if you want to test every config.
  2. pip3 install conan
  3. conan config init
  4. conan profile update settings.compiler=clang default
  5. conan profile update settings.compiler.version=14 default
  6. conan profile update settings.compiler.libcxx=libstdc++11 default
  7. conan config set general.request_timeout=300 our gitlab can be very slow for huge packages
  8. Create api read token
  9. conan remote add just-simple-cmake https://gitlab.cc-asp.fraunhofer.de/api/v4/projects/34353/packages/conan
  10. conan user -r "just-simple-cmake" -p "gitlab_api_read_token" "randomUser"
  11. save line 10 in a conan-relog script in PATH, our gitlab needs sometimes a relog and this makes it way more handy.

how to test normal build?

  1. install conan recipes
    • The current recipes for llvm / wali used are not merged to conan center, therefore we need in install them in our local cache.
    • In the future this shouldn't be necessary, either github supports conan directly or our group could set up a conan artifactory to also share prebuild packages.
    • If you added the repository above, go to step 2
    • If you didn't add the repository ./utils/conan/install_recipes_to_local.sh
  2. cmake --preset debug
    • will build all dependencies according to used conanfile and recipe defaults (full llvm also!)
  3. cmake --build --preset debug
    • will build phasar in release / static build
  4. ctest --preset debug
    • will run all tests and also set jobs/--output-on-failure
    • variants:
      • ctest --preset debug -R IDETSAnalysisOpenSSL
      • ctest --preset debug -L phasar-llvm

Available preset for all stages are:

  • debug
  • debug-shared
  • release
  • release-shared

Phasar in tree build?

  • Created a symlink llvm/tools/phasar
  • Tested as static build worked for me
  • If you want to add changes affecting in tree build there are two places:
    • conanfile.in_llvm_tree.txt
      • Checked if the dependencies have an effect on llvm build, they dont as long as phasar is a subproject.
    • cmake/options.cmake contains a branch for in tree, currently only disables unittests and changes the default conanfile

what is missing?

  • option PHASAR_DEBUG_LIBDEPS not migrated, previously it linked everything privat to resolve missing deps? Shouldn't it be enough to have a executable test per target to get the same information?
  • Install for phasar in tree and normal usage
    • Therefore examples arent tested, requirement is: example should work with find_package without conan.
    • cmake install for more recent versions 3.21 makes runtime requirements easier (at least it states)
  • IDETSAnalysisOpenSSLSecureHeapTest.Memory6 / Memory7 Tests are failing, no idea how to fix them, Disabled them for now => follow up
  • IDEInstInteractionAnalysisTest.* failing for gtest invocation (multiple tests in one process) but are fine in ctest invocation (one test = one process) => follow up
  • Provide a conan recipe for phasar itself, so others can consume easier
  • WPDS is currently disabled due to wali dependency
    • created already a package for wali which worked but someone mentioned you want to get rid of it?
    • For phasar in tree wali recipe would need an adjustment, because it needs llvm as dependency.
    • What did I exclude? Search for #removed wali
  • cleanup files in old/ (my worklist of files which could be affected)

@jusito
Copy link
Contributor Author

jusito commented Mar 8, 2023

As update, conan 2.0 is out so I can continue on this. In essence the cmake integration changed in way I have to rethink a bit. Already looked into different solutions in january and it should be fine.
Also had a look into install part where the config.cmake.in needs some time, but after that should be fine. Iam planning to invest some time in april.

jusito added 18 commits July 10, 2023 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement New feature or request extensive Bigger piece of work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants