tooling: Improve Makefile maintainability

- Fixed incorrect use of .DEFAULT target.

- Refactor .PHONY targets to be stated next to their targets,
  instead of all in one line. This makes it easier to update
  code without forgetting to maintain the .PHONY targets.

Author: cassava

Makefile

@@ -26,8 +26,8 @@ LOCKFILE_OPTION := --lockfile="${CLOE_ROOT}/${BUILD_LOCKFILE}"
 INSTALL_DIR     := /usr/local
 CONAN_OPTIONS   :=
 
+.DEFAULT_GOAL := help
 .PHONY: help
-.DEFAULT: help
 .SILENT: help
 help::
 	$(call print_help_usage)
@@ -37,7 +37,6 @@ help::
 include Makefile.setup
 
 # Workspace targets -----------------------------------------------------------
-.PHONY: lockfile status deploy sphinx doxygen purge-all export-cli smoketest smoketest-deps
 help::
 	$(call print_help_section, "Available workspace targets")
 	$(call print_help_target, status, "show status of each of the Conan packages")
@@ -53,55 +52,65 @@ help::
 ${BUILD_LOCKFILE}:
 	${MAKE} -f Makefile.package SOURCE_CONANFILE=/dev/null LOCKFILE_SOURCE=${LOCKFILE_SOURCE} ${BUILD_LOCKFILE}
 
+.PHONY: lockfile
 lockfile: ${BUILD_LOCKFILE}
 
+.PHONY: status
 status: ${BUILD_LOCKFILE}
 	@for pkg in ${ALL_PKGS}; do \
 		[ -d $${pkg} ] || continue; \
 		${MAKE} LOCKFILE_SOURCE="" LOCKFILE_OPTION=${LOCKFILE_OPTION} -C $${pkg} status || true; \
 	done
 
