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 uninstall functionality to Com api #1909

Merged
merged 15 commits into from
Mar 2, 2022

Conversation

yao-msft
Copy link
Contributor

@yao-msft yao-msft commented Feb 8, 2022

Change

Added uninstall functionality to Com api

Validation

Since the Com packaged tests are not ready yet, I manually verified the newly added uninstall api works on packages from winget source and msstore rest source.

Next I'll work on enabling Com tests as part of the e2e tests.

Microsoft Reviewers: Open in CodeFlow

@yao-msft yao-msft requested a review from a team as a code owner February 8, 2022 01:42
@github-actions
Copy link

github-actions bot commented Feb 24, 2022

@check-spelling-bot Report

Unrecognized words, please review:

  • TOptions
  • TProgress
  • TResult
  • TState
  • TStatus
Previously acknowledged words that are now absent activatable amd Archs dsc Globals hackathon mytool Packagedx parametermap whatif
To accept these unrecognized words as correct (and remove the previously acknowledged and now absent words), run the following commands

... in a clone of the git@github.com:yao-msft/winget-cli.git repository
on the comuninstall branch:

update_files() {
perl -e '
my @expect_files=qw('".github/actions/spelling/expect.txt"');
@ARGV=@expect_files;
my @stale=qw('"$patch_remove"');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
next if /^(?:$re)(?:(?:\r|\n)*$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spelling/expect.txt";
use File::Path qw(make_path);
use File::Basename qw(dirname);
make_path (dirname($new_expect_file));
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"$patch_add"');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a)."-".$a cmp lc($b)."-".$b} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;
system("git", "add", $new_expect_file);
'
}

comment_json=$(mktemp)
curl -L -s -S \
  --header "Content-Type: application/json" \
  "https://api.github.com/repos/microsoft/winget-cli/issues/comments/1049607085" > "$comment_json"
comment_body=$(mktemp)
jq -r .body < "$comment_json" > $comment_body
rm $comment_json

patch_remove=$(perl -ne 'next unless s{^</summary>(.*)</details>$}{$1}; print' < "$comment_body")
  

patch_add=$(perl -e '$/=undef;
$_=<>;
s{<details>.*}{}s;
s{^#.*}{};
s{\n##.*}{};
s{(?:^|\n)\s*\*}{}g;
s{\s+}{ }g;
print' < "$comment_body")
  
update_files
rm $comment_body
git add -u

// Get command queue name based on command name.
std::string_view GetCommandQueueName(std::string_view commandName)
{
if (commandName == "install" || commandName == "uninstall")
Copy link
Member

@JohnMcPMS JohnMcPMS Mar 1, 2022

Choose a reason for hiding this comment

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

These strings should come from the command via a static function rather than be copied here. #Resolved

@@ -48,6 +48,8 @@
</com:Class>
<com:Class Id ="3F85B9F4-487A-4C48-9035-2903F8A6D9E8" DisplayName="PackageMatchFilter Server">
</com:Class>
<com:Class Id ="AA2A5C04-1AD9-46C4-B74F-6B334AD7EB8C" DisplayName="UninstallOptions Server">
Copy link
Member

Choose a reason for hiding this comment

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

Reminder that we need to add it to the other package on pull.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I'll do that on next pull.

enum PackageUninstallProgressState
{
/// The uninstall is queued but not yet active. Cancellation of the IAsyncOperationWithProgress in this
/// state will prevent the package from downloading or installing.
Copy link
Member

@JohnMcPMS JohnMcPMS Mar 1, 2022

Choose a reason for hiding this comment

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

Suggested change
/// state will prevent the package from downloading or installing.
/// state will prevent the package from uninstalling.
``` #Resolved

};

/// Progress object for the uninstall
/// DESIGN NOTE: percentage for the uninstall as a whole is purposefully not included as there is no way to
Copy link
Member

@JohnMcPMS JohnMcPMS Mar 1, 2022

Choose a reason for hiding this comment

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

Remove copied design note as there is a progress value that would satisfy this (if there is any point at which it actually means anything). #Resolved

@JohnMcPMS
Copy link
Member

Forgot to add that you should update the sample caller app to include uninstall and integration with the progress.

@yao-msft
Copy link
Contributor Author

yao-msft commented Mar 1, 2022

Forgot to add that you should update the sample caller app to include uninstall and integration with the progress.

I can do that in a separate pr later.

@yao-msft
Copy link
Contributor Author

yao-msft commented Mar 1, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).


/// Result of the install
[contract(Microsoft.Management.Deployment.WindowsPackageManagerContract, 4)]
runtimeclass UninstallResult
Copy link
Contributor

Choose a reason for hiding this comment

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

UninstallResult

we should make sure when this feature gets checked in we FCIB the DesktopAppInstaller package to SV3, so that dependent callers (backup / restore) are not broken

@yao-msft yao-msft merged commit 79c8c11 into microsoft:master Mar 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants