This is an archive of the discontinued LLVM Phabricator instance.

Differential D79085

[clangd] Add CMake dependencies for Protobuf-generated files
ClosedPublic

Authored by kbobyrev on Apr 29 2020, 6:12 AM.

Details

Reviewers
sammccall
Commits
rG4645ef17ff1a: [clangd] Add CMake dependencies for Protobuf-generated files
Summary

Dependencies ensure that Protobufs are generated before all libraries
depending on the headers are built, not linked.

Diff Detail

Event Timeline

kbobyrev created this revision.Apr 29 2020, 6:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 29 2020, 6:12 AM
Herald added subscribers: cfe-commits, usaxena95, kadircet and 5 others. · View Herald Transcript
kbobyrev added a comment.Apr 29 2020, 6:20 AM

Otherwise sources including Index.pb.h or Index.grpc.pb.h can be compiled before the those headers are generated.

https://github.com/kirillbobyrev/indexing-tools/runs/629305684 (disregard the mess in the repo)

sammccall accepted this revision.Apr 29 2020, 6:35 AM

Haha, CMake :-(

This revision is now accepted and ready to land.Apr 29 2020, 6:35 AM
sammccall added inline comments.Apr 29 2020, 6:50 AM
clang-tools-extra/clangd/index/remote/CMakeLists.txt
26

is this equivalent to DEPENDS RemoteIndexProtos in the library?
Slightly clearer probably.

kbobyrev updated this revision to Diff 260906.Apr 29 2020, 7:05 AM
kbobyrev marked 2 inline comments as done.

Use DEPENDS instead of add_dependencies.

Harbormaster completed remote builds in B55113: Diff 260899.Apr 29 2020, 7:28 AM
kbobyrev added a comment.Apr 29 2020, 7:31 AM
In D79085#2010025, @sammccall wrote:

Haha, CMake :-(

Now that I know this it kind of makes sense (I guess it's faster to just build everything and then link when all deps are ready), just something I didn't think about :)

clang-tools-extra/clangd/index/remote/CMakeLists.txt
26

Ah, true, I had a feeling there was something like this on top of my mind, but forgot about DEPENDS :) Thanks!

Harbormaster failed remote builds in B55119: Diff 260906!Apr 29 2020, 8:00 AM
Closed by commit rG4645ef17ff1a: [clangd] Add CMake dependencies for Protobuf-generated files (authored by kbobyrev). · Explain WhyApr 29 2020, 8:01 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
clang-tools-extra/
clangd/
index/
remote/
3 lines
marshalling/
3 lines
server/
3 lines

Diff 260920

clang-tools-extra/clangd/index/remote/CMakeLists.txt

Show All 13 Lines add_clang_library(clangdRemoteIndex
LINK_LIBS LINK_LIBS
RemoteIndexProtos RemoteIndexProtos
clangdRemoteMarshalling clangdRemoteMarshalling
protobuf protobuf
grpc++ grpc++
clangDaemon clangDaemon
clangdSupport clangdSupport
DEPENDS
RemoteIndexProtos
) )
sammccallUnsubmitted
Done
ReplyInline Actions

is this equivalent to DEPENDS RemoteIndexProtos in the library?
Slightly clearer probably.

sammccall: is this equivalent to `DEPENDS RemoteIndexProtos` in the library? Slightly clearer probably.
kbobyrevAuthorUnsubmitted
Done
ReplyInline Actions

Ah, true, I had a feeling there was something like this on top of my mind, but forgot about DEPENDS :) Thanks!

kbobyrev: Ah, true, I had a feeling there was something like this on top of my mind, but forgot about…
add_subdirectory(marshalling) add_subdirectory(marshalling)
add_subdirectory(server) add_subdirectory(server)
else() else()
# Provides a dummy implementation of clangdRemoteIndex. # Provides a dummy implementation of clangdRemoteIndex.
add_subdirectory(unimplemented) add_subdirectory(unimplemented)
endif() endif()

clang-tools-extra/clangd/index/remote/marshalling/CMakeLists.txt

add_clang_library(clangdRemoteMarshalling add_clang_library(clangdRemoteMarshalling
Marshalling.cpp Marshalling.cpp
LINK_LIBS LINK_LIBS
RemoteIndexProtos RemoteIndexProtos
protobuf protobuf
clangDaemon clangDaemon
clangdSupport clangdSupport
DEPENDS
RemoteIndexProtos
) )

clang-tools-extra/clangd/index/remote/server/CMakeLists.txt

set(LLVM_LINK_COMPONENTS set(LLVM_LINK_COMPONENTS
LineEditor LineEditor
Support Support
) )
add_clang_executable(clangd-index-server add_clang_executable(clangd-index-server
Server.cpp Server.cpp
DEPENDS
RemoteIndexProtos
) )
target_link_libraries(clangd-index-server target_link_libraries(clangd-index-server
PRIVATE PRIVATE
clangDaemon clangDaemon
RemoteIndexProtos RemoteIndexProtos
clangdRemoteMarshalling clangdRemoteMarshalling
grpc++ grpc++
) )