diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 827403fedef21efce1ce76f18251e4496bcbbd09..ced64ab836e3fcdd684e7a66c415a244cf98f8a1 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -3,7 +3,7 @@ repos: rev: 22.3.0 hooks: - id: black - - repo: https://gitlab.com/pycqa/flake8.git + - repo: https://github.com/pycqa/flake8 rev: 3.9.2 hooks: - id: flake8 diff --git a/CHANGELOG.md b/CHANGELOG.md index e5a8827ead32c4588d296cfb498cb54f2de57e5c..4f31d298a5f77a76e5ed1596899a09ff25410270 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Rename `setE3Env.bash` to `activate` * Add alias from `setE3Env.bash` to `activate` * Updated PV-names to be ESS compliant, and remove VERSIONS PV +* Changed from `build number` to `revision number` +* Added revision number parsing for test versions ## [4.0.0] diff --git a/configure/E3/CONFIG b/configure/E3/CONFIG index 6f5efa836efd4aed7cb883f2ac0f302d73265ada..ef6702b6bf64daa9a18128870d6d956c3d9dd347 100644 --- a/configure/E3/CONFIG +++ b/configure/E3/CONFIG @@ -1,6 +1,6 @@ # -*- mode: Makefile;-*- -# Update the module version (for numeric versions) with a build number of 0 if none is specified +# Update the module version (for numeric versions) with a revision number of 0 if none is specified E3_MODULE_VERSION:=$(E3_MODULE_VERSION)$(shell (echo "$(E3_MODULE_VERSION)" | grep -Eq "^[0-9]+\.[0-9]+\.[0-9]+\$$") && echo +0) E3_MODULES_PATH?=$(E3_SITEMODS_PATH) diff --git a/configure/module/RULES_REQUIRE b/configure/module/RULES_REQUIRE index 1f7b9e52dc2c2b0a2dfe509da9af595ea9a4ef37..a054636a09f823c8e7fc46bb602e5da6f772336d 100644 --- a/configure/module/RULES_REQUIRE +++ b/configure/module/RULES_REQUIRE @@ -25,7 +25,7 @@ install: requireconf requireconf: e3-site-path e3-require-path $(QUIET) install -m 755 $(wildcard $(E3_MODULE_SRC_PATH)/tools/*.tcl) $(E3_REQUIRE_TOOLS)/ $(QUIET) install -m 644 $(E3_MODULE_SRC_PATH)/tools/driver.makefile $(E3_REQUIRE_TOOLS)/ - $(QUIET) install -m 755 $(E3_MODULE_SRC_PATH)/tools/build_number $(E3_REQUIRE_TOOLS)/ + $(QUIET) install -m 755 $(E3_MODULE_SRC_PATH)/tools/revision_number $(E3_REQUIRE_TOOLS)/ $(QUIET) install -m 755 $(E3_SHELL_FILES) $(E3_REQUIRE_BIN)/ $(QUIET) install -m 644 $(E3_IOC_CFG_FILES) $(E3_REQUIRE_BIN)/ $(QUIET) install -m 644 $(E3_REQUIRE_CONF_FILES) $(E3_REQUIRE_CONFIG)/ diff --git a/require-ess/src/require.c b/require-ess/src/require.c index 6f14adc3a6811b521e2ab76687e23bcce1e42d27..8ed30e50427bb3555e35ee6a0776e334295e5ec7 100644 --- a/require-ess/src/require.c +++ b/require-ess/src/require.c @@ -65,7 +65,7 @@ int requireDebug; #endif // OS_CLASS #else - #error Only support Unix based distros +#error Only support Unix based distros #endif #include <dlfcn.h> @@ -333,8 +333,8 @@ static void fillModuleListRecord(initHookState state) { (getRecordHandle(":Versions", DBF_STRING, moduleCount, &versions) == 0); moduleListBufferSize += moduleCount * maxModuleNameLength; - have_modver = (getRecordHandle(":ModuleVersions", DBF_CHAR, moduleListBufferSize, - &modver) == 0); + have_modver = (getRecordHandle(":ModuleVersions", DBF_CHAR, + moduleListBufferSize, &modver) == 0); for (m = loadedModules, i = 0; m; m = m->next, i++) { size_t lm = strlen(m->content) + 1; @@ -594,9 +594,7 @@ int libversionShow(const char *outfile) { } #define MISMATCH -1 -#define EXACT 0 #define MATCH 1 -#define TESTVERS 2 #define HIGHER 3 #define debug(...) \ @@ -617,8 +615,7 @@ static int compareDigit(int found, int requested, const char *name) { return MATCH; } -static int compareNumericVersion(semver_t *sv_found, semver_t *sv_request, - int already_matched) { +static int compareNumericVersion(semver_t *sv_found, semver_t *sv_request) { int match; match = compareDigit(sv_found->major, sv_request->major, "major"); @@ -629,25 +626,7 @@ static int compareNumericVersion(semver_t *sv_found, semver_t *sv_request, if (match != MATCH) { return match; } - match = compareDigit(sv_found->patch, sv_request->patch, "patch"); - if (match != MATCH) { - return match; - } - - if (sv_request->build == -1) { - if (already_matched) { - debug( - "require: compareVersions: No build number requested. Returning " - "HIGHER\n"); - return HIGHER; - } else { - debug( - "require: compareVersions: No build number requested. Returning " - "MATCH\n"); - return MATCH; - } - } - return compareDigit(sv_found->build, sv_request->build, "build"); + return compareDigit(sv_found->patch, sv_request->patch, "patch"); } /* @@ -669,19 +648,24 @@ static int compareVersions(const char *found, const char *request, return MISMATCH; } - sv_found = (semver_t *)calloc(1, sizeof(semver_t)); - sv_request = (semver_t *)calloc(1, sizeof(semver_t)); + sv_found = parse_semver(found); + sv_request = parse_semver(request); + if (sv_found == NULL || sv_request == NULL) { + debug("require: compareVersion: failed to allocate semver_t\n"); + return MISMATCH; + } - parse_semver(found, sv_found); - parse_semver(request, sv_request); + // TODO: maybe don't do this. This is only in the case that + // we have found an installed version with no revision number. + if (already_matched && sv_request->revision == -1) sv_request->revision = 0; // test version, look for exact. if (strlen(sv_request->test_str) > 0) { - if (strcmp(found, request) == 0) { + if (strcmp(sv_found->test_str, sv_request->test_str) == 0) { debug( "require: compareVersions: Test version requested and found, " - "matches exactly\n"); - match = EXACT; + "matches\n"); + match = MATCH; } else if (strlen(sv_found->test_str) > 0) { debug( "require: compareVersions: Test versions requested and found, no " @@ -699,7 +683,27 @@ static int compareVersions(const char *found, const char *request, "found\n"); match = MISMATCH; } else { - match = compareNumericVersion(sv_found, sv_request, already_matched); + match = compareNumericVersion(sv_found, sv_request); + } + + // Finally, check revision numbers + if (match == MATCH) { + if (sv_request->revision == -1) { + if (already_matched) { + debug( + "require: compareVersions: No revision number for already found " + "version. Returning HIGHER\n"); + match = HIGHER; + } else { + debug( + "require: compareVersions: No revision number requested. Returning " + "MATCH\n"); + match = MATCH; + } + } else { + match = + compareDigit(sv_found->revision, sv_request->revision, "revision"); + } } cleanup_semver(sv_found); cleanup_semver(sv_request); @@ -953,13 +957,6 @@ static int require_priv( if (loaded) { /* Library already loaded. Check Version. */ switch (compareVersions(loaded, version, FALSE)) { - case TESTVERS: - if (version) - printf( - "Warning: Module %s test version %s already loaded where %s was " - "requested\n", - module, loaded, version); - case EXACT: case MATCH: printf("Module %s version %s already loaded\n", module, loaded); break; @@ -1025,9 +1022,7 @@ static int require_priv( currentFilename, version); switch ((status = compareVersions(currentFilename, version, FALSE))) { - case TESTVERS: /* test version found */ - case EXACT: /* exact match found */ - case MATCH: /* all given numbers match. */ + case MATCH: /* all given numbers match. */ { someArchFound = 1; @@ -1054,15 +1049,6 @@ static int require_priv( continue; } - if (status == EXACT) { - if (requireDebug) - printf("require: %s %s matches %s exactly\n", module, - currentFilename, version); - /* We are done. */ - end = NULL; - break; - } - /* Is it higher than the one we found before? */ if (found && requireDebug) printf( @@ -1088,7 +1074,7 @@ static int require_priv( continue; } } - /* we have found something (EXACT or MATCH) */ + /* we have found something */ free(founddir); /* filename = "<dirname>/[dirlen]<module>/[modulediroffs]..." */ if (asprintf(&founddir, "%.*s%s", modulediroffs, filename, @@ -1096,7 +1082,6 @@ static int require_priv( return errno; /* founddir = "<dirname>/[dirlen]<module>/[modulediroffs]<version>" */ found = founddir + modulediroffs; /* version part in the path */ - if (status == EXACT) break; } END_DIR_LOOP } @@ -1339,4 +1324,4 @@ static void requireRegister(void) { } epicsExportRegistrar(requireRegister); -epicsExportAddress(int, requireDebug); \ No newline at end of file +epicsExportAddress(int, requireDebug); diff --git a/require-ess/src/version.c b/require-ess/src/version.c index bd59fe80a56571ff0126a88543ca9b76cae46e75..4982e43aea63995abe95ec3efb7c3f4e6fe988b5 100644 --- a/require-ess/src/version.c +++ b/require-ess/src/version.c @@ -12,15 +12,15 @@ void cleanup_semver(semver_t *s) { free(s); } -static void init_semver(const char *version, semver_t *s) { +static semver_t *init_semver(const char *version) { + semver_t *s = (semver_t *)calloc(1, sizeof(semver_t)); + s->version_str = calloc(strlen(version) + 1, sizeof(char)); strcpy(s->version_str, version); s->test_str = ""; + s->revision = -1; - s->major = 0; - s->minor = 0; - s->patch = 0; - s->build = -1; + return s; } static void fetch_part(char *version_str, const regmatch_t *groups, @@ -33,37 +33,39 @@ static void fetch_part(char *version_str, const regmatch_t *groups, version_str[groups[ix].rm_eo] = tmp; } -int parse_semver(const char *version, semver_t *s) { +semver_t *parse_semver(const char *version) { static const char *version_regex = - "^(([0-9]+)\\.([0-9]+)\\.([0-9]+)(\\+([0-9]+))?)?(.*)$"; + "^(([0-9]+)\\.([0-9]+)\\.([0-9]+))?([^+]*)(\\+([0-9]+))?$"; static const unsigned int max_regex_groups = 7 + 1; static const unsigned int major_ix = 2; static const unsigned int minor_ix = 3; static const unsigned int patch_ix = 4; - static const unsigned int build_ix = 6; - static const unsigned int test_ix = 7; + static const unsigned int test_label_ix = 5; + static const unsigned int revision_ix = 7; + semver_t *s = init_semver(version); regex_t compiled; regmatch_t groups[max_regex_groups]; - init_semver(version, s); - if (s->version_str == NULL || s->version_str[0] == 0) { - return 1; + return NULL; } if (regcomp(&compiled, version_regex, REG_EXTENDED)) { - return 1; + return NULL; } if (regexec(&compiled, s->version_str, max_regex_groups, groups, 0) == 0) { fetch_part(s->version_str, groups, major_ix, &s->major); fetch_part(s->version_str, groups, minor_ix, &s->minor); fetch_part(s->version_str, groups, patch_ix, &s->patch); - if (groups[build_ix].rm_so != (regoff_t)-1) { - fetch_part(s->version_str, groups, build_ix, &s->build); + if (groups[test_label_ix].rm_so != groups[test_label_ix].rm_eo) { + s->test_str = s->version_str; + } + if (groups[revision_ix].rm_so != (regoff_t)-1) { + fetch_part(s->version_str, groups, revision_ix, &s->revision); + s->version_str[groups[revision_ix].rm_so - 1] = 0; } - s->test_str = s->version_str + groups[test_ix].rm_so; } - return 0; + return s; } diff --git a/require-ess/src/version.h b/require-ess/src/version.h index a3cf1789c7bf3904a363ee808e473d5d45ee82ed..06425c26298902b5d165f366ef07371379867433 100644 --- a/require-ess/src/version.h +++ b/require-ess/src/version.h @@ -7,10 +7,11 @@ typedef struct semver_t { int major; int minor; int patch; - int build; // can be negative; implies that build has not been specified + int revision; // can be negative; implies that revision has not been + // specified char *test_str; } semver_t; void cleanup_semver(semver_t *s); -int parse_semver(const char *version, semver_t *s); +semver_t *parse_semver(const char *version); diff --git a/require-ess/tools/driver.makefile b/require-ess/tools/driver.makefile index e77379a9de459580f5ef126338ab1739f7826067..7d95aca8eccf643eb78e310a6540d22326397d4e 100644 --- a/require-ess/tools/driver.makefile +++ b/require-ess/tools/driver.makefile @@ -90,7 +90,7 @@ RM = rm -f CP = cp MKDIR = mkdir -p -m 775 -# This is to allow for build numbers in recognized versions. +# This is to allow for revision numbers in recognized versions. VERSIONREGEX = [0-9]+\.[0-9]+\.[0-9]+(\+[0-9]+)? # Some generated file names: @@ -135,13 +135,13 @@ define uniq ${seen} endef -# Function that fetches the correct build number from the shared filesystem. +# Function that fetches the correct revision number from the shared filesystem. # Usage: # -# $(call FETCH_BUILD_NUMBER,$(E3_SITEMODS_PATH),module) +# $(call FETCH_REVISION_NUMBER,$(E3_SITEMODS_PATH),module) # -define FETCH_BUILD_NUMBER -$(shell $(MAKEHOME)/build_number $(1) $(2) $($(2)_VERSION)) +define FETCH_REVISION_NUMBER +$(shell $(MAKEHOME)/revision_number $(1) $(2) $($(2)_VERSION)) endef # Functions used for recursive dependency fetching. These are modified from https://github.com/markpiffer/gmtt.git @@ -320,7 +320,7 @@ define fetch_module_versions lm := $$(shell echo $1 | tr '[:upper:]' '[:lower:]') ifneq ($$(strip $$(filter $(INSTALLED_MODULES),$$(lm))),) $$(lm)_VERSION := $($1_DEP_VERSION$2) - $$(lm)_VERSION := $$(lastword $$(call FETCH_BUILD_NUMBER,$(E3_SITEMODS_PATH),$$(lm)) $$(call FETCH_BUILD_NUMBER,$(EPICS_MODULES),$$(lm))) + $$(lm)_VERSION := $$(lastword $$(call FETCH_REVISION_NUMBER,$(E3_SITEMODS_PATH),$$(lm)) $$(call FETCH_REVISION_NUMBER,$(EPICS_MODULES),$$(lm))) MODULES += $$(lm) REQ += $$(lm) else @@ -445,8 +445,8 @@ INSTALL_INCLUDES = # Add include directory of foreign modules to include file search path. # # The default behaviour is to start with <module>_VERSION and to select the highest -# available build number, unless a build no. is specified. This is determined with the -# shell script `build_number` included with require. +# available revision number, unless a build no. is specified. This is determined with the +# shell script `revision_number` included with require. define ADD_INCLUDES_TEMPLATE INSTALL_INCLUDES += $$(patsubst %,-I${2}/${1}/%/include,$${${1}_VERSION}) diff --git a/require-ess/tools/build_number b/require-ess/tools/revision_number similarity index 83% rename from require-ess/tools/build_number rename to require-ess/tools/revision_number index f1e8298fb22ac1e62378979dd79008d9ef3977e1..bad93a192e691cbd7f431b8710deae20ef595f9e 100644 --- a/require-ess/tools/build_number +++ b/require-ess/tools/revision_number @@ -1,6 +1,6 @@ #!/usr/bin/env bash # Small script to fetch current build nuber for a given module. If you pass a numeric version -# but no build number, it fetches the latest build number installed from the target e3 env. +# but no revision number, it fetches the latest revision number installed from the target e3 env. # # Arguments: # $1: require module path e.g. /epics/base-$BASE_VERSION/require/$REQUIRE_VERSION/siteMods diff --git a/tests/test_e3.py b/tests/test_e3.py index 2ed05c5697bd0e07de939bdb2d05f0d5dff824f3..54e1e079488ecb5d658dfb84387c5f8780bc5650 100644 --- a/tests/test_e3.py +++ b/tests/test_e3.py @@ -32,7 +32,7 @@ def test_incorrect_module_name(wrappers): assert f"E3_MODULE_NAME '{module_name}' is not valid" in errs -def test_add_missing_build_number(wrapper: Wrapper): +def test_add_missing_revision_number(wrapper: Wrapper): version = "1.2.3" wrapper.write_dot_local_data("CONFIG_MODULE", {"E3_MODULE_VERSION": version}) diff --git a/tests/test_versions.py b/tests/test_versions.py index b075d4447b64953bd819f1c5f5866f40e20c7d17..055684e60294de8e8b09bc5096429170e316afce 100644 --- a/tests/test_versions.py +++ b/tests/test_versions.py @@ -18,13 +18,13 @@ RE_MODULE_NOT_LOADED = "Module {module} (not available|version {required} not av # Numeric versions should be prioritized over test versions ("", "0.0.1+0", ["test", "0.0.1+0"]), ("", "0.0.1+0", ["0.0.1+0", "test"]), - # Highest build number should be loaded if version is unspecified + # Highest revision number should be loaded if version is unspecified ("", "0.0.1+7", ["0.0.1+0", "0.0.1+7", "0.0.1+3"]), - # Only load the given build number if it is specified + # Only load the given revision number if it is specified ("0.0.1+0", "", ["0.0.1+1"]), - # If no build number is specified, load the highest build number + # If no revision number is specified, load the highest revision number ("0.0.1", "0.0.1+4", ["0.1.0+0", "0.0.1+4", "0.0.1+0"]), - # Build number 0 means load that version exactly + # Revision number 0 means load that version exactly ("0.0.1+0", "0.0.1+0", ["0.0.1+0"]), ("0.0.1+0", "0.0.1+0", ["0.0.1+0", "0.0.1+1", "1.0.0+0"]), # 1-test counts as a test version, as does 1.0 @@ -32,6 +32,12 @@ RE_MODULE_NOT_LOADED = "Module {module} (not available|version {required} not av ("", "0.0.1+0", ["0.0.1+0", "1.0"]), # Numeric version should be prioritised over "higher" test version ("", "0.1.0+0", ["0.1.0+0", "1.0.0-rc1"]), + # Pick the latest test version based on revision number + ("test", "test+2", ["test+0", "test+2"]), + # Don't install an incorrect test version + ("foo", "", ["bar"]), + # Mix of test and numeric + ("1.2.3-test", "1.2.3-test+2", ["1.2.3-test", "1.2.3-test+0", "1.2.3-test+2"]), ], ) def test_version(wrapper: Wrapper, requested, expected, installed):