From 3e51d77782d7684045ec325d8b62b2e809c429b5 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 4 Nov 2020 13:39:23 +0100 Subject: [PATCH 1/6] [keyboard] Check for successful file-write --- src/modules/keyboard/SetKeyboardLayoutJob.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/modules/keyboard/SetKeyboardLayoutJob.cpp b/src/modules/keyboard/SetKeyboardLayoutJob.cpp index 419d6f3fc..8a9f5fe38 100644 --- a/src/modules/keyboard/SetKeyboardLayoutJob.cpp +++ b/src/modules/keyboard/SetKeyboardLayoutJob.cpp @@ -210,7 +210,11 @@ SetKeyboardLayoutJob::writeVConsoleData( const QString& vconsoleConfPath, const } // Write out the existing lines and replace the KEYMAP= line - file.open( QIODevice::WriteOnly | QIODevice::Text ); + if ( !file.open( QIODevice::WriteOnly | QIODevice::Text ) ) + { + cError() << "Could not open" << file.fileName() << "for writing."; + return false; + } QTextStream stream( &file ); bool found = false; foreach ( const QString& existingLine, existingLines ) @@ -243,7 +247,11 @@ bool SetKeyboardLayoutJob::writeX11Data( const QString& keyboardConfPath ) const { QFile file( keyboardConfPath ); - file.open( QIODevice::WriteOnly | QIODevice::Text ); + if ( !file.open( QIODevice::WriteOnly | QIODevice::Text ) ) + { + cError() << "Could not open" << file.fileName() << "for writing."; + return false; + } QTextStream stream( &file ); stream << "# Read and parsed by systemd-localed. It's probably wise not to edit this file\n" From 845fb3f6da8894f40f2966253a847320ffac31fa Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 4 Nov 2020 14:03:14 +0100 Subject: [PATCH 2/6] [keyboard] Refactor writing-various-files - put the writing of each kind of file in its own block -- this should become separate functions -- so that variables become more local and debugging can be improved. - while here, fix the error message for /etc/default/keyboard: it would complain and name the vconsole file path if it ever failed. --- src/modules/keyboard/SetKeyboardLayoutJob.cpp | 97 +++++++++++-------- 1 file changed, 55 insertions(+), 42 deletions(-) diff --git a/src/modules/keyboard/SetKeyboardLayoutJob.cpp b/src/modules/keyboard/SetKeyboardLayoutJob.cpp index 8a9f5fe38..78459d190 100644 --- a/src/modules/keyboard/SetKeyboardLayoutJob.cpp +++ b/src/modules/keyboard/SetKeyboardLayoutJob.cpp @@ -337,60 +337,73 @@ SetKeyboardLayoutJob::exec() Calamares::GlobalStorage* gs = Calamares::JobQueue::instance()->globalStorage(); QDir destDir( gs->value( "rootMountPoint" ).toString() ); - // Get the path to the destination's /etc/vconsole.conf - QString vconsoleConfPath = destDir.absoluteFilePath( "etc/vconsole.conf" ); - - // Get the path to the destination's /etc/X11/xorg.conf.d/00-keyboard.conf - QString xorgConfDPath; - QString keyboardConfPath; - if ( QDir::isAbsolutePath( m_xOrgConfFileName ) ) { - keyboardConfPath = m_xOrgConfFileName; - while ( keyboardConfPath.startsWith( '/' ) ) + // Get the path to the destination's /etc/vconsole.conf + QString vconsoleConfPath = destDir.absoluteFilePath( "etc/vconsole.conf" ); + + // Get the path to the destination's path to the converted key mappings + QString convertedKeymapPath = m_convertedKeymapPath; + if ( !convertedKeymapPath.isEmpty() ) { - keyboardConfPath.remove( 0, 1 ); + while ( convertedKeymapPath.startsWith( '/' ) ) + { + convertedKeymapPath.remove( 0, 1 ); + } + convertedKeymapPath = destDir.absoluteFilePath( convertedKeymapPath ); } - keyboardConfPath = destDir.absoluteFilePath( keyboardConfPath ); - xorgConfDPath = QFileInfo( keyboardConfPath ).path(); - } - else - { - xorgConfDPath = destDir.absoluteFilePath( "etc/X11/xorg.conf.d" ); - keyboardConfPath = QDir( xorgConfDPath ).absoluteFilePath( m_xOrgConfFileName ); - } - destDir.mkpath( xorgConfDPath ); - QString defaultKeyboardPath; - if ( QDir( destDir.absoluteFilePath( "etc/default" ) ).exists() ) - { - defaultKeyboardPath = destDir.absoluteFilePath( "etc/default/keyboard" ); + cDebug() << "Writing VCONSOLE data to" << vconsoleConfPath << convertedKeymapPath; + if ( !writeVConsoleData( vconsoleConfPath, convertedKeymapPath ) ) + { + return Calamares::JobResult::error( tr( "Failed to write keyboard configuration for the virtual console." ), + tr( "Failed to write to %1" ).arg( vconsoleConfPath ) ); + } } - // Get the path to the destination's path to the converted key mappings - QString convertedKeymapPath = m_convertedKeymapPath; - if ( !convertedKeymapPath.isEmpty() ) { - while ( convertedKeymapPath.startsWith( '/' ) ) + // Get the path to the destination's /etc/X11/xorg.conf.d/00-keyboard.conf + QString xorgConfDPath; + QString keyboardConfPath; + if ( QDir::isAbsolutePath( m_xOrgConfFileName ) ) { - convertedKeymapPath.remove( 0, 1 ); + keyboardConfPath = m_xOrgConfFileName; + while ( keyboardConfPath.startsWith( '/' ) ) + { + keyboardConfPath.remove( 0, 1 ); + } + keyboardConfPath = destDir.absoluteFilePath( keyboardConfPath ); + xorgConfDPath = QFileInfo( keyboardConfPath ).path(); } - convertedKeymapPath = destDir.absoluteFilePath( convertedKeymapPath ); - } - - if ( !writeVConsoleData( vconsoleConfPath, convertedKeymapPath ) ) - return Calamares::JobResult::error( tr( "Failed to write keyboard configuration for the virtual console." ), - tr( "Failed to write to %1" ).arg( vconsoleConfPath ) ); + else + { + xorgConfDPath = destDir.absoluteFilePath( "etc/X11/xorg.conf.d" ); + keyboardConfPath = QDir( xorgConfDPath ).absoluteFilePath( m_xOrgConfFileName ); + } + destDir.mkpath( xorgConfDPath ); - if ( !writeX11Data( keyboardConfPath ) ) - return Calamares::JobResult::error( tr( "Failed to write keyboard configuration for X11." ), - tr( "Failed to write to %1" ).arg( keyboardConfPath ) ); + if ( !writeX11Data( keyboardConfPath ) ) + { + return Calamares::JobResult::error( tr( "Failed to write keyboard configuration for X11." ), + tr( "Failed to write to %1" ).arg( keyboardConfPath ) ); + } + } - if ( !defaultKeyboardPath.isEmpty() && m_writeEtcDefaultKeyboard ) { - if ( !writeDefaultKeyboardData( defaultKeyboardPath ) ) - return Calamares::JobResult::error( - tr( "Failed to write keyboard configuration to existing /etc/default directory." ), - tr( "Failed to write to %1" ).arg( keyboardConfPath ) ); + QString defaultKeyboardPath; + if ( QDir( destDir.absoluteFilePath( "etc/default" ) ).exists() ) + { + defaultKeyboardPath = destDir.absoluteFilePath( "etc/default/keyboard" ); + } + + if ( !defaultKeyboardPath.isEmpty() && m_writeEtcDefaultKeyboard ) + { + if ( !writeDefaultKeyboardData( defaultKeyboardPath ) ) + { + return Calamares::JobResult::error( + tr( "Failed to write keyboard configuration to existing /etc/default directory." ), + tr( "Failed to write to %1" ).arg( defaultKeyboardPath ) ); + } + } } return Calamares::JobResult::ok(); From 57da222c7231bce0d7903218e864593001c0519d Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 4 Nov 2020 16:16:28 +0100 Subject: [PATCH 3/6] [keyboard] Improve debug-logging for vconsole-config --- src/modules/keyboard/SetKeyboardLayoutJob.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/modules/keyboard/SetKeyboardLayoutJob.cpp b/src/modules/keyboard/SetKeyboardLayoutJob.cpp index 78459d190..331c11efb 100644 --- a/src/modules/keyboard/SetKeyboardLayoutJob.cpp +++ b/src/modules/keyboard/SetKeyboardLayoutJob.cpp @@ -179,6 +179,8 @@ SetKeyboardLayoutJob::findLegacyKeymap() const bool SetKeyboardLayoutJob::writeVConsoleData( const QString& vconsoleConfPath, const QString& convertedKeymapPath ) const { + cDebug() << "Writing vconsole data to" << vconsoleConfPath; + QString keymap = findConvertedKeymap( convertedKeymapPath ); if ( keymap.isEmpty() ) { @@ -205,6 +207,7 @@ SetKeyboardLayoutJob::writeVConsoleData( const QString& vconsoleConfPath, const file.close(); if ( stream.status() != QTextStream::Ok ) { + cError() << "Could not read lines from" << file.fileName(); return false; } } @@ -217,7 +220,7 @@ SetKeyboardLayoutJob::writeVConsoleData( const QString& vconsoleConfPath, const } QTextStream stream( &file ); bool found = false; - foreach ( const QString& existingLine, existingLines ) + for( const QString& existingLine : qAsConst( existingLines ) ) { if ( existingLine.trimmed().startsWith( "KEYMAP=" ) ) { @@ -237,7 +240,7 @@ SetKeyboardLayoutJob::writeVConsoleData( const QString& vconsoleConfPath, const stream.flush(); file.close(); - cDebug() << "Written KEYMAP=" << keymap << "to vconsole.conf"; + cDebug() << Logger::SubEntry << "Written KEYMAP=" << keymap << "to vconsole.conf" << stream.status(); return ( stream.status() == QTextStream::Ok ); } @@ -352,7 +355,6 @@ SetKeyboardLayoutJob::exec() convertedKeymapPath = destDir.absoluteFilePath( convertedKeymapPath ); } - cDebug() << "Writing VCONSOLE data to" << vconsoleConfPath << convertedKeymapPath; if ( !writeVConsoleData( vconsoleConfPath, convertedKeymapPath ) ) { return Calamares::JobResult::error( tr( "Failed to write keyboard configuration for the virtual console." ), From cbc376a20666e9aab57f9eae46ae5d1b2ca4bd42 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 4 Nov 2020 16:29:57 +0100 Subject: [PATCH 4/6] [keyboard] Improve debug-logging for writing X11 config --- src/modules/keyboard/SetKeyboardLayoutJob.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/modules/keyboard/SetKeyboardLayoutJob.cpp b/src/modules/keyboard/SetKeyboardLayoutJob.cpp index 331c11efb..2588608b1 100644 --- a/src/modules/keyboard/SetKeyboardLayoutJob.cpp +++ b/src/modules/keyboard/SetKeyboardLayoutJob.cpp @@ -249,6 +249,8 @@ SetKeyboardLayoutJob::writeVConsoleData( const QString& vconsoleConfPath, const bool SetKeyboardLayoutJob::writeX11Data( const QString& keyboardConfPath ) const { + cDebug() << "Writing X11 configuration to" << keyboardConfPath; + QFile file( keyboardConfPath ); if ( !file.open( QIODevice::WriteOnly | QIODevice::Text ) ) { @@ -298,8 +300,8 @@ SetKeyboardLayoutJob::writeX11Data( const QString& keyboardConfPath ) const file.close(); - cDebug() << "Written XkbLayout" << m_layout << "; XkbModel" << m_model << "; XkbVariant" << m_variant - << "to X.org file" << keyboardConfPath; + cDebug() << Logger::SubEntry << "Written XkbLayout" << m_layout << "; XkbModel" << m_model << "; XkbVariant" << m_variant + << "to X.org file" << keyboardConfPath << stream.status(); return ( stream.status() == QTextStream::Ok ); } From 705e6dd8d697769000e3a77bfd6a88474a672ea9 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 4 Nov 2020 16:31:35 +0100 Subject: [PATCH 5/6] [keyboard] Improve debug-logging for writing etc/default --- src/modules/keyboard/SetKeyboardLayoutJob.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/modules/keyboard/SetKeyboardLayoutJob.cpp b/src/modules/keyboard/SetKeyboardLayoutJob.cpp index 2588608b1..9cf76d96b 100644 --- a/src/modules/keyboard/SetKeyboardLayoutJob.cpp +++ b/src/modules/keyboard/SetKeyboardLayoutJob.cpp @@ -310,8 +310,14 @@ SetKeyboardLayoutJob::writeX11Data( const QString& keyboardConfPath ) const bool SetKeyboardLayoutJob::writeDefaultKeyboardData( const QString& defaultKeyboardPath ) const { + cDebug() << "Writing default keyboard data to" << defaultKeyboardPath; + QFile file( defaultKeyboardPath ); - file.open( QIODevice::WriteOnly | QIODevice::Text ); + if ( !file.open( QIODevice::WriteOnly | QIODevice::Text ) ) + { + cError() << "Could not open" << defaultKeyboardPath << "for writing"; + return false; + } QTextStream stream( &file ); stream << "# KEYBOARD CONFIGURATION FILE\n\n" @@ -326,8 +332,8 @@ SetKeyboardLayoutJob::writeDefaultKeyboardData( const QString& defaultKeyboardPa file.close(); - cDebug() << "Written XKBMODEL" << m_model << "; XKBLAYOUT" << m_layout << "; XKBVARIANT" << m_variant - << "to /etc/default/keyboard file" << defaultKeyboardPath; + cDebug() << Logger::SubEntry << "Written XKBMODEL" << m_model << "; XKBLAYOUT" << m_layout << "; XKBVARIANT" << m_variant + << "to /etc/default/keyboard file" << defaultKeyboardPath << stream.status(); return ( stream.status() == QTextStream::Ok ); } From 83d2c06f68540cc16bdcf8e1f4425a35a030ffd0 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 4 Nov 2020 16:36:52 +0100 Subject: [PATCH 6/6] [keyboard] Apply coding style --- src/modules/keyboard/SetKeyboardLayoutJob.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/modules/keyboard/SetKeyboardLayoutJob.cpp b/src/modules/keyboard/SetKeyboardLayoutJob.cpp index 9cf76d96b..c80d84e7d 100644 --- a/src/modules/keyboard/SetKeyboardLayoutJob.cpp +++ b/src/modules/keyboard/SetKeyboardLayoutJob.cpp @@ -220,7 +220,7 @@ SetKeyboardLayoutJob::writeVConsoleData( const QString& vconsoleConfPath, const } QTextStream stream( &file ); bool found = false; - for( const QString& existingLine : qAsConst( existingLines ) ) + for ( const QString& existingLine : qAsConst( existingLines ) ) { if ( existingLine.trimmed().startsWith( "KEYMAP=" ) ) { @@ -300,8 +300,8 @@ SetKeyboardLayoutJob::writeX11Data( const QString& keyboardConfPath ) const file.close(); - cDebug() << Logger::SubEntry << "Written XkbLayout" << m_layout << "; XkbModel" << m_model << "; XkbVariant" << m_variant - << "to X.org file" << keyboardConfPath << stream.status(); + cDebug() << Logger::SubEntry << "Written XkbLayout" << m_layout << "; XkbModel" << m_model << "; XkbVariant" + << m_variant << "to X.org file" << keyboardConfPath << stream.status(); return ( stream.status() == QTextStream::Ok ); } @@ -332,8 +332,8 @@ SetKeyboardLayoutJob::writeDefaultKeyboardData( const QString& defaultKeyboardPa file.close(); - cDebug() << Logger::SubEntry << "Written XKBMODEL" << m_model << "; XKBLAYOUT" << m_layout << "; XKBVARIANT" << m_variant - << "to /etc/default/keyboard file" << defaultKeyboardPath << stream.status(); + cDebug() << Logger::SubEntry << "Written XKBMODEL" << m_model << "; XKBLAYOUT" << m_layout << "; XKBVARIANT" + << m_variant << "to /etc/default/keyboard file" << defaultKeyboardPath << stream.status(); return ( stream.status() == QTextStream::Ok ); }