From d09210c08bc8e36ac32727ef12bf3ce35f5374ee Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Mon, 8 Jan 2018 01:39:45 -0800 Subject: [PATCH] [cmake] Untangle the CMake versioning This untangles the CMake versioning issues (I hope) as discussed in #4626. Note most of the advice found on the Internet about how to inject git versions into CMake is just wrong. The behavior we want is to unconditionally run the script build_tools/git_version_gen.sh at build time (i.e. when you invoke ninja or make, and not when you invoke cmake, which is build system generation time). This script is careful to only update the FISH-BUILD-VERSION-FILE if the contents have changed, to avoid spurious rebuilding dependencies of FISH-BUILD-VERSION-FILE. Assuming the git version hasn't changed, the script will run, but not update FISH-BUILD-VERSION-FILE, and therefore fish_version.o will not have to be rebuilt. This might normally rebuild more than is necessary even if the timestamp is not updated, because ninja computes the dependency chain ahead of time. But Ninja also supports the 'restat' option for just this case, and CMake is rad and exposes this via BYPRODUCTS. So mark FISH-BUILD-VERSION-FILE as a byproduct and make the script always update a dummy file (fish-build-version-witness.txt). Note this is the use case for which BYPRODUCTS is designed. We also have fish_version.cpp #include "FISH-BUILD-VERSION-FILE", and do a semi-silly thing and make FISH-BUILD-VERSION-FILE valid C++ (so there's just one version file). This means we have to filter out the quotes in other cases.. --- .gitignore | 2 ++ CMakeLists.txt | 22 +++++++-------- build_tools/git_version_gen.sh | 23 +++++++++------ cmake/Docs.cmake | 4 +-- cmake/Install.cmake | 2 +- cmake/Version.cmake | 51 ++++++++++++++++++++++++++++++++++ configure.ac | 2 +- src/fish_version.cpp | 7 ++++- 8 files changed, 88 insertions(+), 25 deletions(-) create mode 100644 cmake/Version.cmake diff --git a/.gitignore b/.gitignore index f59a91d54..d7b3bc8a1 100644 --- a/.gitignore +++ b/.gitignore @@ -78,6 +78,7 @@ messages.pot /lexicon_filter /toc.txt /version +fish-build-version-witness.txt # File names that can appear below the project root that represent artifacts # from building and testing. @@ -101,4 +102,5 @@ xcuserdata/ *.moved-aside *.xccheckout *.xcscmblueprin +.vscode diff --git a/CMakeLists.txt b/CMakeLists.txt index e31dbdd86..2ffb17956 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -55,17 +55,6 @@ SET(FISH_SRCS # Header files are just globbed. FILE(GLOB FISH_HEADERS src/*.h) -# Set up the version target. -# This creates the file FISH-BUILD-VERSION-FILE which is only modified if necessary. -ADD_CUSTOM_COMMAND(OUTPUT "FISH-BUILD-VERSION-FILE" - DEPENDS CHECK-FISH-BUILD-VERSION-FILE) - -ADD_CUSTOM_TARGET("CHECK-FISH-BUILD-VERSION-FILE" - COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/build_tools/git_version_gen.sh) - -SET(FBVF "FISH-BUILD-VERSION-FILE") - - # Set up config.h INCLUDE(cmake/ConfigureChecks.cmake) INCLUDE(cmake/gettext.cmake) @@ -76,7 +65,6 @@ INCLUDE_DIRECTORIES(${CMAKE_CURRENT_BINARY_DIR}) # Set up standard directories. INCLUDE(GNUInstallDirs) ADD_DEFINITIONS(-D_UNICODE=1 - -DFISH_BUILD_VERSION="${FISH_BUILD_VERSION}" -DLOCALEDIR="${CMAKE_INSTALL_FULL_LOCALEDIR}" -DPREFIX=L"${CMAKE_INSTALL_PREFIX}" -DDATADIR=L"${CMAKE_INSTALL_FULL_DATADIR}" @@ -84,6 +72,16 @@ ADD_DEFINITIONS(-D_UNICODE=1 -DBINDIR=L"${CMAKE_INSTALL_FULL_BINDIR}" -DDOCDIR=L"${CMAKE_INSTALL_FULL_DOCDIR}") +# Set up the machinery around FISH-BUILD-VERSION-FILE +# This defines the FBVF variable. +INCLUDE(Version) + +# Teach fish_version.o to rebuild when FBVF changes. +# The standard C++ include detection machinery misses this. +SET_SOURCE_FILES_PROPERTIES(src/fish_version.cpp + PROPERTIES OBJECT_DEPENDS + ${CMAKE_CURRENT_BINARY_DIR}/${FBVF}) + # Set up PCRE2 INCLUDE(cmake/PCRE2.cmake) diff --git a/build_tools/git_version_gen.sh b/build_tools/git_version_gen.sh index cb208f76c..bbf780b49 100755 --- a/build_tools/git_version_gen.sh +++ b/build_tools/git_version_gen.sh @@ -1,14 +1,11 @@ -#!/bin/sh +#!/bin/bash # Originally from the git sources (GIT-VERSION-GEN) # Presumably (C) Junio C Hamano # Reused under GPL v2.0 # Modified for fish by David Adam -# Obtain directory containing this script in POSIX-compatible manner -# See https://stackoverflow.com/a/43919044/17027 (public domain) -a="/$0"; a="${a%/*}"; a="${a:-.}"; a="${a#/}/"; BASEDIR=$(cd "$a"; pwd) -# Find the fish git directory as two levels up from this directory. -GIT_DIR=$(dirname "$a") +# Find the fish git directory as two levels up from script directory. +GIT_DIR="$( cd "$( dirname $( dirname "${BASH_SOURCE[0]}" ) )" && pwd )" FBVF=FISH-BUILD-VERSION-FILE DEF_VER=unknown @@ -24,11 +21,21 @@ fi if test -r $FBVF then - VC=$(sed -e 's/^FISH_BUILD_VERSION=//' <$FBVF) + VC=$(grep -v '^#' $FBVF | tr -d '"' | sed -e 's/^FISH_BUILD_VERSION=//') else VC=unset fi + +# Output the FBVF +# Note that we are super-double sneaky: we produce a file that is valid bash +# and valid C++, so it may be included from either. Also it may have leading +# hashes filtered out with grep to get a version of the form (for example): +# FISH_BUILD_VERSION=2.7.1-620-g94c9f5c2 test "$VN" = "$VC" || { echo >&2 "FISH_BUILD_VERSION=$VN" - echo "FISH_BUILD_VERSION=$VN" >$FBVF + echo "FISH_BUILD_VERSION=\"$VN\"" >$FBVF } + +# Output the fish-build-version-witness.txt +# See https://cmake.org/cmake/help/v3.4/policy/CMP0058.html +date +%s > fish-build-version-witness.txt diff --git a/cmake/Docs.cmake b/cmake/Docs.cmake index 1ca360166..76a0c89b2 100644 --- a/cmake/Docs.cmake +++ b/cmake/Docs.cmake @@ -70,7 +70,7 @@ ADD_CUSTOM_COMMAND(OUTPUT doc.h # Note we would like to add doc_src/index.hdr.in as a dependency but CMake replaces this with # doc_src/index.hdr; CMake bug? ADD_CUSTOM_COMMAND(OUTPUT toc.txt - COMMAND env `cat ${FBVF}` ${CMAKE_CURRENT_SOURCE_DIR}/build_tools/build_toc_txt.sh + COMMAND env `cat ${FBVF} | tr -d '\"'` ${CMAKE_CURRENT_SOURCE_DIR}/build_tools/build_toc_txt.sh doc_src/index.hdr.in ${HDR_FILES_NO_INDEX} > ${CMAKE_CURRENT_BINARY_DIR}/toc.txt DEPENDS ${FBVF} ${HDR_FILES_NO_INDEX}) @@ -95,7 +95,7 @@ ADD_CUSTOM_TARGET(doc DEPENDS ${FBVF} Doxyfile.user ${DOC_SRC_FILES} doc.h ${HDR_FILES} lexicon_filter) ADD_CUSTOM_COMMAND(OUTPUT share/man/ - COMMAND env `cat ${FBVF}` + COMMAND env `cat ${FBVF} | tr -d '\"' ` INPUT_FILTER=lexicon_filter ${CMAKE_CURRENT_SOURCE_DIR}/build_tools/build_documentation.sh ${CMAKE_CURRENT_SOURCE_DIR}/Doxyfile.help doc_src ./share DEPENDS ${FBVF} ${HELP_SRC} ${CMAKE_CURRENT_BINARY_DIR}/lexicon_filter) diff --git a/cmake/Install.cmake b/cmake/Install.cmake index 37b4c3ce7..18a32e104 100644 --- a/cmake/Install.cmake +++ b/cmake/Install.cmake @@ -111,7 +111,7 @@ FISH_CREATE_DIRS(${rel_datadir}/pkgconfig ${extra_completionsdir} # $v $(INSTALL) -m 644 fish.pc $(DESTDIR)$(datadir)/pkgconfig CONFIGURE_FILE(fish.pc.in fish.pc.noversion) ADD_CUSTOM_COMMAND(OUTPUT fish.pc - COMMAND awk -v `cat ${FBVF}` '/^Version:/ {$$0=$$0 FISH_BUILD_VERSION} 1' fish.pc.noversion > fish.pc + COMMAND awk -v `cat ${FBVF} | tr -d '\"'` '/^Version:/ {$$0=$$0 FISH_BUILD_VERSION} 1' fish.pc.noversion > fish.pc WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} DEPENDS ${FBVF} ${CMAKE_CURRENT_BINARY_DIR}/fish.pc.noversion) diff --git a/cmake/Version.cmake b/cmake/Version.cmake new file mode 100644 index 000000000..446c134eb --- /dev/null +++ b/cmake/Version.cmake @@ -0,0 +1,51 @@ +# This file adds commands to manage the FISH-BUILD-VERSION-FILE (hereafter +# FBVF). This file exists in the build directory and is used to populate the +# documentation and also the version string in fish_version.o (printed with +# `echo $version` and also fish --version). The essential idea is that we are +# going to invoke git_version_gen.sh, which will update the +# FISH-BUILD-VERSION-FILE only if it needs to change; this is what makes +# incremental rebuilds fast. +# +# This code is delicate, with the chief subtlety revolving around Ninja. A +# natural and naive approach would tell the generated build system that FBVF is +# a dependency of fish_version.o, and that git_version_gen.sh updates it. Make +# will then invoke the script, check the timestamp on fish_version.o and FBVF, +# see that FBVF is earlier, and then not rebuild fish_version.o. Ninja, +# however, decides what to build up-front and will unconditionally rebuild +# fish_version.o. +# +# To avoid this with Ninja, we want to hook into its 'restat' option which we +# can do through the BYPRODUCTS feature of CMake. See +# https://cmake.org/cmake/help/latest/policy/CMP0058.html +# +# Unfortunately BYPRODUCTS behaves strangely with the Makefile generator: it +# marks FBVF as generated and then CMake itself will `touch` it on every build, +# meaning that using BYPRODUCTS will cause fish_version.o to be rebuilt +# unconditionally with the Makefile generator. Thus we want to use the +# natural-and-naive approach for Makefiles. + +# **IMPORTANT** If you touch these build rules, please test both Ninja and +# Makefile generators with both a clean and dirty git tree. Verify that both +# generated build systems rebuild fish when the git tree goes from dirty to +# clean (and vice versa), and verify they do NOT rebuild it when the git tree +# stays the same (incremental builds must be fast). + +# Just a handy abbreviation. +SET(FBVF FISH-BUILD-VERSION-FILE) + +# TODO: find a cleaner way to do this. +IF (${CMAKE_GENERATOR} STREQUAL Ninja) + SET(FBVF-OUTPUT fish-build-version-witness.txt) + SET(CFBVF-BYPRODUCTS ${FBVF}) +ELSE(${CMAKE_GENERATOR} STREQUAL Ninja) + SET(FBVF-OUTPUT ${FBVF}) + SET(CFBVF-BYPRODUCTS) +ENDIF(${CMAKE_GENERATOR} STREQUAL Ninja) + +# Set up the version targets +ADD_CUSTOM_TARGET(CHECK-FISH-BUILD-VERSION-FILE + COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/build_tools/git_version_gen.sh + BYPRODUCTS ${CFBVF-BYPRODUCTS}) + +ADD_CUSTOM_COMMAND(OUTPUT ${FBVF-OUTPUT} + DEPENDS CHECK-FISH-BUILD-VERSION-FILE) diff --git a/configure.ac b/configure.ac index c33107b53..4f923266d 100644 --- a/configure.ac +++ b/configure.ac @@ -12,7 +12,7 @@ m4_syscmd([build_tools/git_version_gen.sh 2>/dev/null]) AC_PREREQ([2.60]) AC_INIT(fish, - m4_esyscmd([cut -f 3 -d ' ' FISH-BUILD-VERSION-FILE | tr -d '\n']), + m4_esyscmd([cut -f 2 -d '=' FISH-BUILD-VERSION-FILE | tr -d '"\n']), fish-users@lists.sourceforge.net) ac_clean_files=a.out.dSYM diff --git a/src/fish_version.cpp b/src/fish_version.cpp index d371132f9..e64ba878e 100644 --- a/src/fish_version.cpp +++ b/src/fish_version.cpp @@ -5,7 +5,12 @@ #include "fish_version.h" #ifndef FISH_BUILD_VERSION -#include "fish-build-version.h" +// The contents of FISH-BUILD-VERSION-FILE looks like: +// FISH_BUILD_VERSION="2.7.1-62-gc0480092-dirty" +// Arrange for it to become a variable. +static const char * +#include "FISH-BUILD-VERSION-FILE" + ; #endif /// Return fish shell version.