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

Add resume command and support saving the argument state. #3508

Merged
merged 50 commits into from
Oct 12, 2023
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
291ded4
save work
Jul 14, 2023
c6adb1a
it builds!
Jul 19, 2023
952a799
save work
Jul 21, 2023
2ed8d2c
add checkpoint manager
Jul 24, 2023
166b843
retrieve args from index
Jul 29, 2023
cbc34db
save work
Jul 31, 2023
4786194
clean up index functions
Aug 1, 2023
8982105
resolve merge conflicts
Aug 1, 2023
0f5ce84
add initial unit tests
Aug 3, 2023
e3786aa
save tests
Aug 4, 2023
940599f
fix e2E test and cleanup
Aug 7, 2023
ba3bcb6
fix CLIcore filter
Aug 7, 2023
dace07e
resolve merge conflicts and fix spelling
Aug 7, 2023
8d9a3b4
save work
Aug 10, 2023
e15a142
savework
Aug 10, 2023
22840a5
save work
Aug 11, 2023
35b772f
it builds
Aug 14, 2023
9f7f3f5
capture context data
Aug 14, 2023
e939e90
simplify work and update tests
Aug 15, 2023
a757d8c
fix spelling
Aug 15, 2023
3cf3fb7
remove EF check
Aug 16, 2023
ed47dd1
try again
Aug 16, 2023
3bca5d4
fix client Version
Aug 16, 2023
525bf1c
add table
Aug 18, 2023
623e26c
save work
Aug 22, 2023
f0d86b6
save notes
Aug 22, 2023
5ef5dce
template example
Aug 23, 2023
3775cb0
save work
Aug 28, 2023
44ebf0b
save work
Aug 29, 2023
bbee38e
save work
Aug 30, 2023
b0e64a6
save work
Sep 8, 2023
754cb8f
fix tests
Sep 11, 2023
5a91681
clean up
Sep 11, 2023
c536e20
resolve merge conflicts
Sep 11, 2023
6076097
fix tests and fix spelling
Sep 12, 2023
5107a85
fix path for checkpoints directory
Sep 12, 2023
16ac252
remove all references to index
Sep 12, 2023
1022bc2
actually fix e2e tests
Sep 13, 2023
e04f6bb
rename checkpointRecord to checkpointDatabase
ryfu-msft Sep 19, 2023
38d3f83
save work
ryfu-msft Sep 20, 2023
1d1250b
address rest of comments
ryfu-msft Sep 20, 2023
e57f23e
resolve merge conflicts
ryfu-msft Sep 20, 2023
d73ef7d
fix path
ryfu-msft Sep 20, 2023
3540ba0
Merge branch 'master' of https://github.com/ryfu-msft/winget-cli into…
ryfu-msft Sep 21, 2023
ae2dda0
fix path recursive call
ryfu-msft Sep 21, 2023
a3e72c7
minor fix
ryfu-msft Sep 21, 2023
c133d52
fix resume e2e test
ryfu-msft Sep 25, 2023
8a2da5d
respond to PR feedback
ryfu-msft Oct 10, 2023
792c465
fix error code
ryfu-msft Oct 10, 2023
a8f0ced
resolve merge conflicts
ryfu-msft Oct 11, 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
9 changes: 9 additions & 0 deletions .github/actions/spelling/allow.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ ACCESSDENIED
ACTIONDATA
ACTIONSTART
activatable
addcontext
addfile
addmanifest
addpin
Expand Down Expand Up @@ -60,6 +61,7 @@ cdn
cer
certutil
cguid
checkpointindex
chrono
cin
cla
Expand Down Expand Up @@ -115,6 +117,7 @@ debugbreak
declspec
decltype
declval
deconstructor
defaultlocale
delstore
Demitrius
Expand Down Expand Up @@ -231,6 +234,7 @@ https
HWND
Hyperlink
IAppx
ICheckpoint
IConfiguration
icu
IDX
Expand Down Expand Up @@ -491,6 +495,8 @@ seinfo
selectany
SERVICEPACKMAJOR
SERVICEPACKMINOR
setclientversion
setcommandname
setfill
setschemaversion
setupexitcodes
Expand Down Expand Up @@ -635,10 +641,13 @@ Unmarshal
Unregister
Unregisters
untimes
updateboolargument
updatefile
updatelastcheckpointflag
updatemanifest
updatepin
updateportablefile
updatestringargument
UPLEVEL
upvote
uregex
Expand Down
6 changes: 6 additions & 0 deletions src/AppInstallerCLICore/AppInstallerCLICore.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,7 @@
<ItemGroup>
<ClInclude Include="Argument.h" />
<ClInclude Include="ChannelStreams.h" />
<ClInclude Include="CheckpointManager.h" />
<ClInclude Include="Command.h" />
<ClInclude Include="Commands\COMCommand.h" />
<ClInclude Include="Commands\CompleteCommand.h" />
Expand All @@ -370,6 +371,7 @@
<ClInclude Include="Commands\ShowCommand.h" />
<ClInclude Include="Commands\InstallCommand.h" />
<ClInclude Include="Commands\RootCommand.h" />
<ClInclude Include="Commands\ResumeCommand.h" />
<ClInclude Include="Commands\SourceCommand.h" />
<ClInclude Include="Commands\TestCommand.h" />
<ClInclude Include="Commands\UninstallCommand.h" />
Expand Down Expand Up @@ -412,6 +414,7 @@
<ClInclude Include="Workflows\ShellExecuteInstallerHandler.h" />
<ClInclude Include="Workflows\InstallFlow.h" />
<ClInclude Include="Workflows\ManifestComparator.h" />
<ClInclude Include="Workflows\ResumeFlow.h" />
<ClInclude Include="Workflows\ShowFlow.h" />
<ClInclude Include="Workflows\SourceFlow.h" />
<ClInclude Include="Workflows\UninstallFlow.h" />
Expand All @@ -420,6 +423,7 @@
</ItemGroup>
<ItemGroup>
<ClCompile Include="COMContext.cpp" />
<ClCompile Include="CheckpointManager.cpp" />
<ClCompile Include="Commands\COMCommand.cpp" />
<ClCompile Include="Commands\ConfigureCommand.cpp" />
<ClCompile Include="Commands\ConfigureShowCommand.cpp" />
Expand Down Expand Up @@ -449,6 +453,7 @@
<ClCompile Include="Commands\ShowCommand.cpp" />
<ClCompile Include="Commands\InstallCommand.cpp" />
<ClCompile Include="Commands\RootCommand.cpp" />
<ClCompile Include="Commands\ResumeCommand.cpp" />
<ClCompile Include="Commands\SourceCommand.cpp" />
<ClCompile Include="Commands\UninstallCommand.cpp" />
<ClCompile Include="Commands\UpgradeCommand.cpp" />
Expand Down Expand Up @@ -480,6 +485,7 @@
<ClCompile Include="Workflows\ShellExecuteInstallerHandler.cpp" />
<ClCompile Include="Workflows\InstallFlow.cpp" />
<ClCompile Include="Workflows\ManifestComparator.cpp" />
<ClCompile Include="Workflows\ResumeFlow.cpp" />
<ClCompile Include="Workflows\ShowFlow.cpp" />
<ClCompile Include="Workflows\SourceFlow.cpp" />
<ClCompile Include="Workflows\UninstallFlow.cpp" />
Expand Down
18 changes: 18 additions & 0 deletions src/AppInstallerCLICore/AppInstallerCLICore.vcxproj.filters
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,15 @@
<ClInclude Include="Commands\DownloadCommand.h">
<Filter>Commands</Filter>
</ClInclude>
<ClInclude Include="Commands\ResumeCommand.h">
<Filter>Commands</Filter>
</ClInclude>
<ClInclude Include="Workflows\ResumeFlow.h">
<Filter>Workflows</Filter>
</ClInclude>
<ClInclude Include="CheckpointManager.h">
<Filter>Header Files</Filter>
</ClInclude>
</ItemGroup>
<ItemGroup>
<ClCompile Include="pch.cpp">
Expand Down Expand Up @@ -421,6 +430,15 @@
<ClInclude Include="Commands\DownloadCommand.cpp">
<Filter>Commands</Filter>
</ClInclude>
<ClCompile Include="Commands\ResumeCommand.cpp">
<Filter>Commands</Filter>
</ClCompile>
<ClCompile Include="Workflows\ResumeFlow.cpp">
<Filter>Workflows</Filter>
</ClCompile>
<ClCompile Include="CheckpointManager.cpp">
<Filter>Source Files</Filter>
</ClCompile>
</ItemGroup>
<ItemGroup>
<None Include="PropertySheet.props" />
Expand Down
6 changes: 6 additions & 0 deletions src/AppInstallerCLICore/Argument.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,10 @@ namespace AppInstaller::CLI
case Execution::Args::Type::PinInstalled:
return { type, "installed"_liv, ArgTypeCategory::None };

