Some CMake issues (Are you being served?)

Hello,

I work on CMake upstream. I'd like to find out in what ways CMake upstream
does not fit the needs of llvm/clang, and then fill those gaps. The recent
update of the minimum version to CMake 2.8.8 is a good start. Before being
able to assess what is missing, it should be ensured that the current
codebase is as modern as the minimum version allows, and it needs to be
cleaned up.

1) You need to set the minimum required CMake version before the project()
command.
    
The minimum version affects behavior by setting policies, and can
affect the behavior of the project() command.
    
http://www.cmake.org/cmake/help/git-master/manual/cmake-policies.7.html
    
Eg
    
http://www.cmake.org/cmake/help/git-master/policy/CMP0025.html

-project(LLVM)
cmake_minimum_required(VERSION 2.8.8)
+project(LLVM)

You need to do the same in clang/CMakeLists.txt.

2) You need to clean up tabs vs spaces. At least clang/CMakeLists.txt mixes
them. I didn't look elsewhere but you should clean it all up.

3) In modern cmake code, the end*() commands have no content. See eg

http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=9db31162

You can run a similar command on the llvm cmake files. At least clang
CMakeLists contains

endif(MSVC_IDE OR XCODE)

I didn't check others.

4) There seems to be a 'cmake package config file' generated by the llvm
build.

http://www.cmake.org/cmake/help/git-master/manual/cmake-packages.7.html#package-configuration-file

(the doc is new in master, but it applies almost entirely to CMake 2.8.8
too)

but it doesn't seem to appear in my llvm-3.5-dev ubuntu package.

You need to ensure that the package config file is generated by your
buildsystem. If you support two buildsystems, you need to ensure that it is
generated properly by both of them. As far as I can see, the Makefile
buildsystem in llvm does not generate the files. This is a significant bug.
It is equivalent to not generating a required header file in one of your
supported buildsystems.

You need to ensure that the config file is generated by both buildsystems.
That can mean that you avoid the built-in CMake facilities for creating
packages.

[Sidebar: This is part of the reason why I strongly recommend any project to
have only one buildsystem. If it is impossible for you to drop the Makefile
system, then consider dropping the CMake one. It creates false expectations
of ability to use packages downstream.]

5) When I see things like this:

add_dependencies(clangStaticAnalyzerCheckers
   ClangAttrClasses
   ClangAttrList
   ClangCommentNodes

and this:

set(LLVM_LINK_COMPONENTS
  Support
)

I don't know what those lines are for, but it looks like 'you're doing it
wrong' from a dependency specification point of view, or CMake is not giving
you the interfaces to do it right. If it's the latter, I want to fix that.

6) If you create a proper config file, then you can populate it with
IMPORTED targets and use it in clang. IMPORTED targets record dependencies
and usage requirements (when you start requiring CMake 2.8.9+ at least)
properly.

http://www.cmake.org/cmake/help/git-master/manual/cmake-buildsystem.7.html#imported-targets

The main oddness seems to come from the fact that the Makefile buildsystem
doesn't create the cmake package config files. Fix that first or remove the
Makefile buildsystem. I recall there was a discussion about that a few years
ago.

Thanks,

Steve.

Hi Stephen,

There is an ongoing discussion about compiler-rt where it was
mentioned that with CMake it is hard to use the just-built compiler to
build the runtime libraries:

http://permalink.gmane.org/gmane.comp.compilers.llvm.devel/69951

Dmitri

I added a link to implementing that would be the CMake answer to: What the Makefile system already does is already possible using CMake ExternalProject. However, I wrote that I recommend cleaning up the cmake files first, which is why I listed several cleanups. Let’s focus on that first. Thanks, Steve.

Stephen,

1) You need to set the minimum required CMake version before the project()
command.

Fixed in r200645.

2) You need to clean up tabs vs spaces. At least clang/CMakeLists.txt mixes
them. I didn't look elsewhere but you should clean it all up.

Fixed in r200643 and r200644.

3) In modern cmake code, the end*() commands have no content. See eg

http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=9db31162

In historical reason, IMO, I'd not like to substitute all of them.
We will prune such legacy phrases whenever we touch around them.

4) There seems to be a 'cmake package config file' generated by the llvm
build.

