From afeded38cf913aacd8d625df2b13c73b488df8c9 Mon Sep 17 00:00:00 2001
From: Simon Rose <simon.rose@ess.eu>
Date: Mon, 19 Sep 2022 16:22:32 +0200
Subject: [PATCH] Ensure that EPICS_DRIVER_PATH is set first
Previously, it was the case that running
```
$ iocsh -r module -l cellMods
$ iocsh -l cellMods -r module
```
would both work correctly, but if you looked at the IOC log it was not
clear why that was the case; in the former case it _appeared_ like we
were finding the module before setting `EPICS_DRIVER_PATH`. This only
worked because this variable is exported from the shell.
This change forces the `-l` flag to be processed _before_ all of the
other flags in order to ensure that paths are determined first.
---
require-ess/tools/iocsh | 2 ++
require-ess/tools/iocsh_functions.bash | 48 ++++++++++++++++----------
tests/test_build.py | 19 ++++++----
tests/test_iocsh.py | 48 ++++++++++++++++++++++++++
tests/test_versions.py | 2 +-
tests/utils.py | 16 ++++++---
6 files changed, 106 insertions(+), 29 deletions(-)
create mode 100644 tests/test_iocsh.py
diff --git a/require-ess/tools/iocsh b/require-ess/tools/iocsh
index a23ad25a..ac58d761 100644
--- a/require-ess/tools/iocsh
+++ b/require-ess/tools/iocsh
@@ -124,6 +124,8 @@ trap "softIoc_end ${IOC_STARTUP}" EXIT HUP INT TERM
loadRequire
+ setPaths "$@"
+
loadFiles "$@"
printf "# // Set the IOC Prompt String\n"
diff --git a/require-ess/tools/iocsh_functions.bash b/require-ess/tools/iocsh_functions.bash
index d1443114..89300b49 100644
--- a/require-ess/tools/iocsh_functions.bash
+++ b/require-ess/tools/iocsh_functions.bash
@@ -248,21 +248,37 @@ function check_mandatory_env_settings() {
fi
}
+function setPaths() {
+ while [ "$#" -gt 0 ]; do
+ arg="$1"
+
+ case $arg in
+ -l)
+ shift
+ add_path="$1/$(basename "${EPICS_BASE}")/require-${E3_REQUIRE_VERSION}"
+ printf "epicsEnvSet EPICS_DRIVER_PATH %s:${EPICS_DRIVER_PATH}\n" "$add_path"
+ EPICS_DRIVER_PATH="$add_path:$EPICS_DRIVER_PATH"
+ ;;
+ esac
+ shift
+ done
+}
+
function loadFiles() {
while [ "$#" -gt 0 ]; do
- file=$1
+ arg=$1
- case $file in
+ case $arg in
-rt | -RT | -realtime | --realtime)
REALTIME="RT"
__LOADER__="chrt --fifo 1 "
;;
@*)
- loadFiles "$(cat "${file#@}")"
+ loadFiles "$(cat "${arg#@}")"
;;
*=*)
- echo -n "$file" | awk -F '=' '{printf "epicsEnvSet %s '\''%s'\''\n" $1 $2}'
+ echo -n "$arg" | awk -F '=' '{printf "epicsEnvSet %s '\''%s'\''\n" $1 $2}'
;;
-c)
shift
@@ -294,11 +310,8 @@ function loadFiles() {
shift
echo "require $1"
;;
- -l)
+ -l) # This is taken care of in setPaths()
shift
- add_path="$1/$(basename "${EPICS_BASE}")/require-${E3_REQUIRE_VERSION}"
- printf "epicsEnvSet EPICS_DRIVER_PATH %s:${EPICS_DRIVER_PATH}\n" "$add_path"
- EPICS_DRIVER_PATH="$add_path:$EPICS_DRIVER_PATH"
;;
-dg)
if [[ -n "${2%--dgarg=*}" ]]; then
@@ -333,7 +346,7 @@ function loadFiles() {
help
;;
*.so)
- echo "dlload \"$file\""
+ echo "dlload \"$arg\""
;;
*)
subst=""
@@ -349,26 +362,26 @@ function loadFiles() {
esac
done
subst=${subst#,}
- case $file in
+ case $arg in
*.db | *.template)
- echo "dbLoadRecords '$file','$subst'"
+ echo "dbLoadRecords '$arg','$subst'"
;;
*.subs | *.subst)
- echo "dbLoadTemplate '$file','$subst'"
+ echo "dbLoadTemplate '$arg','$subst'"
;;
*.dbd)
# some dbd files must be loaded before main to take effect
- echo "dbLoadDatabase '$file','$DBD','$subst'"
+ echo "dbLoadDatabase '$arg','$DBD','$subst'"
;;
*)
- set_e3_cmd_top "$file"
- echo "iocshLoad '$file','$subst'"
+ set_e3_cmd_top "$arg"
+ echo "iocshLoad '$arg','$subst'"
# Search for any instance of iocInit at the start of the line.
# If found, do not add the iocInit to the startup script. Any
# other occurrence of iocInit (e.g. in comments) is not matched
# and the script will add an active iocInit.
- if grep -q "^\s*iocInit\b" "$file"; then
+ if grep -q "^\s*iocInit\b" "$arg"; then
init=NO
fi
;;
@@ -379,8 +392,7 @@ function loadFiles() {
shift
done
-} \
- ;
+}
function set_e3_cmd_top() {
local file=$1
diff --git a/tests/test_build.py b/tests/test_build.py
index 6ce80558..901746b5 100644
--- a/tests/test_build.py
+++ b/tests/test_build.py
@@ -176,7 +176,9 @@ def test_updated_dependencies(wrappers):
assert rc == 0
rc, outs, _ = run_ioc_get_output(
- wrapper_main.name, new_version, wrapper_main.path / "cellMods"
+ module=wrapper_main.name,
+ version=new_version,
+ cell_path=wrapper_main.path / "cellMods",
)
assert rc == 0
assert f"Loaded {wrapper_dep.name} version {new_version}" in outs
@@ -421,7 +423,9 @@ def test_architecture_dependent_dependency(wrappers):
assert rc == 0
rc, outs, _ = run_ioc_get_output(
- wrapper_a.name, module_version, wrapper_a.path / "cellMods"
+ module=wrapper_a.name,
+ version=module_version,
+ cell_path=wrapper_a.path / "cellMods",
)
assert rc == 0
assert f"Loaded {wrapper_b.name} version {module_version}" in outs
@@ -461,7 +465,9 @@ def test_recursive_header_include(wrappers):
assert rc == 0
rc, outs, _ = run_ioc_get_output(
- wrapper_a.name, module_version, wrapper_a.path / "cellMods"
+ module=wrapper_a.name,
+ version=module_version,
+ cell_path=wrapper_a.path / "cellMods",
)
assert rc == 0
assert f"Loaded {wrapper_c.name} version {module_version}" in outs
@@ -591,13 +597,14 @@ def test_build_fails_if_nth_architecture_fails(wrapper: Wrapper, archs, failing_
def test_double_install_fails(wrapper: Wrapper, module_version):
RE_ERR_MOD_VER_EXISTS = ".*{module}/{version}.* already exists"
- rc, *_ = wrapper.run_make("install", module_version = module_version)
+ rc, *_ = wrapper.run_make("install", module_version=module_version)
assert rc == 0
- rc, _, errs = wrapper.run_make("install", module_version = module_version)
+ rc, _, errs = wrapper.run_make("install", module_version=module_version)
assert rc == 2
assert re.search(
RE_ERR_MOD_VER_EXISTS.format(
- module=re.escape(wrapper.name), version=re.escape(module_version)),
+ module=re.escape(wrapper.name), version=re.escape(module_version)
+ ),
errs,
)
diff --git a/tests/test_iocsh.py b/tests/test_iocsh.py
new file mode 100644
index 00000000..97dc0c07
--- /dev/null
+++ b/tests/test_iocsh.py
@@ -0,0 +1,48 @@
+import re
+
+from .conftest import Wrapper
+from .utils import run_ioc_get_output
+
+
+def test_multiple_cell_paths(wrapper: Wrapper):
+ cell_path_a = wrapper.path / "cellModsA"
+ wrapper.run_make("cellinstall", cell_path=cell_path_a)
+
+ cell_path_b = wrapper.path / "cellModsB"
+ wrapper.run_make("cellinstall", cell_path=cell_path_b)
+
+ rc, outs, _ = run_ioc_get_output(
+ "-l", cell_path_a, "-l", cell_path_b, module=wrapper.name
+ )
+ assert rc == 0
+
+ regex = r"Module {name} version {version} found in {path}"
+ assert not re.search(
+ regex.format(
+ name=re.escape(wrapper.name),
+ version=re.escape(wrapper.version),
+ path=re.escape(str(cell_path_a)),
+ ),
+ outs,
+ )
+
+ assert re.search(
+ regex.format(
+ name=re.escape(wrapper.name),
+ version=re.escape(wrapper.version),
+ path=re.escape(str(cell_path_b)),
+ ),
+ outs,
+ )
+
+
+def test_cellpath_does_not_depend_on_order(wrapper: Wrapper):
+ cell_path = wrapper.path / "cellMods"
+
+ wrapper.run_make("cellinstall", cell_path=cell_path)
+
+ rc, *_ = run_ioc_get_output("-l", cell_path, "-r", wrapper.name)
+ assert rc == 0
+
+ rc, *_ = run_ioc_get_output("-r", wrapper.name, "-l", cell_path)
+ assert rc == 0
diff --git a/tests/test_versions.py b/tests/test_versions.py
index dcec1f4e..8d6ff943 100644
--- a/tests/test_versions.py
+++ b/tests/test_versions.py
@@ -45,7 +45,7 @@ def test_version(wrapper: Wrapper, requested, expected, installed):
assert returncode == 0
rc, stdout, _ = run_ioc_get_output(
- wrapper.name, requested, wrapper.path / "cellMods"
+ module=wrapper.name, version=requested, cell_path=wrapper.path / "cellMods"
)
if expected:
diff --git a/tests/utils.py b/tests/utils.py
index 2ef7a92d..9b202a31 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -3,12 +3,20 @@ import time
from run_iocsh import IOC
-def run_ioc_get_output(module, version, cell_path):
+def run_ioc_get_output(*args, **kwargs):
"""
Run an IOC and try to load the test module
"""
- with IOC(
- "-r", f"{module},{version}", "-l", cell_path, ioc_executable="iocsh"
- ) as ioc:
+ ioc_args = []
+ module = kwargs.get("module", None)
+ version = kwargs.get("version", None)
+ if module:
+ ioc_args.append("-r")
+ ioc_args.append(f"{module},{version}" if version else module)
+ cell_path = kwargs.get("cell_path", None)
+ if cell_path:
+ ioc_args.extend(["-l", cell_path])
+ ioc_args.extend(args)
+ with IOC(*ioc_args, ioc_executable="iocsh") as ioc:
time.sleep(1)
return ioc.proc.returncode, ioc.outs, ioc.errs
--
GitLab