tools: fix incremental build of topologies. Make incremental build the default.

.github/workflows/pull-request.yml

@@ -108,7 +108,7 @@ jobs:
 
       - name: apt get
         run: sudo apt-get update &&
-             sudo apt-get -y install valgrind alsa-utils libasound2-dev
+             sudo apt-get -y install valgrind alsa-utils libasound2-dev ninja-build
              octave octave-io octave-signal
 
       # testbench needs some topologies.

scripts/build-tools.sh

@@ -10,10 +10,10 @@ SOF_TOP=$(cd "$(dirname "$0")/.." && pwd)
 print_usage()
 {
         cat <<EOFUSAGE
-Deletes and re-builds from scratch CMake projects in the tools/
-directory.
-Attention: the list below is _not_ exhaustive. To re-build _everything_
-from scratch don't select any particular target; this will build the
+
+Configures and builds selected CMake projects in the tools/ directory.
+Attention: the list of selected shortcuts below is _not_ exhaustive. To
+build _everything_ don't select any particular target; this will build
 CMake's default target "ALL".
 
 usage: $0 [-c|-f|-h|-l|-p|-t|-T]
@@ -28,6 +28,8 @@ usage: $0 [-c|-f|-h|-l|-p|-t|-T]
 
        -C No build, only CMake re-configuration. Shows CMake targets.
 EOFUSAGE
+
+        warn_if_incremental_build
 }
 
 # generate Makefiles
@@ -37,12 +39,12 @@ reconfigure_build()
         mkdir -p "$BUILD_TOOLS_DIR"
 
         ( cd "$BUILD_TOOLS_DIR"
-          cmake -DCMAKE_BUILD_TYPE="$CMAKE_BUILD_TYPE" "${SOF_REPO}/tools"
+          cmake -GNinja -DCMAKE_BUILD_TYPE="$CMAKE_BUILD_TYPE" "${SOF_REPO}/tools"
         )
 
         mkdir "$BUILD_TOOLS_DIR/fuzzer"
         ( cd "$BUILD_TOOLS_DIR/fuzzer"
-          cmake -DCMAKE_BUILD_TYPE="$CMAKE_BUILD_TYPE" "${SOF_REPO}/tools/fuzzer"
+          cmake -GNinja -DCMAKE_BUILD_TYPE="$CMAKE_BUILD_TYPE" "${SOF_REPO}/tools/fuzzer"
         )
 }
 
@@ -70,18 +72,32 @@ print_build_info()
        cat <<EOFUSAGE
 
 Build commands for respective tools:
-        ctl:        make -C "$BUILD_TOOLS_DIR" sof-ctl
-        logger:     make -C "$BUILD_TOOLS_DIR" sof-logger
-        probes:     make -C "$BUILD_TOOLS_DIR" sof-probes
-        topologies: make -C "$BUILD_TOOLS_DIR" topologies
-        test tplgs: make -C "$BUILD_TOOLS_DIR" tests
+        ctl:        ninja -C "$BUILD_TOOLS_DIR" sof-ctl
+        logger:     ninja -C "$BUILD_TOOLS_DIR" sof-logger
+        probes:     ninja -C "$BUILD_TOOLS_DIR" sof-probes
+        topologies: ninja -C "$BUILD_TOOLS_DIR" topologies
+        test tplgs: ninja -C "$BUILD_TOOLS_DIR" tests
                (or ./tools/test/topology/tplg-build.sh directly)
 
-        fuzzer:     make -C "$BUILD_TOOLS_DIR/fuzzer"
+        fuzzer:     ninja -C "$BUILD_TOOLS_DIR/fuzzer"
 
         list of targets:
-                    make -C "$BUILD_TOOLS_DIR/" help
+                    ninja -C "$BUILD_TOOLS_DIR/" help
 EOFUSAGE
+
+       warn_if_incremental_build
+}
+
+warn_if_incremental_build()
+{
+        $warn_incremental_build || return 0
+        cat <<EOF
+
+WARNING: building tools/ is now incremental by default!
+         To build from scratch delete: $BUILD_TOOLS_DIR
+         or use the -C option.
+
+EOF
 }
 
 main()
@@ -107,6 +123,9 @@ main()
         DO_BUILD_topologies=false
         CMAKE_ONLY=false
 
+        # better safe than sorry
+        local warn_incremental_build=true
+
         # eval is a sometimes necessary evil
         # shellcheck disable=SC2034
         while getopts "cfhlptTC" OPTION; do
@@ -123,19 +142,25 @@ main()
                 esac
         done
         shift "$((OPTIND - 1))"
-        reconfigure_build
 
         if "$CMAKE_ONLY"; then
+                reconfigure_build
                 print_build_info
                 exit
         fi
 
+        if ! test -e "$BUILD_TOOLS_DIR"/CMakeCache.txt; then
+            warn_incremental_build=false
+            reconfigure_build
+        fi
+
         if "$BUILD_ALL"; then
                 # default CMake targets
                 make_tool # trust set -e
 
