Merge pull request #2910 from alicevision/fix/nodeSourceCodeFolder

[core] Correctly set `_sourceCodeFolder` in `BaseNode`'s constructor

This PR fixes a regression introduced by the refactoring of the command line variables in #2888, where the static nodeSourceCodeFolder is initialized but never set with the correct value once it becomes available.

This is fixed by initializing _sourceCodeFolder directly in the BaseNode's constructor if the description is available (if it isn't, the node will be in Unknown Node compatibility error anyway). This both simplifies what was done prior to #2888 (where _sourceCodeFolder was correctly set later than it should have been) and ensures that _staticExpVars["nodeSourceCodeFolder"] is initialized with the correct value as well.

Additionally, simple unit tests are added to ensure that static variables are always correctly set. A test InputNode is also added to one of the test plugins to cover all cases.

Author: fabiencastan

meshroom/core/node.py

@@ -359,7 +359,7 @@ def name(self):
             return f"{self.node.name}({self.index})"
         else:
             return self.node.name
-    
+
     @property
     def logManager(self):
         if self._logManager is None:
@@ -520,7 +520,7 @@ def process(self, forceCompute=False, inCurrentEnv=False):
         executionStatus = None
         self.statThread = stats.StatisticsThread(self)
         self.statThread.start()
-        
+
         try:
             self.node.nodeDesc.processChunk(self)
             # NOTE: this assumes saving the output attributes for each chunk
@@ -672,7 +672,7 @@ def __init__(self, nodeType: str, position: Position = None, parent: BaseObject
         self.packageName: str = ""
         self.packageVersion: str = ""
         self._internalFolder: str = ""
-        self._sourceCodeFolder: str = ""
+        self._sourceCodeFolder: str = self.nodeDesc.sourceCodeFolder if self.nodeDesc else ""
         self._internalFolderExp = "{cache}/{nodeType}/{uid}"
 
         # temporary unique name for this node
@@ -742,7 +742,7 @@ def getNodeLogLevel(self):
         if self.hasInternalAttribute("nodeDefaultLogLevel"):
             return self.internalAttribute("nodeDefaultLogLevel").value.strip()
         return "info"
-    
+
     def getColor(self):
         """
         Returns:
@@ -1831,7 +1831,6 @@ def __init__(self, nodeType, position=None, parent=None, uid=None, **kwargs):
 
         self.packageName = self.nodeDesc.packageName
         self.packageVersion = self.nodeDesc.packageVersion
-        self._sourceCodeFolder = self.nodeDesc.sourceCodeFolder
 
         for attrDesc in self.nodeDesc.inputs:
             self._attributes.add(attributeFactory(attrDesc, kwargs.get(attrDesc.name, None),

tests/plugins/meshroom/pluginA/PluginAInputNode.py

@@ -0,0 +1,23 @@
+__version__ = "1.0"
+
+from meshroom.core import desc
+
+
+class PluginANodeA(desc.InputNode):
+    inputs = [
+        desc.File(
+            name="input",
+            label="Input",
+            description="",
+            value="",
+        ),
+    ]
+
+    outputs = [
+        desc.File(
+            name="output",
+            label="Output",
+            description="",
+            value="",
+        ),
+    ]

tests/plugins/meshroom/pluginC/PluginCNodeA.py

@@ -6,9 +6,9 @@
 
 class PluginCNodeA(desc.Node):
     """PluginCNodeA"""
-    
+
     author = "testAuthor"
-    
+
     inputs = [
         desc.File(
             name="input",

tests/test_nodes.py

@@ -5,10 +5,8 @@
 from pathlib import Path
 
 from meshroom.core import pluginManager, loadClassesNodes
-from meshroom.core.plugins import Plugin
-from meshroom.core import pluginManager
-from meshroom.core import pluginManager
 from meshroom.core.graph import Graph
+from meshroom.core.plugins import Plugin
 
 from .utils import registerNodeDesc
 
@@ -38,17 +36,68 @@ def test_loadedPlugin(self):
         assert plugin == self.plugin
         node = plugin.nodes["PluginCNodeA"]
         nodeType = node.nodeDescriptor
-        
+
         g = Graph("")
         registerNodeDesc(nodeType)
         node = g.addNewNode(nodeType.__name__)
-        
+
         nodeDocumentation = node.getDocumentation()
         assert nodeDocumentation == "PluginCNodeA"
         nodeInfos = {item["key"]: item["value"] for item in node.getNodeInfos()}
         assert nodeInfos["module"] == "pluginC.PluginCNodeA"
-        plugin_path = os.path.join(self.folder, "pluginC", "PluginCNodeA.py")
-        assert nodeInfos["modulePath"] == Path(plugin_path).as_posix()  # modulePath seems to follow linux convention
+        pluginPath = os.path.join(self.folder, "pluginC", "PluginCNodeA.py")
+        assert nodeInfos["modulePath"] == Path(pluginPath).as_posix()  # modulePath seems to follow linux convention
         assert nodeInfos["author"] == "testAuthor"
         assert nodeInfos["license"] == "no-license"
         assert nodeInfos["version"] == "1.0"
+
+
+class TestNodeVariables:
+    plugin = None
+
+    @classmethod
+    def setup_class(cls):
+        folder = os.path.join(os.path.dirname(__file__), "plugins", "meshroom")
+        package = "pluginA"
+        cls.plugin = Plugin(package, folder)
+        nodes = loadClassesNodes(folder, package)
+        for node in nodes:
+            cls.plugin.addNodePlugin(node)
+        pluginManager.addPlugin(cls.plugin)
+
+    @classmethod
+    def teardown_class(cls):
+        for node in cls.plugin.nodes.values():
+            pluginManager.unregisterNode(node)
+        pluginManager.removePlugin(cls.plugin)
+        cls.plugin = None
+
+    def test_staticVariables(self):
+        g = Graph("")
+
+        for nodeName in self.plugin.nodes.keys():
+            n = g.addNewNode(nodeName)
+            assert nodeName == n._staticExpVars["nodeType"]
+            assert n.sourceCodeFolder
+            assert n.sourceCodeFolder == n._staticExpVars["nodeSourceCodeFolder"]
+
+            self.plugin.nodes[nodeName].reload()
+
+            assert nodeName == n._staticExpVars["nodeType"]
+            assert n.sourceCodeFolder
+            assert n.sourceCodeFolder == n._staticExpVars["nodeSourceCodeFolder"]
+
+    def test_expVariables(self):
+        g = Graph("")
+
+        for nodeName in self.plugin.nodes.keys():
+            n = g.addNewNode(nodeName)
+            assert n._expVars["uid"] == n._uid
+            assert n.internalFolder
+            assert n.internalFolder == n._expVars["nodeCacheFolder"]
+
+            self.plugin.nodes[nodeName].reload()
+
+            assert n._expVars["uid"] == n._uid
+            assert n.internalFolder
+            assert n.internalFolder == n._expVars["nodeCacheFolder"]