Merge pull request #622 from jkloetzke/upper-case-keywords

input: custom properties must not start with lower case

Author: jkloetzke

contrib/plugins/path-fmt.py

@@ -1,15 +1,15 @@
 # Sample plugin to modify path name calculation
 #
 # This plugin introduces two new properties to a recipe:
-#  - checkoutDir: optional string that is appended to the source directory
-#  - platform: optional string that is appenden to the build and dist directories
+#  - CheckoutDir: optional string that is appended to the source directory
+#  - Platform: optional string that is appenden to the build and dist directories
 
 from os.path import join
 from bob.errors import ParseError
 from bob.input import PluginState, PluginProperty
 
 def commonFormatter(step, states):
-    s = states['pathFmt']
+    s = states['PathFmt']
     if step.isCheckoutStep():
         base = step.getPackage().getRecipe().getName()
         ext = s.getCheckoutDir()
@@ -35,15 +35,15 @@ def __init__(self):
         self.__platformDir = None
 
     def onEnter(self, env, properties):
-        # checkoutDir is always taken from current recipe
-        self.__checkoutDir = properties['checkoutDir'].getValue()
+        # CheckoutDir is always taken from current recipe
+        self.__checkoutDir = properties['CheckoutDir'].getValue()
         if self.__checkoutDir is not None:
-            self.__checkoutDir = env.substitute(self.__checkoutDir, "checkoutDir")
+            self.__checkoutDir = env.substitute(self.__checkoutDir, "CheckoutDir")
 
-        # platform is passed down to dependencies
-        platform = properties['platform']
+        # Platform is passed down to dependencies
+        platform = properties['Platform']
         if platform.isPresent():
-            self.__platformDir = env.substitute(platform.getValue(), "platform")
+            self.__platformDir = env.substitute(platform.getValue(), "Platform")
 
     def getCheckoutDir(self):
         return self.__checkoutDir
@@ -66,10 +66,10 @@ def validate(data):
         'jenkinsNameFormatter' : jenkinsFormatter
     },
     'properties' : {
-        "checkoutDir" : StringProperty,
-        "platform" : StringProperty
+        "CheckoutDir" : StringProperty,
+        "Platform" : StringProperty
     },
     'state' : {
-        "pathFmt" : PathFmtState
+        "PathFmt" : PathFmtState
     }
 }

doc/manual/extending.rst

@@ -365,13 +365,13 @@ The following example shows two trivial properties::
     manifest = {
         'apiVersion' : "0.21",
         'properties' : {
-            "checkoutDir" : StringProperty,
-            "platform" : StringProperty
+            "CheckoutDir" : StringProperty,
+            "Platform" : StringProperty
         },
     }
 
-The above example defines two new keywords in recipes: ``checkoutDir`` and
-``platform``. As verified by the ``validate`` method they need to be strings.
+The above example defines two new keywords in recipes: ``CheckoutDir`` and
+``Platform``. As verified by the ``validate`` method, they need to be strings.
 Because :func:`bob.input.PluginProperty.inherit` was not overridden, the recipe
 and higher priority classes will simply replace the value of lower priority
 classes. Other plugin extensions can query the value of a property by calling
@@ -383,6 +383,12 @@ If custom properties need to be propagated in the recipe dependency hierarchy,
 a *property state tracker* is required. A state tracker is a class that is
 invoked on every step when walking the dependency tree to instantiate the
 packages. The state tracker thus has the responsibility to calculate the final
-values associated with the properties for every package. Like properties there
+values associated with the properties for every package. Like properties, there
 can be more than one state tracker. Any state tracker provided by a plugin must
 be derived from :class:`bob.input.PluginState`.
+
+It is not possible to re-define already existing recipe properties. This
+applies both to Bob built-in properties as well as properties defined by other
+plugins.  Because Bob is expected to define new settings in the future, a
+plugin defined properties must not start with a lower case letter. These names
+are reserved for Bob.

pym/bob/input.py

@@ -3470,6 +3470,8 @@ def __loadPlugin(self, mangledName, fileName, name):
         for (i,j) in properties.items():
             if not isinstance(i, str):
                 raise ParseError("Plugin '"+fileName+"': property name must be a string!")