// Resume command
case Execution::Args::Type::ResumeGuid:
return { type, "resume-guid"_liv, 'g', ArgTypeCategory::None };
ryfu-msft marked this conversation as resolved.
Show resolved Hide resolved
ryfu-msft marked this conversation as resolved.
Show resolved Hide resolved

// Configuration commands
case Execution::Args::Type::ConfigurationFile:
return { type, "file"_liv, 'f' };
Expand Down Expand Up @@ -345,6 +349,8 @@ namespace AppInstaller::CLI
return Argument{ type, Resource::String::DownloadDirectoryArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help, false };
case Args::Type::InstallerType:
return Argument{ type, Resource::String::InstallerTypeArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help, false };
case Args::Type::ResumeGuid:
return Argument{ type, Resource::String::ResumeGuidArgumentDescription, ArgumentType::Standard, true };
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to understand the design of this feature a bit more; What was the reason for making this a standard argument and not a positional argument? What happens if the argument is not provided?

default:
THROW_HR(E_UNEXPECTED);
}
Expand Down
221 changes: 221 additions & 0 deletions src/AppInstallerCLICore/CheckpointManager.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,221 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
#include "pch.h"
#include "AppInstallerRuntime.h"
#include "CheckpointManager.h"
#include "Microsoft/CheckpointIndex.h"
#include "Microsoft/SQLiteStorageBase.h"
#include <Argument.h>
#include <Command.h>

