From 782b4699742309c9b4de89c718255331a593cbab Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 7 Sep 2019 12:38:49 +0200 Subject: [PATCH 01/13] [locale] Move all the translation work to a single slot --- src/modules/locale/LocalePage.cpp | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/modules/locale/LocalePage.cpp b/src/modules/locale/LocalePage.cpp index f6cdba436..4a48ecde8 100644 --- a/src/modules/locale/LocalePage.cpp +++ b/src/modules/locale/LocalePage.cpp @@ -215,15 +215,7 @@ LocalePage::LocalePage( QWidget* parent ) } ); - CALAMARES_RETRANSLATE( - m_regionLabel->setText( tr( "Region:" ) ); - m_zoneLabel->setText( tr( "Zone:" ) ); - - updateLocaleLabels(); - - m_localeChangeButton->setText( tr( "&Change..." ) ); - m_formatsChangeButton->setText( tr( "&Change..." ) ); - ) + CALAMARES_RETRANSLATE_SLOT( &LocalePage::updateLocaleLabels ) } @@ -234,6 +226,11 @@ LocalePage::~LocalePage() void LocalePage::updateLocaleLabels() { + m_regionLabel->setText( tr( "Region:" ) ); + m_zoneLabel->setText( tr( "Zone:" ) ); + m_localeChangeButton->setText( tr( "&Change..." ) ); + m_formatsChangeButton->setText( tr( "&Change..." ) ); + LocaleConfiguration lc = m_selectedLocaleConfiguration.isEmpty() ? guessLocaleConfiguration() : m_selectedLocaleConfiguration; From 91f050927284db0bc16d360c996437bb5865c30f Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 7 Sep 2019 15:18:58 +0200 Subject: [PATCH 02/13] [locale] Refactor lambdas to plain methods - Lengthy lambda's doing UI stuff -- that doesn't change -- are easier to read as plain methods. --- src/modules/locale/LocalePage.cpp | 78 ++++++++++++++++--------------- src/modules/locale/LocalePage.h | 3 ++ 2 files changed, 44 insertions(+), 37 deletions(-) diff --git a/src/modules/locale/LocalePage.cpp b/src/modules/locale/LocalePage.cpp index 4a48ecde8..c84cd924e 100644 --- a/src/modules/locale/LocalePage.cpp +++ b/src/modules/locale/LocalePage.cpp @@ -99,43 +99,8 @@ LocalePage::LocalePage( QWidget* parent ) setLayout( mainLayout ); - connect( m_regionCombo, - static_cast< void ( QComboBox::* )( int ) >( &QComboBox::currentIndexChanged ), - [this]( int currentIndex ) - { - Q_UNUSED( currentIndex ) - QHash< QString, QList< LocaleGlobal::Location > > regions = LocaleGlobal::getLocations(); - if ( !regions.contains( m_regionCombo->currentData().toString() ) ) - return; - - m_zoneCombo->blockSignals( true ); - - m_zoneCombo->clear(); - - const QList< LocaleGlobal::Location > zones = regions.value( m_regionCombo->currentData().toString() ); - for ( const LocaleGlobal::Location& zone : zones ) - { - m_zoneCombo->addItem( LocaleGlobal::Location::pretty( zone.zone ), zone.zone ); - } - - m_zoneCombo->model()->sort( 0 ); - - m_zoneCombo->blockSignals( false ); - - m_zoneCombo->currentIndexChanged( m_zoneCombo->currentIndex() ); - } ); - - connect( m_zoneCombo, - static_cast< void ( QComboBox::* )( int ) >( &QComboBox::currentIndexChanged ), - [this]( int currentIndex ) - { - Q_UNUSED( currentIndex ) - if ( !m_blockTzWidgetSet ) - m_tzWidget->setCurrentLocation( m_regionCombo->currentData().toString(), - m_zoneCombo->currentData().toString() ); - - updateGlobalStorage(); - } ); + connect( m_regionCombo, QOverload< int >::of( &QComboBox::currentIndexChanged ), this, &LocalePage::regionChanged ); + connect( m_zoneCombo, QOverload< int >::of( &QComboBox::currentIndexChanged ), this, &LocalePage::zoneChanged ); connect( m_tzWidget, &TimeZoneWidget::locationChanged, [this]( LocaleGlobal::Location location ) @@ -517,3 +482,42 @@ LocalePage::updateGlobalStorage() m_selectedLocaleConfiguration = newLocale; updateLocaleLabels(); } + + +void +LocalePage::regionChanged( int currentIndex ) +{ + Q_UNUSED( currentIndex ) + QHash< QString, QList< LocaleGlobal::Location > > regions = LocaleGlobal::getLocations(); + if ( !regions.contains( m_regionCombo->currentData().toString() ) ) + { + return; + } + + m_zoneCombo->blockSignals( true ); + + m_zoneCombo->clear(); + + const QList< LocaleGlobal::Location > zones = regions.value( m_regionCombo->currentData().toString() ); + for ( const LocaleGlobal::Location& zone : zones ) + { + m_zoneCombo->addItem( LocaleGlobal::Location::pretty( zone.zone ), zone.zone ); + } + + m_zoneCombo->model()->sort( 0 ); + + m_zoneCombo->blockSignals( false ); + + m_zoneCombo->currentIndexChanged( m_zoneCombo->currentIndex() ); +} + +void +LocalePage::zoneChanged( int currentIndex ) +{ + Q_UNUSED( currentIndex ) + if ( !m_blockTzWidgetSet ) + m_tzWidget->setCurrentLocation( m_regionCombo->currentData().toString(), + m_zoneCombo->currentData().toString() ); + + updateGlobalStorage(); +} diff --git a/src/modules/locale/LocalePage.h b/src/modules/locale/LocalePage.h index 20ad444c9..9aa7bf4f8 100644 --- a/src/modules/locale/LocalePage.h +++ b/src/modules/locale/LocalePage.h @@ -65,6 +65,9 @@ private: void updateGlobalStorage(); void updateLocaleLabels(); + void regionChanged( int currentIndex ); + void zoneChanged( int currentIndex ); + TimeZoneWidget* m_tzWidget; QComboBox* m_regionCombo; QComboBox* m_zoneCombo; From 21dde80a6598c040b5b747c316451c822452bca7 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 7 Sep 2019 15:47:12 +0200 Subject: [PATCH 03/13] [locale] Refactor button handling to plain methods --- src/modules/locale/LocalePage.cpp | 160 +++++++++++++++--------------- src/modules/locale/LocalePage.h | 5 + 2 files changed, 87 insertions(+), 78 deletions(-) diff --git a/src/modules/locale/LocalePage.cpp b/src/modules/locale/LocalePage.cpp index c84cd924e..ff9032aed 100644 --- a/src/modules/locale/LocalePage.cpp +++ b/src/modules/locale/LocalePage.cpp @@ -101,84 +101,9 @@ LocalePage::LocalePage( QWidget* parent ) connect( m_regionCombo, QOverload< int >::of( &QComboBox::currentIndexChanged ), this, &LocalePage::regionChanged ); connect( m_zoneCombo, QOverload< int >::of( &QComboBox::currentIndexChanged ), this, &LocalePage::zoneChanged ); - - connect( m_tzWidget, &TimeZoneWidget::locationChanged, - [this]( LocaleGlobal::Location location ) - { - m_blockTzWidgetSet = true; - - // Set region index - int index = m_regionCombo->findData( location.region ); - if ( index < 0 ) - return; - - m_regionCombo->setCurrentIndex( index ); - - // Set zone index - index = m_zoneCombo->findData( location.zone ); - if ( index < 0 ) - return; - - m_zoneCombo->setCurrentIndex( index ); - - m_blockTzWidgetSet = false; - - updateGlobalStorage(); - } ); - - connect( m_localeChangeButton, &QPushButton::clicked, - [this] - { - LCLocaleDialog* dlg = - new LCLocaleDialog( m_selectedLocaleConfiguration.isEmpty() ? - guessLocaleConfiguration().language() : - m_selectedLocaleConfiguration.language(), - m_localeGenLines, - this ); - dlg->exec(); - if ( dlg->result() == QDialog::Accepted && - !dlg->selectedLCLocale().isEmpty() ) - { - m_selectedLocaleConfiguration.setLanguage( dlg->selectedLCLocale() ); - m_selectedLocaleConfiguration.explicit_lang = true; - this->updateGlobalLocale(); - this->updateLocaleLabels(); - } - - dlg->deleteLater(); - } ); - - connect( m_formatsChangeButton, &QPushButton::clicked, - [this] - { - LCLocaleDialog* dlg = - new LCLocaleDialog( m_selectedLocaleConfiguration.isEmpty() ? - guessLocaleConfiguration().lc_numeric : - m_selectedLocaleConfiguration.lc_numeric, - m_localeGenLines, - this ); - dlg->exec(); - if ( dlg->result() == QDialog::Accepted && - !dlg->selectedLCLocale().isEmpty() ) - { - // TODO: improve the granularity of this setting. - m_selectedLocaleConfiguration.lc_numeric = dlg->selectedLCLocale(); - m_selectedLocaleConfiguration.lc_time = dlg->selectedLCLocale(); - m_selectedLocaleConfiguration.lc_monetary = dlg->selectedLCLocale(); - m_selectedLocaleConfiguration.lc_paper = dlg->selectedLCLocale(); - m_selectedLocaleConfiguration.lc_name = dlg->selectedLCLocale(); - m_selectedLocaleConfiguration.lc_address = dlg->selectedLCLocale(); - m_selectedLocaleConfiguration.lc_telephone = dlg->selectedLCLocale(); - m_selectedLocaleConfiguration.lc_measurement = dlg->selectedLCLocale(); - m_selectedLocaleConfiguration.lc_identification = dlg->selectedLCLocale(); - m_selectedLocaleConfiguration.explicit_lc = true; - - this->updateLocaleLabels(); - } - - dlg->deleteLater(); - - } ); + connect( m_tzWidget, &TimeZoneWidget::locationChanged, this, &LocalePage::locationChanged ); + connect( m_localeChangeButton, &QPushButton::clicked, this, &LocalePage::changeLocale ); + connect( m_formatsChangeButton, &QPushButton::clicked, this, &LocalePage::changeFormats ); CALAMARES_RETRANSLATE_SLOT( &LocalePage::updateLocaleLabels ) } @@ -521,3 +446,82 @@ LocalePage::zoneChanged( int currentIndex ) updateGlobalStorage(); } + +void +LocalePage::locationChanged( LocaleGlobal::Location location ) +{ + m_blockTzWidgetSet = true; + + // Set region index + int index = m_regionCombo->findData( location.region ); + if ( index < 0 ) + return; + + m_regionCombo->setCurrentIndex( index ); + + // Set zone index + index = m_zoneCombo->findData( location.zone ); + if ( index < 0 ) + return; + + m_zoneCombo->setCurrentIndex( index ); + + m_blockTzWidgetSet = false; + + updateGlobalStorage(); +} + +void +LocalePage::changeLocale() +{ + LCLocaleDialog* dlg = + new LCLocaleDialog( m_selectedLocaleConfiguration.isEmpty() ? + guessLocaleConfiguration().language() : + m_selectedLocaleConfiguration.language(), + m_localeGenLines, + this ); + dlg->exec(); + if ( dlg->result() == QDialog::Accepted && + !dlg->selectedLCLocale().isEmpty() ) + { + m_selectedLocaleConfiguration.setLanguage( dlg->selectedLCLocale() ); + m_selectedLocaleConfiguration.explicit_lang = true; + this->updateGlobalLocale(); + this->updateLocaleLabels(); + } + + dlg->deleteLater(); +} + + +void +LocalePage::changeFormats() +{ + LCLocaleDialog* dlg = + new LCLocaleDialog( m_selectedLocaleConfiguration.isEmpty() ? + guessLocaleConfiguration().lc_numeric : + m_selectedLocaleConfiguration.lc_numeric, + m_localeGenLines, + this ); + dlg->exec(); + if ( dlg->result() == QDialog::Accepted && + !dlg->selectedLCLocale().isEmpty() ) + { + // TODO: improve the granularity of this setting. + m_selectedLocaleConfiguration.lc_numeric = dlg->selectedLCLocale(); + m_selectedLocaleConfiguration.lc_time = dlg->selectedLCLocale(); + m_selectedLocaleConfiguration.lc_monetary = dlg->selectedLCLocale(); + m_selectedLocaleConfiguration.lc_paper = dlg->selectedLCLocale(); + m_selectedLocaleConfiguration.lc_name = dlg->selectedLCLocale(); + m_selectedLocaleConfiguration.lc_address = dlg->selectedLCLocale(); + m_selectedLocaleConfiguration.lc_telephone = dlg->selectedLCLocale(); + m_selectedLocaleConfiguration.lc_measurement = dlg->selectedLCLocale(); + m_selectedLocaleConfiguration.lc_identification = dlg->selectedLCLocale(); + m_selectedLocaleConfiguration.explicit_lc = true; + + this->updateLocaleLabels(); + } + + dlg->deleteLater(); + +} diff --git a/src/modules/locale/LocalePage.h b/src/modules/locale/LocalePage.h index 9aa7bf4f8..5e4dbe8e9 100644 --- a/src/modules/locale/LocalePage.h +++ b/src/modules/locale/LocalePage.h @@ -21,6 +21,8 @@ #define LOCALEPAGE_H #include "LocaleConfiguration.h" +#include "timezonewidget/localeglobal.h" + #include "Job.h" #include @@ -67,6 +69,9 @@ private: void regionChanged( int currentIndex ); void zoneChanged( int currentIndex ); + void locationChanged( LocaleGlobal::Location location ); + void changeLocale(); + void changeFormats(); TimeZoneWidget* m_tzWidget; QComboBox* m_regionCombo; From 3093f635e2040a34d272b54f13b1aadcbcc56a90 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 7 Sep 2019 15:48:22 +0200 Subject: [PATCH 04/13] [locale] Apply coding style --- src/modules/locale/LocalePage.cpp | 151 ++++++++++++++---------------- src/modules/locale/LocalePage.h | 6 +- 2 files changed, 74 insertions(+), 83 deletions(-) diff --git a/src/modules/locale/LocalePage.cpp b/src/modules/locale/LocalePage.cpp index ff9032aed..ecca8ca28 100644 --- a/src/modules/locale/LocalePage.cpp +++ b/src/modules/locale/LocalePage.cpp @@ -35,8 +35,8 @@ #include #include #include -#include #include +#include LocalePage::LocalePage( QWidget* parent ) : QWidget( parent ) @@ -109,8 +109,7 @@ LocalePage::LocalePage( QWidget* parent ) } -LocalePage::~LocalePage() -{} +LocalePage::~LocalePage() {} void @@ -121,9 +120,8 @@ LocalePage::updateLocaleLabels() m_localeChangeButton->setText( tr( "&Change..." ) ); m_formatsChangeButton->setText( tr( "&Change..." ) ); - LocaleConfiguration lc = m_selectedLocaleConfiguration.isEmpty() ? - guessLocaleConfiguration() : - m_selectedLocaleConfiguration; + LocaleConfiguration lc + = m_selectedLocaleConfiguration.isEmpty() ? guessLocaleConfiguration() : m_selectedLocaleConfiguration; auto labels = prettyLocaleStatus( lc ); m_localeLabel->setText( labels.first ); m_formatsLabel->setText( labels.second ); @@ -131,9 +129,7 @@ LocalePage::updateLocaleLabels() void -LocalePage::init( const QString& initialRegion, - const QString& initialZone, - const QString& localeGenPath ) +LocalePage::init( const QString& initialRegion, const QString& initialZone, const QString& localeGenPath ) { m_regionCombo->blockSignals( true ); m_zoneCombo->blockSignals( true ); @@ -155,19 +151,18 @@ LocalePage::init( const QString& initialRegion, m_regionCombo->currentIndexChanged( m_regionCombo->currentIndex() ); // Default location - auto containsLocation = []( const QList< LocaleGlobal::Location >& locations, - const QString& zone ) -> bool - { + auto containsLocation = []( const QList< LocaleGlobal::Location >& locations, const QString& zone ) -> bool { for ( const LocaleGlobal::Location& location : locations ) { if ( location.zone == zone ) + { return true; + } } return false; }; - if ( keys.contains( initialRegion ) && - containsLocation( regions.value( initialRegion ), initialZone ) ) + if ( keys.contains( initialRegion ) && containsLocation( regions.value( initialRegion ), initialZone ) ) { m_tzWidget->setCurrentLocation( initialRegion, initialZone ); } @@ -185,14 +180,13 @@ LocalePage::init( const QString& initialRegion, QFile supported( "/usr/share/i18n/SUPPORTED" ); QByteArray ba; - if ( supported.exists() && - supported.open( QIODevice::ReadOnly | QIODevice::Text ) ) + if ( supported.exists() && supported.open( QIODevice::ReadOnly | QIODevice::Text ) ) { ba = supported.readAll(); supported.close(); const auto lines = ba.split( '\n' ); - for ( const QByteArray &line : lines ) + for ( const QByteArray& line : lines ) { m_localeGenLines.append( QString::fromLatin1( line.simplified() ) ); } @@ -208,28 +202,32 @@ LocalePage::init( const QString& initialRegion, else { cWarning() << "Cannot open file" << localeGenPath - << ". Assuming the supported languages are already built into " - "the locale archive."; + << ". Assuming the supported languages are already built into " + "the locale archive."; QProcess localeA; localeA.start( "locale", QStringList() << "-a" ); localeA.waitForFinished(); ba = localeA.readAllStandardOutput(); } const auto lines = ba.split( '\n' ); - for ( const QByteArray &line : lines ) + for ( const QByteArray& line : lines ) { - if ( line.startsWith( "## " ) || - line.startsWith( "# " ) || - line.simplified() == "#" ) + if ( line.startsWith( "## " ) || line.startsWith( "# " ) || line.simplified() == "#" ) + { continue; + } QString lineString = QString::fromLatin1( line.simplified() ); if ( lineString.startsWith( "#" ) ) + { lineString.remove( '#' ); + } lineString = lineString.simplified(); if ( lineString.isEmpty() ) + { continue; + } m_localeGenLines.append( lineString ); } @@ -238,41 +236,44 @@ LocalePage::init( const QString& initialRegion, if ( m_localeGenLines.isEmpty() ) { cWarning() << "cannot acquire a list of available locales." - << "The locale and localecfg modules will be broken as long as this " - "system does not provide" - << "\n\t " - << "* a well-formed" - << supported.fileName() - << "\n\tOR" - << "* a well-formed" - << (localeGenPath.isEmpty() ? QLatin1Literal("/etc/locale.gen") : localeGenPath) - << "\n\tOR" - << "* a complete pre-compiled locale-gen database which allows complete locale -a output."; - return; // something went wrong and there's nothing we can do about it. + << "The locale and localecfg modules will be broken as long as this " + "system does not provide" + << "\n\t " + << "* a well-formed" << supported.fileName() << "\n\tOR" + << "* a well-formed" + << ( localeGenPath.isEmpty() ? QLatin1Literal( "/etc/locale.gen" ) : localeGenPath ) << "\n\tOR" + << "* a complete pre-compiled locale-gen database which allows complete locale -a output."; + return; // something went wrong and there's nothing we can do about it. } // Assuming we have a list of supported locales, we usually only want UTF-8 ones // because it's not 1995. for ( auto it = m_localeGenLines.begin(); it != m_localeGenLines.end(); ) { - if ( !it->contains( "UTF-8", Qt::CaseInsensitive ) && - !it->contains( "utf8", Qt::CaseInsensitive ) ) + if ( !it->contains( "UTF-8", Qt::CaseInsensitive ) && !it->contains( "utf8", Qt::CaseInsensitive ) ) + { it = m_localeGenLines.erase( it ); + } else + { ++it; + } } // We strip " UTF-8" from "en_US.UTF-8 UTF-8" because it's redundant redundant. for ( auto it = m_localeGenLines.begin(); it != m_localeGenLines.end(); ++it ) { if ( it->endsWith( " UTF-8" ) ) + { it->chop( 6 ); + } *it = it->simplified(); } updateGlobalStorage(); } -std::pair< QString, QString > LocalePage::prettyLocaleStatus( const LocaleConfiguration& lc ) const +std::pair< QString, QString > +LocalePage::prettyLocaleStatus( const LocaleConfiguration& lc ) const { using CalamaresUtils::Locale::Label; @@ -288,14 +289,11 @@ QString LocalePage::prettyStatus() const { QString status; - status += tr( "Set timezone to %1/%2.
" ) - .arg( m_regionCombo->currentText() ) - .arg( m_zoneCombo->currentText() ); + status += tr( "Set timezone to %1/%2.
" ).arg( m_regionCombo->currentText() ).arg( m_zoneCombo->currentText() ); - LocaleConfiguration lc = m_selectedLocaleConfiguration.isEmpty() ? - guessLocaleConfiguration() : - m_selectedLocaleConfiguration; - auto labels = prettyLocaleStatus(lc); + LocaleConfiguration lc + = m_selectedLocaleConfiguration.isEmpty() ? guessLocaleConfiguration() : m_selectedLocaleConfiguration; + auto labels = prettyLocaleStatus( lc ); status += labels.first + "
"; status += labels.second + "
"; @@ -319,9 +317,8 @@ LocalePage::createJobs() QMap< QString, QString > LocalePage::localesMap() { - return m_selectedLocaleConfiguration.isEmpty() ? - guessLocaleConfiguration().toMap() : - m_selectedLocaleConfiguration.toMap(); + return m_selectedLocaleConfiguration.isEmpty() ? guessLocaleConfiguration().toMap() + : m_selectedLocaleConfiguration.toMap(); } @@ -329,8 +326,7 @@ void LocalePage::onActivate() { m_regionCombo->setFocus(); - if ( m_selectedLocaleConfiguration.isEmpty() || - !m_selectedLocaleConfiguration.explicit_lang ) + if ( m_selectedLocaleConfiguration.isEmpty() || !m_selectedLocaleConfiguration.explicit_lang ) { auto newLocale = guessLocaleConfiguration(); m_selectedLocaleConfiguration.setLanguage( newLocale.language() ); @@ -343,16 +339,15 @@ LocalePage::onActivate() LocaleConfiguration LocalePage::guessLocaleConfiguration() const { - return LocaleConfiguration::fromLanguageAndLocation( QLocale().name(), - m_localeGenLines, - m_tzWidget->getCurrentLocation().country ); + return LocaleConfiguration::fromLanguageAndLocation( + QLocale().name(), m_localeGenLines, m_tzWidget->getCurrentLocation().country ); } void LocalePage::updateGlobalLocale() { - auto *gs = Calamares::JobQueue::instance()->globalStorage(); + auto* gs = Calamares::JobQueue::instance()->globalStorage(); const QString bcp47 = m_selectedLocaleConfiguration.toBcp47(); gs->insert( "locale", bcp47 ); } @@ -361,11 +356,11 @@ LocalePage::updateGlobalLocale() void LocalePage::updateGlobalStorage() { - auto *gs = Calamares::JobQueue::instance()->globalStorage(); + auto* gs = Calamares::JobQueue::instance()->globalStorage(); LocaleGlobal::Location location = m_tzWidget->getCurrentLocation(); - bool locationChanged = ( location.region != gs->value( "locationRegion" ) ) || - ( location.zone != gs->value( "locationZone" ) ); + bool locationChanged + = ( location.region != gs->value( "locationRegion" ) ) || ( location.zone != gs->value( "locationZone" ) ); gs->insert( "locationRegion", location.region ); gs->insert( "locationZone", location.zone ); @@ -378,18 +373,17 @@ LocalePage::updateGlobalStorage() if ( locationChanged && Calamares::Settings::instance()->doChroot() ) { QProcess::execute( "timedatectl", // depends on systemd - { "set-timezone", - location.region + '/' + location.zone } ); + { "set-timezone", location.region + '/' + location.zone } ); } #endif // Preserve those settings that have been made explicit. auto newLocale = guessLocaleConfiguration(); - if ( !m_selectedLocaleConfiguration.isEmpty() && - m_selectedLocaleConfiguration.explicit_lang ) + if ( !m_selectedLocaleConfiguration.isEmpty() && m_selectedLocaleConfiguration.explicit_lang ) + { newLocale.setLanguage( m_selectedLocaleConfiguration.language() ); - if ( !m_selectedLocaleConfiguration.isEmpty() && - m_selectedLocaleConfiguration.explicit_lc ) + } + if ( !m_selectedLocaleConfiguration.isEmpty() && m_selectedLocaleConfiguration.explicit_lc ) { newLocale.lc_numeric = m_selectedLocaleConfiguration.lc_numeric; newLocale.lc_time = m_selectedLocaleConfiguration.lc_time; @@ -455,14 +449,18 @@ LocalePage::locationChanged( LocaleGlobal::Location location ) // Set region index int index = m_regionCombo->findData( location.region ); if ( index < 0 ) + { return; + } m_regionCombo->setCurrentIndex( index ); // Set zone index index = m_zoneCombo->findData( location.zone ); if ( index < 0 ) + { return; + } m_zoneCombo->setCurrentIndex( index ); @@ -474,15 +472,13 @@ LocalePage::locationChanged( LocaleGlobal::Location location ) void LocalePage::changeLocale() { - LCLocaleDialog* dlg = - new LCLocaleDialog( m_selectedLocaleConfiguration.isEmpty() ? - guessLocaleConfiguration().language() : - m_selectedLocaleConfiguration.language(), - m_localeGenLines, - this ); + LCLocaleDialog* dlg + = new LCLocaleDialog( m_selectedLocaleConfiguration.isEmpty() ? guessLocaleConfiguration().language() + : m_selectedLocaleConfiguration.language(), + m_localeGenLines, + this ); dlg->exec(); - if ( dlg->result() == QDialog::Accepted && - !dlg->selectedLCLocale().isEmpty() ) + if ( dlg->result() == QDialog::Accepted && !dlg->selectedLCLocale().isEmpty() ) { m_selectedLocaleConfiguration.setLanguage( dlg->selectedLCLocale() ); m_selectedLocaleConfiguration.explicit_lang = true; @@ -497,15 +493,13 @@ LocalePage::changeLocale() void LocalePage::changeFormats() { - LCLocaleDialog* dlg = - new LCLocaleDialog( m_selectedLocaleConfiguration.isEmpty() ? - guessLocaleConfiguration().lc_numeric : - m_selectedLocaleConfiguration.lc_numeric, - m_localeGenLines, - this ); + LCLocaleDialog* dlg + = new LCLocaleDialog( m_selectedLocaleConfiguration.isEmpty() ? guessLocaleConfiguration().lc_numeric + : m_selectedLocaleConfiguration.lc_numeric, + m_localeGenLines, + this ); dlg->exec(); - if ( dlg->result() == QDialog::Accepted && - !dlg->selectedLCLocale().isEmpty() ) + if ( dlg->result() == QDialog::Accepted && !dlg->selectedLCLocale().isEmpty() ) { // TODO: improve the granularity of this setting. m_selectedLocaleConfiguration.lc_numeric = dlg->selectedLCLocale(); @@ -523,5 +517,4 @@ LocalePage::changeFormats() } dlg->deleteLater(); - } diff --git a/src/modules/locale/LocalePage.h b/src/modules/locale/LocalePage.h index 5e4dbe8e9..73097252b 100644 --- a/src/modules/locale/LocalePage.h +++ b/src/modules/locale/LocalePage.h @@ -39,9 +39,7 @@ public: explicit LocalePage( QWidget* parent = nullptr ); virtual ~LocalePage(); - void init( const QString& initialRegion, - const QString& initialZone, - const QString& localeGenPath ); + void init( const QString& initialRegion, const QString& initialZone, const QString& localeGenPath ); QString prettyStatus() const; @@ -91,4 +89,4 @@ private: bool m_blockTzWidgetSet; }; -#endif // LOCALEPAGE_H +#endif // LOCALEPAGE_H From 5a24e45e3b1522e4f9239ff1a12a4a08c37fe4fc Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 7 Sep 2019 15:58:56 +0200 Subject: [PATCH 05/13] [locale] Factor out a simple lambda - If this was handed to an algorithm it would make more sense as a lambda --- src/modules/locale/LocalePage.cpp | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/modules/locale/LocalePage.cpp b/src/modules/locale/LocalePage.cpp index ecca8ca28..d3829aad3 100644 --- a/src/modules/locale/LocalePage.cpp +++ b/src/modules/locale/LocalePage.cpp @@ -127,6 +127,18 @@ LocalePage::updateLocaleLabels() m_formatsLabel->setText( labels.second ); } +static inline bool +containsLocation( const QList< LocaleGlobal::Location >& locations, const QString& zone ) +{ + for ( const LocaleGlobal::Location& location : locations ) + { + if ( location.zone == zone ) + { + return true; + } + } + return false; +} void LocalePage::init( const QString& initialRegion, const QString& initialZone, const QString& localeGenPath ) @@ -150,18 +162,6 @@ LocalePage::init( const QString& initialRegion, const QString& initialZone, cons m_regionCombo->currentIndexChanged( m_regionCombo->currentIndex() ); - // Default location - auto containsLocation = []( const QList< LocaleGlobal::Location >& locations, const QString& zone ) -> bool { - for ( const LocaleGlobal::Location& location : locations ) - { - if ( location.zone == zone ) - { - return true; - } - } - return false; - }; - if ( keys.contains( initialRegion ) && containsLocation( regions.value( initialRegion ), initialZone ) ) { m_tzWidget->setCurrentLocation( initialRegion, initialZone ); From 0ee8427d5a310a568263f1493d573b888b28661a Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 7 Sep 2019 16:05:09 +0200 Subject: [PATCH 06/13] [locale] Remove old-style GeoIP configuration --- CHANGES | 3 +++ src/modules/locale/LocaleViewStep.cpp | 16 ---------------- 2 files changed, 3 insertions(+), 16 deletions(-) diff --git a/CHANGES b/CHANGES index ad985b1e4..b3a1da805 100644 --- a/CHANGES +++ b/CHANGES @@ -12,6 +12,9 @@ This release contains contributions from (alphabetically by first name): ## Modules ## + - *locale* module no longer recognized the legacy GeoIP configuration. + This has been deprecated since Calamares 3.2.8. + # 3.2.13 (2019-08-30) # diff --git a/src/modules/locale/LocaleViewStep.cpp b/src/modules/locale/LocaleViewStep.cpp index 9dbcb5993..5d9d2df71 100644 --- a/src/modules/locale/LocaleViewStep.cpp +++ b/src/modules/locale/LocaleViewStep.cpp @@ -231,20 +231,4 @@ LocaleViewStep::setConfigurationMap( const QVariantMap& configurationMap ) m_geoipStyle = CalamaresUtils::getString( geoip, "style" ); m_geoipSelector = CalamaresUtils::getString( geoip, "selector" ); } - else - { - // Accommodate deprecated geoip configuration - m_geoipUrl = CalamaresUtils::getString( configurationMap, "geoipUrl" ); - m_geoipStyle = CalamaresUtils::getString( configurationMap, "geoipStyle" ); - m_geoipSelector = CalamaresUtils::getString( configurationMap, "geoipSelector" ); - - if ( !m_geoipUrl.isEmpty() && ( m_geoipStyle.isEmpty() || m_geoipStyle == "legacy" ) ) - { - m_geoipStyle = "json"; - m_geoipUrl.append( "/json/" ); - } - - if ( !m_geoipUrl.isEmpty() ) - cWarning() << "Legacy-style GeoIP configuration is deprecated. Use geoip: map."; - } } From 5cac3ac6ada4682e76c8d9a7c1a516dbfe926ece Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 7 Sep 2019 16:58:37 +0200 Subject: [PATCH 07/13] [locale] Apply coding style --- src/modules/locale/LCLocaleDialog.cpp | 31 ++++---- src/modules/locale/LCLocaleDialog.h | 2 +- src/modules/locale/LocaleConfiguration.cpp | 82 ++++++++++++++-------- src/modules/locale/LocaleConfiguration.h | 20 +++--- src/modules/locale/LocaleViewStep.cpp | 36 ++++++---- src/modules/locale/LocaleViewStep.h | 2 +- src/modules/locale/SetTimezoneJob.cpp | 40 ++++------- src/modules/locale/SetTimezoneJob.h | 3 +- src/modules/locale/Tests.cpp | 25 ++++--- 9 files changed, 127 insertions(+), 114 deletions(-) diff --git a/src/modules/locale/LCLocaleDialog.cpp b/src/modules/locale/LCLocaleDialog.cpp index 9f1b8fbdf..db33f0c3d 100644 --- a/src/modules/locale/LCLocaleDialog.cpp +++ b/src/modules/locale/LCLocaleDialog.cpp @@ -25,9 +25,7 @@ #include #include -LCLocaleDialog::LCLocaleDialog( const QString& guessedLCLocale, - const QStringList& localeGenLines, - QWidget* parent ) +LCLocaleDialog::LCLocaleDialog( const QString& guessedLCLocale, const QStringList& localeGenLines, QWidget* parent ) : QDialog( parent ) { setModal( true ); @@ -41,7 +39,7 @@ LCLocaleDialog::LCLocaleDialog( const QString& guessedLCLocale, upperText->setText( tr( "The system locale setting affects the language and character " "set for some command line user interface elements.
" "The current setting is %1." ) - .arg( guessedLCLocale ) ); + .arg( guessedLCLocale ) ); mainLayout->addWidget( upperText ); setMinimumWidth( upperText->fontMetrics().height() * 24 ); @@ -60,33 +58,32 @@ LCLocaleDialog::LCLocaleDialog( const QString& guessedLCLocale, } } - QDialogButtonBox* dbb = new QDialogButtonBox( QDialogButtonBox::Ok | QDialogButtonBox::Cancel, - Qt::Horizontal, - this ); + QDialogButtonBox* dbb + = new QDialogButtonBox( QDialogButtonBox::Ok | QDialogButtonBox::Cancel, Qt::Horizontal, this ); dbb->button( QDialogButtonBox::Cancel )->setText( tr( "&Cancel" ) ); dbb->button( QDialogButtonBox::Ok )->setText( tr( "&OK" ) ); mainLayout->addWidget( dbb ); - connect( dbb->button( QDialogButtonBox::Ok ), &QPushButton::clicked, - this, &QDialog::accept ); - connect( dbb->button( QDialogButtonBox::Cancel ), &QPushButton::clicked, - this, &QDialog::reject ); + connect( dbb->button( QDialogButtonBox::Ok ), &QPushButton::clicked, this, &QDialog::accept ); + connect( dbb->button( QDialogButtonBox::Cancel ), &QPushButton::clicked, this, &QDialog::reject ); - connect( m_localesWidget, &QListWidget::itemDoubleClicked, - this, &QDialog::accept ); - connect( m_localesWidget, &QListWidget::itemSelectionChanged, - [this, dbb]() - { + connect( m_localesWidget, &QListWidget::itemDoubleClicked, this, &QDialog::accept ); + connect( m_localesWidget, &QListWidget::itemSelectionChanged, [this, dbb]() { if ( m_localesWidget->selectedItems().isEmpty() ) + { dbb->button( QDialogButtonBox::Ok )->setEnabled( false ); + } else + { dbb->button( QDialogButtonBox::Ok )->setEnabled( true ); - + } } ); if ( selected > -1 ) + { m_localesWidget->setCurrentRow( selected ); + } } diff --git a/src/modules/locale/LCLocaleDialog.h b/src/modules/locale/LCLocaleDialog.h index 29005b94b..81e4cb547 100644 --- a/src/modules/locale/LCLocaleDialog.h +++ b/src/modules/locale/LCLocaleDialog.h @@ -37,4 +37,4 @@ private: QListWidget* m_localesWidget; }; -#endif // LCLOCALEDIALOG_H +#endif // LCLOCALEDIALOG_H diff --git a/src/modules/locale/LocaleConfiguration.cpp b/src/modules/locale/LocaleConfiguration.cpp index 8bc2b2c77..2024785e2 100644 --- a/src/modules/locale/LocaleConfiguration.cpp +++ b/src/modules/locale/LocaleConfiguration.cpp @@ -30,16 +30,15 @@ LocaleConfiguration::LocaleConfiguration() LocaleConfiguration::LocaleConfiguration( const QString& localeName, const QString& formatsName ) : LocaleConfiguration() { - lc_numeric = lc_time = lc_monetary = lc_paper = lc_name - = lc_address = lc_telephone = lc_measurement - = lc_identification = formatsName; + lc_numeric = lc_time = lc_monetary = lc_paper = lc_name = lc_address = lc_telephone = lc_measurement + = lc_identification = formatsName; - (void) setLanguage( localeName ); + (void)setLanguage( localeName ); } void -LocaleConfiguration::setLanguage(const QString& localeName ) +LocaleConfiguration::setLanguage( const QString& localeName ) { QString language = localeName.split( '_' ).first(); m_languageLocaleBcp47 = QLocale( language ).bcp47Name().toLower(); @@ -55,30 +54,39 @@ LocaleConfiguration::fromLanguageAndLocation( const QString& languageLocale, QString language = languageLocale.split( '_' ).first(); QStringList linesForLanguage; - for ( const QString &line : availableLocales ) + for ( const QString& line : availableLocales ) { if ( line.startsWith( language ) ) + { linesForLanguage.append( line ); + } } QString lang; if ( linesForLanguage.length() == 0 || languageLocale.isEmpty() ) + { lang = "en_US.UTF-8"; + } else if ( linesForLanguage.length() == 1 ) + { lang = linesForLanguage.first(); + } else { QStringList linesForLanguageUtf; // FIXME: this might be useless if we already filter out non-UTF8 locales foreach ( QString line, linesForLanguage ) { - if ( line.contains( "UTF-8", Qt::CaseInsensitive ) || - line.contains( "utf8", Qt::CaseInsensitive ) ) + if ( line.contains( "UTF-8", Qt::CaseInsensitive ) || line.contains( "utf8", Qt::CaseInsensitive ) ) + { linesForLanguageUtf.append( line ); + } } if ( linesForLanguageUtf.length() == 1 ) + { lang = linesForLanguageUtf.first(); + } } // lang could still be empty if we found multiple locales that satisfy myLanguage @@ -92,8 +100,7 @@ LocaleConfiguration::fromLanguageAndLocation( const QString& languageLocale, # locale categories reflect the selected location. */ if ( language == "pt" || language == "zh" ) { - QString proposedLocale = QString( "%1_%2" ).arg( language ) - .arg( countryCode ); + QString proposedLocale = QString( "%1_%2" ).arg( language ).arg( countryCode ); foreach ( QString line, linesForLanguage ) { if ( line.contains( proposedLocale ) ) @@ -108,7 +115,7 @@ LocaleConfiguration::fromLanguageAndLocation( const QString& languageLocale, // language locale and pick the first result, if any. if ( lang.isEmpty() ) { - for ( const QString &line : availableLocales ) + for ( const QString& line : availableLocales ) { if ( line.startsWith( languageLocale ) ) { @@ -116,13 +123,14 @@ LocaleConfiguration::fromLanguageAndLocation( const QString& languageLocale, break; } } - } // Else we have an unrecognized or unsupported locale, all we can do is go with // en_US.UTF-8 UTF-8. This completes all default language setting guesswork. if ( lang.isEmpty() ) + { lang = "en_US.UTF-8"; + } // The following block was inspired by Ubiquity, scripts/localechooser-apply. @@ -171,10 +179,9 @@ LocaleConfiguration::fromLanguageAndLocation( const QString& languageLocale, // We make a proposed locale based on the UI language and the timezone's country. There is no // guarantee that this will be a valid, supported locale (often it won't). QString lc_formats; - QString combined = QString( "%1_%2" ).arg( language ) - .arg( countryCode ); + QString combined = QString( "%1_%2" ).arg( language ).arg( countryCode ); // We look up if it's a supported locale. - for ( const QString &line : availableLocales ) + for ( const QString& line : availableLocales ) { if ( line.startsWith( combined ) ) { @@ -187,7 +194,7 @@ LocaleConfiguration::fromLanguageAndLocation( const QString& languageLocale, if ( lc_formats.isEmpty() ) { QStringList available; - for ( const QString &line : availableLocales ) + for ( const QString& line : availableLocales ) { if ( line.contains( QString( "_%1" ).arg( countryCode ) ) ) { @@ -248,11 +255,10 @@ LocaleConfiguration::fromLanguageAndLocation( const QString& languageLocale, }; if ( countryToDefaultLanguage.contains( countryCode ) ) { - QString combinedLocale = - QString( "%1_%2" ).arg( countryToDefaultLanguage.value( countryCode ) ) - .arg( countryCode ); + QString combinedLocale + = QString( "%1_%2" ).arg( countryToDefaultLanguage.value( countryCode ) ).arg( countryCode ); - for ( const QString &line : availableLocales ) + for ( const QString& line : availableLocales ) { if ( line.startsWith( combinedLocale ) ) { @@ -267,7 +273,9 @@ LocaleConfiguration::fromLanguageAndLocation( const QString& languageLocale, // If we cannot make a good choice for a given country we go with the LANG // setting, which defaults to en_US.UTF-8 UTF-8 if all else fails. if ( lc_formats.isEmpty() ) + { lc_formats = lang; + } return LocaleConfiguration( lang, lc_formats ); } @@ -276,16 +284,9 @@ LocaleConfiguration::fromLanguageAndLocation( const QString& languageLocale, bool LocaleConfiguration::isEmpty() const { - return m_lang.isEmpty() && - lc_numeric.isEmpty() && - lc_time.isEmpty() && - lc_monetary.isEmpty() && - lc_paper.isEmpty() && - lc_name.isEmpty() && - lc_address.isEmpty() && - lc_telephone.isEmpty() && - lc_measurement.isEmpty() && - lc_identification.isEmpty(); + return m_lang.isEmpty() && lc_numeric.isEmpty() && lc_time.isEmpty() && lc_monetary.isEmpty() && lc_paper.isEmpty() + && lc_name.isEmpty() && lc_address.isEmpty() && lc_telephone.isEmpty() && lc_measurement.isEmpty() + && lc_identification.isEmpty(); } @@ -295,35 +296,54 @@ LocaleConfiguration::toMap() const QMap< QString, QString > map; if ( !m_lang.isEmpty() ) + { map.insert( "LANG", m_lang ); + } if ( !lc_numeric.isEmpty() ) + { map.insert( "LC_NUMERIC", lc_numeric ); + } if ( !lc_time.isEmpty() ) + { map.insert( "LC_TIME", lc_time ); + } if ( !lc_monetary.isEmpty() ) + { map.insert( "LC_MONETARY", lc_monetary ); + } if ( !lc_paper.isEmpty() ) + { map.insert( "LC_PAPER", lc_paper ); + } if ( !lc_name.isEmpty() ) + { map.insert( "LC_NAME", lc_name ); + } if ( !lc_address.isEmpty() ) + { map.insert( "LC_ADDRESS", lc_address ); + } if ( !lc_telephone.isEmpty() ) + { map.insert( "LC_TELEPHONE", lc_telephone ); + } if ( !lc_measurement.isEmpty() ) + { map.insert( "LC_MEASUREMENT", lc_measurement ); + } if ( !lc_identification.isEmpty() ) + { map.insert( "LC_IDENTIFICATION", lc_identification ); + } return map; } - diff --git a/src/modules/locale/LocaleConfiguration.h b/src/modules/locale/LocaleConfiguration.h index 5e99b1f37..4f4fc6b21 100644 --- a/src/modules/locale/LocaleConfiguration.h +++ b/src/modules/locale/LocaleConfiguration.h @@ -21,8 +21,8 @@ #define LOCALECONFIGURATION_H #include -#include #include +#include class LocaleConfiguration { @@ -31,13 +31,14 @@ public: explicit LocaleConfiguration(); /// @brief Create a locale with everything set to the given @p localeName explicit LocaleConfiguration( const QString& localeName /* "en_US.UTF-8" */ ) - : LocaleConfiguration( localeName, localeName ) { } + : LocaleConfiguration( localeName, localeName ) + { + } /// @brief Create a locale with language and formats separate explicit LocaleConfiguration( const QString& localeName, const QString& formatsName ); - static LocaleConfiguration fromLanguageAndLocation( const QString& language, - const QStringList& availableLocales, - const QString& countryCode ); + static LocaleConfiguration + fromLanguageAndLocation( const QString& language, const QStringList& availableLocales, const QString& countryCode ); bool isEmpty() const; @@ -55,8 +56,8 @@ public: // These become all uppercase in locale.conf, but we keep them lowercase here to // avoid confusion with locale.h. - QString lc_numeric, lc_time, lc_monetary, lc_paper, lc_name, lc_address, - lc_telephone, lc_measurement, lc_identification; + QString lc_numeric, lc_time, lc_monetary, lc_paper, lc_name, lc_address, lc_telephone, lc_measurement, + lc_identification; // If the user has explicitly selected language (from the dialog) // or numbers format, set these to avoid implicit changes to them. @@ -67,9 +68,10 @@ private: QString m_languageLocaleBcp47; }; -inline QDebug& operator <<( QDebug& s, const LocaleConfiguration& l ) +inline QDebug& +operator<<( QDebug& s, const LocaleConfiguration& l ) { return s << l.language() << '(' << l.toBcp47() << ") +" << l.lc_numeric; } -#endif // LOCALECONFIGURATION_H +#endif // LOCALECONFIGURATION_H diff --git a/src/modules/locale/LocaleViewStep.cpp b/src/modules/locale/LocaleViewStep.cpp index 5d9d2df71..a9b9da456 100644 --- a/src/modules/locale/LocaleViewStep.cpp +++ b/src/modules/locale/LocaleViewStep.cpp @@ -38,7 +38,7 @@ #include -CALAMARES_PLUGIN_FACTORY_DEFINITION( LocaleViewStepFactory, registerPlugin(); ) +CALAMARES_PLUGIN_FACTORY_DEFINITION( LocaleViewStepFactory, registerPlugin< LocaleViewStep >(); ) LocaleViewStep::LocaleViewStep( QObject* parent ) : Calamares::ViewStep( parent ) @@ -53,19 +53,17 @@ LocaleViewStep::LocaleViewStep( QObject* parent ) m_waitingWidget = new WaitingWidget( tr( "Loading location data..." ) ); mainLayout->addWidget( m_waitingWidget ); - connect( &m_initWatcher, &QFutureWatcher< void >::finished, - this, [=] - { - bool hasInternet = Calamares::JobQueue::instance()->globalStorage() - ->value( "hasInternet" ).toBool(); + connect( &m_initWatcher, &QFutureWatcher< void >::finished, this, [=] { + bool hasInternet = Calamares::JobQueue::instance()->globalStorage()->value( "hasInternet" ).toBool(); if ( m_geoipUrl.isEmpty() || !hasInternet ) setUpPage(); else + { fetchGeoIpTimezone(); - }); + } + } ); - QFuture< void > initFuture = QtConcurrent::run( [=] - { + QFuture< void > initFuture = QtConcurrent::run( [=] { LocaleGlobal::init(); if ( m_geoipUrl.isEmpty() ) return; @@ -90,16 +88,16 @@ LocaleViewStep::LocaleViewStep( QObject* parent ) LocaleViewStep::~LocaleViewStep() { if ( m_widget && m_widget->parent() == nullptr ) + { m_widget->deleteLater(); + } } void LocaleViewStep::setUpPage() { - m_actualWidget->init( m_startingTimezone.first, - m_startingTimezone.second, - m_localeGenPath ); + m_actualWidget->init( m_startingTimezone.first, m_startingTimezone.second, m_localeGenPath ); m_widget->layout()->removeWidget( m_waitingWidget ); m_waitingWidget->deleteLater(); m_widget->layout()->addWidget( m_actualWidget ); @@ -116,10 +114,14 @@ LocaleViewStep::fetchGeoIpTimezone() { m_startingTimezone = h.get(); if ( !m_startingTimezone.isValid() ) + { cWarning() << "GeoIP lookup at" << m_geoipUrl << "failed."; + } } else + { cWarning() << "GeoIP Style" << m_geoipStyle << "is not recognized."; + } setUpPage(); } @@ -198,10 +200,11 @@ LocaleViewStep::onLeave() auto map = m_actualWidget->localesMap(); QVariantMap vm; for ( auto it = map.constBegin(); it != map.constEnd(); ++it ) + { vm.insert( it.key(), it.value() ); + } - Calamares::JobQueue::instance()->globalStorage() - ->insert( "localeConf", vm ); + Calamares::JobQueue::instance()->globalStorage()->insert( "localeConf", vm ); } @@ -216,12 +219,15 @@ LocaleViewStep::setConfigurationMap( const QVariantMap& configurationMap ) } else { - m_startingTimezone = CalamaresUtils::GeoIP::RegionZonePair( QStringLiteral( "America" ), QStringLiteral( "New_York" ) ); + m_startingTimezone + = CalamaresUtils::GeoIP::RegionZonePair( QStringLiteral( "America" ), QStringLiteral( "New_York" ) ); } m_localeGenPath = CalamaresUtils::getString( configurationMap, "localeGenPath" ); if ( m_localeGenPath.isEmpty() ) + { m_localeGenPath = QStringLiteral( "/etc/locale.gen" ); + } bool ok = false; QVariantMap geoip = CalamaresUtils::getSubMap( configurationMap, "geoip", ok ); diff --git a/src/modules/locale/LocaleViewStep.h b/src/modules/locale/LocaleViewStep.h index 8ab50b75d..b04b34651 100644 --- a/src/modules/locale/LocaleViewStep.h +++ b/src/modules/locale/LocaleViewStep.h @@ -83,4 +83,4 @@ private: CALAMARES_PLUGIN_FACTORY_DECLARATION( LocaleViewStepFactory ) -#endif // LOCALEVIEWSTEP_H +#endif // LOCALEVIEWSTEP_H diff --git a/src/modules/locale/SetTimezoneJob.cpp b/src/modules/locale/SetTimezoneJob.cpp index 71a693df7..adcac13c1 100644 --- a/src/modules/locale/SetTimezoneJob.cpp +++ b/src/modules/locale/SetTimezoneJob.cpp @@ -19,11 +19,11 @@ #include -#include "JobQueue.h" #include "GlobalStorage.h" +#include "JobQueue.h" #include "Settings.h" -#include "utils/Logger.h" #include "utils/CalamaresUtilsSystem.h" +#include "utils/Logger.h" #include #include @@ -51,13 +51,13 @@ SetTimezoneJob::exec() // to a running timedated over D-Bus), and we have code that works if ( !Calamares::Settings::instance()->doChroot() ) { - int ec = CalamaresUtils::System::instance()-> - targetEnvCall( { "timedatectl", - "set-timezone", - m_region + '/' + m_zone } ); + int ec = CalamaresUtils::System::instance()->targetEnvCall( + { "timedatectl", "set-timezone", m_region + '/' + m_zone } ); if ( !ec ) + { return Calamares::JobResult::ok(); + } } QString localtimeSlink( "/etc/localtime" ); @@ -72,31 +72,21 @@ SetTimezoneJob::exec() tr( "Bad path: %1" ).arg( zoneFile.absolutePath() ) ); // Make sure /etc/localtime doesn't exist, otherwise symlinking will fail - CalamaresUtils::System::instance()-> - targetEnvCall( { "rm", - "-f", - localtimeSlink } ); + CalamaresUtils::System::instance()->targetEnvCall( { "rm", "-f", localtimeSlink } ); - int ec = CalamaresUtils::System::instance()-> - targetEnvCall( { "ln", - "-s", - zoneinfoPath, - localtimeSlink } ); + int ec = CalamaresUtils::System::instance()->targetEnvCall( { "ln", "-s", zoneinfoPath, localtimeSlink } ); if ( ec ) - return Calamares::JobResult::error( tr( "Cannot set timezone." ), - tr( "Link creation failed, target: %1; link name: %2" ) - .arg( zoneinfoPath ) - .arg( "/etc/localtime" ) ); + return Calamares::JobResult::error( + tr( "Cannot set timezone." ), + tr( "Link creation failed, target: %1; link name: %2" ).arg( zoneinfoPath ).arg( "/etc/localtime" ) ); QFile timezoneFile( gs->value( "rootMountPoint" ).toString() + "/etc/timezone" ); - if ( !timezoneFile.open( QIODevice::WriteOnly | - QIODevice::Text | - QIODevice::Truncate ) ) - return Calamares::JobResult::error( tr( "Cannot set timezone,"), - tr( "Cannot open /etc/timezone for writing")); + if ( !timezoneFile.open( QIODevice::WriteOnly | QIODevice::Text | QIODevice::Truncate ) ) + return Calamares::JobResult::error( tr( "Cannot set timezone," ), + tr( "Cannot open /etc/timezone for writing" ) ); - QTextStream out(&timezoneFile); + QTextStream out( &timezoneFile ); out << m_region << '/' << m_zone << "\n"; timezoneFile.close(); diff --git a/src/modules/locale/SetTimezoneJob.h b/src/modules/locale/SetTimezoneJob.h index 7f50d8744..f7f2331ad 100644 --- a/src/modules/locale/SetTimezoneJob.h +++ b/src/modules/locale/SetTimezoneJob.h @@ -26,8 +26,7 @@ class SetTimezoneJob : public Calamares::Job { Q_OBJECT public: - SetTimezoneJob( const QString& region, - const QString& zone ); + SetTimezoneJob( const QString& region, const QString& zone ); QString prettyName() const override; Calamares::JobResult exec() override; diff --git a/src/modules/locale/Tests.cpp b/src/modules/locale/Tests.cpp index 0e1a3eb48..b8e471339 100644 --- a/src/modules/locale/Tests.cpp +++ b/src/modules/locale/Tests.cpp @@ -25,19 +25,17 @@ QTEST_GUILESS_MAIN( LocaleTests ) -LocaleTests::LocaleTests() +LocaleTests::LocaleTests() {} + +LocaleTests::~LocaleTests() {} + +void +LocaleTests::initTestCase() { } -LocaleTests::~LocaleTests() -{ -} - -void LocaleTests::initTestCase() -{ -} - -void LocaleTests::testEmptyLocaleConfiguration() +void +LocaleTests::testEmptyLocaleConfiguration() { LocaleConfiguration lc; @@ -45,7 +43,8 @@ void LocaleTests::testEmptyLocaleConfiguration() QCOMPARE( lc.toBcp47(), QString() ); } -void LocaleTests::testDefaultLocaleConfiguration() +void +LocaleTests::testDefaultLocaleConfiguration() { LocaleConfiguration lc( "en_US.UTF-8" ); QVERIFY( !lc.isEmpty() ); @@ -58,7 +57,8 @@ void LocaleTests::testDefaultLocaleConfiguration() QCOMPARE( lc2.toBcp47(), "de" ); } -void LocaleTests::testSplitLocaleConfiguration() +void +LocaleTests::testSplitLocaleConfiguration() { LocaleConfiguration lc( "en_US.UTF-8", "de_DE.UTF-8" ); QVERIFY( !lc.isEmpty() ); @@ -76,5 +76,4 @@ void LocaleTests::testSplitLocaleConfiguration() QVERIFY( !lc3.isEmpty() ); QCOMPARE( lc3.toBcp47(), "da" ); QCOMPARE( lc3.lc_numeric, "de_DE.UTF-8" ); - } From d70d418d923fe9848441e7474618a835d15ff6be Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 7 Sep 2019 17:02:59 +0200 Subject: [PATCH 08/13] [locale] Refactor setting of LC entries --- src/modules/locale/LocaleConfiguration.cpp | 68 ++++++---------------- 1 file changed, 19 insertions(+), 49 deletions(-) diff --git a/src/modules/locale/LocaleConfiguration.cpp b/src/modules/locale/LocaleConfiguration.cpp index 2024785e2..3377b11f3 100644 --- a/src/modules/locale/LocaleConfiguration.cpp +++ b/src/modules/locale/LocaleConfiguration.cpp @@ -289,61 +289,31 @@ LocaleConfiguration::isEmpty() const && lc_identification.isEmpty(); } +/// @brief Sets @p value on @p key in the @p map if @p value is non-empty +static inline void +add_lc( QMap< QString, QString >& map, const char* key, const QString& value ) +{ + if ( !value.isEmpty() ) + { + map.insert( key, value ); + } +} QMap< QString, QString > LocaleConfiguration::toMap() const { QMap< QString, QString > map; - if ( !m_lang.isEmpty() ) - { - map.insert( "LANG", m_lang ); - } - - if ( !lc_numeric.isEmpty() ) - { - map.insert( "LC_NUMERIC", lc_numeric ); - } - - if ( !lc_time.isEmpty() ) - { - map.insert( "LC_TIME", lc_time ); - } - - if ( !lc_monetary.isEmpty() ) - { - map.insert( "LC_MONETARY", lc_monetary ); - } - - if ( !lc_paper.isEmpty() ) - { - map.insert( "LC_PAPER", lc_paper ); - } - - if ( !lc_name.isEmpty() ) - { - map.insert( "LC_NAME", lc_name ); - } - - if ( !lc_address.isEmpty() ) - { - map.insert( "LC_ADDRESS", lc_address ); - } - - if ( !lc_telephone.isEmpty() ) - { - map.insert( "LC_TELEPHONE", lc_telephone ); - } - - if ( !lc_measurement.isEmpty() ) - { - map.insert( "LC_MEASUREMENT", lc_measurement ); - } - - if ( !lc_identification.isEmpty() ) - { - map.insert( "LC_IDENTIFICATION", lc_identification ); - } + add_lc( map, "LANG", m_lang ); + add_lc( map, "LC_NUMERIC", lc_numeric ); + add_lc( map, "LC_TIME", lc_time ); + add_lc( map, "LC_MONETARY", lc_monetary ); + add_lc( map, "LC_PAPER", lc_paper ); + add_lc( map, "LC_NAME", lc_name ); + add_lc( map, "LC_ADDRESS", lc_address ); + add_lc( map, "LC_TELEPHONE", lc_telephone ); + add_lc( map, "LC_MEASUREMENT", lc_measurement ); + add_lc( map, "LC_IDENTIFICATION", lc_identification ); return map; } From 0a1dc77f9be6b4c696e05fe218f7ff525d5bc702 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 10 Sep 2019 11:26:47 +0200 Subject: [PATCH 09/13] [locale] Hang on to GeoIP::Handler just once - replace configuration settings by putting them in an object - use unique_ptr to allow us to create one optionally. --- src/modules/locale/LocaleViewStep.cpp | 32 ++++++++++++++++----------- src/modules/locale/LocaleViewStep.h | 8 +++---- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/src/modules/locale/LocaleViewStep.cpp b/src/modules/locale/LocaleViewStep.cpp index a9b9da456..bbd2d07a1 100644 --- a/src/modules/locale/LocaleViewStep.cpp +++ b/src/modules/locale/LocaleViewStep.cpp @@ -45,6 +45,7 @@ LocaleViewStep::LocaleViewStep( QObject* parent ) , m_widget( new QWidget() ) , m_actualWidget( new LocalePage() ) , m_nextEnabled( false ) + , m_geoip( nullptr ) { QBoxLayout* mainLayout = new QHBoxLayout; m_widget->setLayout( mainLayout ); @@ -55,8 +56,10 @@ LocaleViewStep::LocaleViewStep( QObject* parent ) connect( &m_initWatcher, &QFutureWatcher< void >::finished, this, [=] { bool hasInternet = Calamares::JobQueue::instance()->globalStorage()->value( "hasInternet" ).toBool(); - if ( m_geoipUrl.isEmpty() || !hasInternet ) + if ( !m_geoip || !hasInternet ) + { setUpPage(); + } else { fetchGeoIpTimezone(); @@ -65,8 +68,10 @@ LocaleViewStep::LocaleViewStep( QObject* parent ) QFuture< void > initFuture = QtConcurrent::run( [=] { LocaleGlobal::init(); - if ( m_geoipUrl.isEmpty() ) + if ( !m_geoip ) + { return; + } Calamares::GlobalStorage* gs = Calamares::JobQueue::instance()->globalStorage(); @@ -109,19 +114,14 @@ LocaleViewStep::setUpPage() void LocaleViewStep::fetchGeoIpTimezone() { - CalamaresUtils::GeoIP::Handler h( m_geoipStyle, m_geoipUrl, m_geoipSelector ); - if ( h.isValid() ) + if ( m_geoip && m_geoip->isValid() ) { - m_startingTimezone = h.get(); + m_startingTimezone = m_geoip->get(); if ( !m_startingTimezone.isValid() ) { - cWarning() << "GeoIP lookup at" << m_geoipUrl << "failed."; + cWarning() << "GeoIP lookup at" << m_geoip->url() << "failed."; } } - else - { - cWarning() << "GeoIP Style" << m_geoipStyle << "is not recognized."; - } setUpPage(); } @@ -233,8 +233,14 @@ LocaleViewStep::setConfigurationMap( const QVariantMap& configurationMap ) QVariantMap geoip = CalamaresUtils::getSubMap( configurationMap, "geoip", ok ); if ( ok ) { - m_geoipUrl = CalamaresUtils::getString( geoip, "url" ); - m_geoipStyle = CalamaresUtils::getString( geoip, "style" ); - m_geoipSelector = CalamaresUtils::getString( geoip, "selector" ); + QString url = CalamaresUtils::getString( geoip, "url" ); + QString style = CalamaresUtils::getString( geoip, "style" ); + QString selector = CalamaresUtils::getString( geoip, "selector" ); + + m_geoip = std::make_unique< CalamaresUtils::GeoIP::Handler >( style, url, selector ); + if ( !m_geoip->isValid() ) + { + cWarning() << "GeoIP Style" << style << "is not recognized."; + } } } diff --git a/src/modules/locale/LocaleViewStep.h b/src/modules/locale/LocaleViewStep.h index b04b34651..a6c28d78d 100644 --- a/src/modules/locale/LocaleViewStep.h +++ b/src/modules/locale/LocaleViewStep.h @@ -20,6 +20,7 @@ #ifndef LOCALEVIEWSTEP_H #define LOCALEVIEWSTEP_H +#include "geoip/Handler.h" #include "geoip/Interface.h" #include "utils/PluginFactory.h" #include "viewpages/ViewStep.h" @@ -29,6 +30,8 @@ #include #include +#include + class LocalePage; class WaitingWidget; @@ -74,11 +77,8 @@ private: CalamaresUtils::GeoIP::RegionZonePair m_startingTimezone; QString m_localeGenPath; - QString m_geoipUrl; // The URL, depening on style might be modified on lookup - QString m_geoipStyle; // String selecting which kind of geoip data to expect - QString m_geoipSelector; // String selecting data from the geoip lookup - QList< Calamares::job_ptr > m_jobs; + std::unique_ptr< CalamaresUtils::GeoIP::Handler > m_geoip; }; CALAMARES_PLUGIN_FACTORY_DECLARATION( LocaleViewStepFactory ) From 41ece863de6633ac997bdbb55e390b4fbbdc6150 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 10 Sep 2019 06:18:55 -0400 Subject: [PATCH 10/13] [locale] Create widgets when needed instead of at startup - this blocks forever, since now the GeoIP lookup isn't done at all. --- src/modules/locale/LocaleViewStep.cpp | 89 ++++++++++++--------------- 1 file changed, 40 insertions(+), 49 deletions(-) diff --git a/src/modules/locale/LocaleViewStep.cpp b/src/modules/locale/LocaleViewStep.cpp index bbd2d07a1..89baf23c2 100644 --- a/src/modules/locale/LocaleViewStep.cpp +++ b/src/modules/locale/LocaleViewStep.cpp @@ -43,7 +43,8 @@ CALAMARES_PLUGIN_FACTORY_DEFINITION( LocaleViewStepFactory, registerPlugin< Loca LocaleViewStep::LocaleViewStep( QObject* parent ) : Calamares::ViewStep( parent ) , m_widget( new QWidget() ) - , m_actualWidget( new LocalePage() ) + , m_waitingWidget( nullptr ) + , m_actualWidget( nullptr ) , m_nextEnabled( false ) , m_geoip( nullptr ) { @@ -51,41 +52,6 @@ LocaleViewStep::LocaleViewStep( QObject* parent ) m_widget->setLayout( mainLayout ); CalamaresUtils::unmarginLayout( mainLayout ); - m_waitingWidget = new WaitingWidget( tr( "Loading location data..." ) ); - mainLayout->addWidget( m_waitingWidget ); - - connect( &m_initWatcher, &QFutureWatcher< void >::finished, this, [=] { - bool hasInternet = Calamares::JobQueue::instance()->globalStorage()->value( "hasInternet" ).toBool(); - if ( !m_geoip || !hasInternet ) - { - setUpPage(); - } - else - { - fetchGeoIpTimezone(); - } - } ); - - QFuture< void > initFuture = QtConcurrent::run( [=] { - LocaleGlobal::init(); - if ( !m_geoip ) - { - return; - } - - Calamares::GlobalStorage* gs = Calamares::JobQueue::instance()->globalStorage(); - - // Max 10sec wait for RequirementsChecker to finish, assuming the welcome - // module is used. - // If welcome is not used, either "hasInternet" should be set by other means, - // or the GeoIP feature should be disabled. - for ( int i = 0; i < 10; ++i ) - if ( !gs->contains( "hasInternet" ) ) - QThread::sleep( 1 ); - } ); - - m_initWatcher.setFuture( initFuture ); - emit nextStatusChanged( m_nextEnabled ); } @@ -102,10 +68,19 @@ LocaleViewStep::~LocaleViewStep() void LocaleViewStep::setUpPage() { + if ( m_waitingWidget ) + { + m_widget->layout()->removeWidget( m_waitingWidget ); + m_waitingWidget->deleteLater(); + } + + if ( !m_actualWidget ) + { + m_actualWidget = new LocalePage(); + } m_actualWidget->init( m_startingTimezone.first, m_startingTimezone.second, m_localeGenPath ); - m_widget->layout()->removeWidget( m_waitingWidget ); - m_waitingWidget->deleteLater(); m_widget->layout()->addWidget( m_actualWidget ); + m_nextEnabled = true; emit nextStatusChanged( m_nextEnabled ); } @@ -122,7 +97,6 @@ LocaleViewStep::fetchGeoIpTimezone() cWarning() << "GeoIP lookup at" << m_geoip->url() << "failed."; } } - setUpPage(); } @@ -143,6 +117,12 @@ LocaleViewStep::prettyStatus() const QWidget* LocaleViewStep::widget() { + // If none of the inner widgets is already created, create the spinner + if ( !m_actualWidget && !m_waitingWidget ) + { + m_waitingWidget = new WaitingWidget( tr( "Loading location data..." ) ); + m_widget->layout()->addWidget( m_waitingWidget ); + } return m_widget; } @@ -185,7 +165,10 @@ LocaleViewStep::jobs() const void LocaleViewStep::onActivate() { - m_actualWidget->onActivate(); + if ( m_actualWidget ) + { + m_actualWidget->onActivate(); + } } @@ -193,18 +176,26 @@ void LocaleViewStep::onLeave() { m_jobs.clear(); - m_jobs.append( m_actualWidget->createJobs() ); - m_prettyStatus = m_actualWidget->prettyStatus(); - - auto map = m_actualWidget->localesMap(); - QVariantMap vm; - for ( auto it = map.constBegin(); it != map.constEnd(); ++it ) + if ( m_actualWidget ) { - vm.insert( it.key(), it.value() ); - } + m_jobs.append( m_actualWidget->createJobs() ); - Calamares::JobQueue::instance()->globalStorage()->insert( "localeConf", vm ); + m_prettyStatus = m_actualWidget->prettyStatus(); + + auto map = m_actualWidget->localesMap(); + QVariantMap vm; + for ( auto it = map.constBegin(); it != map.constEnd(); ++it ) + { + vm.insert( it.key(), it.value() ); + } + + Calamares::JobQueue::instance()->globalStorage()->insert( "localeConf", vm ); + } + else + { + Calamares::JobQueue::instance()->globalStorage()->remove( "localeConf" ); + } } From 11d3f10e26cb75f2bb9e49537c3d79ee12261e6f Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 10 Sep 2019 14:18:47 +0200 Subject: [PATCH 11/13] [locale] Dispose of waiting widget - Do the async GeoIP checking in the async requirements-checking phase - Do not return any requirements results -- we just need the async bit - Drop the waiting widget, since it's not needed (done by the requirements phase) --- src/modules/locale/LocaleViewStep.cpp | 43 +++++++++++++++++---------- src/modules/locale/LocaleViewStep.h | 5 ++-- 2 files changed, 30 insertions(+), 18 deletions(-) diff --git a/src/modules/locale/LocaleViewStep.cpp b/src/modules/locale/LocaleViewStep.cpp index 89baf23c2..4acfcb2de 100644 --- a/src/modules/locale/LocaleViewStep.cpp +++ b/src/modules/locale/LocaleViewStep.cpp @@ -27,7 +27,7 @@ #include "JobQueue.h" #include "geoip/Handler.h" - +#include "network/Manager.h" #include "utils/CalamaresUtilsGui.h" #include "utils/Logger.h" #include "utils/Variant.h" @@ -43,7 +43,6 @@ CALAMARES_PLUGIN_FACTORY_DEFINITION( LocaleViewStepFactory, registerPlugin< Loca LocaleViewStep::LocaleViewStep( QObject* parent ) : Calamares::ViewStep( parent ) , m_widget( new QWidget() ) - , m_waitingWidget( nullptr ) , m_actualWidget( nullptr ) , m_nextEnabled( false ) , m_geoip( nullptr ) @@ -68,12 +67,6 @@ LocaleViewStep::~LocaleViewStep() void LocaleViewStep::setUpPage() { - if ( m_waitingWidget ) - { - m_widget->layout()->removeWidget( m_waitingWidget ); - m_waitingWidget->deleteLater(); - } - if ( !m_actualWidget ) { m_actualWidget = new LocalePage(); @@ -117,12 +110,6 @@ LocaleViewStep::prettyStatus() const QWidget* LocaleViewStep::widget() { - // If none of the inner widgets is already created, create the spinner - if ( !m_actualWidget && !m_waitingWidget ) - { - m_waitingWidget = new WaitingWidget( tr( "Loading location data..." ) ); - m_widget->layout()->addWidget( m_waitingWidget ); - } return m_widget; } @@ -165,10 +152,11 @@ LocaleViewStep::jobs() const void LocaleViewStep::onActivate() { - if ( m_actualWidget ) + if ( !m_actualWidget ) { - m_actualWidget->onActivate(); + setUpPage(); } + m_actualWidget->onActivate(); } @@ -235,3 +223,26 @@ LocaleViewStep::setConfigurationMap( const QVariantMap& configurationMap ) } } } + +Calamares::RequirementsList +LocaleViewStep::checkRequirements() +{ + LocaleGlobal::init(); + if ( m_geoip && m_geoip->isValid() ) + { + auto& network = CalamaresUtils::Network::Manager::instance(); + if ( network.hasInternet() ) + { + fetchGeoIpTimezone(); + } + else + { + if ( network.synchronousPing( m_geoip->url() ) ) + { + fetchGeoIpTimezone(); + } + } + } + + return Calamares::RequirementsList(); +} diff --git a/src/modules/locale/LocaleViewStep.h b/src/modules/locale/LocaleViewStep.h index a6c28d78d..0bb0e34a6 100644 --- a/src/modules/locale/LocaleViewStep.h +++ b/src/modules/locale/LocaleViewStep.h @@ -61,14 +61,15 @@ public: void setConfigurationMap( const QVariantMap& configurationMap ) override; + /// @brief Do setup (returns empty list) asynchronously + virtual Calamares::RequirementsList checkRequirements(); + private slots: void setUpPage(); private: void fetchGeoIpTimezone(); QWidget* m_widget; - QFutureWatcher< void > m_initWatcher; - WaitingWidget* m_waitingWidget; LocalePage* m_actualWidget; bool m_nextEnabled; From ca351ff7b600f999fdf41dec4e42bda55db6fd34 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 10 Sep 2019 14:26:46 +0200 Subject: [PATCH 12/13] [libcalamares] Apply (some) options to request earlier --- src/libcalamares/network/Manager.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libcalamares/network/Manager.cpp b/src/libcalamares/network/Manager.cpp index 56e36cd55..b4d90c2b8 100644 --- a/src/libcalamares/network/Manager.cpp +++ b/src/libcalamares/network/Manager.cpp @@ -111,6 +111,8 @@ static QNetworkReply* asynchronousRun( const std::unique_ptr< QNetworkAccessManager >& nam, const QUrl& url, const RequestOptions& options ) { QNetworkRequest request = QNetworkRequest( url ); + options.applyToRequest( &request ); + QNetworkReply* reply = nam->get( request ); QTimer* timer = nullptr; @@ -121,7 +123,6 @@ asynchronousRun( const std::unique_ptr< QNetworkAccessManager >& nam, const QUrl return nullptr; } - options.applyToRequest( &request ); if ( options.hasTimeout() ) { timer = new QTimer( reply ); From 57a942d155d2cf8af320c97dad0fb5a0ad5b3ac0 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 14 Sep 2019 07:21:24 -0400 Subject: [PATCH 13/13] [libcalamares] Make a NAM per thread - To avoid warnings about creating requests and replies, parented by the NAM but from another thread, make a NAM per thread. --- src/libcalamares/network/Manager.cpp | 95 +++++++++++++++++++++++++--- 1 file changed, 86 insertions(+), 9 deletions(-) diff --git a/src/libcalamares/network/Manager.cpp b/src/libcalamares/network/Manager.cpp index b4d90c2b8..fefa7c375 100644 --- a/src/libcalamares/network/Manager.cpp +++ b/src/libcalamares/network/Manager.cpp @@ -18,10 +18,15 @@ #include "Manager.h" +#include "utils/Logger.h" + #include +#include +#include #include #include #include +#include #include namespace CalamaresUtils @@ -45,21 +50,91 @@ RequestOptions::applyToRequest( QNetworkRequest* request ) const } } -struct Manager::Private +class Manager::Private : public QObject { + Q_OBJECT +private: std::unique_ptr< QNetworkAccessManager > m_nam; + + using ThreadNam = QPair< QThread*, QNetworkAccessManager* >; + QVector< ThreadNam > m_perThreadNams; + +public slots: + void cleanupNam(); + +public: QUrl m_hasInternetUrl; bool m_hasInternet; Private(); + + QNetworkAccessManager* nam(); }; Manager::Private::Private() : m_nam( std::make_unique< QNetworkAccessManager >() ) , m_hasInternet( false ) { + m_perThreadNams.reserve( 20 ); + m_perThreadNams.append( qMakePair( QThread::currentThread(), m_nam.get() ) ); } +static QMutex* +namMutex() +{ + static QMutex namMutex; + return &namMutex; +} + +QNetworkAccessManager* +Manager::Private::nam() +{ + QMutexLocker lock( namMutex() ); + + auto* thread = QThread::currentThread(); + int index = 0; + for ( const auto& n : m_perThreadNams ) + { + if ( n.first == thread ) + { + return n.second; + } + ++index; + } + + // Need a new NAM for this thread + QNetworkAccessManager* nam = new QNetworkAccessManager(); + m_perThreadNams.append( qMakePair( thread, nam ) ); + QObject::connect( thread, &QThread::finished, this, &Manager::Private::cleanupNam ); + + return nam; +} + +void +Manager::Private::cleanupNam() +{ + QMutexLocker lock( namMutex() ); + + auto* thread = QThread::currentThread(); + bool cleanupFound = false; + int cleanupIndex = 0; + for ( const auto& n : m_perThreadNams ) + { + if ( n.first == thread ) + { + cleanupFound = true; + delete n.second; + break; + } + ++cleanupIndex; + } + if ( cleanupFound ) + { + m_perThreadNams.remove( cleanupIndex ); + } +} + + Manager::Manager() : d( std::make_unique< Private >() ) { @@ -83,9 +158,9 @@ Manager::hasInternet() bool Manager::checkHasInternet() { - bool hasInternet = d->m_nam->networkAccessible() == QNetworkAccessManager::Accessible; + bool hasInternet = d->nam()->networkAccessible() == QNetworkAccessManager::Accessible; - if ( !hasInternet && ( d->m_nam->networkAccessible() == QNetworkAccessManager::UnknownAccessibility ) ) + if ( !hasInternet && ( d->nam()->networkAccessible() == QNetworkAccessManager::UnknownAccessibility ) ) { hasInternet = synchronousPing( d->m_hasInternetUrl ); } @@ -108,7 +183,7 @@ Manager::setCheckHasInternetUrl( const QUrl& url ) * On failure, returns nullptr (e.g. bad URL, timeout). */ static QNetworkReply* -asynchronousRun( const std::unique_ptr< QNetworkAccessManager >& nam, const QUrl& url, const RequestOptions& options ) +asynchronousRun( QNetworkAccessManager* nam, const QUrl& url, const RequestOptions& options ) { QNetworkRequest request = QNetworkRequest( url ); options.applyToRequest( &request ); @@ -143,12 +218,12 @@ asynchronousRun( const std::unique_ptr< QNetworkAccessManager >& nam, const QUrl * is marked for later automatic deletion, so don't store the pointer. */ static QPair< RequestStatus, QNetworkReply* > -synchronousRun( const std::unique_ptr< QNetworkAccessManager >& nam, const QUrl& url, const RequestOptions& options ) +synchronousRun( QNetworkAccessManager* nam, const QUrl& url, const RequestOptions& options ) { auto* reply = asynchronousRun( nam, url, options ); if ( !reply ) { - return qMakePair( RequestStatus( RequestStatus::Failed ), nullptr ); + return qMakePair( RequestStatus( RequestStatus::Failed ), nullptr ); } QEventLoop loop; @@ -177,7 +252,7 @@ Manager::synchronousPing( const QUrl& url, const RequestOptions& options ) return RequestStatus::Failed; } - auto reply = synchronousRun( d->m_nam, url, options ); + auto reply = synchronousRun( d->nam(), url, options ); if ( reply.first ) { return reply.second->bytesAvailable() ? RequestStatus::Ok : RequestStatus::Empty; @@ -196,16 +271,18 @@ Manager::synchronousGet( const QUrl& url, const RequestOptions& options ) return QByteArray(); } - auto reply = synchronousRun( d->m_nam, url, options ); + auto reply = synchronousRun( d->nam(), url, options ); return reply.first ? reply.second->readAll() : QByteArray(); } QNetworkReply* Manager::asynchronouseGet( const QUrl& url, const CalamaresUtils::Network::RequestOptions& options ) { - return asynchronousRun( d->m_nam, url, options ); + return asynchronousRun( d->nam(), url, options ); } } // namespace Network } // namespace CalamaresUtils + +#include "Manager.moc"