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 support for InProc Com invocation #2035

Merged
merged 9 commits into from
Mar 28, 2022
Merged

Conversation

yao-msft
Copy link
Contributor

@yao-msft yao-msft commented Mar 21, 2022

Change

Added a shim dll to support in-proc Com activation of winget Com apis.

Validation

Validated by writing a sample caller consuming winget Com apis through in-proc activation. Codes run fine with in-proc activation.

Next

  • State separation and better support for System caller
  • Add Com e2e tests and remove previous workarounds(i.e. Microsoft.Management.Deployment.Client and Microsoft.Management.Deployment.Server.Test)
Microsoft Reviewers: Open in CodeFlow

@yao-msft yao-msft requested a review from a team as a code owner March 21, 2022 21:14
@github-actions
Copy link

github-actions bot commented Mar 21, 2022

@check-spelling-bot Report

Unrecognized words, please review:

  • CLASSNOTAVAILABLE
  • guiddef
  • OUTOFPROC
  • ppv
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 inproccom 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/1074427235" > "$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

Copy link
Member

@JohnMcPMS JohnMcPMS left a comment

Choose a reason for hiding this comment

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

It would be interesting to have the sample caller app be able to swap to this as well for test purposes.

@@ -191,7 +191,7 @@
<Link>
<SubSystem>Console</SubSystem>
<GenerateWindowsMetadata>false</GenerateWindowsMetadata>
<AdditionalDependencies Condition="'$(Configuration)'=='Debug'">wininet.lib;shell32.lib;winsqlite3.lib;shlwapi.lib;icuuc.lib;icuin.lib;urlmon.lib;Advapi32.lib;winhttp.lib;onecoreuap.lib;msi.lib;%(AdditionalDependencies)</AdditionalDependencies>
<AdditionalDependencies Condition="'$(Configuration)'=='Debug'">%(AdditionalDependencies)</AdditionalDependencies>
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this now a no-op?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I leave the node here in unlikely case we'll need something in the future.


extern "C"
{
BOOL WINDOWS_PACKAGE_MANAGER_API_CALLING_CONVENTION DllMain(
Copy link
Member

Choose a reason for hiding this comment

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

I think we should be using a macro for calling convention that is more in line with what this expects, not just whatever we happen to use.


WINDOWS_PACKAGE_MANAGER_API DllCanUnloadNow()
{
return WindowsPackageManagerInProcModuleTerminate() ? S_OK : S_FALSE;
Copy link
Member

Choose a reason for hiding this comment

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

Is this the right way to do it? It seems like there are potential error paths here, although given the calling patterns it might be ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this pattern is what I found in most other com dlls we wrote in the past.


WINDOWS_PACKAGE_MANAGER_API WindowsPackageManagerInProcModuleInitialize() try
{
::Microsoft::WRL::Module<::Microsoft::WRL::ModuleType::InProc>::Create();
Copy link
Member

Choose a reason for hiding this comment

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

We don't need an initialization of our code? I suppose that might be what the state separation config would do.
We don't need to RegisterObjects? Is that out-of-proc only?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct. RegisterObjects is for out of proc only. In proc automatically recognizes the creator map.

@github-actions
Copy link

github-actions bot commented Mar 28, 2022

@check-spelling-bot Report

Unrecognized words, please review:

  • STDAPI
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 inproccom 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/1080063189" > "$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

@yao-msft yao-msft merged commit c902678 into microsoft:master Mar 28, 2022
@yao-msft yao-msft deleted the inproccom branch March 28, 2022 01:39
@Niranjan-Jyothi
Copy link

Hey, @yao-msft @denelon @Trenly I have been trying to use In-Proc Winget COM APIs directly onto my c# app (.net 8) using CsWinRT to eventually install/search applications but the initial PackageManger object creation itself fails with below message

System.TypeInitializationException: 'The type initializer for 'WinRT.ActivationFactory`1' threw an exception.'
Inner Exception
COMException: Class not registered (0x80040154 (REGDB_E_CLASSNOTREG))

image

Can someone point me the right direction on how to achieve this maybe a straightforward example code snippet or documentation. Analysing the powershell (as it uses In-Proc) was getting a bit complicated and did not see any registrations before hand in the code instead the class objects (PackageManager etc) were directly being created in the
winget-cli\src\PowerShell\Microsoft.WinGet.Client.Engine\Helpers*ManagementDeploymentFactory.cs* file

P.S : The reason for going with this approach was because we wanted to execute the commands in the SYSTEM context and using direct powershell modules was not approved.

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