Skip to content

Commit 3836253

Browse files
DrusTheAxeDrusTheAxe
authored
EnsurePackage*ReadyAsync() doesn't register v2 if v1 is registered and caller asks for minVersion=v1 (#5182)
* https://task.ms/55967280 EnsurePackage*Ready() doesn't register newer package if lower version currently registered. Also, *PackageOptions.ExpectedDigests() didn't jit-create a map on access * Updated tests verifying the fix
1 parent 0706e72 commit 3836253

7 files changed

+104
-22
lines changed

‎dev/PackageManager/API/M.W.M.D.AddPackageOptions.cpp

+4
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,10 @@ namespace winrt::Microsoft::Windows::Management::Deployment::implementation
153153
}
154154
winrt::Windows::Foundation::Collections::IMap<winrt::Windows::Foundation::Uri, hstring> AddPackageOptions::ExpectedDigests()
155155
{
156+
if (!m_expectedDigests)
157+
{
158+
m_expectedDigests = winrt::single_threaded_map<winrt::Windows::Foundation::Uri, hstring>();
159+
}
156160
return m_expectedDigests;
157161
}
158162
bool AddPackageOptions::IsLimitToExistingPackagesSupported()

‎dev/PackageManager/API/M.W.M.D.PackageDeploymentManager.cpp

+86-8
Original file line numberDiff line numberDiff line change
@@ -1929,23 +1929,46 @@ namespace winrt::Microsoft::Windows::Management::Deployment::implementation
19291929
errorText.clear();
19301930
activityId = winrt::guid{};
19311931

1932-
bool isReady{};
1932+
winrt::Microsoft::Windows::Management::Deployment::PackageReadyOrNewerAvailableStatus readyOrNewerStatus{};
19331933
if (options.RegisterNewerIfAvailable())
19341934
{
19351935
// Our caller already verified PackageDeploymentFeature::IsPackageReadyOrNewerAvailable is supported so no need to check again
1936-
isReady = (IsReadyOrNewerAvailable(packageSetItem) == winrt::Microsoft::Windows::Management::Deployment::PackageReadyOrNewerAvailableStatus::Ready);
1936+
readyOrNewerStatus = IsReadyOrNewerAvailable(packageSetItem);
19371937
}
19381938
else
19391939
{
1940-
isReady = IsReady(packageSetItem);
1940+
const bool isReady{ IsReady(packageSetItem) };
1941+
readyOrNewerStatus = isReady ?
1942+
winrt::Microsoft::Windows::Management::Deployment::PackageReadyOrNewerAvailableStatus::Ready :
1943+
winrt::Microsoft::Windows::Management::Deployment::PackageReadyOrNewerAvailableStatus::NotReady;
19411944
}
1942-
if (isReady)
1945+
if (readyOrNewerStatus == winrt::Microsoft::Windows::Management::Deployment::PackageReadyOrNewerAvailableStatus::Ready)
19431946
{
19441947
return S_OK;
19451948
}
19461949

