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

Build with CMake #4455

Merged
merged 17 commits into from
Oct 14, 2017
Merged

Conversation

ridiculousfish
Copy link
Member

@ridiculousfish ridiculousfish commented Oct 7, 2017

TLDR: let's build fish with CMake and eventually replace autotools, so we can drop a ton of cruft. This PR has enough for day-to-day development with CMake. I'm looking forwards to feedback and discussion.

What is CMake?

CMake is a meta-build system. At a high-level it plays the same role as ./configure by generating a Makefile, but it can also output other builds (Ninja, CLion, XCode, CodeBlocks VS, and more).

CMake is mature and battle-tested. It's used by major OS projects like LLVM, KDE, MySQL. It has great docs and is easy to find help on StackOverflow, etc.

KDE switched to CMake, here's their story.

Probably you already know CMake and hopefully don't need to be sold on it. If not there's info on CMake advantages at the bottom.

What does this PR include?

This PR:

  1. Builds fish, fish_indent, fish_key_reader
  2. Builds and runs all tests
  3. Builds the docs and man pages
  4. Can install fish and all the rest (web_config, etc)

What's missing:

  1. Generating release tarballs, .deb files, macOS installer packages, etc
  2. Integration with TravisCI / OBS
  3. New build instructions
  4. The "helpful hints" that the Makefile prints at the end

Try it!

Easy peasy:

  1. git checkout this PR.
  2. Make a build directory: mkdir /tmp/fish-build ; cd /tmp/fish-build
  3. Generate a Makefile: cmake /path/to/fish-shell
  4. make test
  5. mkdir /tmp/fish-install ; env DESTDIR=/tmp/fish-install make install

make VERBOSE=1 for a verbose build.

Advanced command line users will like Ninja:

  1. cmake -G Ninja /path/to/fish-shell; and ninja

Or if you want Xcode:

  1. cmake -G Xcode /path/to/fish-shell

What happens if we merge this?

Here's a sketch of how this could go:

  1. The autotools build system will continue to work until the CMake build is at 100%. Devs can use CMake or the existing autotools system as they prefer. During this time, we'll have to duplicate build system changes across both autotools and CMake.
  2. We'll teach the CMake build how to generate packages, presumably with CPack.
  3. We'll incrementally switch Travis, OBS, homebrew, etc. to using CMake.
  4. We'll remove fish.xcodeproj and most of the stuff in osx/.
  5. We'll remove configure.ac, Makefile.in, m4, and all the rest of the autotools stuff.
  6. (Longer term) we can remove pcre2 and instead rely on CMake's package management. Also clean up the docs build and muParser build to make them more CMake friendly.

How does the CMake build handle:

  1. PCRE2: This PR uses the system PCRE2 if one is installed. If not, it builds the bundled PCRE2 via CMake (which PCRE2 conveniently supports). Once we remove support for autotools we can stop bundling pcre2 and use the CMake package management.

  2. muParser: This builds muParser as an ExternalProject using the muParser autotools build. The path forward here is to fork muParser, and add our own CMake build system.

  3. Docs: The docs are by far the most complicated part of fish build. The autotools path uses a bunch of sed commands and relies on many temporary files. This PR factors those commands into separate shell scripts under build_tools, which both the Makefile and CMake invoke.

    This is admittedly gross, but the docs build was already gross. It should be factored into a monolithic script that builds everything, instead of granular individual targets.

What does CMake bring to the table?

Mature, battle-tested, widely available

This is the main advantage that CMake has over gyp, boost.build, scons, etc. No build system is as ubiquitous as autotools, but CMake is number two.

Much simpler builds

autotools builds are famously difficult to maintain: arcane m4 syntax, workarounds for ancient systems, layers upon layers. CMake's syntax is a little weird, but simple and regular.

Nested projects are very common and easy with CMake. It is very simple to build PCRE2 as a nested project using PCRE2's CMake path. No more "recursive Make considered harmful."

CMake also knows about development artifacts such as header files and libraries. autotools must be manually taught header dependencies ("make depend"), filename extensions (.so vs .dylib), so on.

Actual package management

CMake has built in support for external dependencies. It knows how to look for them locally, fetch them via git, etc. PCRE2 would have been much simpler to add to fish if we were using CMake.

Replaces XCode project

Unlike Make, CMake knows about macOS-isms like bundles and deployment targets. We can support macOS's needs without maintaining the separate XCode project and osx/ contents.

CMake can also generate CLion or VS projects, so we can support a lot more IDEs.

Out-of-source builds

CMake builds in a different directory from the source code. This takes a little bit of getting used to but is actually great. Your source directory stays clean. make distclean becomes rm -Rf *. Multiple builds (debug vs release) are trivial.