http://www.cmake.org/cmake/help/git-master/manual/cmake-packages.7.html#package-configuration-file

(the doc is new in master, but it applies almost entirely to CMake 2.8.8
too)

but it doesn't seem to appear in my llvm-3.5-dev ubuntu package.

You need to ensure that the package config file is generated by your
buildsystem. If you support two buildsystems, you need to ensure that it is
generated properly by both of them. As far as I can see, the Makefile
buildsystem in llvm does not generate the files. This is a significant bug.
It is equivalent to not generating a required header file in one of your
supported buildsystems.

You need to ensure that the config file is generated by both buildsystems.
That can mean that you avoid the built-in CMake facilities for creating
packages.

[Sidebar: This is part of the reason why I strongly recommend any project to
have only one buildsystem. If it is impossible for you to drop the Makefile
system, then consider dropping the CMake one. It creates false expectations
of ability to use packages downstream.]

Brad King is proposing the stuff. Excuse me we haven't accepted his yet.
See also http://llvm.org/bugs/show_bug.cgi?id=15732

5) When I see things like this:

add_dependencies(clangStaticAnalyzerCheckers
   ClangAttrClasses
   ClangAttrList
   ClangCommentNodes

They give dependencies for generated headers. What'd be wrong to you?

I have w-i-p patchset to nuke them (to go more suboptimal)

and this:

set(LLVM_LINK_COMPONENTS
  Support
)

LLVM_LINK_COMPONENTS is for compatibility of autoconf build.
In AddLLVM.cmake and LLVM-Config.cmake, it is translated to
target_link_libraries.

I don't know what those lines are for, but it looks like 'you're doing it
wrong' from a dependency specification point of view, or CMake is not giving
you the interfaces to do it right. If it's the latter, I want to fix that.

You may suppose the former is coming from historical reason. Please be patient.

The latter, I am certain CMake is insufficient.
I know it'd be not easy to describe optimal behavior for generated headers.

FYI, ClangXXX is doing;

  1. Generate clang-tblgen.
  2. Generate foo.inc from foobar.td.
  3. Build object files with generated foo.inc.

Do you think we should give suboptimal dependency,
"Build all files whenever any of *.td were touchd"?

6) If you create a proper config file, then you can populate it with
IMPORTED targets and use it in clang. IMPORTED targets record dependencies
and usage requirements (when you start requiring CMake 2.8.9+ at least)
properly.

http://www.cmake.org/cmake/help/git-master/manual/cmake-buildsystem.7.html#imported-targets

Most of us are building clang in llvm build tree.
Aka "standalone clang" could be supposed as w-i-p.
I think other projects could be good studies for external cmake build.

I agree to update cmake version if LLVMConfig.cmake would work more smart. :wink:

...Takumi

NAKAMURA Takumi wrote:

4) There seems to be a 'cmake package config file' generated by the llvm
build.

http://www.cmake.org/cmake/help/git-master/manual/cmake-packages.7.html#package-configuration-file

(the doc is new in master, but it applies almost entirely to CMake 2.8.8
too)

but it doesn't seem to appear in my llvm-3.5-dev ubuntu package.

You need to ensure that the package config file is generated by your
buildsystem. If you support two buildsystems, you need to ensure that it
is generated properly by both of them. As far as I can see, the Makefile
buildsystem in llvm does not generate the files. This is a significant
bug. It is equivalent to not generating a required header file in one of
your supported buildsystems.

You need to ensure that the config file is generated by both
buildsystems. That can mean that you avoid the built-in CMake facilities
for creating packages.

[Sidebar: This is part of the reason why I strongly recommend any project
[to
have only one buildsystem. If it is impossible for you to drop the
Makefile system, then consider dropping the CMake one. It creates false
expectations of ability to use packages downstream.]

Brad King is proposing the stuff. Excuse me we haven't accepted his yet.

Oh, I didn't know Brad was also working on this stuff in llvm. Good. I
haven't reviewed his patches, but if it's generating the same content with
CMake and Makefiles, that sounds good.

See also http://llvm.org/bugs/show_bug.cgi?id=15732

Some of the bugs are questionable or unclear. I've left some comments.

5) When I see things like this:

add_dependencies(clangStaticAnalyzerCheckers
   ClangAttrClasses
   ClangAttrList
   ClangCommentNodes

They give dependencies for generated headers. What'd be wrong to you?

Oh, ok. I didn't know what they were for, and as I've seen other code for
manually managing dependencies, I thought this was more of the same.

and this:

set(LLVM_LINK_COMPONENTS
  Support
)

LLVM_LINK_COMPONENTS is for compatibility of autoconf build.
In AddLLVM.cmake and LLVM-Config.cmake, it is translated to
target_link_libraries.

What compatibility? A file is generated by a cmake llvm build for autoconf-
using downstreams to use, or something?

I don't know what those lines are for, but it looks like 'you're doing it
wrong' from a dependency specification point of view, or CMake is not
giving you the interfaces to do it right. If it's the latter, I want to
fix that.

You may suppose the former is coming from historical reason. Please be
patient.

Sorry, I did not intend to appear impatient. I'm more looking for gaps in
cmake than in the llvm buildsystem.

The latter, I am certain CMake is insufficient.
I know it'd be not easy to describe optimal behavior for generated
headers.

FYI, ClangXXX is doing;

  1. Generate clang-tblgen.
  2. Generate foo.inc from foobar.td.
  3. Build object files with generated foo.inc.

Do you think we should give suboptimal dependency,
"Build all files whenever any of *.td were touchd"?

Maybe this will be easier to work on after Brads patches are in.

6) If you create a proper config file, then you can populate it with
IMPORTED targets and use it in clang. IMPORTED targets record
dependencies and usage requirements (when you start requiring CMake
2.8.9+ at least) properly.

http://www.cmake.org/cmake/help/git-master/manual/cmake-buildsystem.7.html#imported-targets

Most of us are building clang in llvm build tree.
Aka "standalone clang" could be supposed as w-i-p.
I think other projects could be good studies for external cmake build.

I agree to update cmake version if LLVMConfig.cmake would work more smart.
:wink:

Actually, CMake 2.8.11 would be a better minimum to aim for. It handles
transitive usage requirements, but earlier versions do not. I was wrong
about 2.8.9.

Anyway, from the description, Brads patches add IMPORTED targets, so let's
have another look when they are in.

Thanks,

Steve.

Just to add, the ability to build clang against an installed LLVM tree would make it significantly easier for the FreeBSD LLVM and Clang ports to switch to using CMake. This would make us very happy, as our build infrastructure automatically uses Ninja by default for CMake ports, so we'd get a nice speedup on package builds.

David

+1. This opens a can of worms, but one well worth opening IMHO
because it affects, at least, compiler-rt, libcxx, polly, and lld. If
clang is built against the installed llvm, then it begs the question,
how should a build compose both clang and llvm? Some options:

1) Write a CMake file that invokes two CMake builds, the llvm build,
and then the clang build.

2) Clang's CMakeLists.txt adds 'add_subdirectory(llvm)', and adds a
dependency on its 'install' step and pings the target for its install
directory. But can two builds call 'add_subdirectory(llvm)' in the
same CMake build? Can the llvm install directory be different from
the clang install directory?

3) Clang's CMakeLists.txt has an 'add_subdirectory(llvm)' but no
install step. This suggests that it is somehow possible to build
against /either/ the installed directory or llvm's binary directory.
If so, how can we ensure clang doesn't add a dependency to a file that
is unavailable in the final install directory?

Stephen, I'd appreciate your expert opinion here. Can multiple
independent customers of llvm be composed into a larger CMake build?

Thanks,
Greg

For packagers the separate build could be handled by their dependency
system. For developers or users building locally one could use the
CMake ExternalProject module:

http://www.cmake.org/cmake/help/v2.8.12/cmake.html#module:ExternalProject

to create a "superbuild" project that builds LLVM and then builds
Clang against that LLVM.

This will all be much easier when the patches I proposed here:

http://thread.gmane.org/gmane.comp.compilers.llvm.cvs/173517

are integrated.

-Brad

Greg Fitzgerald wrote:

Stephen, I'd appreciate your expert opinion here. Can multiple
independent customers of llvm be composed into a larger CMake build?

The first step is to commit the patches from Brad. After that, it might be
possible to ask different, more simple questions.

Thanks,

Steve.