[libcalamares] Add a path parameter when creating descriptors

This allows us to print the path of a descriptor file in
error messages, which in turn makes it easier to find
problems with the descriptor files.

src/calamares/testmain.cpp

@@ -210,10 +210,11 @@ ExecViewModule::ExecViewModule()
 {
     // Normally the module-loader gives the module an instance key
     // (out of the settings file, or the descriptor of the module).
-    // We don't have one, so build one -- this gives us "x@x".
+    // We don't have one, so build one -- this gives us "execView@execView".
     QVariantMap m;
-    m.insert( "name", "x" );
+    const QString execView = QStringLiteral( "execView" );
-    Calamares::Module::initFrom( Calamares::ModuleSystem::Descriptor::fromDescriptorData( m ), "x" );
+    m.insert( "name", execView );
+    Calamares::Module::initFrom( Calamares::ModuleSystem::Descriptor::fromDescriptorData( m, execView ), execView );
 }
 
 ExecViewModule::~ExecViewModule() {}
@@ -291,7 +292,7 @@ load_module( const ModuleConfig& moduleConfig )
         return new ExecViewModule;
     }
 
-    QFileInfo fi;
+    QFileInfo fi;  // This is kept around to hold the path of the module descriptor
 
     bool ok = false;
     QVariantMap descriptor;
@@ -357,7 +358,10 @@ load_module( const ModuleConfig& moduleConfig )
     cDebug() << Logger::SubEntry << "Module" << moduleName << "job-configuration:" << configFile;
 
     Calamares::Module* module = Calamares::moduleFromDescriptor(
-        Calamares::ModuleSystem::Descriptor::fromDescriptorData( descriptor ), name, configFile, moduleDirectory );
+        Calamares::ModuleSystem::Descriptor::fromDescriptorData( descriptor, fi.absoluteFilePath() ),
+        name,
+        configFile,
+        moduleDirectory );
 
     return module;
 }

src/libcalamares/modulesystem/Descriptor.cpp

@@ -50,9 +50,10 @@ interfaceNames()
 Descriptor::Descriptor() {}
 
 Descriptor
-Descriptor::fromDescriptorData( const QVariantMap& moduleDesc )
+Descriptor::fromDescriptorData( const QVariantMap& moduleDesc, const QString& descriptorPath )
 {
     Descriptor d;
+    Logger::Once o;
 
     {
         bool typeOk = false;
@@ -60,7 +61,11 @@ Descriptor::fromDescriptorData( const QVariantMap& moduleDesc )
         Type t = typeNames().find( typeValue, typeOk );
         if ( !typeOk )
         {
-            cWarning() << "Module descriptor contains invalid *type*" << typeValue;
+            if ( o )
+            {
+                cWarning() << o << "Descriptor file" << descriptorPath;
+            }
+            cWarning() << o << "Module descriptor contains invalid *type*" << typeValue;
         }
 
         bool interfaceOk = false;
@@ -68,7 +73,11 @@ Descriptor::fromDescriptorData( const QVariantMap& moduleDesc )
         Interface i = interfaceNames().find( interfaceValue, interfaceOk );
         if ( !interfaceOk )
         {
-            cWarning() << "Module descriptor contains invalid *interface*" << interfaceValue;
+            if ( o )
+            {
+                cWarning() << o << "Descriptor file" << descriptorPath;
+            }
+            cWarning() << o << "Module descriptor contains invalid *interface*" << interfaceValue;
         }
 
         d.m_name = moduleDesc.value( "name" ).toString();
@@ -102,7 +111,11 @@ Descriptor::fromDescriptorData( const QVariantMap& moduleDesc )
         d.m_script = CalamaresUtils::getString( moduleDesc, "script" );
         if ( d.m_script.isEmpty() )
         {
-            cWarning() << "Module descriptor contains no *script*" << d.name();
+            if ( o )
+            {
+                cWarning() << o << "Descriptor file" << descriptorPath;
+            }
+            cWarning() << o << "Module descriptor contains no *script*" << d.name();
             d.m_isValid = false;
         }
         consumedKeys << "script";
@@ -117,7 +130,11 @@ Descriptor::fromDescriptorData( const QVariantMap& moduleDesc )
         }
         if ( d.m_script.isEmpty() )
         {
-            cWarning() << "Module descriptor contains no *script*" << d.name();
+            if ( o )
+            {
+                cWarning() << o << "Descriptor file" << descriptorPath;
+            }
+            cWarning() << o << "Module descriptor contains no *script*" << d.name();
             d.m_isValid = false;
         }
         consumedKeys << "command"
@@ -141,7 +158,11 @@ Descriptor::fromDescriptorData( const QVariantMap& moduleDesc )
     }
     if ( !superfluousKeys.isEmpty() )
     {
-        cWarning() << "Module descriptor contains extra keys:" << Logger::DebugList( superfluousKeys );
+        if ( o )
+        {
+            cWarning() << o << "Descriptor file" << descriptorPath;
+        }
+        cWarning() << o << "Module descriptor contains extra keys:" << Logger::DebugList( superfluousKeys );
         d.m_isValid = false;
     }
 