This is possible in princple with autotools, but takes a lot of work.

Disadvantages of CMake

  1. This will add cmake as a dependency for building fish. cmake is widely available but less likely to be installed compared to GNU Make.
  2. CMake's generated Makefiles are not portable by design (e.g. have hard-coded paths), so our tarballs will require CMake to build. Tarball users won't be able to just ./configure ; make ; make install, unless we provide a wrapper.
  3. CMake is not quite as flexible as GNU Make. For example, it is hard to implement the active_test_goals hack from make, which serializes tests but only those that are to be run. However CMake has good support for invoking external scripts, so it ought to be flexible enough.

@mqudsi
Copy link
Contributor

mqudsi commented Oct 7, 2017

Oh my gosh, thank you!

I've locally been building fish with meson for its ninja backend, but that has the extremely unfortunate side effect of introducing python as a build requirement (the meson team feels that's OK, though I disagreed).

cmake or meson piping to ninja would both generate far smarter build files that do not need to rebuild as much or as often when jumping between revisions to test when/where something broke.

With regards to your point:

CMake's generated Makefiles are not portable by design (e.g. have hard-coded paths), so our tarballs will require CMake to build. Tarball users won't be able to just ./configure ; make ; make install, unless we provide a wrapper.

I would argue that this is a dealbreaker, and we shouldn't release without end users being able to ./configure; make; sudo make install; as they've been able to for years with pretty much any FOSS project. It's why I never even whispered about my meson fish builds (despite spending a day replacing the autoconf script with its meson equivalent, feature tests included) as changing that workflow was (for me) a nonstarter.

@ridiculousfish
Copy link
Member Author

ridiculousfish commented Oct 8, 2017

I would argue that this is a dealbreaker, and we shouldn't release without end users being able to ./configure; make; sudo make install

I wonder how other OSS projects handle that. It looks like libpng and pcre2 both manually maintain CMake and autotools systems, which seems nuts. bro has ./configure and Makefile wrappers that call out to CMake, which seems OK. It might be worth asking on StackOverflow or CMake mailing list.

Edit: also found https://github.com/nemequ/configure-cmake

@floam
Copy link
Member

floam commented Oct 8, 2017

Super cool!

@mqudsi
Copy link
Contributor

mqudsi commented Oct 8, 2017

configure-cmake looks interesting, nice find.

