diff --git a/src/libcalamares/packages/Globals.cpp b/src/libcalamares/packages/Globals.cpp index c5e882436..aedbc2119 100644 --- a/src/libcalamares/packages/Globals.cpp +++ b/src/libcalamares/packages/Globals.cpp @@ -12,11 +12,11 @@ #include "GlobalStorage.h" #include "utils/Logger.h" -bool -CalamaresUtils::Packages::setGSPackageAdditions( Calamares::GlobalStorage* gs, - const Calamares::ModuleSystem::InstanceKey& module, - const QVariantList& installPackages, - const QVariantList& tryInstallPackages ) +static bool +additions( Calamares::GlobalStorage* gs, + const QString& key, + const QVariantList& installPackages, + const QVariantList& tryInstallPackages ) { static const char PACKAGEOP[] = "packageOperations"; @@ -25,8 +25,6 @@ CalamaresUtils::Packages::setGSPackageAdditions( Calamares::GlobalStorage* gs, QVariantList packageOperations = gs->contains( PACKAGEOP ) ? gs->value( PACKAGEOP ).toList() : QVariantList(); cDebug() << "Existing package operations length" << packageOperations.length(); - const QString key = module.toString(); - // Clear out existing operations for this module, going backwards: // Sometimes we remove an item, and we don't want the index to // fall off the end of the list. @@ -66,3 +64,25 @@ CalamaresUtils::Packages::setGSPackageAdditions( Calamares::GlobalStorage* gs, } return false; } + +bool +CalamaresUtils::Packages::setGSPackageAdditions( Calamares::GlobalStorage* gs, + const Calamares::ModuleSystem::InstanceKey& module, + const QVariantList& installPackages, + const QVariantList& tryInstallPackages ) +{ + return additions( gs, module.toString(), installPackages, tryInstallPackages ); +} + +bool +CalamaresUtils::Packages::setGSPackageAdditions( Calamares::GlobalStorage* gs, + const Calamares::ModuleSystem::InstanceKey& module, + const QStringList& installPackages ) +{ + QVariantList l; + for ( const auto& s : installPackages ) + { + l << s; + } + return additions( gs, module.toString(), l, QVariantList() ); +} diff --git a/src/libcalamares/packages/Globals.h b/src/libcalamares/packages/Globals.h index a47cf5ae1..a83152ff2 100644 --- a/src/libcalamares/packages/Globals.h +++ b/src/libcalamares/packages/Globals.h @@ -28,6 +28,14 @@ bool setGSPackageAdditions( Calamares::GlobalStorage* gs, const Calamares::ModuleSystem::InstanceKey& module, const QVariantList& installPackages, const QVariantList& tryInstallPackages ); +/** @brief Sets the install-packages GS keys for the given module + * + * This replaces previously-set install-packages lists. Use this with + * plain lists of package names. It does not support try-install. + */ +bool setGSPackageAdditions( Calamares::GlobalStorage* gs, + const Calamares::ModuleSystem::InstanceKey& module, + const QStringList& installPackages ); // void setGSPackageRemovals( const Calamares::ModuleSystem::InstanceKey& key, const QVariantList& removePackages ); } // namespace Packages } // namespace CalamaresUtils diff --git a/src/libcalamares/packages/Tests.cpp b/src/libcalamares/packages/Tests.cpp index 0a9be3a20..09159abdf 100644 --- a/src/libcalamares/packages/Tests.cpp +++ b/src/libcalamares/packages/Tests.cpp @@ -24,7 +24,15 @@ private Q_SLOTS: void initTestCase(); void testEmpty(); + void testAdd_data(); + /** @brief Test various add calls, for a "clean" GS + * + * Check that adding through the variant- and the stringlist-API + * does the same thing. + */ void testAdd(); + /// Test replacement and mixing string-list with variant calls + void testAddMixed(); }; void @@ -46,38 +54,179 @@ PackagesTests::testEmpty() // Adding nothing at all does nothing QVERIFY( !CalamaresUtils::Packages::setGSPackageAdditions( &gs, k, QVariantList(), QVariantList() ) ); QVERIFY( !gs.contains( topKey ) ); + + QVERIFY( !CalamaresUtils::Packages::setGSPackageAdditions( &gs, k, QStringList() ) ); + QVERIFY( !gs.contains( topKey ) ); +} + +void +PackagesTests::testAdd_data() +{ + QTest::addColumn< QStringList >( "packages" ); + + QTest::newRow( "one" ) << QStringList { QString( "vim" ) }; + QTest::newRow( "two" ) << QStringList { QString( "vim" ), QString( "emacs" ) }; + QTest::newRow( "one-again" ) << QStringList { QString( "nano" ) }; + QTest::newRow( "six" ) << QStringList { QString( "vim" ), QString( "emacs" ), QString( "nano" ), + QString( "kate" ), QString( "gedit" ), QString( "sublime" ) }; + // There is no "de-duplication" so this will insert "cim" twice + QTest::newRow( "dups" ) << QStringList { QString( "cim" ), QString( "vim" ), QString( "cim" ) }; } void PackagesTests::testAdd() { Calamares::GlobalStorage gs; + + const QString extraEditor( "notepad++" ); const QString topKey( "packageOperations" ); Calamares::ModuleSystem::InstanceKey k( "this", "that" ); + Calamares::ModuleSystem::InstanceKey otherInstance( "this", "other" ); + + QFETCH( QStringList, packages ); + QVERIFY( !packages.contains( extraEditor ) ); + + { + QVERIFY( !gs.contains( topKey ) ); + QVERIFY( + CalamaresUtils::Packages::setGSPackageAdditions( &gs, k, QVariant( packages ).toList(), QVariantList() ) ); + QVERIFY( gs.contains( topKey ) ); + auto actionList = gs.value( topKey ).toList(); + QCOMPARE( actionList.length(), 1 ); + auto action = actionList[ 0 ].toMap(); + QVERIFY( action.contains( "install" ) ); + auto op = action[ "install" ].toList(); + QCOMPARE( op.length(), packages.length() ); + for ( const auto& s : qAsConst( packages ) ) + { + QVERIFY( op.contains( s ) ); + } + cDebug() << op; + } + { + QVERIFY( CalamaresUtils::Packages::setGSPackageAdditions( &gs, otherInstance, packages ) ); + QVERIFY( gs.contains( topKey ) ); + auto actionList = gs.value( topKey ).toList(); + QCOMPARE( actionList.length(), 2 ); // One for each instance key! + auto action = actionList[ 0 ].toMap(); + auto secondaction = actionList[ 1 ].toMap(); + auto op = action[ "install" ].toList(); + auto secondop = secondaction[ "install" ].toList(); + QCOMPARE( op, secondop ); + } + + { + // Replace one and expect differences + packages << extraEditor; + QVERIFY( CalamaresUtils::Packages::setGSPackageAdditions( &gs, otherInstance, packages ) ); + QVERIFY( gs.contains( topKey ) ); + auto actionList = gs.value( topKey ).toList(); + QCOMPARE( actionList.length(), 2 ); // One for each instance key! + for ( const auto& actionVariant : qAsConst( actionList ) ) + { + auto action = actionVariant.toMap(); + QVERIFY( action.contains( "install" ) ); + QVERIFY( action.contains( "source" ) ); + if ( action[ "source" ].toString() == otherInstance.toString() ) + { + auto op = action[ "install" ].toList(); + QCOMPARE( op.length(), packages.length() ); // changed from original length, though + for ( const auto& s : qAsConst( packages ) ) + { + QVERIFY( op.contains( s ) ); + } + } + else + { + // This is the "original" instance, so it's missing extraEditor + auto op = action[ "install" ].toList(); + QCOMPARE( op.length(), packages.length()-1 ); // changed from original length + QVERIFY( !op.contains( extraEditor ) ); + } + } + } +} - QVERIFY( !gs.contains( topKey ) ); - QVERIFY( - CalamaresUtils::Packages::setGSPackageAdditions( &gs, k, QVariantList { QString( "vim" ) }, QVariantList() ) ); - QVERIFY( gs.contains( topKey ) ); - auto actionList = gs.value( topKey ).toList(); - QCOMPARE( actionList.length(), 1 ); - auto action = actionList[ 0 ].toMap(); - QVERIFY( action.contains( "install" ) ); - auto op = action[ "install" ].toList(); - QCOMPARE( op.length(), 1 ); - cDebug() << op; - - QVERIFY( CalamaresUtils::Packages::setGSPackageAdditions( - &gs, k, QVariantList { QString( "vim" ), QString( "emacs" ) }, QVariantList() ) ); - QVERIFY( gs.contains( topKey ) ); - actionList = gs.value( topKey ).toList(); - QCOMPARE( actionList.length(), 1 ); - action = actionList[ 0 ].toMap(); - QVERIFY( action.contains( "install" ) ); - op = action[ "install" ].toList(); - QCOMPARE( op.length(), 2 ); - QCOMPARE( action[ "source" ].toString(), k.toString() ); - cDebug() << op; +void +PackagesTests::testAddMixed() +{ + Calamares::GlobalStorage gs; + + const QString extraEditor( "notepad++" ); + const QString topKey( "packageOperations" ); + Calamares::ModuleSystem::InstanceKey k( "this", "that" ); + Calamares::ModuleSystem::InstanceKey otherInstance( "this", "other" ); + + // Just one + { + QVERIFY( !gs.contains( topKey ) ); + QVERIFY( CalamaresUtils::Packages::setGSPackageAdditions( + &gs, k, QVariantList { QString( "vim" ) }, QVariantList() ) ); + QVERIFY( gs.contains( topKey ) ); + auto actionList = gs.value( topKey ).toList(); + QCOMPARE( actionList.length(), 1 ); + auto action = actionList[ 0 ].toMap(); + QVERIFY( action.contains( "install" ) ); + auto op = action[ "install" ].toList(); + QCOMPARE( op.length(), 1 ); + QCOMPARE( op[ 0 ], QString( "vim" ) ); + cDebug() << op; + } + + // Replace with two packages + { + QVERIFY( CalamaresUtils::Packages::setGSPackageAdditions( + &gs, k, QVariantList { QString( "vim" ), QString( "emacs" ) }, QVariantList() ) ); + QVERIFY( gs.contains( topKey ) ); + auto actionList = gs.value( topKey ).toList(); + QCOMPARE( actionList.length(), 1 ); + auto action = actionList[ 0 ].toMap(); + QVERIFY( action.contains( "install" ) ); + auto op = action[ "install" ].toList(); + QCOMPARE( op.length(), 2 ); + QCOMPARE( action[ "source" ].toString(), k.toString() ); + QVERIFY( op.contains( QString( "vim" ) ) ); + QVERIFY( op.contains( QString( "emacs" ) ) ); + cDebug() << op; + } + + // Replace with one (different) package + { + QVERIFY( CalamaresUtils::Packages::setGSPackageAdditions( + &gs, k, QVariantList { QString( "nano" ) }, QVariantList() ) ); + QVERIFY( gs.contains( topKey ) ); + auto actionList = gs.value( topKey ).toList(); + QCOMPARE( actionList.length(), 1 ); + auto action = actionList[ 0 ].toMap(); + QVERIFY( action.contains( "install" ) ); + auto op = action[ "install" ].toList(); + QCOMPARE( op.length(), 1 ); + QCOMPARE( action[ "source" ].toString(), k.toString() ); + QCOMPARE( op[ 0 ], QString( "nano" ) ); + cDebug() << op; + } + + // Now we have two sources + { + QVERIFY( CalamaresUtils::Packages::setGSPackageAdditions( &gs, otherInstance, QStringList( extraEditor ) ) ); + QVERIFY( gs.contains( topKey ) ); + auto actionList = gs.value( topKey ).toList(); + QCOMPARE( actionList.length(), 2 ); + + for ( const auto& actionVariant : qAsConst( actionList ) ) + { + auto action = actionVariant.toMap(); + QVERIFY( action.contains( "install" ) ); + QVERIFY( action.contains( "source" ) ); + if ( action[ "source" ].toString() == otherInstance.toString() ) + { + auto op = action[ "install" ].toList(); + QCOMPARE( op.length(), 1 ); + QVERIFY( + op.contains( action[ "source" ] == otherInstance.toString() ? extraEditor : QString( "nano" ) ) ); + } + } + } }