-                make_fuzzer
-                exit $?
+                make_fuzzer # trust set -e
+                warn_if_incremental_build
+                exit 0
         fi
 
         # Keep 'topologies' first because it's the noisiest.
@@ -154,6 +179,8 @@ main()
         if "$DO_BUILD_fuzzer"; then
                 make_fuzzer
         fi
+
+        warn_if_incremental_build
 }
 
 main "$@"

tools/topology/CMakeLists.txt

@@ -28,21 +28,30 @@ macro(add_alsatplg_command)
 	)
 endmacro()
 
-macro(add_alsatplg2_command input_file output_file include_path)
+macro(add_alsatplg2_command conf_header conf_target input_name output_name include_path)
 	# command line definitions are optional
 	# Set defines if there is an optional argument
-	if (${ARGC} GREATER 3)
-		set (defines ${ARGV3})
+	if (${ARGC} GREATER 5)
+		set (defines ${ARGV5})
 	else()
 		set (defines "")
 	endif()
 
 	add_custom_command(
-		MAIN_DEPENDENCY ${input_file}
-		OUTPUT ${output_file}
+		MAIN_DEPENDENCY ${input_name}.conf
+		# The conf_header file is the real dependency that
+		# triggers a rebuild. conf_target is the target that
+		# avoids a race to rebuild conf_header. See the CMake FAQ
+		# or (better) Sam Thursfield's blog about custom
+		# targets.
+		DEPENDS ${conf_header} ${conf_target}
+		OUTPUT ${output_name}.tplg
+
+		COMMAND cat ${conf_header} ${input_name}.conf > ${output_name}.conf
+
 		# -p to pre-process Topology2.0 conf file
 		COMMAND ALSA_CONFIG_DIR=${CMAKE_SOURCE_DIR}/topology/topology2 alsatplg \$\${VERBOSE:+-v 1}
-		        -I ${include_path} -D "'${defines}'" -p -c ${input_file} -o ${output_file}
+		        -I ${include_path} -D "'${defines}'" -p -c ${output_name}.conf -o ${output_name}.tplg
 		USES_TERMINAL
 		)
 endmacro()

tools/topology/topology1/CMakeLists.txt

@@ -9,8 +9,12 @@ file(GLOB TPLG_DEPS
 )
 
 add_custom_target(abi_v1
+	DEPENDS abi.h
+)
+add_custom_command(OUTPUT abi.h
 	COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/get_abi.sh ${SOF_ROOT_SOURCE_DIRECTORY}
-	DEPENDS ${TPLG_DEPS}
+	DEPENDS ${TPLG_DEPS} ${SOF_ROOT_SOURCE_DIRECTORY}/src/include/kernel/abi.h
+	# get_abi.h generates (a smaller) abi.h in the current directory
 	WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
 	VERBATIM
 	USES_TERMINAL
@@ -316,6 +320,8 @@ foreach(tplg ${TPLGS})
 		endforeach()
 	endif()
 
+	# m4
+	# TODO: reduce duplication with subdirectories
 	add_custom_command(
 		OUTPUT production/${output}.conf
 		COMMAND m4 --fatal-warnings
@@ -329,11 +335,16 @@ foreach(tplg ${TPLGS})
 			${CMAKE_CURRENT_SOURCE_DIR}/${input}.m4
 			> production/${output}.conf
 		MAIN_DEPENDENCY ${CMAKE_CURRENT_SOURCE_DIR}/${input}.m4
-		DEPENDS abi_v1
+		# The 'abi.h' file is the real dependency that triggers
+		# a rebuild. 'abi_v1' is the target that avoids a race
+		# to rebuild abi.h. See the CMake FAQ or (better) Sam
+		# Thursfield's blog about custom targets.
+		DEPENDS abi_v1 ${CMAKE_CURRENT_BINARY_DIR}/abi.h
 		VERBATIM
 		USES_TERMINAL
 	)
 
+	# alsatplg
 	add_alsatplg_command(production/${output}.conf production/${output}.tplg)
 
 	add_custom_target(topology1_${output} DEPENDS production/${output}.tplg)

tools/topology/topology1/development/CMakeLists.txt

@@ -89,15 +89,15 @@ foreach(tplg ${TPLGS})
 			${CMAKE_CURRENT_SOURCE_DIR}/../common/abi.m4
 			${CMAKE_CURRENT_SOURCE_DIR}/${input}.m4
 			> ${output}.conf
-		DEPENDS abi_v1
+		DEPENDS abi_v1 ${CMAKE_BINARY_DIR}/topology/topology1/abi.h
 		VERBATIM
 		USES_TERMINAL
 	)
 
 	add_alsatplg_command(${output}.conf ${output}.tplg)
 
-	add_custom_target(topology_${output} DEPENDS ${output}.tplg)
-	add_dependencies(dev_topologies1 topology_${output})
+	add_custom_target(dev_topology_${output} DEPENDS ${output}.tplg)
+	add_dependencies(dev_topologies1 dev_topology_${output})
 endforeach()
 
 # Duplicate of above to handle topologies in parent directory
