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

DynamicDependencies: Use Win11 APIs if available #3704

Merged
merged 26 commits into from
Nov 30, 2023

Conversation

DrusTheAxe
Copy link
Member

@DrusTheAxe DrusTheAxe commented Jul 5, 2023

DynamicDependencies: Use Win11 APIs if available

Leverage the the Win11 Dynamic Dependency API if available, otherwise use WinAppSDK's implementation (aka polyfill). The WinAppSDK DynDep C API handles this, thus benefiting all callers of WinAppSDK's C and WinRT DynDep API (e.g. C# via projections).

This happens on Win11 >= 22H2 (aka SV2).

NOTE: This makes the DDLM irrelevant on Win11 22H2+ (>SV2). A future (separate) PR will change the intaller to not install the DDLM packages on these systems.

NOTE: Persisted definitions aren't imported into the OS after OSupgrade to Win11 22H2+. If you create a persisted definition (LifetimeKind=FilePath|RegistryKey) on older you'll need to re-create them after the OS upgrades.

NOTE: MddGetResolvedPackageFullNameForPackageDependency()'s behavior differed from GetResolvedPackageFullNameForPackageDependency(). Both return null vs package full name when the PackageDependencyId is not resolved vs resolved but the HRESULT return value differed

PackageDependencyId WinAppSDK OS API
unknown HRESULT_FROM_WIN32(ERROR_NO_MATCH) S_OK
not resolved S_OK S_OK
resolved S_OK S_OK

MddGetResolvedPackageFullNameForPackageDependency() has been changed to match the OS API. A new OS API will be needed to distinguish between a known valid PackageDependencyId that's not been resolved yet vs an invalid/unknown PackageDependencyId.

NOTE: This change is compatible with the documented behavior. It'll be noted in the release notes.

NOTE: As a byproduct of return MddGetResolvedPackageFullNameForPackageDependency() error handling the WinRT APIs PackageDependency.GetFromId() and PackageDependency.GetFromIdForSystem() no longer fail for an unknown/invalid id e.g. PackageDependency.GetFromId("not a valid id"). Inappropriate later use will fail e.g.

var pd = PackageDependency.GetFromId("not a valid id");
var context = pd.Add();
// Add() fails and throws an exception

NOTE: A new OS API is needed to determine if a PackageDependencyId is a valid (known) value. WinAppSDK will be updated to use that when available.

https://task.ms/47326537

@DrusTheAxe DrusTheAxe added this to the 1.5 milestone Nov 8, 2023
@DrusTheAxe
Copy link
Member Author

/azp run

Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@DrusTheAxe
Copy link
Member Author

/azp run

Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@DrusTheAxe DrusTheAxe requested a review from BenJKuhn November 15, 2023 22:30
@kythant
Copy link
Contributor

kythant commented Nov 21, 2023

Just a heads up. Develop branch has been renamed to become the new Main Branch and is the default branch from now on.

@Scottj1s
Copy link
Member

Scottj1s commented Nov 21, 2023 via email

@DrusTheAxe
Copy link
Member Author

/azp run

Copy link

@wcheng-msft wcheng-msft left a comment

Choose a reason for hiding this comment

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

:shipit:

@DrusTheAxe
Copy link
Member Author

/azp run

@DrusTheAxe
Copy link
Member Author

/azp run

@DrusTheAxe DrusTheAxe merged commit f0d6f3b into main Nov 30, 2023
@DrusTheAxe DrusTheAxe deleted the user/drustheaxe/dyndep-win11 branch November 30, 2023 17:50
kythant added a commit that referenced this pull request Jan 22, 2024
kythant added a commit that referenced this pull request Jan 23, 2024
kythant added a commit that referenced this pull request Jan 23, 2024
…#4108)

* Revert "48525090 - Skipping a test case to temporarily avoid hitting this bug (#4083)"

This reverts commit 39dcd27.

* Revert "DynamicDependencies: Use Win11 APIs if available (#3704)"

This reverts commit f0d6f3b.
kythant added a commit that referenced this pull request Jan 23, 2024
…#4109)

* Revert "48525090 - Skipping a test case to temporarily avoid hitting this bug (#4083)"

This reverts commit 39dcd27.

* Revert "DynamicDependencies: Use Win11 APIs if available (#3704)"

This reverts commit f0d6f3b.
kythant added a commit that referenced this pull request Jan 26, 2024
kythant added a commit that referenced this pull request Jan 26, 2024
kythant added a commit that referenced this pull request Jan 30, 2024
kythant added a commit that referenced this pull request Jan 30, 2024
DrusTheAxe added a commit that referenced this pull request Jan 31, 2024
DrusTheAxe added a commit that referenced this pull request Feb 5, 2024
…#4083 (#4108)" (#4118)" (#4135)" (#4140)

* Revert "Revert "DynamicDependencies: Use Win11 APIs if available #3704 #4083 (#4108)" (#4118)" (#4135)"

This reverts commit 117ed20.

* Disable DynamicDependency-delegate-to-OS by changing IsSupported() (not reverting the world)

* PackageRuntimeManager tests rely on OS DynDep functionality but Setup was a parallel version check when it should have been IsSupported. Fixed so they're all using the same answer

* Doh! Had the check reversed -- if (foo) should have been if (!foo). Fixed

* Tweak TryCreatePackageDependency when calling the OS with an invaliud package family name we get back STATEREPOSITORY_E_DEPENDENCY_NOT_RESOLVED but should return to the caller E_INVALIDARG

* Remove false skip-tests check. The surgical IsSupported disables all delegation to OS APIs. When that changes we WANT the tests to run and canary any problem

* Bad test passing too long a VersionTag. Tweaked Bootstrapper to spew WIL message and deal instead of FAIL_FAST

* Fixed PackageDependency.m_packageFullName was never set and .PackageId string fields (e.g. .name) weren't properly set, though they weren't used so not noticed or harm (before the resolution fix - next sentence...). Fixed TryCreatePackageDependency() wasn't resolving the dependency unless the MddCreatePackageDependencyOptions_DoNotVerifyDependencyResolution option was specified. Fixed up some test issues as well asupdates in sync with the fixes (above)

* Fixed tests to correctly match (fixed) implementation

* Improved some test logging. Fided up a few tests out of date with the latest behavior
bpulliam pushed a commit that referenced this pull request Feb 7, 2024
…#4083 (#4108)" (#4118)" (#4135)" (#4140)

* Revert "Revert "DynamicDependencies: Use Win11 APIs if available #3704 #4083 (#4108)" (#4118)" (#4135)"

This reverts commit 117ed20.

* Disable DynamicDependency-delegate-to-OS by changing IsSupported() (not reverting the world)

* PackageRuntimeManager tests rely on OS DynDep functionality but Setup was a parallel version check when it should have been IsSupported. Fixed so they're all using the same answer

* Doh! Had the check reversed -- if (foo) should have been if (!foo). Fixed

* Tweak TryCreatePackageDependency when calling the OS with an invaliud package family name we get back STATEREPOSITORY_E_DEPENDENCY_NOT_RESOLVED but should return to the caller E_INVALIDARG

* Remove false skip-tests check. The surgical IsSupported disables all delegation to OS APIs. When that changes we WANT the tests to run and canary any problem

* Bad test passing too long a VersionTag. Tweaked Bootstrapper to spew WIL message and deal instead of FAIL_FAST

* Fixed PackageDependency.m_packageFullName was never set and .PackageId string fields (e.g. .name) weren't properly set, though they weren't used so not noticed or harm (before the resolution fix - next sentence...). Fixed TryCreatePackageDependency() wasn't resolving the dependency unless the MddCreatePackageDependencyOptions_DoNotVerifyDependencyResolution option was specified. Fixed up some test issues as well asupdates in sync with the fixes (above)

* Fixed tests to correctly match (fixed) implementation

* Improved some test logging. Fided up a few tests out of date with the latest behavior
bpulliam added a commit that referenced this pull request Feb 8, 2024
* Revert "Revert "DynamicDependencies: Use Win11 APIs if available #3704 #4083 (#4108)" (#4118)" (#4135)"

This reverts commit 117ed20.

* Disable DynamicDependency-delegate-to-OS by changing IsSupported() (not reverting the world)

* PackageRuntimeManager tests rely on OS DynDep functionality but Setup was a parallel version check when it should have been IsSupported. Fixed so they're all using the same answer

* Doh! Had the check reversed -- if (foo) should have been if (!foo). Fixed

* Tweak TryCreatePackageDependency when calling the OS with an invaliud package family name we get back STATEREPOSITORY_E_DEPENDENCY_NOT_RESOLVED but should return to the caller E_INVALIDARG

* Remove false skip-tests check. The surgical IsSupported disables all delegation to OS APIs. When that changes we WANT the tests to run and canary any problem

* Bad test passing too long a VersionTag. Tweaked Bootstrapper to spew WIL message and deal instead of FAIL_FAST

* Fixed PackageDependency.m_packageFullName was never set and .PackageId string fields (e.g. .name) weren't properly set, though they weren't used so not noticed or harm (before the resolution fix - next sentence...). Fixed TryCreatePackageDependency() wasn't resolving the dependency unless the MddCreatePackageDependencyOptions_DoNotVerifyDependencyResolution option was specified. Fixed up some test issues as well asupdates in sync with the fixes (above)

* Fixed tests to correctly match (fixed) implementation

* Improved some test logging. Fided up a few tests out of date with the latest behavior

Co-authored-by: Howard Kapustein <howardk@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants