Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow multiple apps in a single command #2861

Merged
merged 30 commits into from
Feb 6, 2023
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
a82ea46
Define multi-positional arg type and tests
florelis Jan 7, 2023
c7005fb
Normalize line endings
florelis Jan 7, 2023
a7233a1
Use existing positional count instead...
florelis Jan 7, 2023
d85202a
Support installing multiple packages in single command
florelis Jan 14, 2023
2cdc88c
Rename file
florelis Jan 14, 2023
f455bd5
Allow uninstalling multiple with one command
florelis Jan 17, 2023
db34ed0
Add unit tests
florelis Jan 17, 2023
77b1303
spelling
florelis Jan 17, 2023
ed96bb1
Undo unneeded change
florelis Jan 17, 2023
5133012
Remove extra line
florelis Jan 17, 2023
1069e6f
Fix arg parsing
florelis Jan 17, 2023
5dd4949
Fix failing tests
florelis Jan 17, 2023
e26b6eb
Update strings in tests
florelis Jan 19, 2023
73d7f00
if order
florelis Jan 27, 2023
e27c6e3
RemoveArg()
florelis Jan 27, 2023
22355d1
Use shared code to create search request for single
florelis Jan 27, 2023
6c0c508
not found / found multiple
florelis Jan 27, 2023
bc882da
Remove multi query from list
florelis Jan 27, 2023
98c5f05
Make error more specific
florelis Jan 27, 2023
0ea646d
Fix import tests
florelis Jan 27, 2023
1d1f25e
Fix search request
florelis Jan 27, 2023
2ba14e9
Fix expected error codes in tests
florelis Jan 27, 2023
2ba05d3
Update expected errors in E2E tests
florelis Jan 28, 2023
f73a2f8
Build error...
florelis Jan 30, 2023
2a53cec
Merge branch 'master' into multiinstall
florelis Jan 31, 2023
64411f4
Move logic for converting into single value, add test, add comment
florelis Feb 4, 2023
8951acb
Add test for too many positionals
florelis Feb 4, 2023
8ea449f
Update error strings to be more specific
florelis Feb 4, 2023
8674434
Fix tests
florelis Feb 6, 2023
5d8907c
Merge branch 'master' into multiinstall
florelis Feb 6, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/AppInstallerCLICore/AppInstallerCLICore.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,7 @@
<ClInclude Include="Workflows\ImportExportFlow.h" />
<ClInclude Include="Workflows\MsiInstallFlow.h" />
<ClInclude Include="Workflows\MSStoreInstallerHandler.h" />
<ClInclude Include="Workflows\MultiQueryFlow.h" />
<ClInclude Include="Workflows\PinFlow.h" />
<ClInclude Include="Workflows\PortableFlow.h" />
<ClInclude Include="Workflows\PromptFlow.h" />
Expand Down Expand Up @@ -341,6 +342,7 @@
<ClCompile Include="Workflows\ImportExportFlow.cpp" />
<ClCompile Include="Workflows\MsiInstallFlow.cpp" />
<ClCompile Include="Workflows\MSStoreInstallerHandler.cpp" />
<ClCompile Include="Workflows\MultiQueryFlow.cpp" />
<ClCompile Include="Workflows\PinFlow.cpp" />
<ClCompile Include="Workflows\PortableFlow.cpp" />
<ClCompile Include="Workflows\PromptFlow.cpp" />
Expand Down
6 changes: 6 additions & 0 deletions src/AppInstallerCLICore/AppInstallerCLICore.vcxproj.filters
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,9 @@
<ClInclude Include="Workflows\PinFlow.h">
<Filter>Workflows</Filter>
</ClInclude>
<ClInclude Include="Workflows\MultiQueryFlow.h">
<Filter>Workflows</Filter>
</ClInclude>
</ItemGroup>
<ItemGroup>
<ClCompile Include="pch.cpp">
Expand Down Expand Up @@ -355,6 +358,9 @@
<ClCompile Include="Commands\PinCommand.cpp">
<Filter>Commands</Filter>
</ClCompile>
<ClCompile Include="Workflows\MultiQueryFlow.cpp">
<Filter>Workflows</Filter>
</ClCompile>
</ItemGroup>
<ItemGroup>
<None Include="PropertySheet.props" />
Expand Down
4 changes: 3 additions & 1 deletion src/AppInstallerCLICore/Argument.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ namespace AppInstaller::CLI
{
case Args::Type::Query:
return Argument{ "query"_liv, 'q', Args::Type::Query, Resource::String::QueryArgumentDescription, ArgumentType::Positional};
case Args::Type::MultiQuery:
return Argument{ "query"_liv, 'q', Args::Type::MultiQuery, Resource::String::MultiQueryArgumentDescription, ArgumentType::Positional }.SetCountLimit(128);
case Args::Type::Manifest:
return Argument{ "manifest"_liv, 'm', Args::Type::Manifest, Resource::String::ManifestArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help, Settings::TogglePolicy::Policy::LocalManifestFiles, Settings::AdminSetting::LocalManifestFiles };
case Args::Type::Id:
Expand Down Expand Up @@ -147,7 +149,7 @@ namespace AppInstaller::CLI

void Argument::ValidatePackageSelectionArgumentSupplied(const Execution::Args& args)
{
for (Args::Type type : { Args::Type::Query, Args::Type::Manifest, Args::Type::Id, Args::Type::Name, Args::Type::Moniker, Args::Type::ProductCode, Args::Type::Tag, Args::Type::Command })
for (Args::Type type : { Args::Type::Query, Args::Type::MultiQuery, Args::Type::Manifest, Args::Type::Id, Args::Type::Name, Args::Type::Moniker, Args::Type::ProductCode, Args::Type::Tag, Args::Type::Command })
{
if (args.Contains(type))
{
Expand Down
1 change: 1 addition & 0 deletions src/AppInstallerCLICore/Argument.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ namespace AppInstaller::CLI
Settings::AdminSetting AdminSetting() const { return m_adminSetting; }

Argument& SetRequired(bool required) { m_required = required; return *this; }
Argument& SetCountLimit(size_t countLimit) { m_countLimit = countLimit; return *this; }

private:
// Constructors that set a Feature or Policy are private to force callers to go through the ForType() function.
Expand Down
5 changes: 5 additions & 0 deletions src/AppInstallerCLICore/Command.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,11 @@ namespace AppInstaller::CLI

infoOut << "] <"_liv << arg.Name() << '>';

if (arg.Limit() > 1)
{
infoOut << "..."_liv;
}

if (!arg.Required())
{
infoOut << ']';
Expand Down
4 changes: 3 additions & 1 deletion src/AppInstallerCLICore/Commands/ImportCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "ImportCommand.h"
#include "Workflows/CompletionFlow.h"
#include "Workflows/ImportExportFlow.h"
#include "Workflows/MultiQueryFlow.h"
#include "Workflows/WorkflowBase.h"
#include "Resources.h"

Expand Down Expand Up @@ -46,7 +47,8 @@ namespace AppInstaller::CLI
Workflow::ReadImportFile <<
Workflow::OpenSourcesForImport <<
Workflow::OpenPredefinedSource(Repository::PredefinedSource::Installed) <<
Workflow::SearchPackagesForImport <<
Workflow::GetSearchRequestsForImport <<
Workflow::SearchSubContextsForSingle() <<
Workflow::ReportExecutionStage(Workflow::ExecutionStage::Execution) <<
Workflow::InstallImportedPackages;
}
Expand Down
24 changes: 19 additions & 5 deletions src/AppInstallerCLICore/Commands/InstallCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "Workflows/CompletionFlow.h"
#include "Workflows/InstallFlow.h"
#include "Workflows/UpdateFlow.h"
#include "Workflows/MultiQueryFlow.h"
#include "Workflows/WorkflowBase.h"
#include "Resources.h"

Expand All @@ -18,7 +19,7 @@ namespace AppInstaller::CLI
std::vector<Argument> InstallCommand::GetArguments() const
{
return {
Argument::ForType(Args::Type::Query),
Argument::ForType(Args::Type::MultiQuery),
Argument::ForType(Args::Type::Manifest),
Argument::ForType(Args::Type::Id),
Argument::ForType(Args::Type::Name),
Expand Down Expand Up @@ -63,7 +64,7 @@ namespace AppInstaller::CLI
{
switch (valueType)
{
case Args::Type::Query:
case Args::Type::MultiQuery:
case Args::Type::Manifest:
case Args::Type::Id:
case Args::Type::Name:
Expand Down Expand Up @@ -98,7 +99,7 @@ namespace AppInstaller::CLI
Argument::ValidatePackageSelectionArgumentSupplied(execArgs);

if (execArgs.Contains(Args::Type::Manifest) &&
(execArgs.Contains(Args::Type::Query) ||
(execArgs.Contains(Args::Type::MultiQuery) ||
execArgs.Contains(Args::Type::Id) ||
execArgs.Contains(Args::Type::Name) ||
execArgs.Contains(Args::Type::Moniker) ||
Expand All @@ -109,7 +110,6 @@ namespace AppInstaller::CLI
{
throw CommandException(Resource::String::BothManifestAndSearchQueryProvided);
}

}

void InstallCommand::ExecuteInternal(Context& context) const
Expand All @@ -128,6 +128,7 @@ namespace AppInstaller::CLI
else
florelis marked this conversation as resolved.
Show resolved Hide resolved
{
context <<
Workflow::CheckForMultiQuery <<
Workflow::ReportExecutionStage(ExecutionStage::Discovery) <<
Workflow::OpenSource();

Expand All @@ -137,7 +138,20 @@ namespace AppInstaller::CLI
Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed, false, Repository::CompositeSearchBehavior::AvailablePackages);
}

context << Workflow::InstallOrUpgradeSinglePackage(false);
if (!context.Args.Contains(Execution::Args::Type::MultiQuery))
florelis marked this conversation as resolved.
Show resolved Hide resolved
{
context << Workflow::InstallOrUpgradeSinglePackage(false);
}
else
{
context <<
Workflow::GetMultiSearchRequests <<
Workflow::SearchSubContextsForSingle() <<
Workflow::ReportExecutionStage(Workflow::ExecutionStage::Execution) <<
Workflow::InstallMultiplePackages(
Resource::String::InstallAndUpgradeCommandsReportDependencies,
APPINSTALLER_CLI_ERROR_MULTIPLE_INSTALL_FAILED);
}
}
}
}
4 changes: 2 additions & 2 deletions src/AppInstallerCLICore/Commands/ListCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace AppInstaller::CLI
std::vector<Argument> ListCommand::GetArguments() const
{
return {
Argument::ForType(Execution::Args::Type::Query),
Argument::ForType(Execution::Args::Type::MultiQuery),
Argument::ForType(Execution::Args::Type::Id),
Argument::ForType(Execution::Args::Type::Name),
Argument::ForType(Execution::Args::Type::Moniker),
Expand Down Expand Up @@ -46,7 +46,7 @@ namespace AppInstaller::CLI

switch (valueType)
{
case Execution::Args::Type::Query:
case Execution::Args::Type::MultiQuery:
context <<
Workflow::RequireCompletionWordNonEmpty <<
Workflow::SearchSourceForManyCompletion <<
Expand Down
37 changes: 24 additions & 13 deletions src/AppInstallerCLICore/Commands/UninstallCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "Workflows/UninstallFlow.h"
#include "Workflows/CompletionFlow.h"
#include "Workflows/WorkflowBase.h"
#include "Workflows/MultiQueryFlow.h"
#include "Resources.h"

using AppInstaller::CLI::Execution::Args;
Expand All @@ -16,7 +17,7 @@ namespace AppInstaller::CLI
{
return
{
Argument::ForType(Args::Type::Query),
Argument::ForType(Args::Type::MultiQuery),
Argument::ForType(Args::Type::Manifest),
Argument::ForType(Args::Type::Id),
Argument::ForType(Args::Type::Name),
Expand Down Expand Up @@ -63,7 +64,7 @@ namespace AppInstaller::CLI

switch (valueType)
{
case Execution::Args::Type::Query:
case Execution::Args::Type::MultiQuery:
context <<
Workflow::RequireCompletionWordNonEmpty <<
Workflow::SearchSourceForManyCompletion <<
Expand Down Expand Up @@ -92,7 +93,7 @@ namespace AppInstaller::CLI
Argument::ValidatePackageSelectionArgumentSupplied(execArgs);

if (execArgs.Contains(Execution::Args::Type::Manifest) &&
(execArgs.Contains(Execution::Args::Type::Query) ||
(execArgs.Contains(Execution::Args::Type::MultiQuery) ||
execArgs.Contains(Execution::Args::Type::Id) ||
execArgs.Contains(Execution::Args::Type::Name) ||
execArgs.Contains(Execution::Args::Type::Moniker) ||
Expand Down Expand Up @@ -129,19 +130,29 @@ namespace AppInstaller::CLI
Workflow::GetManifestFromArg <<
Workflow::ReportManifestIdentity <<
Workflow::SearchSourceUsingManifest <<
Workflow::EnsureOneMatchFromSearchResult(true);
Workflow::EnsureOneMatchFromSearchResult(true) <<
Workflow::UninstallSinglePackage;
}
else
{
// search for a single package to uninstall
context <<
Workflow::SearchSourceForSingle <<
Workflow::HandleSearchResultFailures <<
Workflow::EnsureOneMatchFromSearchResult(true) <<
Workflow::ReportPackageIdentity;
// search for specific packages to uninstall
context << Workflow::CheckForMultiQuery;
if (!context.Args.Contains(Execution::Args::Type::MultiQuery))
{
context <<
Workflow::SearchSourceForSingle <<
Workflow::HandleSearchResultFailures <<
Workflow::EnsureOneMatchFromSearchResult(true) <<
Workflow::ReportPackageIdentity <<
Workflow::UninstallSinglePackage;
}
else
{
context <<
Workflow::GetMultiSearchRequests <<
Workflow::SearchSubContextsForSingle(Workflow::SearchSubContextsForSingle::SearchPurpose::Uninstall) <<
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After reading through this again, it seems like the implementation in #2877 of SearchResultType and the SearchPurpose here are similar; Would it make sense to align them both to SearchPurpose and use it more globally rather than inside the namespace of SearchSubContextsForSingle?

If so, I'll update that PR

Workflow::UninstallMultiplePackages;
}
}

context <<
Workflow::UninstallSinglePackage;
}
}
26 changes: 21 additions & 5 deletions src/AppInstallerCLICore/Commands/UpgradeCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "UpgradeCommand.h"
#include "Workflows/CompletionFlow.h"
#include "Workflows/InstallFlow.h"
#include "Workflows/MultiQueryFlow.h"
#include "Workflows/UpdateFlow.h"
#include "Workflows/WorkflowBase.h"
#include "Workflows/DependenciesFlow.h"
Expand All @@ -24,7 +25,7 @@ namespace AppInstaller::CLI
bool HasSearchQueryArguments(Execution::Args& execArgs)
{
// Note that this does not include Manifest (no search) or source related args (used for listing)
return execArgs.Contains(Args::Type::Query) ||
return execArgs.Contains(Args::Type::MultiQuery) ||
execArgs.Contains(Args::Type::Id) ||
execArgs.Contains(Args::Type::Name) ||
execArgs.Contains(Args::Type::Moniker) ||
Expand Down Expand Up @@ -85,7 +86,7 @@ namespace AppInstaller::CLI
std::vector<Argument> UpgradeCommand::GetArguments() const
{
return {
Argument::ForType(Args::Type::Query), // -q
Argument::ForType(Args::Type::MultiQuery), // -q
Argument::ForType(Args::Type::Manifest), // -m
Argument::ForType(Args::Type::Id),
Argument::ForType(Args::Type::Name),
Expand Down Expand Up @@ -143,7 +144,7 @@ namespace AppInstaller::CLI

switch (valueType)
{
case Execution::Args::Type::Query:
case Execution::Args::Type::MultiQuery:
context <<
RequireCompletionWordNonEmpty <<
SearchSourceForManyCompletion <<
Expand Down Expand Up @@ -251,8 +252,23 @@ namespace AppInstaller::CLI
}
else
{
// The remaining case: search for single installed package to update
context << InstallOrUpgradeSinglePackage(true);
// The remaining case: search for specific packages to update
context << Workflow::CheckForMultiQuery;

if (!context.Args.Contains(Execution::Args::Type::MultiQuery))
{
context << Workflow::InstallOrUpgradeSinglePackage(/* isUpgrade */ true);
}
else
{
context <<
Workflow::GetMultiSearchRequests <<
Workflow::SearchSubContextsForSingle(Workflow::SearchSubContextsForSingle::SearchPurpose::Upgrade) <<
Workflow::ReportExecutionStage(Workflow::ExecutionStage::Execution) <<
Workflow::InstallMultiplePackages(
Resource::String::InstallAndUpgradeCommandsReportDependencies,
APPINSTALLER_CLI_ERROR_MULTIPLE_INSTALL_FAILED);
}
}
}
}
6 changes: 6 additions & 0 deletions src/AppInstallerCLICore/ExecutionArgs.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ namespace AppInstaller::CLI::Execution
{
// Args to specify where to get app
Query, // Query to be performed against index
MultiQuery, // Like query, but can take multiple values
Manifest, // Provide the app manifest directly

// Query filtering criteria and query behavior
Expand Down Expand Up @@ -156,6 +157,11 @@ namespace AppInstaller::CLI::Execution
m_parsedArgs[arg].emplace_back(value);
}

bool RemoveArg(Type arg)
florelis marked this conversation as resolved.
Show resolved Hide resolved
{
return m_parsedArgs.erase(arg) > 0;
}

bool Empty()
{
return m_parsedArgs.empty();
Expand Down
14 changes: 11 additions & 3 deletions src/AppInstallerCLICore/ExecutionContextData.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ namespace AppInstaller::CLI::Execution
enum class Data : size_t
{
Source,
SearchRequest, // Only set for multiple installs
SearchResult,
SourceList,
Package,
Expand All @@ -49,8 +50,9 @@ namespace AppInstaller::CLI::Execution
// On export: A collection of packages to be exported to a file
// On import: A collection of packages read from a file
PackageCollection,
// On import and upgrade all: A collection of specific package versions to install
PackagesToInstall,
// When installing multiple packages at once (upgrade all, import, install with multiple args, dependencies):
// A collection of sub-contexts, each of which handles the installation of a single package.
PackageSubContexts,
// On import: Sources for the imported packages
Sources,
ARPCorrelationData,
Expand Down Expand Up @@ -81,6 +83,12 @@ namespace AppInstaller::CLI::Execution
using value_t = Repository::Source;
};

template <>
struct DataMapping<Data::SearchRequest>
{
using value_t = Repository::SearchRequest;
};

template <>
struct DataMapping<Data::SearchResult>
{
Expand Down Expand Up @@ -184,7 +192,7 @@ namespace AppInstaller::CLI::Execution
};

template <>
struct DataMapping<Data::PackagesToInstall>
struct DataMapping<Data::PackageSubContexts>
{
using value_t = std::vector<std::unique_ptr<Context>>;
};
Expand Down
Loading