I was actually thinking of a configure script that would take take a cmake-generated makefile and plug in prefix and co without needing to shell out to cmake at all (so it's only a dev and not a build dependency) via some sed voodoo, but I'm not sure how practical that is (presumably it would have been done by others if it were easy?).

neovim wraps cmake in a Makefile that shells out to cmake and ninja, no ./configure required (cmake build time dependency though, of course): https://github.com/neovim/neovim/blob/master/Makefile

(When I was deciding between meson, cmake, and other "modern" build systems for $work I came across tup which is downright awesome (and generates dependency-free build scripts for distribution tarballs) and written by folk that obviously have their head on straight, but it has a dev (not build) dependency on fuse (which is how it operates; inspecting which files are opened during build and automatically configuring them as dependencies), but the fuse dependency is a huge non-starter and makes Windows support (even via WSL) out of the question.)

@faho
Copy link
Member

faho commented Oct 8, 2017

I've locally been building fish with meson for its ninja backend

Hehe... so we've had at least two people trying out meson.

I did as well, and here's what I wrote up then:

So, I've got fish building and installing with meson. It takes about half the time autotools takes, and I find the file much more readable than both configure.ac and Makefile.in. However, documentation is sparse and it won't build on travis (since even the "experimental" Ubuntu they use is too old - the regular one is actually been EOL'd last week).

The remaining issues:

  • Localization seems to require a file that lists all files to be localized
  • Version handling is weird - they have a "vcs_tag" function to get a version via git, but that won't expose it as a variable, only writing it to a file
  • Incorporating doxygen likely requires scripting
  • This would exclude Debian Jessie (i.e. the current one IIRC) and Ubuntu < 16.04, plus possibly many others.

So all-in-all, unless there's interest in a second build system for development (i.e. test with meson, keep autotools for distributions), I think I'm abandoning this experiment for now. If anyone cares, see my meson branch.

So unlike @mqudsi, I never really had an issue with the python dependency, though my concern is mostly linux systems, and those tend to have python anyway.

What I did have an issue with is that meson is still so new that it's adding functionality left and right, so it's kinda hard to use it in such a way that it still works on old OSen - if they have it at all.


So, as you might see, cmake in this implementation solves all of those issues - it already has the doc stuff (that I never really attempted since I gave up after figuring out that it was a non-starter for Debian), the version generation works, cmake has "GLOB" to avoid specifying every .po file and .fish script. It can also still use ninja, which manages to make the build quite a bit faster (though cmake's makefiles are apparently bad, since a cmake-with-make build takes 50% longer on my system than with our autoconf one).

So, while I like meson's syntax a bit more than cmake (and I dislike having to explicitly pick ninja), cmake solves some real problems we'd have with it.

It's also just simply more well-known than meson.

(Thwough of course it's possible that @mqudsi's meson work is better than mine, since I am not well-versed in build systems)


So, I am apparently not alone in not loving autostuff/make. IMHO, the main advantage it has is that it's ubiquitous. However, cmake is also available on every linux distribution, and has been for quite a while. It's available on windows and I'd imagine the BSDs. I don't think availability is going to be an issue.

There is one thing that I dislike in pretty much anything that isn't autotools, and that's configuring directories. I'm currently rewriting my PKGBUILD (the thing that gives me nice archlinux packages to install) to use cmake if available, and I find it really hard to pass the right things. I.e.

cmake -G Ninja -DCMAKE_INSTALL_PREFIX=/usr -DCMAKE_INSTALL_DATADIR=/share -DCMAKE_INSTALL_DOCDIR=/share/doc/fish ..

is noticeably uglier than and doesn't work as a replacement for

./configure --prefix=/usr --sysconfdir=/etc --docdir=/usr/share/doc/fish

@floam
Copy link
Member

floam commented Oct 9, 2017

For some reason, the fish binary generated with CMake+ninja is way larger than the autotools build here, like 4595444 bytes vs. 2066896 bytes. I don't see any pcre2 symbols in the cmake-produced binary, which means it wasn't my first guess, that it might building that in statically instead of using the system library. Stripping the binary doesn't seem to make a big difference.

@floam
Copy link
Member

floam commented Oct 9, 2017

Surprised that the CMake build is linking libform, which the autotools build does not.

$ otool -l fish
...
Load command 13
          cmd LC_LOAD_DYLIB
      cmdsize 56
         name /usr/lib/libform.5.4.dylib (offset 24)
   time stamp 2 Wed Dec 31 16:00:02 1969
      current version 5.4.0
compatibility version 5.4.0
...

@ridiculousfish
Copy link
Member Author

@floam the CMake build by default is -O0, the autotools build is -O2. Can you repeat your experiment except run cmake -DCMAKE_BUILD_TYPE=Release /path/to/fish-shell?

libform is probably being picked up by CMake's FIND_CURSES implementation. Nice idea to check the libs!

@floam
Copy link
Member

floam commented Oct 10, 2017

Yeah, it was just the optimization level. Release build is way smaller.

@mqudsi
Copy link
Contributor

mqudsi commented Oct 10, 2017

Do we want to set the default level via CMake to the same we currently use?

@mqudsi
Copy link
Contributor

mqudsi commented Oct 10, 2017

@zanchey what do you think about the switch to CMake?

If we have consensus, we should probably merge these changes to a cmake branch upstream so we can make PRs against it.

@ridiculousfish
Copy link
Member Author

@mqudsi IMO the CMake default of -O0 is better for day to day development: faster builds and easier debugging. I'd like to keep it.

If we decide to go with this I don't think we need an upstream branch, we can just merge to master. This doesn't break the autotools build.

@zanchey
Copy link
Member

zanchey commented Oct 11, 2017

SGTM!

@mqudsi
Copy link
Contributor

mqudsi commented Oct 12, 2017

@ridiculousfish does the cmake build also depend on pkg-config? I keep forgetting to double check and then update the readme to note that building fish via autotools currently indirectly depends on pkg-config.

@floam
Copy link
Member

floam commented Oct 12, 2017

We depend on pkg-config? I don't have it and have never had trouble with anything.

@mqudsi
Copy link
Contributor

mqudsi commented Oct 12, 2017

I recall being unable to build on solaris without installing it. But like I said, I'd have to double-check.

That said, I'm surprised you don't have it installed; it usually ends up being installed regardless of the package manager you use on most modern *nix systems.

@floam
Copy link
Member

floam commented Oct 12, 2017

I use MacOS, FWIW.

@mqudsi
Copy link
Contributor

mqudsi commented Oct 12, 2017

Hmmm, reviewing my logs it seems that I did have to install pkg-config but removing it now allows fish to build. Perhaps I need it to build a dependency or something... no matter.

@floam floam added this to the fish-3.0 milestone Oct 13, 2017
@floam
Copy link
Member

floam commented Oct 13, 2017

Looking over FindCurses, it looks like CURSES_LIBRARIES contains all of CURSES_LIBRARY, CURSES_EXTRA_LIBRARY (libtinfo), and CURSES_FORM_LIBRARY that it can find. This seems to make it stop linking the unnecessary libform library here:

--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -90,7 +90,7 @@ ENDFUNCTION(FISH_LINK_DEPS)
 ADD_LIBRARY(fishlib STATIC ${FISH_SRCS})
 TARGET_SOURCES(fishlib PRIVATE ${FISH_HEADERS})
 TARGET_LINK_LIBRARIES(fishlib
-  ${CURSES_LIBRARIES} Threads::Threads ${CMAKE_DL_LIBS}
+  ${CURSES_LIBRARY} ${CURSES_EXTRA_LIBRARY} Threads::Threads ${CMAKE_DL_LIBS}
   ${PCRE2_LIB} muparser)
 
 # builtin_math.cpp needs to see muParser's built header.

This adds a basic CMakeLists.txt. It also adds a ConfigureChecks.cmake
and config_cmake.h.in that produces a config.h.
This adds files MuParser.cmake and PCRE2.cmake. PCRE2 is built using
its own CMake path, while MuParser uses ExternalProject.
This adds a basic Tests.cmake that can build and run fish_tests.
It also adds a 'test' target.
This adds CMake targets for fish_indent and fish_key_reader.
This adds a new script build_tools/build_lexicon_filter.sh
that builds the lexicon filter. It is factored out from the Makefile,
and both the Makefile and CMake build invoke it.
As part of factoring out the documentation building parts of the fish
build, add a new file build_commands_hdr.sh that builds the commands.hdr
file. Invoke it from both the Makefile and CMake build.
As part of factoring out the documentation building parts of the fish
build, add a new file build_index_hdr.sh that builds the index.hdr
file. Invoke it from both the Makefile and CMake build.
As part of factoring out the documentation building parts of the fish
build, add a new file build_index_hdr.sh that builds the index.hdr
file. Invoke it from both the Makefile and CMake build.
As part of factoring out the documentation building parts of the fish
build, add a new file build_user_doc.sh that builds the user_doc directory.
Invoke it from both the Makefile and CMake build.
This adds support for creating the FISH-BUILD-VERSION-FILE in the CMake
build. A FISH-BUILD-VERSION-FILE is created in the CMake directory
and only updated when necessary.
This adds a file Install.cmake for installing fish. It is
quite incomplete; in particular it does not support building
the docs.
This adds a new library fishlib, which the CMake build builds.
This library is linked by the tests, fish, and fish_indent, so
that object files do not have to be built separately for each
of them.
This adds support in Tests.cmake for running the script and other
"high level" tests, in addition to the unit tests fish_tests.
This adds intelligent groups and hides unused files when generating
IDE projects (Xcode, CLion, etc) in the CMake build.
This links against CMAKE_DL_LIBS, reflecting the recent change
73f2992.
Switch to ${CURSES_LIBRARY} and ${CURSES_EXTRA_LIBRARY}
instead of ${CURSES_LIBRARIES} to reduce the linked libraries
in the CMake build.
@ridiculousfish
Copy link
Member Author

Thanks @floam, added that as 2ca7b88

@ridiculousfish ridiculousfish merged commit 2ca7b88 into fish-shell:master Oct 14, 2017
@ridiculousfish ridiculousfish deleted the cmake_experiment branch October 14, 2017 20:16
@zanchey
Copy link
Member

zanchey commented Nov 6, 2017

One problem I've run into is that the CMakeFiles directory is used by CMake for the actual build process, so in-tree builds (i.e. cmake .) fail because the include files get wiped out by the build process.

git mv CMakeFiles cmake_includes and related fixups make in-tree builds work.

@ridiculousfish
Copy link
Member Author

I didn't know in-tree builds were even a possibility outside of a dedicated ./build directory, but if it's easy to fix let's do it!

@zanchey
Copy link
Member

zanchey commented Nov 11, 2017

I have a patch series in the works that fixes that, among other things.

@zanchey
Copy link
Member

zanchey commented Nov 13, 2017

See #4535 for further work.

Still remaining:

  • checking for fixed-argument tparm (the old "Solaris kludge"), which is still required on NetBSD unless we add a TPARM_VARARGS define, and possibly on IllumOS or other Solaris-derived systems
  • checking for and enabling Gettext - at present USE_GETTEXT is always off
  • supporting make install without Doxygen

@zanchey
Copy link
Member

zanchey commented Nov 4, 2018

AFAICT the only thing still requiring autotools is the build_tarball script

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants