[llvm-commits] [PATCH][CMake] Removing LLVMLibDeps.cmake and exporting libraries for CMake clients.

[Douglas Gregor]

On Sep 11, 2010, at 3:31 PM, Óscar Fuentes wrote:

> Hello Doug. Glad to see my confidence on the sociability of the LLVM
> developers restored ;-)

:)

> Douglas Gregor <dgregor at apple.com> writes:
> 
>>> Your patch switches from automatic to manually maintained library
>>> dependencies. On the case of LLVM, that increases the workload of the
>>> maintainer and it is the CMake equivalent of spaghetti code. Only a
>>> strong benefit would justify that.
>> 
>> There are numerous benefits to explicitly-maintained library dependencies:
>> 	- It's clear what those dependencies are, which leads to better
>> 	understanding of the system as a wholea
> 
> The file cmake/modules/LLVMLibDeps.cmake concentrates all those
> dependencies, so if you want to get the whole picture of the library
> dependencies, you have a single point to look at, instead of dozens of
> files scattered all around the source tree.

If you want the whole picture, it's slightly better to have one large list of dependencies. That said, text is a horrible format for such a task, and it's far more likely that someone wanted to look at the whole picture would actually want a picture, e.g., as generated by GraphViz.

I do have a few issues with putting all of the dependencies in a single file:

	1) People forget about it when they make changes. LLVM developers have gotten fairly good at updating CMakeLists.txt when they change the corresponding makefile (finally!), because they live alongside the Makefiles. This other file off in an apparently-unrelated subdirectory gets forgotten.

	2) Most of the time, we want to reason about dependencies locally ("I added this feature, and now my library depends on..."), and a single file makes that harder.

	3) It's unclear how a "single file" works for multiple projects. For example, LLVMLibDeps didn't have Clang's dependencies, because Clang is in a different repository. Should Clang have its own ClangLibDeps? Should they all be merged together? We actually had a mix of LLVMLibDeps dependencies and per-CMakeLists.txt dependencies.

> It is possible to make that file more redable, if you wish.
> 
>> 	- Changes in dependencies, which are fundamentally architectural
>> 	changes, become obvious when they happen
> 
> Again, looking at the history of LLVMLibDeps.cmake you can see all those
> changes at a glance.

Yes, agreed.

>> 	- Dependencies can make their way into other tools far more
>> 	easily, e.g., component-based CPack installers (e.g.,
>> 	http://www.paraview.org/Wiki/CMake:Component_Install_With_CPack)
> 
> CPack works fine with the automatic library dependency system. Please
> note that the automatic system ends using target_link_libraries anyways,
> so putting it explicitly on the corresponding CMakeLists.txt makes no
> difference for tools that require its existente.

I'm specifically referring to dependencies in component-based installers, which are separate from those dependencies and can be at a higher level of granularity (e.g., dependencies between groups of libraries in the graphical installer).

>> For one, find_package is the standard CMake way of finding a package,
>> so we should be providing that to CMake users.
> 
> That sounds good, but we shall think on terms of the specific
> characteristics of LLVM. find_package works fine for libraries such as
> zlib, that is, components that you install on standard places because
> they are required by some other package and which are stable and remain
> unchanged until a bug is found or a new version is required.
> 
> I don't think LLVM fits there. Either you have LLVM for working *on* it,
> or you are tracking its development. In any case LLVM is not just "a
> library," but THE library. And given how LLVM is developed you end
> working with it as if it were work-in-progress, i.e. everything but
> stable (in the sense of "frozen") not mentioning that you can easily end
> with several installs around and wanting to use uninstalled builds
> too. AFAIK find_package doesn't shine on those scenarios.

My understanding of CMake's export feature is that it helps with this, too; you can use CMake variables to push find_package toward, for example, a build tree of LLVM in your project that depends on LLVM.

> For the average user, I don't think that find_package offers significant
> advantages over the method documented in
> 
> http://www.llvm.org/docs/CMake.html#embedding

I think the advantage is familiarity; we'd fit better into the general CMake ecosystem if we follow their conventions.

> That said, I *think* that the automatic library dependencies system is
> not an impediment for supporting find_package.

No, it should be separate. Indeed, these two pieces of the patch (dependencies changes and export changes) should have been separated.

>> CMake's export is designed to make it easy to get find_package working
>> well, with versioning, the ability to directly depend on the exported
>> project's targets, etc. One huge benefit to doing
>> exporting/find_package well is that it would make it easier to
>> separate the LLVM core from, e.g., Clang, because LLVM could export
>> its targets and Clang could just import them via find_package. The
>> rest of Clang's CMakeLists.txt files shouldn't have to change.
> 
> At some point on the past, I had a patch for doing that without the need
> of find_package. It removed the requirement of putting clang under the
> LLVM source tree. It wasn't committed because I wanted to improve some
> aesthetics and becuase I didn't perceived too much enthusiasm about that
> feature.

You'll likely get more enthusiasm as Clang and LLVM get bigger and bigger. The Xcode project for Clang+LLVM is *huge*, and things will get much more interesting when we get a CMake'ified lldb (which depends on Clang+LLVM). We have a lot more subprojects than we had even a year ago, and they're not small.

> The issue here is that this patch goes against the main goal of the
> CMake build as I implemented it: simple maintenance. As configure&&make
> is the most used system by the developers, the CMake build will
> inevitably go out of sync with the actual state of the source.

Recently, this hasn't been much of a problem. LLVM developers have gotten accustomed to updating CMakeLists.txt files, and we now have buildbots that use the CMake build system (on Windows), so I find myself having to fix CMakeLists.txt files far less often than even a few months ago. Plus, there are at least several Clang developers that use the CMake build system on a daily basis. When problems crop up, they get fixed fast.

> So
> easying the job of fixing it is paramount. On the case of a removed or
> added source file the fix is easy, something anyone can do without
> having a cmake build hence doable for those who use conf&&make. Not so
> for a change on a library dependency: figuring out what's happening
> *and* the optimal point for fixing the broken dependency is not so easy
> and may require quite a few tentative builds. Furthermore, unnecessary
> dependencis will accumulate either due to failures at reflecting the
> optimal dependencies (i.e. putting the dependency on libraries A and B
> when it should go to C) or due to dependencies which are no longer
> required. This goes against your goal of having an easy way of seeing
> the project structure.

Mistakes can creep in anywhere, and when we find them, we'll fix them. Automatic checking (as Rafael mentioned) would certainly help, but a little common sense goes a long way toward keeping things consistent, and I believe that LLVM developers are careful enough that these issues won't turn into serious problems.

> There are other more subtle cases of possible
> brokenness, such as removed libraries that are forgotten on the list of
> dependencies: the problem would only be visible if you do "make clean
> all".

The buildbots do a "make clean all" every hour or so; we'd catch such an error quickly.

> Overall, my opinion is that removing automatic library dependencies will
> create more broken builds and will increase the time required for fixing
> the breakage. It will be seen as a problem too if at some point someone
> proposes to switch to CMake and ditch configure&&make (just remember how
> much fuss was about explicitly adding and removing source files to the
> build.) I think that the best thing we can do for our users is to
> provide a reliable build, even if that requires renouncing to some nifty
> features. At least until CMake is adopted as the official LLVM build.


Let's say that we adopt CMake as the official LLVM build system tomorrow. Would you still want implicit dependencies, or would you then favor explicit dependencies?

	- Doug