PR#26: Add test: Find LLVM package with CMake - tests/llvm

tests / llvm

#26 Add test: Find LLVM package with CMake

Merged 8 days ago by tstellar. Opened 9 days ago by kkleine.

tests/ kkleine/llvm find-package-llvm into main

Added newlines and moved dependencies

Konrad Kleine • 9 days ago

9f1c4ac

Add test: Find LLVM package with CMake

Konrad Kleine • 9 days ago

5b893df

tests/find-package-llvm/CMakeLists.txt

file added

		`@@ -0,0 +1,8 @@`
		`+ # See https://llvm.org/docs/CMake.html#embedding-llvm-in-your-project`
		`+ cmake_minimum_required(VERSION 3.20.0)`
		`+ project(SimpleProject)`
		`+`
		`+ find_package(LLVM REQUIRED CONFIG)`
		`+`
		`+ message(STATUS "Found LLVM (version = ${LLVM_PACKAGE_VERSION})")`
		`+ message(STATUS "Using LLVMConfig.cmake in: ${LLVM_DIR}")`

tests/find-package-llvm/main.fmf

file added

+25

		`@@ -0,0 +1,25 @@`
		`+ summary: Find LLVM package with CMake`
		`+ description: \|`
		`+ Find LLVM package with CMake. This is to guarantee that building the packages`
		`+ together (aka "big-merge") but only installing them partially (e.g. llvm`
		`+ without clang) doesn't break CMake because of missing binaries. Therefore we`
		`+ install llvm and clang, and find the package "LLVM" in cmake. But before we`
		`+ rename a binary, namely clang-tblgen, from the clang-devel package. We expect`
		`+ CMake to still be able to find LLVM. We know that this works in standalone`
		`+ build mode and we expect this to work in the big-merge build as well. In order`
		`+ to monitor that upstream doesn't change here and still generates separate`
		`+ CMake export files for both, llvm and clang, we keep this test around.`
		`+ contact:`
		`+ - Konrad Kleine <kkleine@redhat.com>`
		`+ framework: beakerlib`
		`+ tier: 1`
		`+ require+:`
		`+ - git`
		`+ - make`
		`+ - cmake`
		`+ - llvm-devel`
		`+ - clang`
		`+ - clang-devel`
tstellar commented 8 days ago We don't want to require clang or clang-devel here. We want to make sure the test runs with only llvm-devel installed.
		`+ adjust+:`
		`+ - enabled: false`
		`+ when: collection is defined`

tests/find-package-llvm/test.sh

file added

+25

		`@@ -0,0 +1,25 @@`
		`+ #!/bin/bash`
		`+ # vim: dict+=/usr/share/beakerlib/dictionary.vim cpt=.,w,b,u,t,i,k`
		`+ . /usr/share/beakerlib/beakerlib.sh \|\| exit 1`
		`+`
		`+ rlJournalStart`
		`+ rlPhaseStartSetup`
		`+ rlRun "set -o pipefail"`
		`+ rlRun "loc_old=$(which clang-tblgen)"`
		`+ rlRun "loc_new=$(mktemp)"`
		`+ rlRun "tmp_dir=$(mktemp -d -p /var/tmp)"`
		`+ rlRun "mv $loc_old $loc_new" 0 "Move $loc_old to $loc_new to fool CMake"`
		`+ rlRun "cp ./CMakeLists.txt $tmp_dir" 0 "Copy CMakeLists.txt to temporary directory"`
		`+ rlRun "pushd $tmp_dir" 0 "Enter temporary directory"`
		`+ rlPhaseEnd`
		`+`
		`+ rlPhaseStartTest`
		`+ rlRun "cmake ." 0 "Configure project with CMake that tries to find LLVM"`
		`+ rlPhaseEnd`
		`+`
		`+ rlPhaseStartCleanup`
		`+ rlRun "popd" 0 "Pop out of temporary directory"`
		`+ rlRun "rm -rf $tmp_dir" 0 "Remove temporary directory: $tmp_dir"`
		`+ rlRun "mv $loc_new $loc_old" 0 "Move clang binary back to original location"`
		`+ rlPhaseEnd`
		`+ rlJournalEnd`

