Skip to content

Add uninstall functionality to Com api - #1909

Merged
yao-msft merged 15 commits into
microsoft:masterfrom
yao-msft:comuninstall
Mar 2, 2022
Merged

yao-msft merged 15 commits into
microsoft:masterfrom
yao-msft:comuninstall

Conversation

@yao-msft

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

Copy link
Copy Markdown
Contributor

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

github-actions Bot commented Feb 24, 2022 •

Copy link
Copy Markdown

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 [email protected]: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")

@JohnMcPMS JohnMcPMS Mar 1, 2022 •

Copy link
Copy Markdown
Member

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

</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
Copy Markdown
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
Copy Markdown
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.

@JohnMcPMS JohnMcPMS Mar 1, 2022 •

Copy link
Copy Markdown
Member

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

@JohnMcPMS JohnMcPMS Mar 1, 2022 •

Copy link
Copy Markdown
Member

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
Copy Markdown
Member

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

@yao-msft

yao-msft commented Mar 1, 2022

Copy link
Copy Markdown
Contributor Author

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

yao-msft commented Mar 1, 2022

Copy link
Copy Markdown
Contributor Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).


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

Copy link
Copy Markdown
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