commit 9a291758856428eeb11c5cd3c05d16b2ae9fbb2c
parent fb0db2ae4ef8d9216b2f501edd9a95b66a9ba423
Author: JohnMcPMS <johnmcp@microsoft.com>
Date: Wed, 8 Apr 2020 23:40:41 -0700
Fixes for deploying optional package while running (#81)
Diffstat:
7 files changed, 191 insertions(+), 82 deletions(-)
diff --git a/src/AppInstallerCLICore/VTSupport.cpp b/src/AppInstallerCLICore/VTSupport.cpp
@@ -24,7 +24,9 @@ namespace AppInstaller::CLI::VirtualTerminal
{
if (!GetConsoleMode(hOut, &m_previousMode))
{
- LOG_LAST_ERROR();
+ // If the user redirects output, the handle will be invalid for this function.
+ // Don't log it in that case.
+ LOG_LAST_ERROR_IF(GetLastError() != ERROR_INVALID_HANDLE);
}
else
{
diff --git a/src/AppInstallerCLITests/MsixInfo.cpp b/src/AppInstallerCLITests/MsixInfo.cpp
@@ -17,10 +17,10 @@ TEST_CASE("MsixInfo_GetPackageFamilyName", "[msixinfo]")
TestDataFile index(s_MsixFile_1);
Msix::MsixInfo msix(index.GetPath().u8string());
- std::string expectedFamilyName = "AppInstallerCLITestsFakeIndex_125rzkzqaqjwj";
- std::string actualFamilyName = msix.GetPackageFamilyName();
+ std::string expectedFullName = "AppInstallerCLITestsFakeIndex_1.0.0.0_neutral__125rzkzqaqjwj";
+ std::string actualFullName = msix.GetPackageFullName();
- REQUIRE(expectedFamilyName == actualFamilyName);
+ REQUIRE(expectedFullName == actualFullName);
}
TEST_CASE("MsixInfo_WriteManifestAndCompareToSelf", "[msixinfo]")
diff --git a/src/AppInstallerCommonCore/Deployment.cpp b/src/AppInstallerCommonCore/Deployment.cpp
@@ -8,6 +8,9 @@
namespace AppInstaller::Deployment
{
+ using namespace winrt::Windows::Foundation;
+ using namespace winrt::Windows::Management::Deployment;
+
namespace
{
size_t GetDeploymentOperationId()
@@ -15,6 +18,72 @@ namespace AppInstaller::Deployment
static std::atomic_size_t s_deploymentId = 0;
return s_deploymentId.fetch_add(1);
}
+
+ void WaitForDeployment(
+ IAsyncOperationWithProgress<DeploymentResult, DeploymentProgress>& deployOperation,
+ size_t id,
+ IProgressCallback& callback)
+ {
+ AsyncOperationProgressHandler<DeploymentResult, DeploymentProgress> progressCallback(
+ [&callback](const IAsyncOperationWithProgress<DeploymentResult, DeploymentProgress>&, DeploymentProgress progress)
+ {
+ callback.OnProgress(progress.percentage, 100, ProgressType::Percent);
+ }
+ );
+
+ // Set progress callback.
+ deployOperation.Progress(progressCallback);
+
+ auto removeCancel = callback.SetCancellationFunction([&]() { deployOperation.Cancel(); });
+ auto deployResult = deployOperation.get();
+
+ if (!SUCCEEDED(deployResult.ExtendedErrorCode()))
+ {
+ AICLI_LOG(Core, Error, << "Deployment failed #" << id << ": " << Utility::ConvertToUTF8(deployResult.ErrorText()));
+
+ // Note that while the format string is char*, it gets converted to wchar before being used and thus %s needs a wchar.
+ THROW_HR_MSG(deployResult.ExtendedErrorCode(), "Install failed: %s", deployResult.ErrorText().c_str());
+ }
+ else
+ {
+ AICLI_LOG(Core, Info, << "Successfully deployed #" << id);
+ }
+ }
+
+ // Type that exists simply to enabled a fire and forget register call as we exit.
+ struct DelayRegisterStorage
+ {
+ DelayRegisterStorage() = default;
+
+ ~DelayRegisterStorage()
+ {
+ PackageManager packageManager;
+
+ for (const auto& fn : m_familyNames)
+ {
+ size_t id = GetDeploymentOperationId();
+ AICLI_LOG(Core, Info, << "Starting RegisterPackageByFamilyName operation #" << id << ": " << fn);
+
+ winrt::hstring familyName = Utility::ConvertToUTF16(fn).c_str();
+ (void)packageManager.RegisterPackageByFamilyNameAsync(
+ familyName,
+ nullptr,
+ winrt::Windows::Management::Deployment::DeploymentOptions::None,
+ nullptr,
+ nullptr);
+ }
+ }
+
+ void Add(std::string_view familyName)
+ {
+ m_familyNames.emplace_back(familyName);
+ }
+
+ private:
+ std::vector<std::string> m_familyNames;
+ };
+
+ DelayRegisterStorage s_delayRegisterStorage;
}
void RequestAddPackageAsync(
@@ -22,9 +91,6 @@ namespace AppInstaller::Deployment
winrt::Windows::Management::Deployment::DeploymentOptions options,
IProgressCallback& callback)
{
- using namespace winrt::Windows::Foundation;
- using namespace winrt::Windows::Management::Deployment;
-
size_t id = GetDeploymentOperationId();
AICLI_LOG(Core, Info, << "Starting RequestAddPackage operation #" << id << ": " << Utility::ConvertToUTF8(uri.AbsoluteUri().c_str()));
@@ -39,36 +105,39 @@ namespace AppInstaller::Deployment
nullptr, /*optionalAndRelatedPackageFamilyNames*/
nullptr /*relatedPackageUris*/);
- AsyncOperationProgressHandler<DeploymentResult, DeploymentProgress> progressCallback(
- [&callback](const IAsyncOperationWithProgress<DeploymentResult, DeploymentProgress>&, DeploymentProgress progress)
- {
- callback.OnProgress(progress.percentage, 100, ProgressType::Percent);
- }
- );
+ WaitForDeployment(deployOperation, id, callback);
+ }
+
+ void StageAndDelayRegisterPackageAsync(
+ std::string_view packageFamilyName,
+ const winrt::Windows::Foundation::Uri& uri,
+ winrt::Windows::Management::Deployment::DeploymentOptions stageOptions,
+ winrt::Windows::Management::Deployment::DeploymentOptions,
+ IProgressCallback& callback)
+ {
+ size_t id = GetDeploymentOperationId();
+ AICLI_LOG(Core, Info, << "Starting StagePackage operation #" << id << ": " << Utility::ConvertToUTF8(uri.AbsoluteUri().c_str()));
- // Set progress callback.
- deployOperation.Progress(progressCallback);
+ PackageManager packageManager;
- auto removeCancel = callback.SetCancellationFunction([&]() { deployOperation.Cancel(); });
- auto deployResult = deployOperation.get();
+ // RequestAddPackageAsync will invoke smart screen.
+ auto deployOperation = packageManager.StagePackageAsync(
+ uri,
+ nullptr, /*dependencyPackageUris*/
+ stageOptions,
+ nullptr, /*targetVolume*/
+ nullptr, /*optionalAndRelatedPackageFamilyNames*/
+ nullptr /*relatedPackageUris*/);
- if (!SUCCEEDED(deployResult.ExtendedErrorCode()))
- {
- AICLI_LOG(Core, Error, << "Deployment failed #" << id << ": " << Utility::ConvertToUTF8(deployResult.ErrorText()));
+ WaitForDeployment(deployOperation, id, callback);
- // Note that while the format string is char*, it gets converted to wchar before being used and thus %s needs a wchar.
- THROW_HR_MSG(deployResult.ExtendedErrorCode(), "Install failed: %s", deployResult.ErrorText().c_str());
- }
- else
- {
- AICLI_LOG(Core, Info, << "Successfully deployed #" << id);
- }
+ s_delayRegisterStorage.Add(packageFamilyName);
}
- void RemovePackageFireAndForget(winrt::hstring packageFullName)
+ void RemovePackageFireAndForget(std::string_view packageFullName)
{
- using namespace winrt::Windows::Management::Deployment;
PackageManager packageManager;
- (void)packageManager.RemovePackageAsync(packageFullName, RemovalOptions::None);
+ winrt::hstring fullName = Utility::ConvertToUTF16(packageFullName).c_str();
+ (void)packageManager.RemovePackageAsync(fullName, RemovalOptions::None);
}
}
diff --git a/src/AppInstallerCommonCore/MsixInfo.cpp b/src/AppInstallerCommonCore/MsixInfo.cpp
@@ -188,6 +188,44 @@ namespace AppInstaller::Msix
THROW_IF_FAILED(appxFactory->CreateManifestReader(inputStream, reader));
}
+ std::string GetPackageFamilyNameFromFullName(std::string_view fullName)
+ {
+ std::wstring result;
+ result.resize(PACKAGE_FAMILY_NAME_MAX_LENGTH + 1);
+ UINT32 size = static_cast<UINT32>(result.size());
+ THROW_IF_WIN32_ERROR(PackageFamilyNameFromFullName(Utility::ConvertToUTF16(fullName).c_str(), &size, &result[0]));
+ result.resize(size - 1);
+ return Utility::ConvertToUTF8(result);
+ }
+
+ std::optional<std::filesystem::path> GetPackageLocationFromFullName(std::string_view fullName)
+ {
+ std::wstring fn = Utility::ConvertToUTF16(fullName);
+
+ UINT32 length = 0;
+ LONG returnVal = GetStagedPackagePathByFullName(fn.c_str(), &length, nullptr);
+ if (returnVal != ERROR_INSUFFICIENT_BUFFER)
+ {
+ LOG_WIN32(returnVal);
+ return {};
+ }
+
+ THROW_HR_IF(E_UNEXPECTED, length == 0);
+
+ std::wstring result;
+ result.resize(length);
+
+ returnVal = GetStagedPackagePathByFullName(fn.c_str(), &length, &result[0]);
+ if (returnVal != ERROR_SUCCESS)
+ {
+ LOG_WIN32(returnVal);
+ return {};
+ }
+
+ result.resize(length - 1);
+ return { result };
+ }
+
MsixInfo::MsixInfo(std::string_view uriStr)
{
if (Utility::IsUrlRemote(uriStr))
@@ -255,7 +293,7 @@ namespace AppInstaller::Msix
return signatureContent;
}
- std::string MsixInfo::GetPackageFamilyName()
+ std::string MsixInfo::GetPackageFullName()
{
ComPtr<IAppxManifestPackageId> packageId;
if (m_isBundle)
@@ -271,10 +309,10 @@ namespace AppInstaller::Msix
THROW_IF_FAILED(manifestReader->GetPackageId(&packageId));
}
- wil::unique_cotaskmem_string familyName;
- THROW_IF_FAILED(packageId->GetPackageFamilyName(&familyName));
+ wil::unique_cotaskmem_string fullName;
+ THROW_IF_FAILED(packageId->GetPackageFullName(&fullName));
- return Utility::ConvertToUTF8(familyName.get());
+ return Utility::ConvertToUTF8(fullName.get());
}
bool MsixInfo::IsNewerThan(const std::filesystem::path& otherManifest)
diff --git a/src/AppInstallerCommonCore/Public/AppInstallerDeployment.h b/src/AppInstallerCommonCore/Public/AppInstallerDeployment.h
@@ -13,8 +13,18 @@ namespace AppInstaller::Deployment
winrt::Windows::Management::Deployment::DeploymentOptions options,
IProgressCallback& callback);
+ // Stages the package, and then attempts to register it without waiting.
+ // This enables us to work around the fact that we cannot call SetPackageInUse,
+ // and thus cannot actually update an optional package while we are running.
+ void StageAndDelayRegisterPackageAsync(
+ std::string_view packageFamilyName,
+ const winrt::Windows::Foundation::Uri& uri,
+ winrt::Windows::Management::Deployment::DeploymentOptions stageOptions,
+ winrt::Windows::Management::Deployment::DeploymentOptions registerOptions,
+ IProgressCallback& callback);
+
// Calls winrt::Windows::Management::Deployment::PackageManager::RemovePackageAsync,
// but *DOES NOT WAIT FOR A RESULT*. As this is used for removing an optional package
- // we will simply complete our actions
- void RemovePackageFireAndForget(winrt::hstring packageFullName);
+ // we will simply complete our actions and exit the process.
+ void RemovePackageFireAndForget(std::string_view packageFullName);
}
diff --git a/src/AppInstallerCommonCore/Public/AppInstallerMsixInfo.h b/src/AppInstallerCommonCore/Public/AppInstallerMsixInfo.h
@@ -5,6 +5,7 @@
#include <AppxPackaging.h>
#include <wrl/client.h>
#include <filesystem>
+#include <optional>
#include <string>
#include <string_view>
#include <vector>
@@ -28,6 +29,12 @@ namespace AppInstaller::Msix
IStream* inputStream,
IAppxManifestReader** reader);
+ // Gets the package family name from the given full name.
+ std::string GetPackageFamilyNameFromFullName(std::string_view fullName);
+
+ // Gets the package location from the given full name.
+ std::optional<std::filesystem::path> GetPackageLocationFromFullName(std::string_view fullName);
+
// MsixInfo class handles all appx/msix related query.
struct MsixInfo
{
@@ -47,8 +54,8 @@ namespace AppInstaller::Msix
// Full content of AppxSignature.p7x
std::vector<byte> GetSignature();
- // Gets the package family name.
- std::string GetPackageFamilyName();
+ // Gets the package full name.
+ std::string GetPackageFullName();
// Gets a value indicating whether the referenced info is newer than the given manifest.
bool IsNewerThan(const std::filesystem::path& otherManifest);
diff --git a/src/AppInstallerRepositoryCore/Microsoft/PreIndexedPackageSourceFactory.cpp b/src/AppInstallerRepositoryCore/Microsoft/PreIndexedPackageSourceFactory.cpp
@@ -35,6 +35,13 @@ namespace AppInstaller::Repository::Microsoft
std::string GetPackageFamilyNameFromDetails(const SourceDetails& details)
{
THROW_HR_IF(E_UNEXPECTED, details.Data.empty());
+ return Msix::GetPackageFamilyNameFromFullName(details.Data);
+ }
+
+ // Gets the package full name from the details.
+ std::string GetPackageFullNameFromDetails(const SourceDetails& details)
+ {
+ THROW_HR_IF(E_UNEXPECTED, details.Data.empty());
return details.Data;
}
@@ -86,9 +93,9 @@ namespace AppInstaller::Repository::Microsoft
// If not initialized, we need to open the package and get the family name.
Msix::MsixInfo packageInfo(packageLocation);
THROW_HR_IF(APPINSTALLER_CLI_ERROR_PACKAGE_IS_BUNDLE, packageInfo.GetIsBundle());
- details.Data = packageInfo.GetPackageFamilyName();
+ details.Data = packageInfo.GetPackageFullName();
- AICLI_LOG(Repo, Info, << "Found package family name: " << details.Name << " => " << details.Data);
+ AICLI_LOG(Repo, Info, << "Found package full name: " << details.Name << " => " << details.Data);
}
auto lock = Synchronization::CrossProcessReaderWriteLock::LockForWrite(CreateNameForCPRWL(details));
@@ -115,44 +122,21 @@ namespace AppInstaller::Repository::Microsoft
struct PackagedContextFactory : public PreIndexedFactoryBase
{
// *Should only be called when under a CrossProcessReaderWriteLock*
- auto GetPackageFromDetails(const SourceDetails& details)
+ std::optional<std::filesystem::path> GetPackageLocationFromDetails(const SourceDetails& details)
{
- using Package = winrt::Windows::ApplicationModel::Package;
-
- std::wstring packageFamilyName = Utility::ConvertToUTF16(GetPackageFamilyNameFromDetails(details));
-
- Package currentPackage = Package::Current();
- auto dependencies = currentPackage.Dependencies();
- for (uint32_t i = 0; i < dependencies.Size(); ++i)
- {
- Package package = dependencies.GetAt(i);
- if (package.Id().FamilyName() == packageFamilyName)
- {
- if (package.IsOptional())
- {
- return package;
- }
- else
- {
- AICLI_LOG(Repo, Error, << "Source references a non-optional package: " << details.Name << " => " << GetPackageFamilyNameFromDetails(details));
- return Package{ nullptr };
- }
- }
- }
- AICLI_LOG(Repo, Error, << "Source references an unknown package: " << details.Name << " => " << GetPackageFamilyNameFromDetails(details));
- return Package{ nullptr };
+ return Msix::GetPackageLocationFromFullName(GetPackageFullNameFromDetails(details));
}
std::shared_ptr<ISource> CreateInternal(const SourceDetails& details, Synchronization::CrossProcessReaderWriteLock&& lock) override
{
- auto optionalPackage = GetPackageFromDetails(details);
+ auto optionalPackage = GetPackageLocationFromDetails(details);
if (!optionalPackage)
{
- AICLI_LOG(Repo, Info, << "Package not found by family name " << details.Data);
+ AICLI_LOG(Repo, Info, << "Package not found " << details.Data);
THROW_HR(APPINSTALLER_CLI_ERROR_SOURCE_DATA_MISSING);
}
- std::filesystem::path packageLocation = optionalPackage.InstalledLocation().Path().c_str();
+ std::filesystem::path packageLocation = optionalPackage.value();
packageLocation /= s_PreIndexedPackageSourceFactory_IndexFileName;
SQLiteIndex index = SQLiteIndex::Open(packageLocation.u8string(), SQLiteIndex::OpenDisposition::Immutable);
@@ -164,7 +148,7 @@ namespace AppInstaller::Repository::Microsoft
{
// Check if the package is newer before calling into deployment.
// This can save us a lot of time over letting deployment detect same version.
- auto optionalPackage = GetPackageFromDetails(details);
+ auto optionalPackage = GetPackageLocationFromDetails(details);
if (optionalPackage)
{
Msix::MsixInfo packageInfo(packageLocation);
@@ -176,7 +160,7 @@ namespace AppInstaller::Repository::Microsoft
return;
}
- std::filesystem::path packagePath = optionalPackage.InstalledLocation().Path().c_str();
+ std::filesystem::path packagePath = optionalPackage.value();
std::filesystem::path manifestPath = packagePath / s_PreIndexedPackageSourceFactory_AppxManifestFileName;
if (!packageInfo.IsNewerThan(manifestPath))
@@ -184,6 +168,8 @@ namespace AppInstaller::Repository::Microsoft
AICLI_LOG(Repo, Info, << "Remote source data was not newer than existing, no update needed");
return;
}
+
+ details.Data = packageInfo.GetPackageFullName();
}
if (progress.IsCancelled())
@@ -193,23 +179,20 @@ namespace AppInstaller::Repository::Microsoft
}
winrt::Windows::Foundation::Uri uri(Utility::ConvertToUTF16(packageLocation));
- Deployment::RequestAddPackageAsync(uri, winrt::Windows::Management::Deployment::DeploymentOptions::None, progress);
+ Deployment::StageAndDelayRegisterPackageAsync(
+ GetPackageFamilyNameFromDetails(details),
+ uri,
+ winrt::Windows::Management::Deployment::DeploymentOptions::None,
+ winrt::Windows::Management::Deployment::DeploymentOptions::None,
+ progress);
}
void RemoveInternal(const SourceDetails& details, IProgressCallback&) override
{
- // Get the package referenced by the details
- auto optionalPackage = GetPackageFromDetails(details);
- if (!optionalPackage)
- {
- AICLI_LOG(Repo, Info, << "Package not found by family name " << details.Data);
- return;
- }
-
// Begin package removal, but let it run its course without waiting.
// This pattern is required due to the inability to use SetInUseAsync from a full trust process.
- AICLI_LOG(Repo, Info, << "Removing package " << Utility::ConvertToUTF8(optionalPackage.Id().FullName()));
- Deployment::RemovePackageFireAndForget(optionalPackage.Id().FullName());
+ AICLI_LOG(Repo, Info, << "Removing package " << GetPackageFullNameFromDetails(details));
+ Deployment::RemovePackageFireAndForget(GetPackageFullNameFromDetails(details));
}
};
@@ -232,7 +215,7 @@ namespace AppInstaller::Repository::Microsoft
if (!std::filesystem::exists(packageLocation))
{
- AICLI_LOG(Repo, Info, << "Data not found by family name " << details.Data);
+ AICLI_LOG(Repo, Info, << "Data not found at " << packageLocation);
THROW_HR(APPINSTALLER_CLI_ERROR_SOURCE_DATA_MISSING);
}
