Merge branch 'E3-879' into 'master'

E3-879: driver.Makefile: Add check for inconsistent versions between dependencies

See merge request e3/e3-require!88

CHANGELOG.md

@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   * Module dependecies are now fetched from `CONFIG_MODULE` in the sense that `X_DEP_VERSION` is parsed as a
     dependency on the module `x`
   * It is no longer necessary to specify `REQUIRED += ...` nor `x_VERSION = $(X_DEP_VERSION)` within the module makefile
+* Check inconsistent versions between dependencies at build time. If an inconsistency is found the build will fail.
 
 ### Bugfixes
 * Fixed issue where updated dependencies of substitution files did not retrigger a .db expansion.

require-ess/tools/driver.makefile

@@ -454,6 +454,10 @@ REQ :=
 INSTALLED_MODULES := $(sort $(notdir $(wildcard $(E3_SITEMODS_PATH)/* $(EPICS_MODULES)/*)))
 
 # Converts all of the X_DEP_VERSIONs to x_VERSION and records them
+
+# Create the module_VERSION variables giving the module and check if the module
+# is actually installed. Add the module to the MODULES and REQ variables. That
+# will be used later.
 define fetch_module_versions
   lm := $$(shell echo $1 | tr '[:upper:]' '[:lower:]')
   ifneq ($$(strip $$(filter $(INSTALLED_MODULES),$$(lm))),)
@@ -464,30 +468,51 @@ define fetch_module_versions
     $$(warning Invalid dependency "$1_DEP_VERSION$2"; pruning)
   endif
 endef
+# 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))))\
 )
 export REQ
 
+
 # Fetches the data from .dep files to be parsed by the above
 define fetch_deps
 $(shell cat $(lastword $(wildcard $(addsuffix /$1/$($1_VERSION)/lib/$(T_A)/$1.dep,$(E3_SITEMODS_PATH) $(EPICS_MODULES)))) | sed '1d')
 endef
 
-# Used to recurse through versions: recursively fetches all of the dependencies from the given module
+# 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))
-  $$(foreach mm,$$($$m_DEPS),$$(eval $$(mm)_VERSION := $$(call select,2,$$($$m_TBL),$$$$(call str-eq,$$$$1,$$(mm)))))
+  # 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 3: T_A = $(T_A) ====================="
 	@echo "BINS = $(BINS)"

tests/test_build.py

@@ -161,6 +161,187 @@ def test_updated_dependencies(wrappers):
     assert f"Loaded {wrapper_dep.name} version {new_version}" in outs
 
 
+def test_match_versions(wrappers):
+    """Test match version scenario.
+
+    This test checks if inconsistent versions are correctly verified during
+    build time. This tests if the dependecies B->A->C and B->C with A and B
+    both requesting the same version of C will be correctly built.
+    """
+    wrapper_dep = wrappers.get()
+    wrapper_a = wrappers.get()
+    wrapper_b = wrappers.get()
+
+    cell_path = wrapper_b.path / "cellmods"
+
+    dep_version = "1.0.0+0"
+
+    a_version = "0.0.0+0"
+    b_version = "0.0.0+0"
+
+    # Wrapper a dependes on dep,1.0.0+0
+    wrapper_a.add_var_to_config_module(
+        f"{wrapper_dep.name}_DEP_VERSION", dep_version, modifier=""
+    )
+
+    # Wrapper b depends on dep,1.0.0+0
+    wrapper_b.add_var_to_config_module(
+        f"{wrapper_dep.name}_DEP_VERSION", dep_version, modifier=""
+    )
+
+    # Wrapper b also depends on a
+    wrapper_b.add_var_to_config_module(
+        f"{wrapper_a.name}_DEP_VERSION", a_version, modifier=""
+    )
+
+    rc, *_ = wrapper_dep.run_make(
+        "cellinstall", module_version=dep_version, cell_path=cell_path
+    )
+    assert rc == 0
+
+    rc, *_ = wrapper_a.run_make(
+        "cellinstall", module_version=a_version, cell_path=cell_path
+    )
+    assert rc == 0
+
+    # As wrappers a and b both depends on dep,1.0.0+0 this build should finish
+    # corretly.
+    rc, *_ = wrapper_b.run_make(
+        "cellinstall", module_version=b_version, cell_path=cell_path
+    )
+    assert rc == 0
+
+
+def test_unmatching_versions(wrappers):
+    """Test unmatching version scenarion.
+
+    This test checks if inconsistent versions are correctly verified during
+    build time. This checks for the scenarion where B->A->C and B->C however
+    A depends on a version of C different than B.
+    """
+    wrapper_dep = wrappers.get()
+    wrapper_a = wrappers.get()
+    wrapper_b = wrappers.get()
+
+    cell_path = wrapper_b.path / "cellmods"
+
+    dep_version_1 = "1.0.0+0"
+    dep_version_2 = "2.0.0+0"
+
+    a_version = "0.0.0+0"
+    b_version = "0.0.0+0"
+
+    # Wrapper a dependes on dep v1
+    wrapper_a.add_var_to_config_module(
+        f"{wrapper_dep.name}_DEP_VERSION", dep_version_1, modifier=""
+    )
+
+    # Wrapper b depends on dep v2
+    wrapper_b.add_var_to_config_module(
+        f"{wrapper_dep.name}_DEP_VERSION", dep_version_2, modifier=""
+    )
+
+    # Wrapper b also depends on wrapper_a
+    wrapper_b.add_var_to_config_module(
+        f"{wrapper_a.name}_DEP_VERSION", a_version, modifier=""
+    )
+
+    rc, *_ = wrapper_dep.run_make(
+        "cellinstall", module_version=dep_version_1, cell_path=cell_path
+    )
+    assert rc == 0
+
+    rc, *_ = wrapper_a.run_make(
+        "cellinstall", module_version=a_version, cell_path=cell_path
+    )
+    assert rc == 0
+
+    # Now a second installation of wrapper_dep but with version 2
+    rc, *_ = wrapper_dep.run_make(
+        "cellinstall", module_version=dep_version_2, cell_path=cell_path
+    )
+    assert rc == 0
+
+    # This next installation should fail because B depends on A
+    # that depends on DEP. However A needs DEP 1.0.0+0 and B
+    # needs DEP 2.0.0+0
+    rc, *_ = wrapper_b.run_make(
+        "cellinstall", module_version=b_version, cell_path=cell_path
+    )
+    assert rc != 0
+
+
+def test_indirect_unmatching_versions(wrappers):
+    """Test indirect unmatching version scenarion.
+
+    This test checks if inconsistend versions are correctly verified during
+    build time. This checks for the scenarion where B->A->C and B->D->C
+    however A depends on a version of C different than D.
+    """
+
+    wrapper_c = wrappers.get()
+    wrapper_a = wrappers.get()
+    wrapper_b = wrappers.get()
+    wrapper_d = wrappers.get()
+
+    cell_path = wrapper_b.path / "cellmods"
+
+    c_version_a = "1.0.0+0"
+    c_version_d = "2.0.0+0"
+
+    a_version = "0.0.0+0"
+    d_version = "0.0.0+0"
+    b_version = "0.0.0+0"
+
+    # Wrapper a dependes on c
+    wrapper_a.add_var_to_config_module(
+        f"{wrapper_c.name}_DEP_VERSION", c_version_a, modifier=""
+    )
+
+    # Wrapper d dependes on c
+    wrapper_d.add_var_to_config_module(
+        f"{wrapper_c.name}_DEP_VERSION", c_version_d, modifier=""
+    )
+
+    # Wrapper b depends on d
+    wrapper_b.add_var_to_config_module(
+        f"{wrapper_d.name}_DEP_VERSION", d_version, modifier=""
+    )
+
+    # Wrapper b also depends on a
+    wrapper_b.add_var_to_config_module(
+        f"{wrapper_a.name}_DEP_VERSION", a_version, modifier=""
+    )
+
+    rc, *_ = wrapper_c.run_make(
+        "cellinstall", module_version=c_version_a, cell_path=cell_path
+    )
+    assert rc == 0
+
+    rc, *_ = wrapper_a.run_make(
+        "cellinstall", module_version=a_version, cell_path=cell_path
+    )
+    assert rc == 0
+
+    # Now a second installation of wrapper_dep but with version 2
+    rc, *_ = wrapper_c.run_make(
+        "cellinstall", module_version=c_version_d, cell_path=cell_path
+    )
+    assert rc == 0
+
+    rc, *_ = wrapper_d.run_make(
+        "cellinstall", module_version=d_version, cell_path=cell_path
+    )
+    assert rc == 0
+
+    # This next installation should fail because A depends on C
+    # with a different version that D depends on C.
+    rc, *_ = wrapper_b.run_make(
+        "cellinstall", module_version=b_version, cell_path=cell_path
+    )
+    assert rc != 0
+
+
 def test_automated_dependency(wrappers):
     wrapper_a = wrappers.get()
     wrapper_b = wrappers.get()