BUG: openmp/cmake/HandleOpenMPOptions.cmake uses CMake function append without defining it

[h-vetinari]

While building openmp 18.1.0-rc1 on windows, I get:

CMake Error at cmake/HandleOpenMPOptions.cmake:47 (append):
  Unknown CMake command "append".

This is due to https://github.com/llvm/llvm-project/blame/release/18.x/openmp/cmake/HandleOpenMPOptions.cmake using append since 15fdc76, but not defining it -- in contrast to append_if which explicitly duplicates the definition from HandleLLVMOptions.cmake:

llvm-project/openmp/cmake/HandleOpenMPOptions.cmake

Lines 1 to 10 in 400a02b

	if (NOT COMMAND append_if)
	# From HandleLLVMOptions.cmake
	function(append_if condition value)
	if (${condition})
	foreach(variable ${ARGN})
	set(${variable} "${${variable}} ${value}" PARENT_SCOPE)
	endforeach(variable)
	endif()
	endfunction()
	endif()

I don't know why this file does not (or cannot) do include(HandleLLVMOptions), but if that's not an option, then it needs to similarly vendor the definition of append.

CC @aganea

[aganea]

Thanks for the report, I will take a look today.

[aganea]

Could you also please explain what cmake version are you using and the arguments used to build LLVM?

[llvmbot]

@llvm/issue-subscribers-openmp

Author: None (h-vetinari)

While [building](https://github.com/conda-forge/openmp-feedstock/pull/113) openmp 18.1.0-rc1 on windows, I get: ``` CMake Error at cmake/HandleOpenMPOptions.cmake:47 (append): Unknown CMake command "append". ```

This is due to https://github.com/llvm/llvm-project/blame/release/18.x/openmp/cmake/HandleOpenMPOptions.cmake using append since 15fdc76, but not defining it -- in contrast to append_if which explicitly duplicates the definition from HandleLLVMOptions.cmake:

llvm-project/openmp/cmake/HandleOpenMPOptions.cmake

Lines 1 to 10 in 400a02b

	if (NOT COMMAND append_if)
	# From HandleLLVMOptions.cmake
	function(append_if condition value)
	if (${condition})
	foreach(variable ${ARGN})
	set(${variable} "${${variable}} ${value}" PARENT_SCOPE)
	endforeach(variable)
	endif()
	endfunction()
	endif()

I don't know why this file does not (or cannot) do include(HandleLLVMOptions), but if that's not an option, then it needs to similarly vendor the definition of append.

CC @aganea

[h-vetinari]

Could you also please explain what cmake version are you using and the arguments used to build LLVM?

We use the newest available CMake (3.28.2) and the main build invocation is:

cd openmp
mkdir build
cd build

cmake -G "Ninja" ^
    -DCMAKE_BUILD_TYPE="Release" ^
    -DCMAKE_PREFIX_PATH=%LIBRARY_PREFIX% ^
    -DCMAKE_INSTALL_PREFIX:PATH=%LIBRARY_PREFIX% ^
    ..

The truth is slightly more complicated (for various reasons incl. CI limitations, we build LLVM component-wise; originally that was done with the subproject tarballs, now we ensure that just ./openmp and ./cmake are in the source directory). Most importantly, we build against an already-built llvm library that lives in %LIBRARY_PREFIX%.

Edit: fixed a braino in referring to the wrong component

[h-vetinari]

Thanks for the report, I will take a look today.

Thank you for the quick response! 🙏

[jhuber6]

IIRC the reason we can't just include(LLVM) is because we somewhat support a standalone usage. I.e. you can just get openmp and build it through CMake if you have an LLVM installation somewhere on your system.

[h-vetinari]

IIRC the reason we can't just include(LLVM) is because we somewhat support a standalone usage. I.e. you can just get openmp and build it through CMake if you have an LLVM installation somewhere on your system.

I mean, the waters have muddied on this substantially in recent years, c.f. here and here. My understanding of the former is that using /cmake is allowed in /openmp, and not in conflict with the standalone builds. CC @tstellar as RFC author.

I do care about building openmp "standalone", but I don't mind if I need to provide the corresponding CMake sources alongside (this has been the status quo for several other subprojects as well).

[shiltian]

Even so, host OpenMP still supports standalone build w/o LLVM. That's why we are pretty careful about adding dependencies for libomp. libomptarget is completely a LLVM library, which is beyond the scope here.

[aganea]

@h-vetinari Can you check please if #80174 fixes the issue?

[h-vetinari]

Thanks a lot @aganea! Tested the state of this patch on top of the release/18.x branch, and it works! 🙏

[aganea]

/cherry-pick d2565bb

[llvmbot]

/pull-request #80291

[tstellar]

PR has been created, we will track the status there.