Merge branch 'fix_make_clean' into 'master'

E3-1378: Fix make clean

See merge request e3/e3-require!150

require-ess/tools/driver.makefile

@@ -59,6 +59,11 @@ MAKEHOME:=$(dir $(lastword ${MAKEFILE_LIST}))
 # Get the name of the Makefile that included this file.
 USERMAKEFILE:=$(lastword $(filter-out $(lastword ${MAKEFILE_LIST}), ${MAKEFILE_LIST}))
 
+# These are the targets that we will pass through to the next stages of require's
+# recursive build process. For each of these targets we will perform all three
+# of the build runs listed above; for others (e.g. `make clean`) we only perform
+# a single pass.
+RECURSE_TARGETS = install build debug db_internal
 
 ##---## In E3, We only use one version of EPICS base when compiling modules.
 ##---## EPICS_LOCATION is (by default) /epics/base-${EPICSVERSION}
@@ -160,6 +165,12 @@ define fetch_module_versions
   endif
 endef
 
+# This is used to ensure that relative/absolute paths in e.g. USR_DBFLAGS are
+# correct, relative to the build directory.
+define fix_relative_paths
+$(foreach t,$(patsubst -I%,-I %,$1),$(if $(filter -I,$t),$t,$(if $(filter /%,$t),$t,../$t)))
+endef
+
 # Functions used for recursive dependency fetching. These are modified from https://github.com/markpiffer/gmtt.git
 space := $(strip) $(strip)#
 comma := ,#
@@ -177,6 +188,38 @@ select = $(strip $(call -select,$(strip $2),$1,$3))
 str-eq = $(if $(subst x$1,,x$2),,t)
 # End of functions from https://github.com/markpiffer/gmtt.git
 
+# Fetches the data from .dep files to be parsed by the above
+define fetch_deps
+$(shell cat $(or \
+  $(lastword $(wildcard $(addsuffix /$1/$($1_VERSION)/lib/$(T_A)/$1.dep,$(E3_SITEMODS_PATH) $(EPICS_MODULES)))),\
+  $(error Module '$1' version '$($1_VERSION)' does not exist.)) \
+  | sed '1d')
+endef
+
+# Used to recurse through versions: recursively fetches all of the dependencies
+# from the given module
+define update_dep_versions
+  m := $$(firstword $$(MODULES))
+  PROCESSED_MODULES += $$m
+  $$m_TBL := $$(call fetch_deps,$$m)
+  $$m_DEPS := $$(call select,1,$$($$m_TBL),1)
+  MODULES := $$(filter-out $$(PROCESSED_MODULES),$$(MODULES) $$($$m_DEPS))
+  # Fetch dependency versions on the .dep table then check it agains already
+  # processed requirements.
+  # If module_VERSION don't exists create it with module_FETCHED_VERSION value.
+  # If module_VERSION already exists fails if it doesn't matches
+  # module_FETCHED_VERSION.
+  $$(foreach mm,$$($$m_DEPS),\
+    $$(eval $$(mm)_FETCHED_VERSION := $$(call select,2,$$($$m_TBL),$$$$(call str-eq,$$$$1,$$(mm))))\
+    $$(if $$($$(mm)_VERSION),\
+       $$(if $$(filter-out $$($$(mm)_FETCHED_VERSION),$$($$(mm)_VERSION)),\
+         $$(error "$$(m) depends on $$(mm),$$($$(mm)_FETCHED_VERSION) but $$(mm),$$($$(mm)_VERSION) is also needed"),\
+         ),\
+      $$(eval $$(mm)_VERSION := $$($$(mm)_FETCHED_VERSION))\
+      )\
+    )
+endef
+
 # Some TOP and EPICS_BASE tweeking necessary to work around release check in 3.14.10+.
 EB:=${EPICS_BASE}
 TOP:=${EPICS_BASE}
