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 all 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: 4 additions & 0 deletions src/AppInstallerCLICore/Argument.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ namespace AppInstaller::CLI
// Args to specify where to get app
case Execution::Args::Type::Query:
return { type, "query"_liv, 'q', ArgTypeCategory::PackageQuery | ArgTypeCategory::SinglePackageQuery };
case Execution::Args::Type::MultiQuery:
return { type, "query"_liv, 'q', ArgTypeCategory::PackageQuery };
case Execution::Args::Type::Manifest:
return { type, "manifest"_liv, 'm', ArgTypeCategory::Manifest };

Expand Down Expand Up @@ -215,6 +217,8 @@ namespace AppInstaller::CLI
{
case Args::Type::Query:
return Argument{ type, Resource::String::QueryArgumentDescription, ArgumentType::Positional};
case Args::Type::MultiQuery:
return Argument{ type, Resource::String::MultiQueryArgumentDescription, ArgumentType::Positional }.SetCountLimit(128);
case Args::Type::Manifest:
return Argument{ type, Resource::String::ManifestArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help, Settings::TogglePolicy::Policy::LocalManifestFiles, Settings::AdminSetting::LocalManifestFiles };
case Args::Type::Id:
Expand Down
1 change: 1 addition & 0 deletions src/AppInstallerCLICore/Argument.h
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,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
16 changes: 10 additions & 6 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 Expand Up @@ -405,14 +410,10 @@ namespace AppInstaller::CLI
const CLI::Argument* ParseArgumentsStateMachine::NextPositional()
{
// Find the next appropriate positional arg if the current itr isn't one or has hit its limit.
if (m_positionalSearchItr != m_arguments.end() &&
while (m_positionalSearchItr != m_arguments.end() &&
florelis marked this conversation as resolved.
Show resolved Hide resolved
(m_positionalSearchItr->Type() != ArgumentType::Positional || m_executionArgs.GetCount(m_positionalSearchItr->ExecArgType()) == m_positionalSearchItr->Limit()))
{
do
{
++m_positionalSearchItr;
}
while (m_positionalSearchItr != m_arguments.end() && m_positionalSearchItr->Type() != ArgumentType::Positional);
++m_positionalSearchItr;
}

if (m_positionalSearchItr == m_arguments.end())
Expand Down Expand Up @@ -600,6 +601,9 @@ namespace AppInstaller::CLI
{
stateMachine.ThrowIfError();
}

// Special handling for multi-query arguments:
execArgs.MoveMultiQueryToSingleQueryIfNeeded();
}

void Command::ValidateArguments(Execution::Args& execArgs) const
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
20 changes: 17 additions & 3 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 @@ -123,7 +124,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))
{
context <<
Workflow::GetMultiSearchRequests <<
Workflow::SearchSubContextsForSingle() <<
Workflow::ReportExecutionStage(Workflow::ExecutionStage::Execution) <<
Workflow::InstallMultiplePackages(
Resource::String::InstallAndUpgradeCommandsReportDependencies,
APPINSTALLER_CLI_ERROR_MULTIPLE_INSTALL_FAILED);
}
else
{
context << Workflow::InstallOrUpgradeSinglePackage(false);
}
}
}
}
34 changes: 22 additions & 12 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 @@ -110,19 +111,28 @@ 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
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;
}
}
24 changes: 19 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 Down Expand Up @@ -31,14 +32,14 @@ namespace AppInstaller::CLI
// Valid arguments for list are only those related to the sources and which packages to include (e.g. --include-unknown).
// Instead of checking for them, we check that there aren't any other arguments present.
return !args.Contains(Args::Type::All) &&
WI_AreAllFlagsClear(argCategories, ArgTypeCategory::Manifest | ArgTypeCategory::SinglePackageQuery | ArgTypeCategory::InstallerBehavior);
WI_AreAllFlagsClear(argCategories, ArgTypeCategory::Manifest | ArgTypeCategory::PackageQuery | ArgTypeCategory::InstallerBehavior);
}
}

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 @@ -96,7 +97,7 @@ namespace AppInstaller::CLI

