Improved control over code changes (#925)

Implemented the formalised code changes described in PR #920.

**Benefits**: Make it possible to improve our control over code changes. Provide predictability for when changes will happen and give users a time-span to a given versions number to adapt to new changes.

* Improved deprecation warnings and initial implementation of behavior.
* Added a simple Python class for accessing behavior from Python
* Added Python interface to behaviours.
* Added initialisation of behavior table and simplified the logics.
* Fixed a bug in natoi()
* Fix documentation strings.
* Documented environment variables DLITE_BEHAVIOR_*
* Added more instructions to warning in dlite_behavior_get()

---------

Co-authored-by: Francesca L. Bleken <48128015+francescalb@users.noreply.github.com>

bindings/python/dlite-behavior-python.i

@@ -0,0 +1,61 @@
+/* -*- Python -*-  (not really, but good for syntax highlighting) */
+
+/* Python-specific extensions to dlite-behavior.i */
+
+
+%extend _DLiteBehavior {
+  %pythoncode %{
+    def asdict(self):
+        return {
+            "name": self.name,
+            "value": bool(self.value),
+            "version_added": self.version_added,
+            "version_new": self.version_new,
+            "version_remove": self.version_remove,
+            "description": self.description,
+        }
+
+    def __str__(self):
+        import json
+        return json.dumps(self.asdict(), indent=2)
+  %}
+}
+
+
+%pythoncode %{
+
+class _Behavior():
+    """A class that provides easy access for setting and getting behavior
+    settings.
+
+    It is used via the singleton `dlite.Behavior`.
+
+    The different behavior values can be accessed as attributes.
+
+    """
+    def __getattr__(self, name):
+        return bool(_dlite._behavior_get(name))
+
+    def __setattr__(self, name, value):
+        _dlite._behavior_set(name, 1 if value else 0)
+
+    def __dir__(self):
+        return object.__dir__(self) + list(self.get_names())
+
+    def get_names(self):
+        """Return a generator over all registered behavior names."""
+        for i in range(_dlite._behavior_nrecords()):
+            yield _dlite._behavior_recordno(i).name
+
+    def get_record(self, name):
+        """Return a record with information about the given behavior."""
+        rec = _dlite._behavior_record(name)
+        if not rec:
+            raise DLiteKeyError(f"No such behavior record: {name}")
+        return rec
+
+
+# A singleton for accessing behavior settings.
+Behavior = _Behavior()
+
+%}

bindings/python/dlite-behavior.i

@@ -0,0 +1,59 @@
+/* -*- C -*-  (not really, but good for syntax highlighting) */
+%{
+#include "dlite-behavior.h"
+%}
+
+%rename(_BehaviorRecord) _DLiteBehavior;
+struct _DLiteBehavior {
+  %immutable;
+  char *name;
+  char *version_added;
+  char *version_new;
+  char *version_remove;
+  char *description;
+  %mutable;
+  int value;
+};
+
+%rename("%(strip:[dlite])s") "";
+
+
+%feature("docstring", "\
+Return the number of registered behaviors.
+") dlite_behavior_nrecords;
+size_t dlite_behavior_nrecords(void);
+
+%feature("docstring", "\
+Return a pointer to record with the given number or NULL if `n` is out of
+range.
+") dlite_behavior_recordno;
+const struct _DLiteBehavior *dlite_behavior_recordno(size_t n);
+
+%feature("docstring", "\
+Return a pointer to the given behavior record, or NULL if `name` is
+not in the behavior table.
+") dlite_behavior_record;
+const struct _DLiteBehavior *dlite_behavior_record(const char *name);
+
+%feature("docstring", "\
+Get value of given behavior.
+
+Returns 1 if the behavior is on, 0 if it is off and a negative
+value on error.
+") dlite_behavior_get;
+int dlite_behavior_get(const char *name);
+
+%feature("docstring", "\
+Enable given behavior if `value` is non-zero.  Disable if `value` is zero.
+
+Returns non-zero on error.
+") dlite_behavior_set;
+int dlite_behavior_set(const char *name, int value);
+
+
+/* -----------------------------------
+ * Target language-spesific extensions
+ * ----------------------------------- */
+#ifdef SWIGPYTHON
+%include "dlite-behavior-python.i"
+#endif

bindings/python/dlite-misc-python.i

@@ -5,6 +5,8 @@
 %pythoncode %{
 
 import atexit
+import warnings
+
 
 atexit.register(_mark_python_atexit)
 
@@ -77,4 +79,40 @@ class errctl():
 
 
 silent = errctl(filename="None")
+
+# A set for keeping track of already issued deprecation warnings
+_deprecation_warning_record = set()
+
+def deprecation_warning(version_removed, descr):
+    """Issues a deprecation warning.
+
+    Arguments:
+        version_removed: The version the deprecated feature is
+            expected to be finally removed.
+        descr: Description of the deprecated feature.
+
+    Returns non-zero on error.
+    """
+    if descr in _deprecation_warning_record:
+        return
+    _deprecation_warning_record.add(descr)
+
+    warnings.warn(descr, DeprecationWarning, stacklevel=2)
+
+    dlite_version = get_version()
+    if chk_semver(version_removed) < 0:
+        raise SystemError(
+            f"argument `version_removed='{version_removed}'` must be a "
+            "valid semantic version number",
+        )
+    if chk_semver(dlite_version) < 0:
+        raise SystemError(
+            f"DLite version number is not semantic: {dlite_version}"
+        )
+    if cmp_semver(version_removed, dlite_version) <= 0:
+        raise SystemError(
+            "deprecated feature was supposed to be removed in version "
+            f"{version_removed}: {descr}"
+        )
+
 %}

bindings/python/dlite-misc.i

@@ -34,6 +34,16 @@
   /* Just check for errors, do nothing else. */
   void errcheck(void) {}
 
+  int chk_semver(const char *version, int n) {
+    if (n >= 0) return strnchk_semver(version, n);
+    return strchk_semver(version);
+  }
+
+  int cmp_semver(const char *v1, const char *v2, int n) {
+    if (n >= 0) return strncmp_semver(v1, v2, n);
+    return strcmp_semver(v1, v2);
+  }
+
 %}
 
 %include <stdint.i>
@@ -211,6 +221,28 @@ Set error stream.
 bool asbool(const char *str);
 
 
+%feature("docstring", "\
+Check if `version` is a valid semantic version 2.0.0 number.
+
+By default, the whole string is checked.  If `n` is non-negative, only
+the initial part of `version` is checked, at most `n` characters.
+
+Returns the length of the valid part of `version` or -1 if `version` is
+not a valid semantic number.
+") chk_semver;
+int chk_semver(const char *version, int n=-1);
+
+%feature("docstring", "\
+Compare semantic version strings `v1` and `v2`.
+
+Returns -1 if `v1 < v2`, 0 if `v1 == v2` and 1 if `v1 > v2`.
+
+By default, the whole of `v1` and `v2` are compared.  If `n` is non-negative,
+only the initial part of `v1` and `v2` are compared, at most `n` characters.
+") cmp_semver;
+int cmp_semver(const char *v1, const char *v2, int n=-1);
+
+
 
 /* -----------------------------------
  * Target language-spesific extensions

bindings/python/dlite.i

@@ -112,3 +112,4 @@
 %include "dlite-collection.i"
 %include "dlite-path.i"
 %include "dlite-mapping.i"
+%include "dlite-behavior.i"

bindings/python/tests/test_misc.py

@@ -1,6 +1,7 @@
 #!/usr/bin/env python
 # -*- coding: utf-8 -*-
 import dlite
+from dlite.testutils import raises
 
 
 assert dlite.get_uuid_version() == 4
@@ -63,3 +64,32 @@
         dlite.Instance.from_location("-", "__non-existing__")
 except dlite.DLiteStorageOpenError:
     pass
+
+
+# Test checking and comparing semantic version numbers
+assert dlite.chk_semver(dlite.__version__) > 0
+assert dlite.chk_semver("1.12.2-rc1.alpha+34") == 19
+assert dlite.chk_semver("1.12.2-rc1.alpha+34 ") < 0
+assert dlite.chk_semver("1.12.2-rc1.alpha+34 ", 6) == 6
+assert dlite.chk_semver("1.12.2-rc1.alpha+34 ", 19) == 19
+
+assert dlite.cmp_semver("1.3.11", "1.3.3") > 0
+assert dlite.cmp_semver("1.3.11", "1.3.13") < 0
+assert dlite.cmp_semver("1.3.11", "1.3.13", 5) == 0
+
+
+# Test deprecation warnings
+dlite.deprecation_warning("100.3.2", "My deprecated feature...")
+dlite.deprecation_warning("100.3.2", "My deprecated feature...")
+dlite.deprecation_warning("100.3.2", "My deprecated feature...")
+
+with raises(SystemError):
+    dlite.deprecation_warning("0.0.1", "My deprecated feature 2...")
+
+# Issuing the same deprecation warning a second or third time should not
+# raise an exception
+dlite.deprecation_warning("0.0.1", "My deprecated feature 2...")
+dlite.deprecation_warning("0.0.1", "My deprecated feature 2...")
+
+with raises(SystemError):
+    dlite.deprecation_warning("0.0.x", "My deprecated feature 3...")

doc/contributors_guide/code_changes.md

@@ -1,15 +1,16 @@
 Code changes
 ============
-Although that DLite haven't reached version 1.0.0, it strives to be stable and not introduce unnecessary backward-incompatible changes.
+Although that DLite has not reached version 1.0.0, the intention is for it to be stable and not introduce unnecessary backward-incompatible changes.
 
 
 Versioning
 ----------
-Before reaching v 1.0.0, DLite follows the following versioning rules that are well-known from Python:
+Before reaching version 1.0.0, DLite follows the following versioning rules that are well-known from Python:
 
 * **patch release**: Only backward-compatible changes.
   - Addition of new backward-compatible features.
   - New deprecation warnings can be added.
+  - Bug fixes.
 
 * **minor release**: Backward-incompatible changes.
   - Addition of new backward-incompatible features.
@@ -21,11 +22,10 @@ After version 1.0.0, DLite should strictly follow the [semantic versioning] rule
 
 Feature deprecation
 -------------------
-In Python, use `dlite.deprecation_warning()` (function to be added) to mark deprecation warnings.
+In Python, use `dlite.deprecation_warning()` to mark deprecation warnings.
 This function is a simple wrapper around `warnings.warn(msg, DeprecationWarning, stacklevel=2)` with the release version when the feature is planned to be removed as an additional argument.
 
-In C, use the `deprecation_warning()` function (to be added) to mark a
-deprecation warning.
+In C, use the `dlite_deprecation_warning()` function to mark a deprecation warning.
 
 The idea with these functions is to make it easy to grep for deprecation warnings that should be remove in a new backward-incompatible release.
 The following command can e.g. be used to find all deprecation warnings:
@@ -43,7 +43,7 @@ Adding new arguments to an existing function or new methods to existing classes
 **Note**, new (positional) arguments should be added to the end of the argument list, such that existing code using positional arguments will be unaffected by the change.
 
 ### New behavior
-All changes in behavior should be considered backward-incompatible.
+All changes in behavior in existing code should be considered backward-incompatible.
 
 Where it make sense, DLite will try to optionally keep the new/old behaviour over some releases in order for users to adapt to the new behavior.
 
@@ -53,23 +53,29 @@ Each optional new behavior should have a:
 - release number for when the behavior was introduced (keeping old behavior as default)
 - release number for when the new behavior should be the default (managed automatically)
 - expected release number for when the old behavior should be removed (require developer effort)
+- value: True means that the behavior is enabled.
 
-All behaviors are described in a single source file `src/dlite-behavior.c`.
+All behavior changes will be described in a single source file `src/dlite-behavior.c`.
 
-Behavior can be selected in several ways:
+Whether to enable a behavior can be configured in several ways:
 
 - **programmatically from Python**:
 
-      >>> from dlite.behavior import Behavior
-      >>> Behavior.<NAME> = True  # true means use the new behavior
+      >>> dlite.Behavior.<NAME> = True
 
 - **programmatically from C**:
 
-      dlite_behavior("<NAME>", 1);
+      dlite_behavior_set("<NAME>", 1);
 
 - **via environment variables**:
 
-      export DLITE_BEHAVIOR_<NAME>
+      export DLITE_BEHAVIOR_<NAME>=1
+
+  The empty string or any of the following values will enable the behavior:
+  "true", ".true.", "on", "yes", 1
+
+  Any of the following values will disable the behavior:
+  "false", ".false.", "off", "no", 0
 
 A warning will automatically be issued if a behavior is not selected explicitly.
 

doc/user_guide/environment_variables.md

@@ -52,6 +52,14 @@ DLite-specific environment variables
   - **DLITE_ATEXIT_FREE**: Free memory at exit.  This might be useful to avoid
     getting false positive when tracking down memory leaks with tools like valgrind.
 
+  - **DLITE_BEHAVIOR_<name>**: Enables/disables the behavior `<name>`.
+
+    The empty string or any of the following values will enable the behavior:
+    "true", ".true.", "on", "yes", 1
+
+    Any of the following values will disable the behavior:
+    "false", ".false.", "off", "no", 0
+
 
 ### Specific paths
 These environment variables can be used to provide additional search

src/CMakeLists.txt

@@ -45,6 +45,7 @@ set(sources
   dlite-getlicense.c
   dlite-json.c
   dlite-bson.c
+  dlite-behavior.c
   getuuid.c
   pathshash.c
   triple.c