kkleine commented 9 days ago

Find LLVM package with CMake. This is to guarantee that building the packages together (aka "big-merge") but only installing them partially (e.g. llvm
without clang) doesn't break CMake because of missing binaries. Therefore we install llvm and clang, and find the package "LLVM" in cmake. But before we
rename a binary, namely clang-tblgen, from the clang-devel package. We expect CMake to still be able to find LLVM. We know that this works in standalone
build mode and we expect this to work in the big-merge build as well. In order to monitor that upstream doesn't change here and still generates separate
CMake export files for both, llvm and clang, we keep this test around.

tstellar commented on line 15 of tests/find-package-llvm/main.fmf 9 days ago

Can we make this a tmt test?

jcheca commented on line 15 of tests/find-package-llvm/main.fmf 9 days ago

@tstellar I don't follow. This is already a tmt test. Beakerlib tests should be preferred over plain shell tests as they provide better log readability, is the standard across all QE and is used in all the components.

jcheca commented on line 10 of tests/find-package-llvm/CMakeLists.txt 9 days ago

Please add a newline :)

jcheca commented on line 27 of tests/find-package-llvm/main.fmf 9 days ago

Please add a newline :)

jcheca commented on line 23 of tests/find-package-llvm/main.fmf 9 days ago

You don't really need to require llvm-devel and clang, all the tests require it by default from https://src.fedoraproject.org/tests/llvm/blob/main/f/tests/main.fmf#_29. If you still want to put these reqs here for some reason it's fine though :)

jcheca commented on line 21 of tests/find-package-llvm/main.fmf 9 days ago

I'm not sure about this adjustment. If this test is not relevant to collections, then you should move these components outside of the adjust list defined in line 17. Then you would disable the test for collection. Something like this:

require+:
  - git
  - make
  - cmake
  - clang-devel
adjust+:
  - enabled: false
    when: collection is defined

If you do want this test to be run in collections (that should be only llvm-toolset-15.0) then you need to leave this adjustment as is but also need to add the corresponding requirements for llvm-toolset-15.0-* packages. That should look like this:

require+:
  - git
  - make
adjust+:
  - require+:
      - cmake
      - clang-devel
    when: collection is not defined

    - require+:
        - llvm-toolset-15.0-cmake
        - llvm-toolset-15.0-clang-devel
      when: collection == llvm-toolset-15.0

1 new commit added

Added newlines and moved dependencies

9 days ago

kkleine commented 9 days ago

@jcheca I've adjusted the files to include newlines and I opted for the exclusion of testing the toolsets. If anything happens to break this it will be due to the way we change how we build LLVM and those errors are specific to new versions of LLVM in particular.

jcheca commented 9 days ago

@jcheca I've adjusted the files to include newlines and I opted for the exclusion of testing the toolsets. If anything happens to break this it will be due to the way we change how we build LLVM and those errors are specific to new versions of LLVM in particular.

@kkleine thanks. Regarding collections, I agree with you. Moreover I'd say if anything breaks (in collections or in other env), this test from the integration test suite should cover it. But I still think it's a good idea to have a dedicated test here too.

jcheca commented 9 days ago

Oh, looks good to me btw :)

tstellar commented 8 days ago

@tstellar I don't follow. This is already a tmt test. Beakerlib tests should be preferred over plain shell tests as they provide better log readability, is the standard across all QE and is used in all the components.

Ok, makes sense.

tstellar commented on line 23 of tests/find-package-llvm/main.fmf 8 days ago

We don't want to require clang or clang-devel here. We want to make sure the test runs with only llvm-devel installed.

kkleine commented 8 days ago

We don't want to require clang or clang-devel here. We want to make sure the test runs with only llvm-devel installed.

I agree that we don't want clang or clang-devel here. The problem is, that tmt installs all required packages from all tests in a plan in a prepare-stage. And we do have other tests like tests/integration-test-suite that do require clang and more. Even the main.fmf file from which things tests inherited, it requires clang. Considering this fact I thought it is best to require clang explicitly to make sure it has its binaries installed and we can rename them manually.

After talking to you @tstellar I'm going to merge this now.