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 in-proc Com state separation #2068

Merged
merged 9 commits into from
Apr 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/actions/spelling/expect.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
abcd
accepteula
adjacents
activatable
adml
Expand Down
2 changes: 1 addition & 1 deletion azure-pipelines.nuget.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ parameters:
type: string

pool:
vmImage: "windows-2019"
vmImage: "windows-latest"

variables:
solution: "src/AppInstallerCLI.sln"
Expand Down
22 changes: 14 additions & 8 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ pr:
- src/*

pool:
vmImage: 'windows-2019'
vmImage: 'windows-latest'

variables:
solution: 'src\AppInstallerCLI.sln'
Expand Down Expand Up @@ -143,13 +143,19 @@ jobs:
inputs:
packageFeedSelector: 'nugetOrg'

# - task: CmdLine@2
# displayName: Run Unit Tests Unpackaged
# inputs:
# script: |
# AppInstallerCLITests.exe -logto $(artifactsDir)\AICLI-Unpackaged.log -s -r junit -o $(artifactsDir)\TEST-AppInstallerCLI-Unpackaged.xml
# workingDirectory: '$(buildOutDir)\AppInstallerCLITests'
# continueOnError: true
- task: DownloadSecureFile@1
name: PsExec
displayName: 'Download PsExec.exe'
inputs:
secureFile: 'PsExec.exe'

- task: CmdLine@2
displayName: Run Unit Tests Unpackaged Under System Context
inputs:
script: |
$(PsExec.secureFilePath) -accepteula -s -i $(buildOutDir)\AppInstallerCLITests\AppInstallerCLITests.exe -logto $(artifactsDir)\AICLI-Unpackaged-System.log -s -r junit -o $(artifactsDir)\TEST-AppInstallerCLI-Unpackaged-System.xml
workingDirectory: '$(buildOutDir)\AppInstallerCLITests'
continueOnError: true

- task: PowerShell@2
displayName: Run Unit Tests Packaged
Expand Down
2 changes: 2 additions & 0 deletions src/AppInstallerCommonCore/Public/AppInstallerRuntime.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ namespace AppInstaller::Runtime
PortableAppMachineRootX86,
};

void SetRuntimePathStateName(std::string name);

// Gets the path to the requested location.
std::filesystem::path GetPathTo(PathName path);

Expand Down
7 changes: 5 additions & 2 deletions src/AppInstallerCommonCore/Public/winget/UserSettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ namespace AppInstaller::Settings
Standard,
// Loaded settings.json.backup
Backup,
// Loaded from custom settings content
Custom,
};

// The visual style of the progress bar.
Expand Down Expand Up @@ -172,7 +174,7 @@ namespace AppInstaller::Settings
bool IsFieldWarning = true;
};

static UserSettings const& Instance();
static UserSettings const& Instance(const std::optional<std::string>& content = std::nullopt);

static std::filesystem::path SettingsFilePath();

Expand Down Expand Up @@ -205,10 +207,11 @@ namespace AppInstaller::Settings
std::vector<Warning> m_warnings;
std::map<Setting, details::SettingVariant> m_settings;

UserSettings();
UserSettings(const std::optional<std::string>& content = std::nullopt);
~UserSettings() = default;
};

const UserSettings* TryGetUser();
UserSettings const& User();
bool TryInitializeCustomUserSettings(std::string content);
}
34 changes: 34 additions & 0 deletions src/AppInstallerCommonCore/Runtime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ namespace AppInstaller::Runtime
constexpr std::string_view s_SecureSettings_Relative_Packaged = "pkg"sv;
#endif
constexpr std::string_view s_PreviewBuildSuffix = "-preview"sv;
constexpr std::string_view s_RuntimePath_Unpackaged_DefaultState = "defaultState"sv;

static std::optional<std::string> s_runtimePathStateName;
static wil::srwlock s_runtimePathStateNameLock;

// Gets a boolean indicating whether the current process has identity.
bool DoesCurrentProcessHaveIdentity()
Expand Down Expand Up @@ -139,6 +143,24 @@ namespace AppInstaller::Runtime
THROW_IF_WIN32_BOOL_FALSE(ConvertSidToStringSidW(userToken->User.Sid, &sidString));
return { sidString.get() };
}

std::string GetRuntimePathStateName()
{
std::string result;
auto lock = s_runtimePathStateNameLock.lock_shared();

if (s_runtimePathStateName.has_value())
{
result = s_runtimePathStateName.value();
}

if (Utility::IsEmptyOrWhitespace(result))
{
result = s_RuntimePath_Unpackaged_DefaultState;
}

return result;
}
}

bool IsRunningInPackagedContext()
Expand Down Expand Up @@ -239,6 +261,13 @@ namespace AppInstaller::Runtime
}
#endif

void SetRuntimePathStateName(std::string name)
{
auto suitablePathPart = MakeSuitablePathPart(name);
auto lock = s_runtimePathStateNameLock.lock_exclusive();
s_runtimePathStateName.emplace(std::move(suitablePathPart));
}

std::filesystem::path GetPathTo(PathName path)
{
std::filesystem::path result;
Expand Down Expand Up @@ -358,26 +387,31 @@ namespace AppInstaller::Runtime
{
result = GetPathToUserTemp();
result /= s_DefaultTempDirectory;
result /= GetRuntimePathStateName();
}
break;
case PathName::DefaultLogLocationForDisplay:
result.assign("%TEMP%");
result /= s_DefaultTempDirectory;
result /= GetRuntimePathStateName();
create = false;
break;
case PathName::LocalState:
result = GetPathToAppDataDir(s_AppDataDir_State);
result /= GetRuntimePathStateName();
break;
case PathName::StandardSettings:
case PathName::UserFileSettings:
result = GetPathToAppDataDir(s_AppDataDir_Settings);
result /= GetRuntimePathStateName();
break;
case PathName::SecureSettings:
result = GetKnownFolderPath(FOLDERID_ProgramData);
result /= s_SecureSettings_Base;
result /= GetUserSID();
result /= s_SecureSettings_UserRelative;
result /= s_SecureSettings_Relative_Unpackaged;
result /= GetRuntimePathStateName();
create = false;
break;
case PathName::UserProfile:
Expand Down
99 changes: 66 additions & 33 deletions src/AppInstallerCommonCore/UserSettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,25 +72,31 @@ namespace AppInstaller::Settings
return convertedValue;
}

std::optional<Json::Value> ParseSettingsContent(const std::string& content, std::string_view settingName, std::vector<UserSettings::Warning>& warnings)
{
Json::Value root;
Json::CharReaderBuilder builder;
const std::unique_ptr<Json::CharReader> reader(builder.newCharReader());
std::string error;

if (reader->parse(content.c_str(), content.c_str() + content.size(), &root, &error))
{
return root;
}

AICLI_LOG(Core, Error, << "Error parsing " << settingName << ": " << error);
warnings.emplace_back(StringResource::String::SettingsWarningParseError, settingName, error, false);

return {};
}

std::optional<Json::Value> ParseFile(const StreamDefinition& setting, std::vector<UserSettings::Warning>& warnings)
{
auto stream = Stream{ setting }.Get();
if (stream)
{
Json::Value root;
Json::CharReaderBuilder builder;
const std::unique_ptr<Json::CharReader> reader(builder.newCharReader());

std::string settingsContentStr = Utility::ReadEntireStream(*stream);
std::string error;

if (reader->parse(settingsContentStr.c_str(), settingsContentStr.c_str() + settingsContentStr.size(), &root, &error))
{
return root;
}

AICLI_LOG(Core, Error, << "Error parsing " << setting.Name << ": " << error);
warnings.emplace_back(StringResource::String::SettingsWarningParseError, setting.Name, error, false);
return ParseSettingsContent(settingsContentStr, setting.Name, warnings);
}

return {};
Expand Down Expand Up @@ -382,7 +388,7 @@ namespace AppInstaller::Settings
static std::atomic_bool s_userSettingsInitialized{ false };
static std::atomic_bool s_userSettingsInInitialization{ false };

UserSettings const& UserSettings::Instance()
UserSettings const& UserSettings::Instance(const std::optional<std::string>& content)
{
#ifndef AICLI_DISABLE_TEST_HOOKS
if (s_UserSettings_Override)
Expand All @@ -395,7 +401,7 @@ namespace AppInstaller::Settings
s_userSettingsInInitialization = true;
}

static UserSettings userSettings;
static UserSettings userSettings(content);
s_userSettingsInitialized = true;
s_userSettingsInInitialization = false;

Expand Down Expand Up @@ -423,40 +429,67 @@ namespace AppInstaller::Settings
return UserSettings::Instance();
}

UserSettings::UserSettings() : m_type(UserSettingsType::Default)
bool TryInitializeCustomUserSettings(std::string content)
{
if (s_userSettingsInitialized || s_userSettingsInInitialization)
Copy link
Member

Choose a reason for hiding this comment

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

This is probably the right approach for now, but we should have a test to make sure that our suggested method for initialization is working so that we don't load the settings earlier than that.

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 verified manually it's working as expected, and it should and will be part of Com e2e test.

{
return false;
}

return UserSettings::Instance(std::move(content)).GetType() == UserSettingsType::Custom;
}

UserSettings::UserSettings(const std::optional<std::string>& content) : m_type(UserSettingsType::Default)
{
Json::Value settingsRoot = Json::Value::nullSingleton();

// Settings can be loaded from settings.json or settings.json.backup files.
// 0 - Use default (empty) settings if disabled by group policy.
// 1 - Use settings.json if exists and passes parsing.
// 2 - Use settings.backup.json if settings.json fails to parse.
// 3 - Use default (empty) if both settings files fail to load.
// if
// 1 - Use passed in settings content if available.
// else
// 2 - Use settings.json if exists and passes parsing.
// 3 - Use settings.backup.json if settings.json fails to parse.
// finally
// 4 - Use default (empty) if both settings files fail to load.

if (!GroupPolicies().IsEnabled(TogglePolicy::Policy::Settings))
{
AICLI_LOG(Core, Info, << "Ignoring settings file due to group policy. Using default values.");
return;
}

auto settingsJson = ParseFile(Stream::PrimaryUserSettings, m_warnings);
if (settingsJson.has_value())
if (content.has_value())
{
AICLI_LOG(Core, Info, << "Settings loaded from " << Stream::PrimaryUserSettings.Name);
m_type = UserSettingsType::Standard;
settingsRoot = settingsJson.value();
auto settingsJson = ParseSettingsContent(content.value(), "CustomSettings", m_warnings);
if (settingsJson.has_value())
{
AICLI_LOG(Core, Info, << "Settings loaded from custom settings");
m_type = UserSettingsType::Custom;
settingsRoot = settingsJson.value();
}
}

// Settings didn't parse or doesn't exist, try with backup.
if (settingsRoot.isNull())
else
{
auto settingsBackupJson = ParseFile(Stream::BackupUserSettings, m_warnings);
if (settingsBackupJson.has_value())
auto settingsJson = ParseFile(Stream::PrimaryUserSettings, m_warnings);
if (settingsJson.has_value())
{
AICLI_LOG(Core, Info, << "Settings loaded from " << Stream::BackupUserSettings.Name);
m_warnings.emplace_back(StringResource::String::SettingsWarningLoadedBackupSettings);
m_type = UserSettingsType::Backup;
settingsRoot = settingsBackupJson.value();
AICLI_LOG(Core, Info, << "Settings loaded from " << Stream::PrimaryUserSettings.Name);
m_type = UserSettingsType::Standard;
settingsRoot = settingsJson.value();
}

// Settings didn't parse or doesn't exist, try with backup.
if (settingsRoot.isNull())
{
auto settingsBackupJson = ParseFile(Stream::BackupUserSettings, m_warnings);
if (settingsBackupJson.has_value())
{
AICLI_LOG(Core, Info, << "Settings loaded from " << Stream::BackupUserSettings.Name);
m_warnings.emplace_back(StringResource::String::SettingsWarningLoadedBackupSettings);
m_type = UserSettingsType::Backup;
settingsRoot = settingsBackupJson.value();
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#pragma once
#include "PackageManagerSettings.g.h"

namespace winrt::Microsoft::Management::Deployment::factory_implementation
{
struct PackageManagerSettings : PackageManagerSettingsT<PackageManagerSettings, implementation::PackageManagerSettings>
{
auto ActivateInstance() const
{
return winrt::create_instance<winrt::Microsoft::Management::Deployment::PackageManagerSettings>(__uuidof(implementation::PackageManagerSettings), CLSCTX_ALL);
}
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@
</Link>
</ItemDefinitionGroup>
<ItemGroup>
<ClInclude Include="Client.PackageManagerSettings.h" />
<ClInclude Include="Client.UninstallOptions.h" />
<ClInclude Include="pch.h" />
<ClInclude Include="Client.CreateCompositePackageCatalogOptions.h" />
Expand All @@ -141,6 +142,7 @@
<ClCompile Include="CreateCompositePackageCatalogOptions.cpp" />
<ClCompile Include="FindPackagesOptions.cpp" />
<ClCompile Include="InstallOptions.cpp" />
<ClCompile Include="PackageManagerSettings.cpp" />
<ClCompile Include="PackageMatchFilter.cpp" />
<ClCompile Include="pch.cpp">
<PrecompiledHeader>Create</PrecompiledHeader>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
#include "pch.h"
#pragma warning( push )
#pragma warning ( disable : 4467 6388)
#include <PackageManagerSettings.h>
#include <Client.PackageManagerSettings.h>
#pragma warning( pop )
#include "PackageManagerSettings.g.cpp"

namespace winrt::Microsoft::Management::Deployment::implementation
{
bool PackageManagerSettings::SetCallerIdentifier(hstring const&)
{
throw hresult_not_implemented();
}
bool PackageManagerSettings::SetStateIdentifier(hstring const&)
{
throw hresult_not_implemented();
}
bool PackageManagerSettings::SetUserSettings(hstring const&)
{
throw hresult_not_implemented();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@
clsid="{57DC8962-7343-42CD-B91C-04F6A25DB1D0}"
threadingModel="Both"
description="PackageMatchFilter"/>
<comClass
clsid="{80CF9D63-5505-4342-B9B4-BB87895CA8BB}"
threadingModel="Both"
description="PackageManagerSettings"/>
</file>

</assembly>
Loading