Differential D145228
[clangd] Add clangd headers to install targets Authored by ivanmurashko on Mar 3 2023, 3:49 AM. Tokens
Details The install target for clang distributes the clangd static libs but missing corresponding headers. The diff adds necessary headers. That opens a possibility to create custom clangd builds outside LLVM repo. Test Plan: ninja install-clangd-headers see the headers installed at the install folder
Diff Detail
Event TimelineComment Actions
I don't think this was ever intended, looks like an accidental side-effect of using LLVM's many cmake macros. Comment Actions Why not allow people building custom clangd outside of LLVM repo? There was exactly the same issue with clang-tidy (see D73236) and it was fixed with adding header to the distribution. In comparison with headers, libraries takes much more space so it is just very tiny increase in size and significant convenience for people who need this feature. Comment Actions Clangd isn't designed as a collection of libraries.
Sure, if clang-tidy maintainers are happy to support clang-tidy as engine + library checks for running against ASTs, in addition to the clang-tidy tool per se, I guess that makes sense. I don't think it follows that it makes sense for clangd.
As I said, including the libraries looks like an oversight, the intent was to distribute clangd as a binary only. Comment Actions i agree with Sam's concerns here. clangd isn't designed to be consumed as a library, but rather as a binary through LSP. increasing surface are here and letting people build applications on top of clangd internals would create extra maintenance burden that we're not equipped to support. do you have any specific use case that's pending on this change, or is this a change that might allow things in the future? if you have a specific need it would be better if you can share it directly and we can look for other solutions instead. Comment Actions I understand concerns about maintenance cost for the change. But I dare to ask why you think it is so high? Perhaps there are different expectations from the feature. I’m not asking to support any API stability or anything like this. IMHO, the maintenance cost for the feature should be on people who would like to use it. Clangd can only export headers and libraries as they are with one addition to allow main function overriding. Users of the feature should be prepared that it might change significantly at any point even within release. There is smaller but similar problem with other many other LLVM APIs, that are significantly changed especially between releases. I hope this expectation should make the maintenance cost for the change smaller and we are happy to support them if it is needed. There are some additional context about the change. We have a separate build system (not CMake) for in-house apps. The LVVM is considered as a third-party with corresponding libs and headers. The approach allows us to create in-house clang tools such as lint and refactoring tools compiling from the same sources against multiple clang versions and variants. The approach is supported by LLVM modular structure that gives an ability to combine different pieces to create powerful customized in-house apps. Use forks of LLVM is one of the obvious way to use the feature, but it has high maintenance cost and it makes harder to contribute back bug fixes and features. Because we have to pay this cost anyway we can support this feature in upstream for everybody and make LLVM even more re-usable. If we need to support it in another build system, we can do it as well (up to the degree we can in LLVM). What do you think about it? Have I missed something important? Comment Actions clangd is not my area of expertise, so I don't intend to step into the middle of this with my comment, but: one of the tenants of the LLVM project is that it's a modular set of reusable components. clangd being designed such that it cannot be used as a library goes against the grain in that regard, so I'm surprised to see push-back against attempts to make a more modular, component-based design. What am I missing? Comment Actions If someone wants to work on producing a more modular, component-based design, that's probably a good idea (but tricky to get right). Probably a good place to start is working out what pieces can be moved into separate libraries with e.g. a coherent layering story, thinking about how they might be reused in other tools etc. Exposing all clangd's headers as-is is not that. I don't think the idea that llvm-project is modular is at odds with the idea that leaf subprojects exist, though clangd is big enough that it may make sense to split up. And private headers (those that live next to source files in the tree) are not installed (with the recent exception of clang-tidy, which IMO should have been restructured to publish in this way). FWIW, I do think clang is a fair example of a project where the default of "everything is a public library" has made the library design somewhat incoherent and hard to evolve. OTOH it's enabled tons of useful stuff, so... Yes, my experience is that in practice people object to e.g. removing things without replacement, even in an "unstable" API. I think this is a bit abstract though. Concretely, what API do you need here? e.g. which headers do you want to include, to what end? My impression so far is that you actually don't need an API, but rather to link some extra libraries into an otherwise-unmodified clangd binary, and that being able to call clangdMain() from an external source file would make this easier in your build system. That seems like a problem worth solving, but installing all clangd's private headers into /usr/lib doesn't seem like a particularly direct solution. Comment Actions Ah, okay, thank you for the clarification! I agree we have to go about splitting the project up into libraries very carefully and this approach isn't appropriate. But I was getting the impression there was push-back against the idea of making clangd into a series of libraries in general and that worried me.
IMO, we've not really had too many issues evolving Clang's libraries, but I do think there's room for improvement (it's hard to figure out how to split up Sema, for example). As you say, it's not an easy problem to solve but it enables a lot of really useful stuff we wouldn't otherwise be able to achieve as easily. Comment Actions If we consider the bare minimum with the only goal to build outside LLVM source tree then we don’t need to copy all internal headers. At the case the D145302 can be modified and we can end up with the only one header that is required to copy: clang-tools-extra/clangd/tool/ClangdMain.h. For further customization one might require additional headers to be installed. That can be done once it’s required or via one diff that install all possible headers. What do you think about it? Comment Actions
I just updated D145302 with the changes. The D145228 can be abandoned if the changes are OK. |