+.PHONY: deploy
 deploy:
 	$(call print_header, "Deploying binaries to ${INSTALL_DIR}...")
 	conan install ${CONAN_OPTIONS} --install-folder ${BUILD_DIR}/deploy -g deploy .
 	mkdir -p ${INSTALL_DIR}
 	cp -r ${BUILD_DIR}/deploy/cloe-*/* ${INSTALL_DIR}/
 
+.PHONY: deploy-cli
 deploy-cli:
 	$(call print_header, "Deploying cloe-launch binary with pip...")
 	${MAKE} -C cli install
 
+.PHONY: export-cli
 export-cli:
 	${MAKE} -C cli conan-profile
 
+.PHONY: sphinx
 sphinx:
 	$(call print_header, "Generating Sphinx documentation...")
 	mkdir -p ${BUILD_DIR}/sphinx
 	${MAKE} -C docs html
 
-doxygen:keyboard tilt
+.PHONY: doxygen
+doxygen:
 	$(call print_header, "Generating Doxygen documentation...")
 	mkdir -p ${BUILD_DIR}/doxygen
 	doxygen Doxyfile
 
+.PHONY: smoketest-deps
 smoketest-deps: export-cli smoketest-deps-select
 
+.PHONY: smoketest
 smoketest: smoketest-select
 
+.PHONY: purge-all
 purge-all:
 	$(call print_header, "Removing all cloe Conan packages...")
 	conan remove -f 'cloe-*'
 	conan remove -f 'cloe'
 	conan remove -f 'fable'
 
 # Development targets ---------------------------------------------------------
-.PHONY: format todos find-missing-eol sanitize-files
 help::
 	$(call print_help_section, "Available development targets")
 	$(call print_help_target, format, "format Cloe source code with clang-format")
 	$(call print_help_target, todos, "show all TODOs in Cloe source code")
 	echo
 
+.PHONY: format
 format:
 	# When you do this, isolate the change in a single commit and then add the
 	# commit hash to a file such as .git-blame-ignore-revs, so that git-blame
@@ -110,17 +119,21 @@ format:
 	# See: https://www.moxio.com/blog/43/ignoring-bulk-change-commits-with-git-blame
 	find . -type f -not -path '*/\.git/*' -and \( -name '*.cpp' -o -name '*.hpp' \) -exec ${CLANG_FORMAT} ${CLANG_FORMAT_ARGS} -i {} \;
 
+.PHONY: todos
 todos:
 	${AG} TODO
 	${AG} FIXME
 	${AG} XXX
 
+.PHONY: grep-uuids
 grep-uuids:
 	${AG} "\b[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}\b"
 
+.PHONY: find-missing-eol
 find-missing-eol:
 	find . -type f -size +0 -exec gawk 'ENDFILE{if ($0 == "") print FILENAME}' {} \;
 
+.PHONY: sanitize-files
 sanitize-files:
 	git grep --cached -Ilz '' | while IFS= read -rd '' f; do tail -c1 < "$$f" | read -r _ || echo >> "$$f"; done
 

Makefile.all

@@ -137,8 +137,8 @@ endef
 
 # --------------------------------------------------------------------------- #
 
+.DEFAULT_GOAL := help
 .PHONY: help
-.DEFAULT: help
 .SILENT: help
 help::
 	$(call print_help_section, "Available build targets")

Makefile.help

@@ -90,14 +90,57 @@ define print_help_define_align
 	@printf "    ${_grn}%-16s${_rst}=${_dim} %s${_rst}\n" ${1} ${2}
 endef
 
-# Usage: $(call print_help_define_lines, VAR-NAME, ${VAR-NAME}}
+# Usage: $(call print_help_define_lines, VAR-NAME, ${VAR-NAME})
+#
+# For example, given a call with the following:
+#
+#     $(call print_help_define_lines, FROM_VERSIONS, 18.04 20.04 22.04)
+#
+# we will get the following output:
+#
+#     FROM_VERSIONS=\
+#       18.04
+#       20.04
+#       22.04
+#
+# If the contents of ${VAR-NAME} is empty or only contains whitespace,
+# then no extra newline is printed, and it behaves as print_help_define.
 define print_help_define_lines
-	@printf "    ${_grn}%s${_rst}=${_dim}\\ \n" ${1}
+	@printf "    ${_grn}%s${_rst}=${_dim}" ${1}
+	@if [ -n "$(strip ${2})" ]; then printf "\\ \n"; fi
 	@printf "%s" "$$(echo "     ${2}" | fmt -5)"
 	@printf "${_rst}\n"
 endef
 
+# Usage: $(call print_help_args_lines, VAR-NAME, ${VAR-NAME})
+#
+# This function behaves as print_help_define_lines, except that
+# it doesn't split on words, but on arguments. Each argument (leading with -)
+# is prepended with a newline.
+#
+# For example, given a call with the following:
+#
+#     $(call print_help_define_lines, DOCKER_ARGS, -it --rm --network host)
+#
+# we will get the following output:
+#
+#     DOCKER_ARGS=\
+#         -it
+#         --rm
+#         --network host
+#
+# If the contents of ${VAR-NAME} is empty, same behavior as print_help_define.
+define print_help_args_lines
+	@printf "    ${_grn}%s${_rst}=${_dim}" ${1}
+	@if [ -n "$(strip ${2})" ]; then printf "\\ \n"; fi
+	@printf "%s" "$$(echo "     ${2}" | sed -r -e 's/\W--?[^-]/\n     \0/g' -e 's/^\s*\n//')"
+	@printf "${_rst}\n"
+endef
+
 # Usage: $(call print_help_usage)
+#
+# This should be called only once, before all other print_help_* calls,
+# so that the help message starts with a usage statement.
 define print_help_usage
 	@printf "Usage: make ${_blu}target${_rst}\n"
 endef

Makefile.package

@@ -132,8 +132,8 @@ endif
 
 # INFORMATIONAL TARGETS -------------------------------------------------------
 .DEFAULT_GOAL := help
-.SILENT: help status parse-info parse-editable info-name info-version info-channel info-fqn info smoketest smoketest-deps
-.PHONY: help status parse-info parse-editable info-name info-version info-channel info-fqn info smoketest smoketest-deps
+.SILENT: help
+.PHONY: help
 help:: parse-info parse-editable
 	$(call print_help_usage)
 	echo
@@ -193,6 +193,8 @@ endif
 	$(call print_help_define_align, GIT_COMMIT_HASH, ${GIT_COMMIT_HASH})
 	echo
 
+.PHONY: parse-info
+.SILENT: parse-info
 parse-info: ${BUILD_LOCKFILE}
 	# Fetch package information from Conan.
 	#
@@ -205,9 +207,13 @@ parse-info: ${BUILD_LOCKFILE}
 	$(eval PACKAGE_DIR := $(shell echo -e "${PACKAGE_INFO}" | sed -rn 's/^ *package_folder: *(.*)$$/\1/p'))
 	$(eval PACKAGE_DATE := $(shell echo -e "${PACKAGE_INFO}" | sed -rn 's/^ *Creation date: *(.*)$$/\1/p'))
 
+.PHONY: parse-editable
+.SILENT: parse-editable
 parse-editable:
 	$(eval PACKAGE_EDITABLE := $(shell [ -e ~/.conan/editable_packages.json ] && jq -r "${JQ_PACKAGE_EDITABLE}" ~/.conan/editable_packages.json || echo "not-editable"))
 
+.PHONY: status
+.SILENT: status
 status: parse-info parse-editable
 	# Show the *approximate* status of each package in the cloe workspace.
 	#
@@ -226,42 +232,54 @@ status: parse-info parse-editable
 		fi \
 	fi
 
+.PHONY: info-name
+.SILENT: info-name
 info-name:
 	echo ${PACKAGE_NAME}
 
+.PHONY: info-version
+.SILENT: info-version
 info-version:
 	echo ${PACKAGE_VERSION}
 
+.PHONY: info-channel
+.SILENT: info-channel
 info-channel:
 	echo ${PACKAGE_CHANNEL}
 
+.PHONY: info-fqn
+.SILENT: info-fqn
 info-fqn:
 	echo ${PACKAGE_FQN}
 
+.PHONY: info
+.SILENT: info
 info: parse-info
 	if [ -z "${PACKAGE_INFO}" ]; then \
 		echo "Errors occurred, no output available."; \
 	else \
 		echo ${PACKAGE_INFO}; \
 	fi
 
+.PHONY: smoketest
 smoketest:
+	# Ensure that you have built all smoketest dependencies!
 	@for profile in tests/profile_*.py; do \
 		test -f "$${profile}" || continue; \
 		printf "Running BATS tests with profile: $${profile}\n\n"; \
 		SHELL=/bin/bash ${CLOE_LAUNCH} shell -P "$${profile}" -- -c "source ${PROJECT_ROOT}/tests/setup_testname.bash; bats tests"; \
 	done
 
 smoketest-deps:
+	# Ensure that you have exported all relevant packages!
 	@for profile in tests/profile_*.py; do \
 		test -f "$${profile}" || continue; \
 		echo "Building dependencies for profile: $${profile}"; \
 		${CLOE_LAUNCH} prepare -P "$${profile}"; \
 	done
 
 # CONFIGURATION TARGETS -------------------------------------------------------
-.PHONY: editable uneditable
-
+.PHONY: editable
 editable:
 	# Conan will now resolve references to the in-source build.
 	#
@@ -274,6 +292,7 @@ editable:
 	#
 	conan editable add . --layout "${BUILD_LAYOUT}" ${PACKAGE_FQN}
 
+.PHONY: uneditable
 uneditable:
 	# Conan will now resolve references to the package in the cache.
 	#
@@ -283,8 +302,7 @@ uneditable:
 	conan editable remove ${PACKAGE_FQN}
 
 # CONAN TARGETS ---------------------------------------------------------------
-.PHONY: export package package-all package-missing package-outdated purge list
-
+.PHONY: export
 export: parse-editable
 	# Export sources to Conan cache.
 	#
@@ -293,6 +311,7 @@ export: parse-editable
 	#
 	if [ "${PACKAGE_EDITABLE}" = "not-editable" ] || [ "${PACKAGE_EDITABLE}" = "editable-other-name" ]; then conan export . ${PACKAGE_FQN}; fi
 
+.PHONY: download
 download:
 	# Try to download the package to Conan cache.
 	#
@@ -307,6 +326,7 @@ download:
 	conan create . ${PACKAGE_FQN} --build=never ${ALL_OPTIONS} || \
 	conan create . ${PACKAGE_FQN} --build=${BUILD_POLICY} --build=${PACKAGE_NAME} ${ALL_OPTIONS}
 
+.PHONY: package
 package: ${BUILD_LOCKFILE}
 	# Build the package in Conan cache unconditionally.
 	#
@@ -318,6 +338,7 @@ package: ${BUILD_LOCKFILE}
 	conan create . ${PACKAGE_FQN} \
 		--build=${BUILD_POLICY} --build=${PACKAGE_NAME} ${ALL_OPTIONS}
 
+.PHONY: package-all
 package-all: ${BUILD_LOCKFILE}
 	# Build the package in Conan cache unconditionally.
 	#
@@ -326,6 +347,7 @@ package-all: ${BUILD_LOCKFILE}
 	#
 	conan create . ${PACKAGE_FQN} --build ${ALL_OPTIONS}
 
+.PHONY: package-outdated
 package-outdated: ${BUILD_LOCKFILE}
 	# Build the package in Conan cache if it is outdated.
 	#
@@ -335,6 +357,7 @@ package-outdated: ${BUILD_LOCKFILE}
 	#
 	conan create . ${PACKAGE_FQN} --build=outdated ${ALL_OPTIONS}
 
+.PHONY: purge
 purge:
 	# Remove all instances of this package in the Conan cache.
 	#
@@ -344,14 +367,14 @@ purge:
 	#
 	-conan remove -f ${PACKAGE_FQN}
 
+.PHONY: list
 list: parse-info
 	# List all files in the Conan cache package directory.
 	#
 	@tree ${PACKAGE_DIR}
 
 # IN-SOURCE TARGETS -----------------------------------------------------------
-.PHONY: all clean conan configure test export-pkg
-
+.PHONY: clean
 clean:
 	# Clean the build directory and Python cache files.
 	#
@@ -362,20 +385,25 @@ ifeq (${CLEAN_SOURCE_DIR}, true)
 endif
 
 ifeq (${BUILD_IN_SOURCE}, false)
+.PHONY: all conan configure test export-pkg
 all conan configure test export-pkg ${SOURCE_DIR} ${SOURCE_CMAKELISTS} ${BUILD_CONANINFO} ${BUILD_CMAKECACHE}:
 	@echo "Error: [in-source] targets are not supported for this package."
 	@echo "Note: please use [conan-cache] targets, such as 'package'."
 	exit 1
 else
+.PHONY: all
 all: ${BUILD_CONANINFO} | ${SOURCE_DIR}
 	# Build the package in-source.
 	#
 	conan build . --source-folder="${SOURCE_DIR}" --build-folder="${BUILD_DIR}"
 
+.PHONY: conan
 conan: ${BUILD_CONANINFO}
 
+.PHONY: configure
 configure: ${BUILD_CMAKECACHE}
 
+.PHONY: test
 test:
 	# Run tests available to CMake ctest.
 	#
@@ -387,6 +415,7 @@ test:
 		true; \
 	fi
 
+.PHONY: export-pkg
 export-pkg:
 	# Export in-source build artifacts to Conan cache.
 	#