From 5fdaeaa8993726cfb4cfd2af52b5295cbfdfd2a3 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 18 Jun 2019 11:58:03 +0200 Subject: [PATCH 1/6] [libcalamaresui] Improve wording when module is missing configuration --- src/libcalamaresui/modulesystem/Module.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcalamaresui/modulesystem/Module.cpp b/src/libcalamaresui/modulesystem/Module.cpp index d05245384..44798b32b 100644 --- a/src/libcalamaresui/modulesystem/Module.cpp +++ b/src/libcalamaresui/modulesystem/Module.cpp @@ -198,7 +198,7 @@ Module::loadConfigurationFile( const QString& configFileName ) //throws YAML::Ex return; } } - cDebug() << "No config file found in" << Logger::DebugList( configCandidates ); + cDebug() << "No config file for" << m_name << "found anywhere at" << Logger::DebugList( configCandidates ); } From 34b1a250bae02896c14d2e0cf7d32c5013e1a650 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 18 Jun 2019 12:06:13 +0200 Subject: [PATCH 2/6] [libcalamares] Improve warnings when module descriptor files are bad --- .../modulesystem/ModuleManager.cpp | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/libcalamaresui/modulesystem/ModuleManager.cpp b/src/libcalamaresui/modulesystem/ModuleManager.cpp index 185ec37e9..a030e55cd 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.cpp +++ b/src/libcalamaresui/modulesystem/ModuleManager.cpp @@ -90,10 +90,14 @@ ModuleManager::doInit() if ( success ) { QFileInfo descriptorFileInfo( currentDir.absoluteFilePath( QLatin1Literal( "module.desc") ) ); - if ( ! ( descriptorFileInfo.exists() && descriptorFileInfo.isReadable() ) ) + if ( !descriptorFileInfo.exists() ) { - cDebug() << Q_FUNC_INFO << "unreadable file: " - << descriptorFileInfo.absoluteFilePath(); + cDebug() << "ModuleManager expected descriptor is missing:" << descriptorFileInfo.absoluteFilePath(); + continue; + } + if ( !descriptorFileInfo.isReadable() ) + { + cDebug() << "ModuleManager descriptor file is unreadable:" << descriptorFileInfo.absoluteFilePath(); continue; } @@ -111,13 +115,14 @@ ModuleManager::doInit() } else { - cWarning() << "Cannot cd into module directory " - << path << "/" << subdir; + cWarning() << "ModuleManager module directory is not accessible:" << path << "/" << subdir; } } } else - cDebug() << "ModuleManager bad search path" << path; + { + cDebug() << "ModuleManager module search path does not exist:" << path; + } } // At this point m_availableModules is filled with whatever was found in the // search paths. @@ -359,7 +364,7 @@ ModuleManager::checkDependencies() m_availableDescriptorsByModuleName.erase( it ); failed << moduleName; cWarning() << "Module" << moduleName << "requires modules" << Logger::DebugList( unmet ); - cWarning() << Logger::SubEntry << "but these are not available (listed in settings, or installed)."; + cWarning() << Logger::SubEntry << "but these are not available (listed in settings, or installed)."; break; } } From 6183c4e2f40eb49b40e0bdcc6cb866729d0afc3b Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 18 Jun 2019 12:17:25 +0200 Subject: [PATCH 3/6] [libcalamares] Add accessors for GeoIP handler attributes --- src/libcalamares/geoip/Handler.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libcalamares/geoip/Handler.h b/src/libcalamares/geoip/Handler.h index 5c3207b60..8e435b5b7 100644 --- a/src/libcalamares/geoip/Handler.h +++ b/src/libcalamares/geoip/Handler.h @@ -25,7 +25,7 @@ #include #include -namespace CalamaresUtils +namespace CalamaresUtils { namespace GeoIP { @@ -80,6 +80,8 @@ public: bool isValid() const { return m_type != Type::None; } Type type() const { return m_type; } + QString url() const { return m_url; } + QString selector() const { return m_selector; } private: Type m_type; From 3967f6c5ae75ac23c6b97bcbeb5258285caf6b23 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 18 Jun 2019 12:24:30 +0200 Subject: [PATCH 4/6] [welcome] Log where GeoIP information came from, if it's unusable - This helps chase down broken GeoIP configurations, since you can check the URL and handler type shown in the log. --- src/modules/welcome/WelcomeViewStep.cpp | 15 +++++++++++++-- src/modules/welcome/WelcomeViewStep.h | 13 +++++++++++-- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/modules/welcome/WelcomeViewStep.cpp b/src/modules/welcome/WelcomeViewStep.cpp index a27cc4cf0..cccf49458 100644 --- a/src/modules/welcome/WelcomeViewStep.cpp +++ b/src/modules/welcome/WelcomeViewStep.cpp @@ -133,7 +133,7 @@ WelcomeViewStep::setConfigurationMap( const QVariantMap& configurationMap ) { QString countryResult = f->future().result(); cDebug() << "GeoIP result for welcome=" << countryResult; - view->setCountry( countryResult ); + view->setCountry( countryResult, h ); f->deleteLater(); delete h; } ); @@ -156,12 +156,22 @@ WelcomeViewStep::checkRequirements() return m_requirementsChecker->checkRequirements(); } +static inline void +logGeoIPHandler( CalamaresUtils::GeoIP::Handler* handler ) +{ + if ( handler ) + { + cDebug() << Logger::SubEntry << "Obtained from" << handler->url() << " (" << static_cast( handler->type() ) << handler->selector() << ')'; + } +} + void -WelcomeViewStep::setCountry( const QString& countryCode ) +WelcomeViewStep::setCountry( const QString& countryCode, CalamaresUtils::GeoIP::Handler* handler ) { if ( countryCode.length() != 2 ) { cDebug() << "Unusable country code" << countryCode; + logGeoIPHandler( handler ); return; } @@ -169,6 +179,7 @@ WelcomeViewStep::setCountry( const QString& countryCode ) if ( c_l.first == QLocale::Country::AnyCountry ) { cDebug() << "Unusable country code" << countryCode; + logGeoIPHandler( handler ); return; } else diff --git a/src/modules/welcome/WelcomeViewStep.h b/src/modules/welcome/WelcomeViewStep.h index 7deed2167..5d27d7aad 100644 --- a/src/modules/welcome/WelcomeViewStep.h +++ b/src/modules/welcome/WelcomeViewStep.h @@ -32,6 +32,14 @@ class WelcomePage; class GeneralRequirements; +namespace CalamaresUtils +{ + namespace GeoIP + { + class Handler; + } +} // namespace + class PLUGINDLLEXPORT WelcomeViewStep : public Calamares::ViewStep { Q_OBJECT @@ -57,9 +65,10 @@ public: /** @brief Sets the country that Calamares is running in. * * This (ideally) sets up language and locale settings that are right for - * the given 2-letter country code. + * the given 2-letter country code. Uses the handler's information (if + * given) for error reporting. */ - void setCountry( const QString& ); + void setCountry( const QString&, CalamaresUtils::GeoIP::Handler* handler ); Calamares::RequirementsList checkRequirements() override; From 68dc1f5e318ae59bc80690d525de35e6af5802ec Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 18 Jun 2019 12:33:56 +0200 Subject: [PATCH 5/6] [libcalamares] Warn about badly-configured GeoIP - Warn when type will be none - Re-order warnings from general to specific --- src/libcalamares/geoip/Handler.cpp | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/libcalamares/geoip/Handler.cpp b/src/libcalamares/geoip/Handler.cpp index 192ab1a7c..df033f476 100644 --- a/src/libcalamares/geoip/Handler.cpp +++ b/src/libcalamares/geoip/Handler.cpp @@ -64,17 +64,21 @@ Handler::Handler( const QString& implementation, const QString& url, const QStri { bool ok = false; m_type = handlerTypes().find( implementation, ok ); -#if !defined(QT_XML_LIB) - if ( m_type == Type::XML ) - { - m_type = Type::None; - cWarning() << "GeoIP style XML is not supported in this version of Calamares."; - } -#endif if ( !ok ) { - cWarning() << "GeoIP Style" << implementation << "is not recognized."; + cWarning() << "GeoIP style" << implementation << "is not recognized."; } + else if ( m_type == Type::None ) + { + cWarning() << "GeoIP style *none* does not do anything."; + } +#if !defined(QT_XML_LIB) + else if ( m_type == Type::XML ) + { + m_type = Type::None; + cWarning() << "GeoIP style *xml* is not supported in this version of Calamares."; + } +#endif } Handler::~Handler() From 0f66a892363a6f0972726da14fd7dfae4e75f8bc Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 18 Jun 2019 12:34:52 +0200 Subject: [PATCH 6/6] [welcome] Only do GeoIP query if it's useful - If badly-configured, then type is none; this is warned about in the constructor of Handler() - Only run the query if it's a useful type. --- src/modules/welcome/WelcomeViewStep.cpp | 26 ++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/modules/welcome/WelcomeViewStep.cpp b/src/modules/welcome/WelcomeViewStep.cpp index cccf49458..938fe1f45 100644 --- a/src/modules/welcome/WelcomeViewStep.cpp +++ b/src/modules/welcome/WelcomeViewStep.cpp @@ -128,16 +128,24 @@ WelcomeViewStep::setConfigurationMap( const QVariantMap& configurationMap ) CalamaresUtils::getString( geoip, "style" ), CalamaresUtils::getString( geoip, "url" ), CalamaresUtils::getString( geoip, "selector" ) ); - auto* future = new FWString(); - connect( future, &FWString::finished, [view=this, f=future, h=handler]() + if ( handler->type() != CalamaresUtils::GeoIP::Handler::Type::None ) { - QString countryResult = f->future().result(); - cDebug() << "GeoIP result for welcome=" << countryResult; - view->setCountry( countryResult, h ); - f->deleteLater(); - delete h; - } ); - future->setFuture( handler->queryRaw() ); + auto* future = new FWString(); + connect( future, &FWString::finished, [view=this, f=future, h=handler]() + { + QString countryResult = f->future().result(); + cDebug() << "GeoIP result for welcome=" << countryResult; + view->setCountry( countryResult, h ); + f->deleteLater(); + delete h; + } ); + future->setFuture( handler->queryRaw() ); + } + else + { + // Would not produce useful country code anyway. + delete handler; + } }