@@ -273,25 +316,6 @@ BUILD_ARCHS = $(filter-out $(addprefix %,${EXCLUDE_ARCHS}),\
 SRCS_Linux = ${SOURCES_Linux}
 export SRCS_Linux
 
-# Perform default database expansion of .substitions/.templates into $(COMMON_DIR)
-db_internal: $(COMMON_DIR)
-
--include $(COMMON_DIR)/*.db.d
-
-VPATH += $(dir $(TMPS))
-VPATH += $(dir $(SUBS))
-
-$(COMMON_DIR)/%.db: %.substitutions
-	@printf "Inflating database ... %44s >>> %40s \n" "$^" "$@"
-	$(QUIET)$(MSI) -D $(USR_DBFLAGS) -o $(COMMON_DIR)/$(notdir $(basename $@).db) -S $^ > $(COMMON_DIR)/$(notdir $(basename $@).db).d
-	$(QUIET)$(MSI)    $(USR_DBFLAGS) -o $(COMMON_DIR)/$(notdir $(basename $@).db) -S $^
-
-$(COMMON_DIR)/%.db: %.template
-	@printf "Inflating database ... %44s >>> %40s \n" "$^" "$@"
-	$(QUIET)$(MSI) -D $(USR_DBFLAGS) -o $(COMMON_DIR)/$(notdir $(basename $@).db) $^ > $(COMMON_DIR)/$(notdir $(basename $@).db).d
-	$(QUIET)$(MSI)    $(USR_DBFLAGS) -o $(COMMON_DIR)/$(notdir $(basename $@).db) $^
-
-
 ${COMMON_DIR}/${METAFILE}:
 	@echo "wrapper_url: '$(${PRJ}_E3_GIT_URL)'" > $@
 	@echo "wrapper_git_desc: '$(${PRJ}_E3_GIT_DESC)'" >> $@
@@ -301,9 +325,11 @@ ${INSTALL_META}: ${COMMON_DIR}/${METAFILE}
 	@echo "Installing metadata file $@"
 	$(INSTALL) -d -m444 $< $(@D)
 
-install build debug::
+$(RECURSE_TARGETS)::
 	@echo "MAKING EPICS VERSION ${EPICSVERSION}"
 
+build db_internal:: $(COMMON_DIR)
+
 debug::
 	@echo "===================== Pass 1 ====================="
 	@echo "BUILDCLASSES = ${BUILDCLASSES}"
@@ -320,36 +346,21 @@ debug::
 # Create e.g. build-$(T_A) rules for each architecture, so that we can just do
 #   build: build-arch1 build-arch2
 define target_rule
-$1-%: | $(COMMON_DIR)
+$1-%:
 	$${MAKE} -f $${USERMAKEFILE} T_A=$$* $1
 endef
-$(foreach target,install build debug,$(eval $(call target_rule,$(target))))
+$(foreach target,$(RECURSE_TARGETS),$(eval $(call target_rule,$(target))))
 
-MODULES :=
-REQ :=
 INSTALLED_MODULES := $(sort $(notdir $(wildcard $(E3_SITEMODS_PATH)/* $(EPICS_MODULES)/*)))
 export INSTALLED_MODULES
 
-# Converts all of the X_DEP_VERSIONs to x_VERSION and records them
-# Calls fetch_module_version for each module that appears in a
-# module_DEP_VERSION variable. This is defined on the wrapper CONFIG_MODULE
-$(foreach m,$(patsubst %_DEP_VERSION,%,$(filter %_DEP_VERSION,$(.VARIABLES))),$(eval $(call fetch_module_versions,$m)))
-export REQ
-export MODULES
 
-$(foreach m,$(MODULES),$(eval export $m_VERSION))
 
 .SECONDEXPANSION:
 
-# This has to fit under .SECONDEXPANSION in order to catch TMPS and SUBS, which are typically defined
-# _after_ driver.makefile is included.
-db_internal: $$(addprefix $(COMMON_DIR)/,$$(notdir $$(patsubst %.template,%.db,$$(TMPS))))
-
-db_internal: $$(addprefix $(COMMON_DIR)/,$$(notdir $$(patsubst %.substitutions,%.db,$$(SUBS))))
-
 # This has to be after .SECONDEXPANSION since BUILD_ARCHS will be modified based on EXCLUDE_ARCHS
 # which is defined _after_ driver.makefile.
-$(foreach target,install build debug,$(eval $(target):: $$$$(foreach arch,$$$${BUILD_ARCHS},$(target)-$$$${arch})))
+$(foreach target,$(RECURSE_TARGETS),$(eval $(target):: $$$$(foreach arch,$$$${BUILD_ARCHS},$(target)-$$$${arch})))
 
 # Create and install the metadata file as the very last step
 build:: ${COMMON_DIR}/${METAFILE}
@@ -382,54 +393,9 @@ ARCH_PARTS = ${T_A} $(subst -, ,${T_A}) ${OS_CLASS}
 VAR_EXTENSIONS = ${EPICSVERSION} ${ARCH_PARTS} ${ARCH_PARTS:%=${EPICSVERSION}_%}
 export VAR_EXTENSIONS
 
-# Look for dependencies on extension files the same way as above
-$(foreach x,$(VAR_EXTENSIONS),\
-  $(foreach m,$(patsubst %_DEP_VERSION_$(x),%,$(filter %_DEP_VERSION_$(x),$(.VARIABLES))),$(eval $(call fetch_module_versions,$m,_$(x))))\
-)
-export REQ
-export MODULES
-
-PROCESSED_MODULES :=
-# Fetches the data from .dep files to be parsed by the above
-define fetch_deps
-$(shell cat $(or \
-  $(lastword $(wildcard $(addsuffix /$1/$($1_VERSION)/lib/$(T_A)/$1.dep,$(E3_SITEMODS_PATH) $(EPICS_MODULES)))),\
-  $(error Module '$1' version '$($1_VERSION)' does not exist.)) \
-  | sed '1d')
-endef
-
-# Used to recurse through versions: recursively fetches all of the dependencies
-# from the given module
-define update_dep_versions
-  m := $$(firstword $$(MODULES))
-  PROCESSED_MODULES += $$m
-  $$m_TBL := $$(call fetch_deps,$$m)
-  $$m_DEPS := $$(call select,1,$$($$m_TBL),1)
-  MODULES := $$(filter-out $$(PROCESSED_MODULES),$$(MODULES) $$($$m_DEPS))
-  # Fetch dependency versions on the .dep table then check it agains already
-  # processed requirements.
-  # If module_VERSION don't exists create it with module_FETCHED_VERSION value.
-  # If module_VERSION already exists fails if it doesn't matches
-  # module_FETCHED_VERSION.
-  $$(foreach mm,$$($$m_DEPS),\
-    $$(eval $$(mm)_FETCHED_VERSION := $$(call select,2,$$($$m_TBL),$$$$(call str-eq,$$$$1,$$(mm))))\
-    $$(if $$($$(mm)_VERSION),\
-       $$(if $$(filter-out $$($$(mm)_FETCHED_VERSION),$$($$(mm)_VERSION)),\
-         $$(error "$$(m) depends on $$(mm),$$($$(mm)_FETCHED_VERSION) but $$(mm),$$($$(mm)_VERSION) is also needed"),\
-         ),\
-      $$(eval $$(mm)_VERSION := $$($$(mm)_FETCHED_VERSION))\
-      )\
-    )
-endef
-$(call while,$$(MODULES),$(update_dep_versions))
-
-$(foreach m,$(PROCESSED_MODULES),$(eval export $m_VERSION))
-
-
 debug::
 	@echo "===================== Pass 2: T_A = $(T_A) ====================="
 	@echo "BINS = $(BINS)"
-	@echo "REQ = $(REQ)"
 	@echo "VLIBS = $(VLIBS)"
 
 ifeq ($(filter ${OS_CLASS},${OS_CLASS_LIST}),)
@@ -453,7 +419,7 @@ else
 install:: build
 	$(if $(wildcard ${MODULE_LOCATION}/lib/${T_A}),$(error ${MODULE_LOCATION}/lib/${T_A} already exists. If you really want to overwrite then uninstall first.))
 
-install build debug:: O.${EPICSVERSION}_${T_A}
+$(RECURSE_TARGETS):: O.${EPICSVERSION}_${T_A}
 	@${MAKE} -C O.${EPICSVERSION}_${T_A} -f ../${USERMAKEFILE} $@
 
 endif
@@ -468,10 +434,30 @@ export BINS
 VLIBS = $(VENDOR_LIBS) $(foreach x,$(VAR_EXTENSIONS),$(VENDOR_LIBS_$x))
 export VLIBS
 
+export USR_DBFLAGS
+export TMPS
+export SUBS
+
 else # in O.*
 ## RUN 3
 # In build directory.
 
+MODULES :=
+REQ :=
+
+# Converts all of the X_DEP_VERSIONs to x_VERSION and records them
+# Calls fetch_module_version for each module that appears in a
+# module_DEP_VERSION variable. This is defined on the wrapper CONFIG_MODULE
+$(foreach m,$(patsubst %_DEP_VERSION,%,$(filter %_DEP_VERSION,$(.VARIABLES))),$(eval $(call fetch_module_versions,$m)))
+
+# Look for dependencies on extension files the same way as above
+$(foreach x,$(VAR_EXTENSIONS),\
+  $(foreach m,$(patsubst %_DEP_VERSION_$(x),%,$(filter %_DEP_VERSION_$(x),$(.VARIABLES))),$(eval $(call fetch_module_versions,$m,_$(x))))\
+)
+
+PROCESSED_MODULES :=
+$(call while,$$(MODULES),$(update_dep_versions))
+
 # Add macros like USR_CFLAGS_Linux.
 EXTENDED_VARS=INCLUDES CFLAGS CXXFLAGS CPPFLAGS CODE_CXXFLAGS LDFLAGS
 $(foreach v,${EXTENDED_VARS},$(foreach x,${VAR_EXTENSIONS},$(eval $v+=$${$v_$x}) $(eval USR_$v+=$${USR_$v_$x})))
@@ -600,6 +586,8 @@ build: MODULEINFOS
 build: ${MODULEDBD}
 build: ${DEPFILE}
 
+db_internal:
+
 COMMON_INC = ${RECORDS:%=${COMMON_DIR}/%.h}
 
 # Include default EPICS Makefiles (version dependent).
@@ -615,6 +603,26 @@ ifeq (,$(findstring debug,${MAKECMDGOALS}))
   endif
 endif
 
+DBDEPENDS_FILES = $(wildcard $(COMMON_DIR)/*.db.d)
+ifneq (,$(DBDEPENDS_FILES))
+-include $(DBDEPENDS_FILES)
+endif
+
+USR_DBFLAGS := $(call fix_relative_paths,$(USR_DBFLAGS))
+
+define SUBS_EXPAND
+db_internal: $(COMMON_DIR)/$(notdir $(basename $2).db)
+
+# Note that this rule overrides the one from RULES.Db from EPICS_BASE
+$(COMMON_DIR)/$(notdir $(basename $2).db): $(if $(filter /%,$2),$2,../$2)
+	@printf "Inflating database ... %44s >>> %40s \n" "$$<" "$$@"
+	$(MSI) -D $$(USR_DBFLAGS) -o $(COMMON_DIR)/$$(notdir $$(basename $2).db) $1 $$< > $(COMMON_DIR)/$$(notdir $$(basename $2).db).d
+	$(MSI)    $$(USR_DBFLAGS) -o $(COMMON_DIR)/$$(notdir $$(basename $2).db) $1 $$<
+endef
+
+$(foreach file,$(TMPS),$(eval $(call SUBS_EXPAND,,$(file))))
+$(foreach file,$(SUBS),$(eval $(call SUBS_EXPAND,-S,$(file))))
+
 # Fix incompatible release rules.
 RELEASE_DBDFLAGS = -I ${EPICS_BASE}/dbd
 RELEASE_INCLUDES = -I${EPICS_BASE}/include

tests/test_build.py

@@ -485,6 +485,7 @@ def test_recursive_header_include(wrappers):
 
 def test_updated_template_files(wrapper: Wrapper):
     wrapper.add_var_to_module_makefile("SUBS", "x.substitutions")
+    wrapper.add_var_to_module_makefile("USR_DBFLAGS", "-I.")
 
     substitution_file = wrapper.module_dir / "x.substitutions"
     substitution_file.write_text("file x.template {pattern {x} {y}}")
@@ -492,8 +493,7 @@ def test_updated_template_files(wrapper: Wrapper):
     template_file = wrapper.module_dir / "x.template"
     template_file.write_text("record(ai, initial) {}")
 
-    base_version = wrapper.get_env_var("EPICS_VERSION_NUMBER")
-    db_file = wrapper.module_dir / f"O.{base_version}_Common" / "x.db"
+    db_file = wrapper.module_dir / f"O.{wrapper.base_version}_Common" / "x.db"
 
     rc, *_ = wrapper.run_make("db_internal")
     assert rc == 0
@@ -529,8 +529,7 @@ def test_expand_db_files(wrapper: Wrapper):
 """
     )
 
-    base_version = wrapper.get_env_var("EPICS_VERSION_NUMBER")
-    common_dir = wrapper.module_dir / f"O.{base_version}_Common"
+    common_dir = wrapper.module_dir / f"O.{wrapper.base_version}_Common"
 
     rc, *_ = wrapper.run_make("db_internal")
     assert rc == 0
@@ -586,6 +585,7 @@ def test_substitution_template_priority(wrapper: Wrapper):
 
     wrapper.add_var_to_module_makefile("TMPS", "a.template")
     wrapper.add_var_to_module_makefile("SUBS", "a.substitutions")
+    wrapper.add_var_to_module_makefile("USR_DBFLAGS", "-I.")
 
     template_file = wrapper.module_dir / "a.template"
     template_file_contents = "record (ai, $(FOO)) {}"
@@ -601,8 +601,7 @@ def test_substitution_template_priority(wrapper: Wrapper):
 """
     )
 
-    base_version = wrapper.get_env_var("EPICS_VERSION_NUMBER")
-    common_dir = wrapper.module_dir / f"O.{base_version}_Common"
+    common_dir = wrapper.module_dir / f"O.{wrapper.base_version}_Common"
 
     rc, *_ = wrapper.run_make("db_internal")
     assert rc == 0

tests/test_e3.py

@@ -72,3 +72,10 @@ def test_debug_arch_is_used_when_specified(wrapper: Wrapper):
 
     # Note that output from run-iocsh goes to stderr
     assert f'EPICS_HOST_ARCH = "{debug_arch}"' in errs
+
+
+def test_make_clean_should_run_correctly_with_missing_dependencies(wrapper: Wrapper):
+    wrapper.add_var_to_config_module("FOO_DEP_VERSION", "bar")
+
+    rc, *_ = wrapper.run_make("clean")
+    assert rc == 0