@@ -125,7 +125,7 @@ foreach(tplg ${TPLGS_UP})
 			${CMAKE_CURRENT_SOURCE_DIR}/../common/abi.m4
 			${CMAKE_CURRENT_SOURCE_DIR}/../${input}.m4
 			> ${output}.conf
-		DEPENDS abi_v1
+		DEPENDS abi_v1 ${CMAKE_BINARY_DIR}/topology/topology1/abi.h
 		VERBATIM
 		USES_TERMINAL
 	)

tools/topology/topology1/dsp_enhancement/CMakeLists.txt

@@ -34,13 +34,13 @@ foreach(tplg ${TPLGS_UP})
 			${CMAKE_CURRENT_SOURCE_DIR}/../common/abi.m4
 			${CMAKE_CURRENT_SOURCE_DIR}/../${input}.m4
 			> ${output}.conf
-		DEPENDS abi_v1
+		DEPENDS abi_v1 ${CMAKE_BINARY_DIR}/topology/topology1/abi.h
 		VERBATIM
 		USES_TERMINAL
 	)
 
 	add_alsatplg_command(${output}.conf ${output}.tplg)
 
-	add_custom_target(topology_${output} DEPENDS ${output}.tplg)
-	add_dependencies(dsp_topologies1 topology_${output})
+	add_custom_target(dsp_topology_${output} DEPENDS ${output}.tplg)
+	add_dependencies(dsp_topologies1 dsp_topology_${output})
 endforeach()

tools/topology/topology1/kernel_dependent/v5.19/CMakeLists.txt

@@ -73,12 +73,12 @@ foreach(tplg ${TPLGS_UP})
 			${CMAKE_CURRENT_SOURCE_DIR}/../../common/abi.m4
 			${CMAKE_CURRENT_SOURCE_DIR}/../../${input}.m4
 			> ${output}.conf
-		DEPENDS abi_v1
+		DEPENDS abi_v1 ${CMAKE_BINARY_DIR}/topology/topology1/abi.h
 		VERBATIM
 		USES_TERMINAL
 	)
 	add_alsatplg_command(${output}.conf ${output}.tplg)
 
-	add_custom_target(topology_${output} DEPENDS ${output}.tplg)
-	add_dependencies(kernel_dependent_v5_19_topologies1 topology_${output})
+	add_custom_target(k519_topology_${output} DEPENDS ${output}.tplg)
+	add_dependencies(kernel_dependent_v5_19_topologies1 k519_topology_${output})
 endforeach()

tools/topology/topology2/cavs/CMakeLists.txt

@@ -46,9 +46,12 @@ PREPROCESS_PLUGINS=nhlt,NHLT_BIN=nhlt-ace-mtl-nocodec.bin"
 )
 
 # generate ABI for IPC4
-execute_process(
-	COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/../get_abi.sh ${SOF_ROOT_SOURCE_DIRECTORY} "ipc4"
-	OUTPUT_FILE ${CMAKE_CURRENT_BINARY_DIR}/abi.conf
+add_custom_command(OUTPUT abi.conf
+	COMMAND > abi.conf  ${CMAKE_CURRENT_SOURCE_DIR}/../get_abi.sh ${SOF_ROOT_SOURCE_DIRECTORY} "ipc4"
+	DEPENDS ${SOF_ROOT_SOURCE_DIRECTORY}/src/include/kernel/abi.h
+)
+add_custom_target(cavs_abi_target
+	DEPENDS abi.conf
 )
 
 add_custom_target(topology2_cavs)
@@ -68,15 +71,9 @@ foreach(tplg ${TPLGS})
 		list(GET tplg ${last_index} defines)
 	endif()
 
-	# copy ABI and input conf file contents
-	#
-	# BUG: move this from CMake configuration time to build time to
-	# fix incremental builds
-	configure_file(${CMAKE_CURRENT_BINARY_DIR}/abi.conf ${CMAKE_CURRENT_BINARY_DIR}/${output}.conf)
-	file(READ ${CMAKE_CURRENT_SOURCE_DIR}/${input}.conf CONTENTS)
-	file(APPEND ${CMAKE_CURRENT_BINARY_DIR}/${output}.conf "${CONTENTS}")
-
-	add_alsatplg2_command("${output}.conf" "${output}.tplg" "${CMAKE_CURRENT_SOURCE_DIR}" "${defines}")
+	add_alsatplg2_command("${CMAKE_CURRENT_BINARY_DIR}/abi.conf" cavs_abi_target
+	  "${CMAKE_CURRENT_SOURCE_DIR}/${input}" "${output}"
+	  "${CMAKE_CURRENT_SOURCE_DIR}" "${defines}")
 	add_custom_target(topology2_${output} DEPENDS ${output}.tplg)
 	add_dependencies(topology2_cavs topology2_${output})
 endforeach()

tools/topology/topology2/get_abi.sh

@@ -36,4 +36,4 @@ case "$2" in
 	ipc4) print_2bytes "$ABI_PATCH" ;;
 esac
 
-printf '\"\n\t}\n}'
+printf '\"\n\t}\n}\n'