+            if i[:1].islower():
+                raise ParseError(f"Plugin '{fileName}': property '{i}' must not start lower case!")
             if not issubclass(j, PluginProperty):
                 raise ParseError("Plugin '"+fileName+"': property '" +i+"' has wrong type!")
             if i in self.__properties:
@@ -3482,8 +3484,8 @@ def __loadPlugin(self, mangledName, fileName, name):
         for (i,j) in states.items():
             if not isinstance(i, str):
                 raise ParseError("Plugin '"+fileName+"': state tracker name must be a string!")
-            if i in ["environment", "tools", "result", "deps", "sandbox"]:
-                raise ParseError("Plugin '"+fileName+"': state tracker has reserved name!")
+            if i[:1].islower():
+                raise ParseError(f"Plugin '{fileName}': state tracker '{i}' must not start lower case!")
             if not issubclass(j, PluginState):
                 raise ParseError("Plugin '"+fileName+"': state tracker '" +i+"' has wrong type!")
             if i in self.__states:

test/black-box/plugin-states/plugins/path-fmt.py

@@ -1,15 +1,15 @@
 # Sample plugin to modify path name calculation
 #
 # This plugin introduces two new properties to a recipe:
-#  - checkoutDir: optional string that is appended to the source directory
-#  - platform: optional string that is appenden to the build and dist directories
+#  - CheckoutDir: optional string that is appended to the source directory
+#  - Platform: optional string that is appenden to the build and dist directories
 
 from os.path import join
 from bob.errors import ParseError
 from bob.input import PluginState, PluginProperty
 
 def commonFormatter(step, states):
-    s = states['pathFmt']
+    s = states['PathFmt']
     if step.isCheckoutStep():
         base = step.getPackage().getRecipe().getName()
         ext = s.getCheckoutDir()
@@ -35,15 +35,15 @@ def __init__(self):
         self.__platformDir = None
 
     def onEnter(self, env, properties):
-        # checkoutDir is always taken from current recipe
-        self.__checkoutDir = properties['checkoutDir'].getValue()
+        # CheckoutDir is always taken from current recipe
+        self.__checkoutDir = properties['CheckoutDir'].getValue()
         if self.__checkoutDir is not None:
-            self.__checkoutDir = env.substitute(self.__checkoutDir, "checkoutDir")
+            self.__checkoutDir = env.substitute(self.__checkoutDir, "CheckoutDir")
 
-        # platform is passed down to dependencies
-        platform = properties['platform']
+        # Platform is passed down to dependencies
+        platform = properties['Platform']
         if platform.isPresent():
-            self.__platformDir = env.substitute(platform.getValue(), "platform")
+            self.__platformDir = env.substitute(platform.getValue(), "Platform")
 
     def getCheckoutDir(self):
         return self.__checkoutDir
@@ -66,10 +66,10 @@ def validate(data):
         'jenkinsNameFormatter' : jenkinsFormatter
     },
     'properties' : {
-        "checkoutDir" : StringProperty,
-        "platform" : StringProperty
+        "CheckoutDir" : StringProperty,
+        "Platform" : StringProperty
     },
     'state' : {
-        "pathFmt" : PathFmtState
+        "PathFmt" : PathFmtState
     }
 }

test/black-box/plugin-states/recipes/lib1.yaml

@@ -1,5 +1,5 @@
 # set property for special checkout subdirectory
-checkoutDir: "special"
+CheckoutDir: "special"
 checkoutVars: [PLATFORM]
 checkoutScript: |
   echo "lib1 $PLATFORM"

test/black-box/plugin-states/recipes/root.yaml

@@ -10,13 +10,13 @@ buildScript: |
 packageScript: |
   true
 
-# set two different "platform" properties that are inherited
+# set two different "Platform" properties that are inherited
 multiPackage:
   alpha:
-    platform: "alpha"
+    Platform: "alpha"
     environment:
       PLATFORM: "alpha"
   bravo:
-    platform: "bravo"
+    Platform: "bravo"
     environment:
       PLATFORM: "bravo"