diff --git a/src/libcalamares/utils/Logger.h b/src/libcalamares/utils/Logger.h index 6736ba740..fb49248e1 100644 --- a/src/libcalamares/utils/Logger.h +++ b/src/libcalamares/utils/Logger.h @@ -58,7 +58,7 @@ public: virtual ~CDebug(); friend CDebug& operator<<( CDebug&&, const FuncSuppressor& ); - friend CDebug& operator<<( CDebug&&, Once& ); + friend CDebug& operator<<( CDebug&&, const Once& ); private: QString m_msg; @@ -306,14 +306,14 @@ public: : m( true ) { } - friend CDebug& operator<<( CDebug&&, Once& ); + friend CDebug& operator<<( CDebug&&, const Once& ); private: - bool m = false; + mutable bool m = false; }; inline CDebug& -operator<<( CDebug&& s, Once& o ) +operator<<( CDebug&& s, const Once& o ) { if ( o.m ) { diff --git a/src/modules/partition/CMakeLists.txt b/src/modules/partition/CMakeLists.txt index 07f1eaf5d..bffb2128c 100644 --- a/src/modules/partition/CMakeLists.txt +++ b/src/modules/partition/CMakeLists.txt @@ -8,8 +8,9 @@ # want to allow unsafe partitioning choices (e.g. doing things to the # current disk). Set DEBUG_PARTITION_UNSAFE to allow that (it turns off # some filtering of devices). If you **do** allow unsafe partitioning, -# it will error out unless you **also** switch **off** DEBUG_PARTITION_LAME, -# at which point you are welcome to shoot yourself in the foot. +# it will error out at runtime unless you **also** switch **off** +# DEBUG_PARTITION_LAME, at which point you are welcome to shoot +# yourself in the foot. option( DEBUG_PARTITION_UNSAFE "Allow unsafe partitioning choices." OFF ) option( DEBUG_PARTITION_LAME "Unsafe partitioning will error out on exec." ON ) diff --git a/src/modules/partition/core/DeviceList.cpp b/src/modules/partition/core/DeviceList.cpp index 2fce62e9d..f7d1d4835 100644 --- a/src/modules/partition/core/DeviceList.cpp +++ b/src/modules/partition/core/DeviceList.cpp @@ -10,14 +10,8 @@ #include "DeviceList.h" -#include "PartitionCoreModule.h" -#include "core/DeviceModel.h" -#include "core/KPMHelpers.h" - -#include "GlobalStorage.h" -#include "JobQueue.h" -#include "partition/PartitionIterator.h" #include "utils/Logger.h" +#include "partition/PartitionIterator.h" #include #include @@ -25,7 +19,6 @@ #include #include -#include using CalamaresUtils::Partition::PartitionIterator; @@ -118,8 +111,6 @@ erase( DeviceList& l, DeviceList::iterator& it ) QList< Device* > getDevices( DeviceType which ) { - bool writableOnly = ( which == DeviceType::WritableOnly ); - CoreBackend* backend = CoreBackendManager::self()->backend(); #if defined( WITH_KPMCORE4API ) DeviceList devices = backend->scanDevices( /* not includeReadOnly, not includeLoopback */ ScanFlag( 0 ) ); @@ -127,47 +118,66 @@ getDevices( DeviceType which ) DeviceList devices = backend->scanDevices( /* excludeReadOnly */ true ); #endif + /* The list of devices is cleaned up for use: + * - some devices can **never** be used (e.g. floppies, nullptr) + * - some devices can be used if unsafe mode is on, but not in normal operation + * Two lambda's are defined, + * - removeInAllModes() + * - removeInSafeMode() + * To handle the difference. + */ #ifdef DEBUG_PARTITION_UNSAFE cWarning() << "Allowing unsafe partitioning choices." << devices.count() << "candidates."; #ifdef DEBUG_PARTITION_LAME - cDebug() << Logger::SubEntry << "it has been lamed, and will fail."; + cDebug() << Logger::SubEntry << "unsafe partitioning has been lamed, and will fail."; #endif + + // Unsafe partitioning + auto removeInAllModes = []( DeviceList& l, DeviceList::iterator& it) { return erase(l, it); }; + auto removeInSafeMode = []( DeviceList&, DeviceList::iterator& it) { return ++it; }; #else + // Safe partitioning + auto removeInAllModes = []( DeviceList& l, DeviceList::iterator& it) { return erase(l, it); }; + auto& removeInSafeMode = removeFromAll; +#endif + cDebug() << "Removing unsuitable devices:" << devices.count() << "candidates."; + bool writableOnly = ( which == DeviceType::WritableOnly ); // Remove the device which contains / from the list for ( DeviceList::iterator it = devices.begin(); it != devices.end(); ) + { if ( !( *it ) ) { cDebug() << Logger::SubEntry << "Skipping nullptr device"; - it = erase( devices, it ); + it = removeInAllModes( devices, it ); } else if ( isZRam( *it ) ) { cDebug() << Logger::SubEntry << "Removing zram" << it; - it = erase( devices, it ); + it = removeInAllModes( devices, it ); } else if ( isFloppyDrive( ( *it ) ) ) { cDebug() << Logger::SubEntry << "Removing floppy disk" << it; - it = erase( devices, it ); + it = removeInAllModes( devices, it ); } else if ( writableOnly && hasRootPartition( *it ) ) { cDebug() << Logger::SubEntry << "Removing device with root filesystem (/) on it" << it; - it = erase( devices, it ); + it = removeInSafeMode( devices, it ); } else if ( writableOnly && isIso9660( *it ) ) { cDebug() << Logger::SubEntry << "Removing device with iso9660 filesystem (probably a CD) on it" << it; - it = erase( devices, it ); + it = removeInSafeMode( devices, it ); } else { ++it; } -#endif - + } + cDebug() << Logger::SubEntry << "there are" << devices.count() << "devices left."; return devices; } diff --git a/src/modules/partition/core/DeviceModel.cpp b/src/modules/partition/core/DeviceModel.cpp index bf5541756..46a181b33 100644 --- a/src/modules/partition/core/DeviceModel.cpp +++ b/src/modules/partition/core/DeviceModel.cpp @@ -99,7 +99,7 @@ DeviceModel::data( const QModelIndex& index, int role ) const return CalamaresUtils::defaultPixmap( CalamaresUtils::PartitionDisk, CalamaresUtils::Original, - QSize( CalamaresUtils::defaultIconSize().width() * 3, CalamaresUtils::defaultIconSize().height() * 3 ) ); + QSize( CalamaresUtils::defaultIconSize().width() * 2, CalamaresUtils::defaultIconSize().height() * 2 ) ); default: return QVariant(); } diff --git a/src/modules/partition/core/PartUtils.cpp b/src/modules/partition/core/PartUtils.cpp index 065f88d99..368d04af2 100644 --- a/src/modules/partition/core/PartUtils.cpp +++ b/src/modules/partition/core/PartUtils.cpp @@ -72,15 +72,15 @@ getRequiredStorageGiB( bool& ok ) } bool -canBeReplaced( Partition* candidate ) +canBeReplaced( Partition* candidate, const Logger::Once& o ) { if ( !candidate ) { - cDebug() << "Partition* is NULL"; + cDebug() << o << "Partition* is NULL"; return false; } - cDebug() << "Checking if" << convenienceName( candidate ) << "can be replaced."; + cDebug() << o << "Checking if" << convenienceName( candidate ) << "can be replaced."; if ( candidate->isMounted() ) { cDebug() << Logger::SubEntry << "NO, it is mounted."; @@ -100,7 +100,7 @@ canBeReplaced( Partition* candidate ) if ( availableStorageB > requiredStorageB ) { - cDebug() << "Partition" << convenienceName( candidate ) << "authorized for replace install."; + cDebug() << o << "Partition" << convenienceName( candidate ) << "authorized for replace install."; return true; } else @@ -117,15 +117,15 @@ canBeReplaced( Partition* candidate ) bool -canBeResized( Partition* candidate ) +canBeResized( Partition* candidate, const Logger::Once& o ) { if ( !candidate ) { - cDebug() << "Partition* is NULL"; + cDebug() << o << "Partition* is NULL"; return false; } - cDebug() << "Checking if" << convenienceName( candidate ) << "can be resized."; + cDebug() << o << "Checking if" << convenienceName( candidate ) << "can be resized."; if ( !candidate->fileSystem().supportGrow() || !candidate->fileSystem().supportShrink() ) { cDebug() << Logger::SubEntry << "NO, filesystem" << candidate->fileSystem().name() @@ -177,7 +177,7 @@ canBeResized( Partition* candidate ) if ( availableStorageB > advisedStorageB ) { - cDebug() << "Partition" << convenienceName( candidate ) << "authorized for resize + autopartition install."; + cDebug() << o << "Partition" << convenienceName( candidate ) << "authorized for resize + autopartition install."; return true; } else @@ -196,9 +196,9 @@ canBeResized( Partition* candidate ) bool -canBeResized( DeviceModel* dm, const QString& partitionPath ) +canBeResized( DeviceModel* dm, const QString& partitionPath, const Logger::Once& o ) { - cDebug() << "Checking if" << partitionPath << "can be resized."; + cDebug() << o << "Checking if" << partitionPath << "can be resized."; QString partitionWithOs = partitionPath; if ( partitionWithOs.startsWith( "/dev/" ) ) { @@ -208,7 +208,7 @@ canBeResized( DeviceModel* dm, const QString& partitionPath ) Partition* candidate = CalamaresUtils::Partition::findPartitionByPath( { dev }, partitionWithOs ); if ( candidate ) { - return canBeResized( candidate ); + return canBeResized( candidate, o ); } } cDebug() << Logger::SubEntry << "no Partition* found for" << partitionWithOs; @@ -357,6 +357,8 @@ findPartitionPathForMountPoint( const FstabEntryList& fstab, const QString& moun OsproberEntryList runOsprober( DeviceModel* dm ) { + Logger::Once o; + QString osproberOutput; QProcess osprober; osprober.setProgram( "os-prober" ); @@ -411,18 +413,18 @@ runOsprober( DeviceModel* dm ) QString homePath = findPartitionPathForMountPoint( fstabEntries, "/home" ); osproberEntries.append( - { prettyName, path, file, QString(), canBeResized( dm, path ), lineColumns, fstabEntries, homePath } ); + { prettyName, path, file, QString(), canBeResized( dm, path, o ), lineColumns, fstabEntries, homePath } ); osproberCleanLines.append( line ); } } if ( osproberCleanLines.count() > 0 ) { - cDebug() << "os-prober lines after cleanup:" << Logger::DebugList( osproberCleanLines ); + cDebug() << o << "os-prober lines after cleanup:" << Logger::DebugList( osproberCleanLines ); } else { - cDebug() << "os-prober gave no output."; + cDebug() << o << "os-prober gave no output."; } Calamares::JobQueue::instance()->globalStorage()->insert( "osproberLines", osproberCleanLines ); @@ -481,7 +483,6 @@ findFS( QString fsName, FileSystem::Type* fsType ) FileSystem::Type tmpType = FileSystem::typeForName( fsName, fsLanguage ); if ( tmpType != FileSystem::Unknown ) { - cDebug() << "Found filesystem" << fsName; if ( fsType ) { *fsType = tmpType; @@ -496,7 +497,6 @@ findFS( QString fsName, FileSystem::Type* fsType ) if ( 0 == QString::compare( fsName, FileSystem::nameForType( t, fsLanguage ), Qt::CaseInsensitive ) ) { QString fsRealName = FileSystem::nameForType( t, fsLanguage ); - cDebug() << "Filesystem name" << fsName << "translated to" << fsRealName; if ( fsType ) { *fsType = t; @@ -505,7 +505,7 @@ findFS( QString fsName, FileSystem::Type* fsType ) } } - cDebug() << "Filesystem" << fsName << "not found, using ext4"; + cWarning() << "Filesystem" << fsName << "not found, using ext4"; fsName = QStringLiteral( "ext4" ); // fsType can be used to check whether fsName was a valid filesystem. if ( fsType ) diff --git a/src/modules/partition/core/PartUtils.h b/src/modules/partition/core/PartUtils.h index f210cc3ab..aec345882 100644 --- a/src/modules/partition/core/PartUtils.h +++ b/src/modules/partition/core/PartUtils.h @@ -24,6 +24,10 @@ class DeviceModel; class Partition; +namespace Logger +{ + class Once; +} namespace PartUtils { @@ -41,26 +45,29 @@ QString convenienceName( const Partition* const candidate ); * @brief canBeReplaced checks whether the given Partition satisfies the criteria * for replacing it with the new OS. * @param candidate the candidate partition to replace. + * @param o applied to debug-logging. * @return true if the criteria are met, otherwise false. */ -bool canBeReplaced( Partition* candidate ); +bool canBeReplaced( Partition* candidate, const Logger::Once& o ); /** * @brief canBeReplaced checks whether the given Partition satisfies the criteria * for resizing (shrinking) it to make room for a new OS. * @param candidate the candidate partition to resize. + * @param o applied to debug-logging. * @return true if the criteria are met, otherwise false. */ -bool canBeResized( Partition* candidate ); +bool canBeResized( Partition* candidate, const Logger::Once& o ); /** * @brief canBeReplaced checks whether the given Partition satisfies the criteria * for resizing (shrinking) it to make room for a new OS. * @param dm the DeviceModel instance. * @param partitionPath the device path of the candidate partition to resize. + * @param o applied to debug-logging. * @return true if the criteria are met, otherwise false. */ -bool canBeResized( DeviceModel* dm, const QString& partitionPath ); +bool canBeResized( DeviceModel* dm, const QString& partitionPath, const Logger::Once& o ); /** * @brief runOsprober executes os-prober, parses the output and writes relevant diff --git a/src/modules/partition/core/PartitionCoreModule.cpp b/src/modules/partition/core/PartitionCoreModule.cpp index 27aceec60..912d46546 100644 --- a/src/modules/partition/core/PartitionCoreModule.cpp +++ b/src/modules/partition/core/PartitionCoreModule.cpp @@ -255,13 +255,22 @@ PartitionCoreModule::doInit() DeviceList devices = PartUtils::getDevices( PartUtils::DeviceType::WritableOnly ); cDebug() << "LIST OF DETECTED DEVICES:"; - cDebug() << "node\tcapacity\tname\tprettyName"; + cDebug() << Logger::SubEntry << "node\tcapacity\tname\tprettyName"; for ( auto device : devices ) { - // Gives ownership of the Device* to the DeviceInfo object - auto deviceInfo = new DeviceInfo( device ); - m_deviceInfos << deviceInfo; - cDebug() << device->deviceNode() << device->capacity() << device->name() << device->prettyName(); + cDebug() << Logger::SubEntry << Logger::Pointer(device); + if ( device ) + { + // Gives ownership of the Device* to the DeviceInfo object + auto deviceInfo = new DeviceInfo( device ); + m_deviceInfos << deviceInfo; + cDebug() << Logger::SubEntry << device->deviceNode() << device->capacity() << device->name() + << device->prettyName(); + } + else + { + cDebug() << Logger::SubEntry << "(skipped null device)"; + } } cDebug() << Logger::SubEntry << devices.count() << "devices detected."; m_deviceModel->init( devices ); @@ -659,10 +668,10 @@ PartitionCoreModule::dumpQueue() const cDebug() << "# Queue:"; for ( auto info : m_deviceInfos ) { - cDebug() << "## Device:" << info->device->name(); + cDebug() << Logger::SubEntry << "## Device:" << info->device->name(); for ( const auto& job : info->jobs() ) { - cDebug() << "-" << job->prettyName(); + cDebug() << Logger::SubEntry << "-" << job->prettyName(); } } } diff --git a/src/modules/partition/gui/ChoicePage.cpp b/src/modules/partition/gui/ChoicePage.cpp index f6b49cb2f..245ee0b92 100644 --- a/src/modules/partition/gui/ChoicePage.cpp +++ b/src/modules/partition/gui/ChoicePage.cpp @@ -134,6 +134,28 @@ ChoicePage::ChoicePage( Config* config, QWidget* parent ) ChoicePage::~ChoicePage() {} +/** @brief Sets the @p model for the given @p box and adjusts UI sizes to match. + * + * The model provides data for drawing the items in the model; the + * drawing itself is done by the delegate, which may end up drawing a + * different width in the popup than in the collapsed combo box. + * + * Make the box wide enough to accomodate the whole expanded delegate; + * this avoids cases where the popup would truncate data being drawn + * because the overall box is sized too narrow. + */ +void setModelToComboBox( QComboBox* box, QAbstractItemModel* model ) +{ + box->setModel( model ); + if ( model->rowCount() > 0 ) + { + QStyleOptionViewItem options; + options.initFrom( box ); + auto delegateSize = box->itemDelegate()->sizeHint(options, model->index(0, 0) ); + box->setMinimumWidth( delegateSize.width() ); + } +} + void ChoicePage::init( PartitionCoreModule* core ) { @@ -145,10 +167,10 @@ ChoicePage::init( PartitionCoreModule* core ) // We need to do this because a PCM revert invalidates the deviceModel. connect( core, &PartitionCoreModule::reverted, this, [=] { - m_drivesCombo->setModel( core->deviceModel() ); + setModelToComboBox( m_drivesCombo, core->deviceModel() ); m_drivesCombo->setCurrentIndex( m_lastSelectedDeviceIndex ); } ); - m_drivesCombo->setModel( core->deviceModel() ); + setModelToComboBox( m_drivesCombo, core->deviceModel() ); connect( m_drivesCombo, static_cast< void ( QComboBox::* )( int ) >( &QComboBox::currentIndexChanged ), @@ -482,8 +504,6 @@ ChoicePage::applyActionChoice( InstallChoice choice ) [] {}, this ); } - updateNextEnabled(); - connect( m_beforePartitionBarsView->selectionModel(), SIGNAL( currentRowChanged( QModelIndex, QModelIndex ) ), this, @@ -507,7 +527,6 @@ ChoicePage::applyActionChoice( InstallChoice choice ) }, this ); } - updateNextEnabled(); connect( m_beforePartitionBarsView->selectionModel(), SIGNAL( currentRowChanged( QModelIndex, QModelIndex ) ), @@ -519,6 +538,7 @@ ChoicePage::applyActionChoice( InstallChoice choice ) case InstallChoice::Manual: break; } + updateNextEnabled(); updateActionChoicePreview( choice ); } @@ -985,7 +1005,7 @@ ChoicePage::updateActionChoicePreview( InstallChoice choice ) SelectionFilter filter = []( const QModelIndex& index ) { return PartUtils::canBeResized( - static_cast< Partition* >( index.data( PartitionModel::PartitionPtrRole ).value< void* >() ) ); + static_cast< Partition* >( index.data( PartitionModel::PartitionPtrRole ).value< void* >() ), Logger::Once() ); }; m_beforePartitionBarsView->setSelectionFilter( filter ); m_beforePartitionLabelsView->setSelectionFilter( filter ); @@ -1074,7 +1094,7 @@ ChoicePage::updateActionChoicePreview( InstallChoice choice ) { SelectionFilter filter = []( const QModelIndex& index ) { return PartUtils::canBeReplaced( - static_cast< Partition* >( index.data( PartitionModel::PartitionPtrRole ).value< void* >() ) ); + static_cast< Partition* >( index.data( PartitionModel::PartitionPtrRole ).value< void* >() ), Logger::Once() ); }; m_beforePartitionBarsView->setSelectionFilter( filter ); m_beforePartitionLabelsView->setSelectionFilter( filter ); @@ -1219,10 +1239,12 @@ operator<<( QDebug& s, PartitionIterator& it ) void ChoicePage::setupActions() { + Logger::Once o; + Device* currentDevice = selectedDevice(); OsproberEntryList osproberEntriesForCurrentDevice = getOsproberEntriesForDevice( currentDevice ); - cDebug() << "Setting up actions for" << currentDevice->deviceNode() << "with" + cDebug() << o << "Setting up actions for" << currentDevice->deviceNode() << "with" << osproberEntriesForCurrentDevice.count() << "entries."; if ( currentDevice->partitionTable() ) @@ -1268,12 +1290,12 @@ ChoicePage::setupActions() for ( auto it = PartitionIterator::begin( currentDevice ); it != PartitionIterator::end( currentDevice ); ++it ) { - if ( PartUtils::canBeResized( *it ) ) + if ( PartUtils::canBeResized( *it, o ) ) { cDebug() << Logger::SubEntry << "contains resizable" << it; atLeastOneCanBeResized = true; } - if ( PartUtils::canBeReplaced( *it ) ) + if ( PartUtils::canBeReplaced( *it, o ) ) { cDebug() << Logger::SubEntry << "contains replaceable" << it; atLeastOneCanBeReplaced = true; @@ -1402,7 +1424,7 @@ ChoicePage::setupActions() } else { - cDebug() << "Replace button suppressed because none can be replaced."; + cDebug() << "No partitions available for replace-action."; force_uncheck( m_grp, m_replaceButton ); } @@ -1412,7 +1434,7 @@ ChoicePage::setupActions() } else { - cDebug() << "Alongside button suppressed because none can be resized."; + cDebug() << "No partitions available for resize-action."; force_uncheck( m_grp, m_alongsideButton ); } @@ -1422,8 +1444,8 @@ ChoicePage::setupActions() } else { - cDebug() << "Erase button suppressed" - << "mount?" << atLeastOneIsMounted << "raid?" << isInactiveRAID; + cDebug() << "No partitions (" + << "any-mounted?" << atLeastOneIsMounted << "is-raid?" << isInactiveRAID << ") for erase-action."; force_uncheck( m_grp, m_eraseButton ); } diff --git a/src/modules/partition/gui/PartitionPage.h b/src/modules/partition/gui/PartitionPage.h index 81c2cd983..462822346 100644 --- a/src/modules/partition/gui/PartitionPage.h +++ b/src/modules/partition/gui/PartitionPage.h @@ -20,7 +20,6 @@ class PartitionCoreModule; class Ui_PartitionPage; class Device; -class DeviceModel; class Partition; /** diff --git a/src/modules/partition/gui/PartitionViewStep.cpp b/src/modules/partition/gui/PartitionViewStep.cpp index 738c5d25d..b4eefb3b0 100644 --- a/src/modules/partition/gui/PartitionViewStep.cpp +++ b/src/modules/partition/gui/PartitionViewStep.cpp @@ -16,45 +16,27 @@ #include "core/BootLoaderModel.h" #include "core/Config.h" #include "core/DeviceModel.h" -#include "core/KPMHelpers.h" -#include "core/OsproberEntry.h" -#include "core/PartUtils.h" -#include "core/PartitionActions.h" #include "core/PartitionCoreModule.h" -#include "core/PartitionModel.h" #include "gui/ChoicePage.h" #include "gui/PartitionBarsView.h" #include "gui/PartitionLabelsView.h" #include "gui/PartitionPage.h" #include "Branding.h" -#include "CalamaresVersion.h" #include "GlobalStorage.h" -#include "Job.h" #include "JobQueue.h" #include "utils/CalamaresUtilsGui.h" #include "utils/Logger.h" -#include "utils/NamedEnum.h" #include "utils/QtCompat.h" #include "utils/Retranslator.h" -#include "utils/Units.h" #include "utils/Variant.h" #include "widgets/WaitingWidget.h" - -#include #include -#include -#include -#include #include -#include -#include #include -#include #include -#include #include PartitionViewStep::PartitionViewStep( QObject* parent ) @@ -560,6 +542,8 @@ PartitionViewStep::onLeave() void PartitionViewStep::setConfigurationMap( const QVariantMap& configurationMap ) { + Logger::Once o; + m_config->setConfigurationMap( configurationMap ); // Copy the efiSystemPartition setting to the global storage. It is needed not only in @@ -570,7 +554,7 @@ PartitionViewStep::setConfigurationMap( const QVariantMap& configurationMap ) // Set up firmwareType global storage entry. This is used, e.g. by the bootloader module. QString firmwareType( PartUtils::isEfiSystem() ? QStringLiteral( "efi" ) : QStringLiteral( "bios" ) ); - cDebug() << "Setting firmwareType to" << firmwareType; + cDebug() << o << "Setting firmwareType to" << firmwareType; gs->insert( "firmwareType", firmwareType ); // Read and parse key efiSystemPartitionSize @@ -610,7 +594,7 @@ PartitionViewStep::setConfigurationMap( const QVariantMap& configurationMap ) QString fsRealName = PartUtils::findFS( fsName, &fsType ); if ( fsRealName == fsName ) { - cDebug() << "Partition-module setting *defaultFileSystemType*" << fsRealName; + cDebug() << o << "Partition-module setting *defaultFileSystemType*" << fsRealName; } else if ( fsType != FileSystem::Unknown ) {