switch (valueType)
{
case Execution::Args::Type::Query:
case Execution::Args::Type::MultiQuery:
context <<
RequireCompletionWordNonEmpty <<
SearchSourceForManyCompletion <<
Expand Down Expand Up @@ -188,8 +189,21 @@ 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
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);
}
}
}
}
19 changes: 18 additions & 1 deletion 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 @@ -160,7 +161,7 @@ namespace AppInstaller::CLI::Execution
m_parsedArgs[arg].emplace_back(value);
}

bool Empty() const
bool Empty()
{
return m_parsedArgs.empty();
}
Expand All @@ -182,6 +183,22 @@ namespace AppInstaller::CLI::Execution
return types;
}

// If we get a single value for multi-query, we remove the argument and add it back as a single query.
// This way the rest of the code can assume that if there is a MultiQuery we will always have multiple values,
// and if there is a single one it will be in the Query type.
// This is the only case where we modify the parsed args from user input.
void MoveMultiQueryToSingleQueryIfNeeded()
{
auto itr = m_parsedArgs.find(Type::MultiQuery);
if (itr != m_parsedArgs.end() && itr->second.size() == 1)
{
// A test ensures that commands don't have both Query and MultiQuery arguments,
// so if we had a MultiQuery value, we can be sure there is no Query value
m_parsedArgs[Type::Query].emplace_back(std::move(itr->second[0]));
m_parsedArgs.erase(itr);
}
}

private:
std::map<Type, std::vector<std::string>> m_parsedArgs;
};
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
7 changes: 5 additions & 2 deletions src/AppInstallerCLICore/Resources.h
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,6 @@ namespace AppInstaller::CLI::Resource
WINGET_DEFINE_RESOURCE_STRINGID(ImportIgnorePackageVersionsArgumentDescription);
WINGET_DEFINE_RESOURCE_STRINGID(ImportIgnoreUnavailableArgumentDescription);
WINGET_DEFINE_RESOURCE_STRINGID(ImportInstallFailed);
WINGET_DEFINE_RESOURCE_STRINGID(ImportPackageAlreadyInstalled);
WINGET_DEFINE_RESOURCE_STRINGID(ImportSearchFailed);
WINGET_DEFINE_RESOURCE_STRINGID(ImportSourceNotInstalled);
WINGET_DEFINE_RESOURCE_STRINGID(IncludeUnknownArgumentDescription);
WINGET_DEFINE_RESOURCE_STRINGID(IncompatibleArgumentsProvided);
Expand Down Expand Up @@ -215,6 +213,11 @@ namespace AppInstaller::CLI::Resource
WINGET_DEFINE_RESOURCE_STRINGID(MultipleInstalledPackagesFound);
WINGET_DEFINE_RESOURCE_STRINGID(MultipleNonPortableNestedInstallersSpecified);
WINGET_DEFINE_RESOURCE_STRINGID(MultiplePackagesFound);
WINGET_DEFINE_RESOURCE_STRINGID(MultiQueryArgumentDescription);
WINGET_DEFINE_RESOURCE_STRINGID(MultiQueryPackageAlreadyInstalled);
WINGET_DEFINE_RESOURCE_STRINGID(MultiQueryPackageNotFound);
WINGET_DEFINE_RESOURCE_STRINGID(MultiQuerySearchFailed);
WINGET_DEFINE_RESOURCE_STRINGID(MultiQuerySearchFoundMultiple);
WINGET_DEFINE_RESOURCE_STRINGID(NameArgumentDescription);
WINGET_DEFINE_RESOURCE_STRINGID(NestedInstallerNotFound);
WINGET_DEFINE_RESOURCE_STRINGID(NestedInstallerNotSpecified);
Expand Down
Loading