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

Unable to use LLVM 16 as a library from apt.llvm.org #58075

Closed
rymiel opened this issue Sep 30, 2022 · 15 comments · Fixed by apple/llvm-project#5431
Closed

Unable to use LLVM 16 as a library from apt.llvm.org #58075

rymiel opened this issue Sep 30, 2022 · 15 comments · Fixed by apple/llvm-project#5431
Labels
cmake Build system in general and CMake in particular

Comments

@rymiel
Copy link
Member

rymiel commented Sep 30, 2022

Installing the llvm-dev package for LLVM 16 from apt.llvm.org, then attempting to use LLVM as a CMake library using find_package(LLVM 16 REQUIRED CONFIG) always fails with the following error:

CMake Error at /usr/lib/llvm-16/lib/cmake/llvm/LLVMExports.cmake:1649 (message):
  The imported target "clangdRemoteIndexProto" references the file
     "/usr/lib/llvm-16/lib/libclangdRemoteIndexProto.a"
  but this file does not exist.  Possible reasons include:
  * The file was deleted, renamed, or moved to another location.

I don't have enough cmake knowledge to be sure of the cause, but i assume this to be caused by (or related to) https://reviews.llvm.org/D131593.
Since that changed generate_protos to call add_llvm_library, and generate_protos is used by https://github.com/llvm/llvm-project/blob/894c0e94f9c62413feef88fd577c430839abaea7/clang-tools-extra/clangd/index/remote/CMakeLists.txt, those now appear in the LLVM_AVAILABLE_LIBS list in /lib/cmake/llvm/LLVMConfig.cmake.
This can be manually verified by the existence of those entries in the cmake file, which were not there in previous versions.

Note that installing clangd alongside LLVM also fails, presumably because the LLVM cmake config is looking for packages under /usr/lib/llvm-16/, but clangd is installed somewhere else.

I am using ubuntu-22.04 (jammy) in GitHub actions (of which I can provide the build file in case further reproduction is required), but I was able to note the existence of clangd entries in the LLVM config for other distributions as well.

@cachemeifyoucan
Copy link
Collaborator

@akyrtzi. This seems to be something try to be fixed here: https://reviews.llvm.org/D132033 but that got reverted because it breaks clangd build. We need to take a look at external builds since that is not tested by our CI system.

Do you have a reproducing steps for your problem, like where to get the package and how the packages are built?

@cachemeifyoucan
Copy link
Collaborator

My initial impression is that the clangdRmoteIndexProto is configured but not built/shipped.

@rymiel
Copy link
Member Author

rymiel commented Oct 11, 2022

Sounds about right. I don't know how the packages are built, but they come from https://apt.llvm.org; its setup is described there.

Here's a minimal-ish "repro": https://github.com/rymiel/llvm-58075-repro
It tries to build via github actions, with the following config: https://github.com/rymiel/llvm-58075-repro/blob/master/.github/workflows/cmake.yml
This is the failure: https://github.com/rymiel/llvm-58075-repro/actions/runs/3229398893/jobs/5286650528 (note it never reaches the build, it fails at configuring)
And this is it with a ridiculous amount of trace output: https://github.com/rymiel/llvm-58075-repro/actions/runs/3229418711/jobs/5286713031#check-step-5

@akyrtzi
Copy link
Contributor

akyrtzi commented Oct 11, 2022