1947-
winrt::Windows::Management::Deployment::AddPackageOptions addOptions{ ToOptions(options) };
1950+
// If the package family's known but a newer version's available on the machine than currently registered
1951+
// to the user we need to register the family to ensure the newer package is used.
1952+
//
1953+
// PackageManager.AddPackage.*Async() SHOULD do that but it doesn't. If the user has foo-v2 registered and foo-v3 is staged
1954+
// on the machine Add(foo-v2) is a NOOP. This does NOT reigster foo-v3 for the user. That'll require an OS change,
1955+
// 'cause, backwards compatibility (e.g. Register(foo-v2, options.VersionSupercedence=true)).
1956+
//
1957+
// https://task.ms/55967280 tracks this workaround registering the package family.
1958+
// https://task.ms/56383047 tracks enhancing PackageManager.AddPackage*Async() to support options.VersionSupercedence=true
1959+
// https://task.ms/56385363 tracks revising this code back to PackageManager.AddPackageByUriAsync() once it supports VersionSupercedence=true (https://task.ms/56383047)
1960+
if (readyOrNewerStatus == winrt::Microsoft::Windows::Management::Deployment::PackageReadyOrNewerAvailableStatus::NewerAvailable)
1961+
{
1962+
// We know there's a package registered to the user >=MinVersion AND there's a newer package than that available on the machine (staged, registered to other users, etc)
1963+
// IOW we want VersionSupercedence behavior and only RegisterPackageByPackageFamilyName() respects VersionSupercedence so let's do it...
1964+
winrt::Microsoft::Windows::Management::Deployment::RegisterPackageOptions registerOptions{ ToRegisterOptions(options) };
1965+
RETURN_IF_FAILED(RegisterPackageByPackageFamilyName(packageSetItem.PackageFamilyName(), registerOptions,
1966+
packageDeploymentProgress, progress, progressMaxPerPackageSetItem, extendedError, errorText, activityId));
1967+
return S_OK;
1968+
}
1969+
19481970
const auto progressBefore{ packageDeploymentProgress.Progress };
1971+
winrt::Windows::Management::Deployment::AddPackageOptions addOptions{ ToOptions(options) };
19491972
auto deploymentOperation{ m_packageManager.AddPackageByUriAsync(packageUri, addOptions) };
19501973
deploymentOperation.Progress([&](winrt::Windows::Foundation::IAsyncOperationWithProgress<
19511974
winrt::Windows::Management::Deployment::DeploymentResult,
@@ -3391,7 +3414,7 @@ namespace winrt::Microsoft::Windows::Management::Deployment::implementation
33913414
if (options.IsExpectedDigestsSupported())
33923415
{
33933416
const auto expectedDigests{ options.ExpectedDigests() };
3394-
if (expectedDigests)
3417+
if (expectedDigests.Size() > 0)
33953418
{
33963419
auto toExpectedDigests{ toOptions.ExpectedDigests() };
33973420
for (const auto expectedDigest : expectedDigests)
@@ -3446,7 +3469,7 @@ namespace winrt::Microsoft::Windows::Management::Deployment::implementation
34463469
if (options.IsExpectedDigestsSupported())
34473470
{
34483471
const auto expectedDigests{ options.ExpectedDigests() };
3449-
if (expectedDigests)
3472+
if (expectedDigests.Size() > 0)
34503473
{
34513474
auto toExpectedDigests{ toOptions.ExpectedDigests() };
34523475
for (const auto expectedDigest : expectedDigests)
@@ -3490,7 +3513,7 @@ namespace winrt::Microsoft::Windows::Management::Deployment::implementation
34903513
if (options.IsExpectedDigestsSupported())
34913514
{
34923515
const auto expectedDigests{ options.ExpectedDigests() };
3493-
if (expectedDigests)
3516+
if (expectedDigests.Size() > 0)
34943517
{
34953518
auto toExpectedDigests{ toOptions.ExpectedDigests() };
34963519
for (const auto expectedDigest : expectedDigests)
@@ -3575,6 +3598,61 @@ namespace winrt::Microsoft::Windows::Management::Deployment::implementation
35753598
return toOptions;
35763599
}
35773600

3601+
winrt::Microsoft::Windows::Management::Deployment::RegisterPackageOptions PackageDeploymentManager::ToRegisterOptions(winrt::Microsoft::Windows::Management::Deployment::EnsureReadyOptions const& options) const
3602+
{
3603+
winrt::Microsoft::Windows::Management::Deployment::RegisterPackageOptions toOptions;
3604+
const auto addOptions{ options.AddPackageOptions() };
3605+
3606+
// RegisterPackageOptions has no TargetVolume -- no need (the packge is already staged however it's staged on the machine)
3607+
for (const auto uri : addOptions.DependencyPackageUris())
3608+
{
3609+
toOptions.DependencyPackageUris().Append(uri);
3610+
}
3611+
for (const auto packageFamilyName : addOptions.OptionalPackageFamilyNames())
3612+
{
3613+
toOptions.OptionalPackageFamilyNames().Append(packageFamilyName);
3614+
}
3615+
// RegisterPackageOptions has no OptionalPackageUris -- warn if any as we won't (can't) use them
3616+
for (const auto uri : addOptions.OptionalPackageUris())
3617+
{
3618+
const auto uriAsString{ uri.ToString() };
3619+
(void) LOG_HR_MSG(MDD_E_WINDOWSAPPRUNTIME_PACKAGEMANAGER_WARNING_OPTION_IGNORED, "OptionalPackageUri: %ls", uriAsString.c_str());
3620+
}
3621+
// RegisterPackageOptions has no RelatedPackageUris -- warn if any as we won't (can't) use them
3622+
for (const auto uri : addOptions.RelatedPackageUris())
3623+
{
3624+
const auto uriAsString{ uri.ToString() };
3625+
(void) LOG_HR_MSG(MDD_E_WINDOWSAPPRUNTIME_PACKAGEMANAGER_WARNING_OPTION_IGNORED, "RelatedPackageUri: %ls", uriAsString.c_str());
3626+
}
3627+
const auto externalLocationUri{ addOptions.ExternalLocationUri() };
3628+
if (externalLocationUri)
3629+
{
3630+
toOptions.ExternalLocationUri(externalLocationUri);
3631+
}
3632+
// RegisterPackageOptions has no StubPackageOption -- no need (the packge is already staged however it's staged on the machine)
3633+
toOptions.AllowUnsigned(addOptions.AllowUnsigned());
3634+
toOptions.DeveloperMode(addOptions.DeveloperMode());
3635+
toOptions.ForceAppShutdown(addOptions.ForceAppShutdown());
3636+
toOptions.ForceTargetAppShutdown(addOptions.ForceTargetAppShutdown());
3637+
toOptions.ForceUpdateFromAnyVersion(addOptions.ForceUpdateFromAnyVersion());
3638+
toOptions.InstallAllResources(addOptions.InstallAllResources());
3639+
// RegisterPackageOptions has no RequiredContentGroupOnly -- no need (the packge is already staged however it's staged on the machine)
3640+
// RegisterPackageOptions has no RetainFilesOnFailure -- no need (the packge is already staged however it's staged on the machine)
3641+
toOptions.StageInPlace(addOptions.StageInPlace());
3642+
toOptions.DeferRegistrationWhenPackagesAreInUse(addOptions.DeferRegistrationWhenPackagesAreInUse());
3643+
const auto expectedDigests{ addOptions.ExpectedDigests() };
3644+
if (expectedDigests)
3645+
{
3646+
auto toExpectedDigests{ addOptions.ExpectedDigests() };
3647+
for (const auto expectedDigest : expectedDigests)
3648+
{
3649+
toExpectedDigests.Insert(expectedDigest.Key(), expectedDigest.Value());
3650+
}
3651+
}
3652+
// RegisterPackageOptions has no LimitToExistingPackages -- no need (the packge is already staged however it's staged on the machine)
3653+
return toOptions;
3654+
}
3655+
35783656
double PackageDeploymentManager::PercentageToProgress(uint32_t percentage, const double progressMaxPerItem)
35793657
{
35803658
return (static_cast<double>(percentage) / 100.0) * progressMaxPerItem;

‎dev/PackageManager/API/M.W.M.D.PackageDeploymentManager.h

+1
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,7 @@ namespace winrt::Microsoft::Windows::Management::Deployment::implementation
290290
winrt::Windows::Management::Deployment::AddPackageOptions ToOptions(winrt::Microsoft::Windows::Management::Deployment::EnsureReadyOptions const& options) const;
291291
winrt::Windows::Management::Deployment::DeploymentOptions ToDeploymentOptions(winrt::Microsoft::Windows::Management::Deployment::RegisterPackageOptions const& options) const;
292292
winrt::Windows::Management::Deployment::PackageAllUserProvisioningOptions ToOptions(winrt::Microsoft::Windows::Management::Deployment::ProvisionPackageOptions const& options) const;
293+
winrt::Microsoft::Windows::Management::Deployment::RegisterPackageOptions ToRegisterOptions(winrt::Microsoft::Windows::Management::Deployment::EnsureReadyOptions const& options) const;
293294
static double PercentageToProgress(uint32_t percentage, const double progressMaxPerItem);
294295
static bool IsUriEndsWith(winrt::Windows::Foundation::Uri const& packageUri, PCWSTR target);
295296
static winrt::Windows::Foundation::Uri GetEffectivePackageUri(

‎dev/PackageManager/API/M.W.M.D.RegisterPackageOptions.cpp

+4
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,10 @@ namespace winrt::Microsoft::Windows::Management::Deployment::implementation
121121
}
122122
winrt::Windows::Foundation::Collections::IMap<winrt::Windows::Foundation::Uri, hstring> RegisterPackageOptions::ExpectedDigests()
123123
{
124+
if (!m_expectedDigests)
125+
{
126+
m_expectedDigests = winrt::single_threaded_map<winrt::Windows::Foundation::Uri, hstring>();
127+
}
124128
return m_expectedDigests;
125129
}
126130
}

‎dev/PackageManager/API/M.W.M.D.StagePackageOptions.cpp

+4
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,10 @@ namespace winrt::Microsoft::Windows::Management::Deployment::implementation
121121
}
122122
winrt::Windows::Foundation::Collections::IMap<winrt::Windows::Foundation::Uri, hstring> StagePackageOptions::ExpectedDigests()
123123
{
124+
if (!m_expectedDigests)
125+
{
126+
m_expectedDigests = winrt::single_threaded_map<winrt::Windows::Foundation::Uri, hstring>();
127+
}
124128
return m_expectedDigests;
125129
}
126130
}

‎dev/PackageManager/API/MsixPackageManager.h

+3
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,7 @@
99
// Reserved: _HRESULT_TYPEDEF_(0x80040303L)
1010
// Reserved: _HRESULT_TYPEDEF_(0x80040304L)
1111

12+
/// Package Manager HRESULT: Warning option is ignored
13+
#define MDD_E_WINDOWSAPPRUNTIME_PACKAGEMANAGER_WARNING_OPTION_IGNORED _HRESULT_TYPEDEF_(0x80040305L)
14+
1215
#endif // MSIXPACKAGEMANAGER_H

‎test/PackageManager/API/PackageDeploymentManagerTests_EnsureReady_RegisterNewerIfAvailable.cpp

+2-14
Original file line numberDiff line numberDiff line change
@@ -494,14 +494,8 @@ namespace Test::PackageManager::Tests
494494
auto deploymentResult{ WaitForDeploymentOperation(deploymentOperation) };
495495
TPMT::VerifyDeploymentSucceeded(deploymentResult, __FILE__, __LINE__, __FUNCTION__);
496496

497-
#ifndef TODO_55967280_EnsurePackageSetReadyAsync_doesnt_register_newer_package_if_lower_version_is_currently_registered
498-
WEX::Logging::Log::Comment(L"Bug https://task.ms/55967171 Ensure*() doesn't account for package status");
499-
VERIFY_IS_TRUE(IsPackageRegistered_Red());
500-
VERIFY_IS_FALSE(IsPackageRegistered_Redder());
501-
#else
502497
VERIFY_IS_FALSE(IsPackageRegistered_Red());
503498
VERIFY_IS_TRUE(IsPackageRegistered_Redder());
504-
#endif
505499
VERIFY_IS_TRUE(packageDeploymentManager.IsPackageSetReady(packageSet));
506500

507501
RemovePackage_Redder();
@@ -534,14 +528,8 @@ namespace Test::PackageManager::Tests
534528
auto deploymentResult{ WaitForDeploymentOperation(deploymentOperation) };
535529
TPMT::VerifyDeploymentSucceeded(deploymentResult, __FILE__, __LINE__, __FUNCTION__);
536530

537-
#ifndef TODO_55967280_EnsurePackageSetReadyAsync_doesnt_register_newer_package_if_lower_version_is_currently_registered
538-
WEX::Logging::Log::Comment(L"Bug https://task.ms/55967280 RegisterNewerIfAvailable(true) isn't honored");
539-
VERIFY_IS_TRUE(IsPackageRegistered_Red());
540-
VERIFY_IS_FALSE(IsPackageRegistered_Redder());
541-
#else
542531
VERIFY_IS_FALSE(IsPackageRegistered_Red());
543532
VERIFY_IS_TRUE(IsPackageRegistered_Redder());
544-
#endif
545533
VERIFY_IS_TRUE(IsPackageRegistered_Green());
546534
VERIFY_IS_TRUE(packageDeploymentManager.IsPackageSetReady(packageSet));
547535

@@ -620,7 +608,7 @@ namespace Test::PackageManager::Tests
620608
TPMT::VerifyDeploymentSucceeded(deploymentResult, __FILE__, __LINE__, __FUNCTION__);
621609

622610
VERIFY_IS_TRUE(IsPackageRegistered_Red());
623-
#ifndef TODO_55967280_EnsurePackageSetReadyAsync_doesnt_register_newer_package_if_lower_version_is_currently_registered
611+
#ifndef TODO_55967171_IsPackageReady_OrNewerAvailable_doesnt_account_for_packagestatus
624612
WEX::Logging::Log::Comment(L"Bug https://task.ms/55967171 Ensure*() doesn't account for package status");
625613
VERIFY_IS_FALSE(packageDeploymentManager.IsPackageSetReady(packageSet));
626614
#else
@@ -655,7 +643,7 @@ namespace Test::PackageManager::Tests
655643
auto deploymentResult{ WaitForDeploymentOperation(deploymentOperation) };
656644
TPMT::VerifyDeploymentSucceeded(deploymentResult, __FILE__, __LINE__, __FUNCTION__);
657645

658-
#ifndef TODO_55967280_EnsurePackageSetReadyAsync_doesnt_register_newer_package_if_lower_version_is_currently_registered
646+
#ifndef TODO_55967171_EnsurePackageSetReadyAsync_doesnt_account_for_package_status
659647
WEX::Logging::Log::Comment(L"Bug https://task.ms/55967171 Ensure*() doesn't account for package status");
660648
VERIFY_IS_FALSE(packageDeploymentManager.IsPackageReady(::TPF::Green::GetPackageFullName()));
661649
VERIFY_IS_FALSE(packageDeploymentManager.IsPackageSetReady(packageSet));

0 commit comments

Comments
 (0)