namespace AppInstaller::CLI::Checkpoint
{
void CheckpointManager::Initialize(GUID checkpointId)
{
// Initialize should only be called once.
THROW_HR_IF(E_UNEXPECTED, m_checkpointId != GUID_NULL);

bool createCheckpointIndex = (checkpointId == GUID_NULL);
if (createCheckpointIndex)
{
std::ignore = CoCreateGuid(&m_checkpointId);
AICLI_LOG(CLI, Info, << "Creating checkpoint index with guid: " << m_checkpointId);
}
else
{
m_checkpointId = checkpointId;
AICLI_LOG(CLI, Info, << "Opening checkpoint index with guid: " << m_checkpointId);
}

auto openDisposition = AppInstaller::Repository::Microsoft::SQLiteStorageBase::OpenDisposition::ReadWrite;
auto checkpointIndex = AppInstaller::Repository::Microsoft::CheckpointIndex::OpenOrCreateDefault(m_checkpointId, openDisposition);
if (!checkpointIndex)
{
AICLI_LOG(CLI, Error, << "Unable to open checkpoint index.");
THROW_HR_MSG(HRESULT_FROM_WIN32(ERROR_NOT_FOUND), "The checkpoint state could not be found.");
ryfu-msft marked this conversation as resolved.
Show resolved Hide resolved
}

Copy link
Member

Choose a reason for hiding this comment

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

nit: extre newline

m_checkpointIndex = std::move(checkpointIndex);

if (createCheckpointIndex)
{
m_checkpointIndex->SetClientVersion(AppInstaller::Runtime::GetClientVersion());
}
}

void CheckpointManager::Checkpoint(Execution::Context& context, Execution::CheckpointFlag checkpointFlag)
{
Execution::CheckpointFlag contextCheckpointFlag = context.GetTargetCheckpoint();

// If the context target checkpoint is ahead or equal to the current checkpoint, we have previously executed this state, load checkpoint state from index.
// If the context target checkpoint is behind the current checkpoint, we have not yet processed this state, save checkpoint state to index.
if (contextCheckpointFlag > checkpointFlag || contextCheckpointFlag == checkpointFlag)
{
LoadCheckpoint(context, checkpointFlag);
ryfu-msft marked this conversation as resolved.
Show resolved Hide resolved
}
else if (contextCheckpointFlag < checkpointFlag)
{
SaveCheckpoint(context, checkpointFlag);
}
}

bool CheckpointManager::HasContext()
{
return !m_checkpointIndex->IsEmpty();
}

void CheckpointManager::AddContext(int contextId)
{
m_checkpointIndex->AddContext(contextId);
}

void CheckpointManager::RemoveContext(int contextId)
{
m_checkpointIndex->RemoveContext(contextId);
}

std::string CheckpointManager::GetClientVersion()
{
return m_checkpointIndex->GetClientVersion();
}

std::string CheckpointManager::GetCommandName(int contextId)
{
return m_checkpointIndex->GetCommandName(contextId);
}

int CheckpointManager::GetFirstContextId()
{
return m_checkpointIndex->GetFirstContextId();
}

Execution::CheckpointFlag CheckpointManager::GetLastCheckpoint(int contextId)
{
return static_cast<Execution::CheckpointFlag>(m_checkpointIndex->GetLastCheckpoint(contextId));
}

#ifndef AICLI_DISABLE_TEST_HOOKS
static bool s_MockCheckpointManagerCleanUp_Override = false;

void TestHook_MockCheckpointManagerCleanUp_Override(bool status)
{
s_MockCheckpointManagerCleanUp_Override = status;
}
#endif

void CheckpointManager::CleanUpIndex()
{
#ifndef AICLI_DISABLE_TEST_HOOKS
// Unit tests will handle clean up so this hook is needed to avoid attempting to reset and clean up again.
if (s_MockCheckpointManagerCleanUp_Override)
{
return;
}
#endif

bool isIndexEmpty = m_checkpointIndex->IsEmpty();

m_checkpointIndex.reset();

if (isIndexEmpty)
{
const auto& checkpointIndexPath = AppInstaller::Repository::Microsoft::CheckpointIndex::GetCheckpointIndexPath(m_checkpointId);
if (std::filesystem::remove(checkpointIndexPath))
ryfu-msft marked this conversation as resolved.
Show resolved Hide resolved
{
AICLI_LOG(CLI, Info, << "Checkpoint index deleted: " << checkpointIndexPath);
}
}
}

CheckpointManager::~CheckpointManager()
{
CleanUpIndex();
ryfu-msft marked this conversation as resolved.
Show resolved Hide resolved
}

void CheckpointManager::SaveCheckpoint(Execution::Context& context, Execution::CheckpointFlag flag)
{
switch (flag)
{
case Execution::CheckpointFlag::CommandArguments:
ryfu-msft marked this conversation as resolved.
Show resolved Hide resolved
RecordContextArgsToIndex(context);
break;
default:
THROW_HR(E_UNEXPECTED);
}

m_checkpointIndex->SetLastCheckpoint(context.GetContextId(), static_cast<int>(flag));
context.SetCurrentCheckpoint(flag);
}

void CheckpointManager::LoadCheckpoint(Execution::Context& context, Execution::CheckpointFlag flag)
{
switch (flag)
{
case Execution::CheckpointFlag::CommandArguments:
PopulateContextArgsFromIndex(context);
break;
default:
THROW_HR(E_UNEXPECTED);
}

context.SetCurrentCheckpoint(flag);
}

void CheckpointManager::PopulateContextArgsFromIndex(Execution::Context& context)
{
int contextId = context.GetContextId();

const auto& executingCommand = context.GetExecutingCommand();
if (executingCommand != nullptr)
{
const auto& commandArguments = executingCommand->GetArguments();
for (const auto& argument : commandArguments)
{
if (m_checkpointIndex->ContainsArgument(contextId, argument.Name()))
{
Execution::Args::Type executionArgsType = argument.ExecArgType();
if (argument.Type() == ArgumentType::Flag)
{
if (m_checkpointIndex->GetBoolArgument(contextId, argument.Name()))
{
context.Args.AddArg(executionArgsType);
}
}
else
{
context.Args.AddArg(executionArgsType, m_checkpointIndex->GetStringArgument(contextId, argument.Name()));
}
}
}
}
}

void CheckpointManager::RecordContextArgsToIndex(Execution::Context& context)
{
int contextId = context.GetContextId();

const auto& executingCommand = context.GetExecutingCommand();
if (executingCommand != nullptr)
{
m_checkpointIndex->SetCommandName(contextId, executingCommand->Name());

const auto& commandArguments = executingCommand->GetArguments();
for (const auto& argument : commandArguments)
{
Execution::Args::Type type = argument.ExecArgType();
if (context.Args.Contains(type))
{
if (argument.Type() == ArgumentType::Flag)
{
m_checkpointIndex->UpdateArgument(contextId, argument.Name(), true);
}
else
{
const auto& argumentValue = context.Args.GetArg(type);
m_checkpointIndex->UpdateArgument(contextId, argument.Name(), argumentValue);
}
}
}
}
}
}
Loading