From 91ab638242f4582f73f3ece278c03156ff1de1b4 Mon Sep 17 00:00:00 2001
From: "Lucas A. M. Magalhaes" <lucas.magalhaes@ess.eu>
Date: Mon, 1 Aug 2022 10:54:34 +0200
Subject: [PATCH] driver.Makefile: Add check for inconsistent versions between
dependencies
In a dependency chain like B->A->C and also B->C where A needs a version
of C different than B the build should fail.
This commit adds this check during dependency treatment on the built
target.
---
CHANGELOG.md | 1 +
require-ess/tools/driver.makefile | 29 ++++-
tests/test_build.py | 181 ++++++++++++++++++++++++++++++
3 files changed, 209 insertions(+), 2 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 91d52757..4372671d 100644
--- a/CHANGELOG.md
+++ b/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.
diff --git a/require-ess/tools/driver.makefile b/require-ess/tools/driver.makefile
index 7aceaba6..71c43c9f 100644
--- a/require-ess/tools/driver.makefile
+++ b/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)"
diff --git a/tests/test_build.py b/tests/test_build.py
index f58cb98c..c7875620 100644
--- a/tests/test_build.py
+++ b/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()
--
GitLab