@rymiel does it fix the issue if you change add_llvm_library(${LibraryName} ${GeneratedProtoSource} to add_llvm_library(${LibraryName} ${GeneratedProtoSource} BUILDTREE_ONLY inside llvm/cmake/modules/FindGRPC.cmake file?

@cachemeifyoucan
Copy link
Collaborator

I think BUILDTREE_ONLY is going to hit the same problem as the previously reverted change, unless we make everything depend on grpc library to be BUILDTREE_ONLY.

My change kind of sink libclangdRemoteIndexProto.a into llvm library, which means it shows up in LLVMExports.cmake instead of ClangTargets.cmake. I am interested to look at how the package was built and what is shipped but I can't find any information for now.

@akyrtzi
Copy link
Contributor

akyrtzi commented Oct 11, 2022

Note that this particular error seems a bit different that the error The following imported targets are referenced, but are missing: grpc++ protobuf::libprotobuf which prompted https://reviews.llvm.org/D132033.

@cachemeifyoucan
Copy link
Collaborator

I see. I found where the package is built, probably here: https://llvm-jenkins.debian.net/view/Ubuntu%20Jammy/job/llvm-toolchain-jammy-binaries/

Looks like the missing library is built (and it should be part of the install-llvm-libraries target even it is named libclangdRemoteProto.a). I don't know how .deb packages are created and what rule do they follow to determine which binary goes into which package.

@cachemeifyoucan
Copy link
Collaborator

I assume the example should use llvm-16-dev package right?

The package list is basically here: https://salsa.debian.org/pkg-llvm-team/llvm-toolchain/-/blob/11/debian/llvm-X.Y-dev.install.in

So the name libclangd* makes it being excluded from the llvm-dev package but unfortunately still in the cmake export list. @sam-mccall I didn't foresee the layering change of the protocol definition library. Do you have any opinion on the layering change?

@cachemeifyoucan
Copy link
Collaborator

Maybe we just be more flexible and do this: https://reviews.llvm.org/D135712

@sam-mccall
Copy link
Collaborator

My change kind of sink libclangdRemoteIndexProto.a into llvm library, which means it shows up in LLVMExports.cmake instead of ClangTargets.cmake

FWIW this seems like it's the wrong direction to me. This proto is part of clangd, which it not part of llvm (as opposed to llvm-project). Ideally it'd be possible to write a CMake macro without having to couple it to either "ships as part of llvm" or "ships as part of clang", but maybe it isn't. This library certainly shouldn't be built or exported as part of LLVM. If the change was doing this, quite likely I didn't understand it :-(

@sam-mccall I didn't foresee the layering change of the protocol definition library. Do you have any opinion on the layering change?

Sorry, I'm not sure exactly what you mean, can you be more specific?

Maybe we just be more flexible and do this: https://reviews.llvm.org/D135712

This dumps a bunch of implementation details into every callsite, we lose a lot of the value of the build rule :-(

If we can't write a rule that works well for both llvm and clang libraries, I'd probably prefer having FindGRPC.cmake exporting separate generate_llvm_protos and generate_clang_protos entrypoints, and having them share source generation stuff as an implementation detail.

@cachemeifyoucan
Copy link
Collaborator

I don't think the original layering is correct since referring add_clang_library from llvm is not really good behavior. We can have generate_protos (only generates GRPC source), generate_llvm_protos (also generates library) in llvm/FindGRPC, then add generate_clang_protos to a cmake module in clang. That is probably the best solution.

@sam-mccall
Copy link
Collaborator

I don't think the original layering is correct since referring add_clang_library from llvm is not really good behavior.

Ah, got it. Agree, sorry about that (the original commit was generic using add_library, that didn't work for shared lib build and we forgot to move it :-()

We can have generate_protos (only generates GRPC source), generate_llvm_protos (also generates library) in llvm/FindGRPC

Sure (nit: generate_proto_sources? since we probably don't want to encourage using it directly).

While we're worried about layering, the former should go in cmake/ rather than llvm/cmake/ too (that's relatively new but seems appropriate here). But also happy to defer that.

@EugeneZelenko EugeneZelenko added cmake Build system in general and CMake in particular and removed packaging platform:linux labels Oct 12, 2022
cachemeifyoucan added a commit to cachemeifyoucan/llvm-project that referenced this issue Oct 12, 2022
Take the library target out of `generate_protos` function so the caller
can decide where to layer the library using the source generated from
the function. Use `add_llvm_protos_library` and
`add_clang_protos_library` to decide which layer is the library
exported.

Fixes: llvm#58075

Differential Revision: https://reviews.llvm.org/D135712
cachemeifyoucan added a commit to apple/llvm-project that referenced this issue Oct 12, 2022
Take the library target out of `generate_protos` function so the caller
can decide where to layer the library using the source generated from
the function. Use `add_llvm_protos_library` and
`add_clang_protos_library` to decide which layer is the library
exported.

Fixes: llvm#58075

Differential Revision: https://reviews.llvm.org/D135712
cachemeifyoucan added a commit to cachemeifyoucan/llvm-project that referenced this issue Oct 12, 2022
Take the library target out of `generate_protos` function so the caller
can decide where to layer the library using the source generated from
the function. Use `add_llvm_protos_library` and
`add_clang_protos_library` to decide which layer is the library
exported.

Fixes: llvm#58075

Differential Revision: https://reviews.llvm.org/D135712
@rymiel
Copy link
Member Author

rymiel commented Oct 13, 2022

Thank you a lot for looking into this, unfortunately I cannot test if it's been fixed for apt.llvm.org because it appears the builds for LLVM 16 have been failing for ~week. I assume this is know about @sylvestre ?

@sylvestre
Copy link
Collaborator

yeah, some various regressions with openmp and other things lately :/
I will try to fix that in the next few days

cachemeifyoucan added a commit to apple/llvm-project that referenced this issue Oct 13, 2022
Take the library target out of `generate_protos` function so the caller
can decide where to layer the library using the source generated from
the function. Use `add_llvm_protos_library` and
`add_clang_protos_library` to decide which layer is the library
exported.

Fixes: llvm#58075

Differential Revision: https://reviews.llvm.org/D135712
@rymiel
Copy link
Member Author

rymiel commented Oct 14, 2022

Thank you everyone, this specific problem seems to be fixed! However #58317 remains for 16

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmake Build system in general and CMake in particular
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants