[docs] Fix build-docs.sh

[thieta]

Summary

If libcxxabi is not included CMake will error out:

Cannot find target libcxxabi-SHARED

I ran into this doing the 15.0.0 release

[ldionne]

This is weird, I just tried locally and I don't get the same issue. Does it reproduce for you when running this from the root of the monorepo? For me, it doesn't.

mkdir -p build
cmake -S runtimes -B build -GNinja \
            -DLLVM_ENABLE_RUNTIMES="libcxx" \
            -DLLVM_ENABLE_SPHINX=ON \
            -DSPHINX_WARNINGS_AS_ERRORS=OFF
ninja -C build docs-libcxx-html

I'm not against the change, but I'd like to understand what's going on instead of blindly hiding the issue.

[thieta]

In D133475#3777035, @ldionne wrote:

This is weird, I just tried locally and I don't get the same issue. Does it reproduce for you when running this from the root of the monorepo? For me, it doesn't.

mkdir -p build
cmake -S runtimes -B build -GNinja \
            -DLLVM_ENABLE_RUNTIMES="libcxx" \
            -DLLVM_ENABLE_SPHINX=ON \
            -DSPHINX_WARNINGS_AS_ERRORS=OFF
ninja -C build docs-libcxx-html

I'm not against the change, but I'd like to understand what's going on instead of blindly hiding the issue.

I ran the exact same commands:

CMake Error at /home/tobias/code/llvm-project/libcxx/src/CMakeLists.txt:261 (add_custom_command):
  Error evaluating generator expression:

    $<TARGET_PROPERTY:libcxx-abi-shared,IMPORTED_LIBNAME>

  Target "libcxx-abi-shared" not found.


CMake Error at /home/tobias/code/llvm-project/libcxx/src/CMakeLists.txt:261 (add_custom_command):
  Error evaluating generator expression:

    $<TARGET_PROPERTY:libcxx-abi-shared,IMPORTED_LIBNAME>

  Target "libcxx-abi-shared" not found.

Could it be linux specific issue?

[thieta]

@ldionne any ideas? would like to fix this before 15.0.1 next week so I don't have to manually build the docs.

[ldionne]

In D133475#3791649, @thieta wrote:

@ldionne any ideas? would like to fix this before 15.0.1 next week so I don't have to manually build the docs.

Okay, I looked into it and the issue is basically that libcxx-abi-shared does not exist when libcxxabi is not in LLVM_ENABLE_RUNTIMES. That's normally okay, except when we generate a linker script in place of libc++.so (aka on Linux), since that requires resolving the ABI library's target to know to add -lc++abi. I would normally expect that generator expressions inside the linker script generation are only evaluated if we actually perform the POST_BUILD step it's associated to, but that's not how CMake works. It seems to eagerly evaluate the generator expressions, leading to problems since at least one of them is invalid.

Potential fixes:

1. When specifying LIBCXX_CXX_ABI=libcxxabi (which is also the default), we could implicitly add LLVM_ENABLE_RUNTIMES=libcxxabi.
2. We could mandate that users pass LLVM_ENABLE_RUNTIMES=libcxxabi when they select LIBCXX_CXX_ABI=libcxxabi. We would add a proper diagnostic to avoid confusing users. Since LIBCXX_CXX_ABI=libcxxabi is the default, that would mean that most users would suddenly have to specify LLVM_ENABLE_RUNTIMES=libcxxabi if they don't already do it.
3. Add a narrow workaround for the problem by e.g. not creating a linker script if libcxx-abi-shared is not present. This is kind of hacky, since it would mean that e.g. running ninja install-cxx would result in a malfunctioning libc++ installation on Linux, and that would be silent.

(2) seems like a lot of breakage and user-facing complexity for little to no benefit.
(3) seems like the worst of both worlds, where we set ourselves up for silent problems down the road.

Therefore, I conclude that the best way forward would be (1). I also think it's kind of weird to do that, but all the other options really don't make sense to me.

So as far as I'm concerned, you can ship this to unblock you in the short term. However, I'll look into doing (1) and landing that for LLVM 16.

[thieta]

In D133475#3800522, @ldionne wrote:

Okay, I looked into it and the issue is basically that libcxx-abi-shared does not exist when libcxxabi is not in LLVM_ENABLE_RUNTIMES. That's normally okay, except when we generate a linker script in place of libc++.so (aka on Linux), since that requires resolving the ABI library's target to know to add -lc++abi. I would normally expect that generator expressions inside the linker script generation are only evaluated if we actually perform the POST_BUILD step it's associated to, but that's not how CMake works. It seems to eagerly evaluate the generator expressions, leading to problems since at least one of them is invalid.

I have solved similar problems in CMake before by having a separate script that's called from POST_BUILD with cmake -E script <script_path> where the script can be configured with a configure_file() in cmake to inject variables. It's not great, but it's a working portable solution.

Potential fixes:

1. When specifying LIBCXX_CXX_ABI=libcxxabi (which is also the default), we could implicitly add LLVM_ENABLE_RUNTIMES=libcxxabi.
2. We could mandate that users pass LLVM_ENABLE_RUNTIMES=libcxxabi when they select LIBCXX_CXX_ABI=libcxxabi. We would add a proper diagnostic to avoid confusing users. Since LIBCXX_CXX_ABI=libcxxabi is the default, that would mean that most users would suddenly have to specify LLVM_ENABLE_RUNTIMES=libcxxabi if they don't already do it.
3. Add a narrow workaround for the problem by e.g. not creating a linker script if libcxx-abi-shared is not present. This is kind of hacky, since it would mean that e.g. running ninja install-cxx would result in a malfunctioning libc++ installation on Linux, and that would be silent.

(2) seems like a lot of breakage and user-facing complexity for little to no benefit.
(3) seems like the worst of both worlds, where we set ourselves up for silent problems down the road.

Therefore, I conclude that the best way forward would be (1). I also think it's kind of weird to do that, but all the other options really don't make sense to me.

I think you are right - but it would have to be logged as a warning I think.

So as far as I'm concerned, you can ship this to unblock you in the short term. However, I'll look into doing (1) and landing that for LLVM 16.

Thanks, will do.

llvm/utils/release/build-docs.sh

ninja -C $builddir \
                doxygen-clang \
                doxygen-clang-tools \
                doxygen-flang \
                doxygen-llvm \
                doxygen-mlir \
                doxygen-polly
 
 cmake -G Ninja $srcdir/../runtimes -B $builddir/runtimes-doc \
-               -DLLVM_ENABLE_RUNTIMES="libcxx" \
+               -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \
                -DLLVM_ENABLE_SPHINX=ON \
                -DSPHINX_WARNINGS_AS_ERRORS=OFF
 
 ninja -C $builddir/runtimes-doc \
                docs-libcxx-html \
 
 package_doxygen llvm .
 package_doxygen clang tools/clang