src/libcalamares/modulesystem/Descriptor.h

@@ -58,9 +58,12 @@ public:
     Descriptor();
 
     /** @brief Fills a descriptor from the loaded (YAML) data.
+     *
+     * The @p descriptorPath is used only for debug messages, the
+     * data is only read from @p moduleDesc.
      *
      */
-    static Descriptor fromDescriptorData( const QVariantMap& moduleDesc );
+    static Descriptor fromDescriptorData( const QVariantMap& moduleDesc, const QString& descriptorPath );
 
     bool isValid() const { return m_isValid; }
 

src/libcalamares/modulesystem/Tests.cpp

@@ -132,9 +132,10 @@ ModuleSystemTests::testBadFromStringCases()
 void
 ModuleSystemTests::testBasicDescriptor()
 {
+    const QString path = QStringLiteral( "/bogus.desc" );
     {
         QVariantMap m;
-        auto d = Calamares::ModuleSystem::Descriptor::fromDescriptorData( m );
+        auto d = Calamares::ModuleSystem::Descriptor::fromDescriptorData( m, path );
 
         QVERIFY( !d.isValid() );
         QVERIFY( d.name().isEmpty() );
@@ -142,7 +143,7 @@ ModuleSystemTests::testBasicDescriptor()
     {
         QVariantMap m;
         m.insert( "name", QVariant() );
-        auto d = Calamares::ModuleSystem::Descriptor::fromDescriptorData( m );
+        auto d = Calamares::ModuleSystem::Descriptor::fromDescriptorData( m, path );
 
         QVERIFY( !d.isValid() );
         QVERIFY( d.name().isEmpty() );
@@ -150,7 +151,7 @@ ModuleSystemTests::testBasicDescriptor()
     {
         QVariantMap m;
         m.insert( "name", 17 );
-        auto d = Calamares::ModuleSystem::Descriptor::fromDescriptorData( m );
+        auto d = Calamares::ModuleSystem::Descriptor::fromDescriptorData( m, path );
 
         QVERIFY( !d.isValid() );
         QVERIFY( !d.name().isEmpty() );
@@ -161,7 +162,7 @@ ModuleSystemTests::testBasicDescriptor()
         m.insert( "name", "welcome" );
         m.insert( "type", "viewmodule" );
         m.insert( "interface", "qtplugin" );
-        auto d = Calamares::ModuleSystem::Descriptor::fromDescriptorData( m );
+        auto d = Calamares::ModuleSystem::Descriptor::fromDescriptorData( m, path );
 
         // QVERIFY( !d.isValid() );
         QCOMPARE( d.name(), QStringLiteral( "welcome" ) );

src/libcalamaresui/modulesystem/ModuleManager.cpp

@@ -104,11 +104,20 @@ ModuleManager::doInit()
                     if ( ok && !moduleName.isEmpty() && ( moduleName == currentDir.dirName() )
                          && !m_availableDescriptorsByModuleName.contains( moduleName ) )
                     {
-                        auto descriptor
+                        auto descriptor = Calamares::ModuleSystem::Descriptor::fromDescriptorData(
-                            = Calamares::ModuleSystem::Descriptor::fromDescriptorData( moduleDescriptorMap );
+                            moduleDescriptorMap, descriptorFileInfo.absoluteFilePath() );
                         descriptor.setDirectory( descriptorFileInfo.absoluteDir().absolutePath() );
                         m_availableDescriptorsByModuleName.insert( moduleName, descriptor );
                     }
+                    else
+                    {
+                        // Duplicate modules are ok; other issues like empty name or dir-mismatch are reported.
+                        if ( !m_availableDescriptorsByModuleName.contains( moduleName ) )
+                        {
+                            cWarning() << deb << "ModuleManager module descriptor"
+                                       << descriptorFileInfo.absoluteFilePath() << "has bad name" << moduleName;
+                        }
+                    }
                 }
                 else
                 {