From f231adc1673a902e3be1f012411de7b34f2d6e4c Mon Sep 17 00:00:00 2001 From: David Bennett Date: Mon, 1 Dec 2025 08:26:35 -0800 Subject: [PATCH 01/19] Add Edit Source --- .../Commands/SourceCommand.cpp | 34 +++++ .../Commands/SourceCommand.h | 15 +++ src/AppInstallerCLICore/Resources.h | 4 + .../Workflows/SourceFlow.cpp | 33 +++++ .../Workflows/SourceFlow.h | 6 + .../Shared/Strings/en-us/winget.resw | 14 ++ src/AppInstallerCLITests/Sources.cpp | 45 +++++++ src/AppInstallerCLITests/TestSource.cpp | 15 +++ src/AppInstallerCLITests/TestSource.h | 4 + .../ConfigurableTestSourceFactory.cpp | 5 + .../PreIndexedPackageSourceFactory.cpp | 9 ++ .../PredefinedInstalledSourceFactory.cpp | 5 + .../PredefinedWriteableSourceFactory.cpp | 5 + .../PackageTrackingCatalog.cpp | 5 + .../Public/winget/RepositorySource.h | 10 ++ .../RepositorySource.cpp | 73 +++++++++++ .../Rest/RestSourceFactory.cpp | 6 + .../SourceFactory.h | 4 + src/AppInstallerRepositoryCore/SourceList.cpp | 120 ++++++++++++++++++ src/AppInstallerRepositoryCore/SourceList.h | 13 +- 20 files changed, 424 insertions(+), 1 deletion(-) diff --git a/src/AppInstallerCLICore/Commands/SourceCommand.cpp b/src/AppInstallerCLICore/Commands/SourceCommand.cpp index 4b0b557ffb..871758938f 100644 --- a/src/AppInstallerCLICore/Commands/SourceCommand.cpp +++ b/src/AppInstallerCLICore/Commands/SourceCommand.cpp @@ -21,6 +21,7 @@ namespace AppInstaller::CLI std::make_unique(FullName()), std::make_unique(FullName()), std::make_unique(FullName()), + std::make_unique(FullName()), std::make_unique(FullName()), std::make_unique(FullName()), }); @@ -312,4 +313,37 @@ namespace AppInstaller::CLI Workflow::GetSourceListWithFilter << Workflow::ExportSourceList; } + + // Source Edit Command + + std::vector SourceEditCommand::GetArguments() const + { + return { + Argument::ForType(Args::Type::SourceName).SetRequired(true), + Argument::ForType(Args::Type::SourceExplicit), + }; + } + + Resource::LocString SourceEditCommand::ShortDescription() const + { + return { Resource::String::SourceEditCommandShortDescription }; + } + + Resource::LocString SourceEditCommand::LongDescription() const + { + return { Resource::String::SourceEditCommandLongDescription }; + } + + Utility::LocIndView SourceEditCommand::HelpLink() const + { + return s_SourceCommand_HelpLink; + } + + void SourceEditCommand::ExecuteInternal(Context& context) const + { + context << + Workflow::EnsureRunningAsAdmin << + Workflow::GetSourceListWithFilter << + Workflow::EditSources; + } } diff --git a/src/AppInstallerCLICore/Commands/SourceCommand.h b/src/AppInstallerCLICore/Commands/SourceCommand.h index 4656bfb62c..2385155126 100644 --- a/src/AppInstallerCLICore/Commands/SourceCommand.h +++ b/src/AppInstallerCLICore/Commands/SourceCommand.h @@ -121,4 +121,19 @@ namespace AppInstaller::CLI protected: void ExecuteInternal(Execution::Context& context) const override; }; + + struct SourceEditCommand final : public Command + { + SourceEditCommand(std::string_view parent) : Command("edit", {}, parent, Settings::TogglePolicy::Policy::AllowedSources) {} + + std::vector GetArguments() const override; + + Resource::LocString ShortDescription() const override; + Resource::LocString LongDescription() const override; + + Utility::LocIndView HelpLink() const override; + + protected: + void ExecuteInternal(Execution::Context& context) const override; + }; } diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h index 7f07b0c142..c81f79889c 100644 --- a/src/AppInstallerCLICore/Resources.h +++ b/src/AppInstallerCLICore/Resources.h @@ -687,6 +687,10 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(SourceArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(SourceCommandLongDescription); WINGET_DEFINE_RESOURCE_STRINGID(SourceCommandShortDescription); + WINGET_DEFINE_RESOURCE_STRINGID(SourceEditCommandLongDescription); + WINGET_DEFINE_RESOURCE_STRINGID(SourceEditCommandShortDescription); + WINGET_DEFINE_RESOURCE_STRINGID(SourceEditOne); + WINGET_DEFINE_RESOURCE_STRINGID(SourceEditNoChanges); WINGET_DEFINE_RESOURCE_STRINGID(SourceExplicitArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(SourceExportCommandLongDescription); WINGET_DEFINE_RESOURCE_STRINGID(SourceExportCommandShortDescription); diff --git a/src/AppInstallerCLICore/Workflows/SourceFlow.cpp b/src/AppInstallerCLICore/Workflows/SourceFlow.cpp index 7d8df61ffd..13b75d5e5e 100644 --- a/src/AppInstallerCLICore/Workflows/SourceFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/SourceFlow.cpp @@ -272,6 +272,39 @@ namespace AppInstaller::CLI::Workflow } } + void EditSources(Execution::Context& context) + { + // We are assuming there is only one match, as SourceName is a required parameter. + const std::vector& sources = context.Get(); + bool isExplicit = context.Args.Contains(Args::Type::SourceExplicit); + for (const auto& sd : sources) + { + // Get the current source with this name. + Repository::Source currentSource{ sd.Name }; + + // Only permitting Explicit to be edited at this time. + Repository::Source source{ sd.Name, isExplicit }; + + // Is anything being changed?? + if (!source.IsEditOfSource(currentSource)) + { + context.Reporter.Info() << Resource::String::SourceEditNoChanges(Utility::LocIndView{ sd.Name }) << std::endl; + continue; + } + + context.Reporter.Info() << Resource::String::SourceEditOne(Utility::LocIndView{ sd.Name }, Utility::LocIndView{ Utility::ConvertBoolToString(isExplicit) }) << std::endl; + auto editFunction = [&](IProgressCallback& progress)->bool { return source.Edit(progress); }; + if (context.Reporter.ExecuteWithProgress(editFunction)) + { + context.Reporter.Info() << Resource::String::Done << std::endl; + } + else + { + context.Reporter.Info() << Resource::String::Cancelled << std::endl; + } + } + } + void QueryUserForSourceReset(Execution::Context& context) { if (!context.Args.Contains(Execution::Args::Type::ForceSourceReset)) diff --git a/src/AppInstallerCLICore/Workflows/SourceFlow.h b/src/AppInstallerCLICore/Workflows/SourceFlow.h index f3d651148c..e3a92a04a8 100644 --- a/src/AppInstallerCLICore/Workflows/SourceFlow.h +++ b/src/AppInstallerCLICore/Workflows/SourceFlow.h @@ -83,4 +83,10 @@ namespace AppInstaller::CLI::Workflow // Inputs: None // Outputs: None void ForceInstalledCacheUpdate(Execution::Context& context); + + // Edits a source in SourceList. + // Required Args: SourceName + // Inputs: SourceList + // Outputs: None + void EditSources(Execution::Context& context); } diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index 02388bd06f..8bb77f2743 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -545,6 +545,20 @@ They can be configured through the settings file 'winget settings'. Manage sources of packages + + Edit properties of an existing source. A source provides the data for you to discover and install packages. Only add a new source if you trust it as a secure location. + + + Edit properties of a source + + + Changing source: {0} to Explicit={1}... + {Locked="{0}","{1}"} Message displayed to inform the user about a registered repository source that is currently being edited. {0} is a placeholder replaced by the repository source name. {1} is a placeholder replaced by the changed value. + + + The source named '{0}' is already in that desired state. + {Locked="{0}"} Message displayed to inform the user about a registered repository source that is currently being edited. {0} is a placeholder replaced by the repository source name. + Argument Value given to source. diff --git a/src/AppInstallerCLITests/Sources.cpp b/src/AppInstallerCLITests/Sources.cpp index 1ca3ee51ab..b9fd72b15d 100644 --- a/src/AppInstallerCLITests/Sources.cpp +++ b/src/AppInstallerCLITests/Sources.cpp @@ -66,6 +66,17 @@ constexpr std::string_view s_SingleSource = R"( IsTombstone: false )"sv; +constexpr std::string_view s_SingleSourceOverride = R"( +Sources: + - Name: winget-font + Type: "" + Arg: "" + Data: "" + IsTombstone: false + IsOverride: true + Explicit: false +)"sv; + constexpr std::string_view s_SingleSourceMetadata = R"( Sources: - Name: testName @@ -291,6 +302,40 @@ TEST_CASE("RepoSources_DefaultSourcesTombstoned", "[sources]") REQUIRE(sources.empty()); } + +TEST_CASE("RepoSources_DefaultSourceOverride", "[sources]") +{ + SetSetting(Stream::UserSources, s_EmptySources); + + // Default font has explicit to true. + // Font is at index 2 as it is the third one added. + auto beforeOverride = GetSources(); + REQUIRE(beforeOverride.size() == c_DefaultSourceCount); + REQUIRE(beforeOverride[2].Name == "winget-font"); + REQUIRE(beforeOverride[2].Arg == "https://cdn.winget.microsoft.com/fonts"); + REQUIRE(beforeOverride[2].Data == "Microsoft.Winget.Fonts.Source_8wekyb3d8bbwe"); + REQUIRE(beforeOverride[2].Type == "Microsoft.PreIndexed.Package"); + REQUIRE(beforeOverride[2].Origin == SourceOrigin::Default); + REQUIRE(beforeOverride[2].Explicit == true); + + SetSetting(Stream::UserSources, s_SingleSourceOverride); + auto afterOverride = GetSources(); + + // The override will change the index value as the Default will be replaced by the override. + // User sources have higher priority so the override will be at index 0. + // We expect the same count, and the Name, Arg, Data, and Type properties to all be identical. + // Only the name is defined in the override setting so all others should be propertly populated. + REQUIRE(afterOverride.size() == c_DefaultSourceCount); + REQUIRE(afterOverride[0].Name == beforeOverride[2].Name); + REQUIRE(afterOverride[0].Arg == beforeOverride[2].Arg); + REQUIRE(afterOverride[0].Data == beforeOverride[2].Data); + REQUIRE(afterOverride[0].Type == beforeOverride[2].Type); + + // The only properties we expect to be different are the Origin, which is now User, and Explicit. + REQUIRE(afterOverride[0].Origin == SourceOrigin::User); + REQUIRE(afterOverride[0].Explicit == false); +} + TEST_CASE("RepoSources_SingleSource", "[sources]") { SetSetting(Stream::UserSources, s_SingleSource); diff --git a/src/AppInstallerCLITests/TestSource.cpp b/src/AppInstallerCLITests/TestSource.cpp index 6544f99395..0281f2726c 100644 --- a/src/AppInstallerCLITests/TestSource.cpp +++ b/src/AppInstallerCLITests/TestSource.cpp @@ -454,6 +454,15 @@ namespace TestCommon return true; } + bool TestSourceFactory::Edit(const SourceDetails& details, IProgressCallback&) + { + if (OnEdit) + { + OnEdit(details); + } + return true; + } + // Make copies of self when requested. TestSourceFactory::operator std::function()>() { @@ -466,6 +475,12 @@ namespace TestCommon return source.Add(progress); } + bool EditSource(const AppInstaller::Repository::SourceDetails& details, AppInstaller::IProgressCallback& progress) + { + Repository::Source source{ details.Name, details.Explicit }; + return source.Edit(progress); + } + bool UpdateSource(std::string_view name, AppInstaller::IProgressCallback& progress) { Repository::Source source{ name }; diff --git a/src/AppInstallerCLITests/TestSource.h b/src/AppInstallerCLITests/TestSource.h index bc5aca74f7..4dc8a33192 100644 --- a/src/AppInstallerCLITests/TestSource.h +++ b/src/AppInstallerCLITests/TestSource.h @@ -187,6 +187,7 @@ namespace TestCommon using AddFunctor = std::function; using UpdateFunctor = std::function; using RemoveFunctor = std::function; + using EditFunctor = std::function; TestSourceFactory(OpenFunctor open) : OnOpen(std::move(open)) {} TestSourceFactory(OpenFunctorWithCustomHeader open) : OnOpenWithCustomHeader(std::move(open)) {} @@ -197,6 +198,7 @@ namespace TestCommon bool Add(AppInstaller::Repository::SourceDetails& details, AppInstaller::IProgressCallback&) override; bool Update(const AppInstaller::Repository::SourceDetails& details, AppInstaller::IProgressCallback&) override; bool Remove(const AppInstaller::Repository::SourceDetails& details, AppInstaller::IProgressCallback&) override; + bool Edit(const AppInstaller::Repository::SourceDetails& details, AppInstaller::IProgressCallback&) override; // Make copies of self when requested. operator std::function()>(); @@ -207,9 +209,11 @@ namespace TestCommon AddFunctor OnAdd; UpdateFunctor OnUpdate; RemoveFunctor OnRemove; + EditFunctor OnEdit; }; bool AddSource(const AppInstaller::Repository::SourceDetails& details, AppInstaller::IProgressCallback& progress); + bool EditSource(const AppInstaller::Repository::SourceDetails& details, AppInstaller::IProgressCallback& progress); bool UpdateSource(std::string_view name, AppInstaller::IProgressCallback& progress); bool RemoveSource(std::string_view name, AppInstaller::IProgressCallback& progress); AppInstaller::Repository::Source OpenSource(std::string_view name, AppInstaller::IProgressCallback& progress); diff --git a/src/AppInstallerRepositoryCore/Microsoft/ConfigurableTestSourceFactory.cpp b/src/AppInstallerRepositoryCore/Microsoft/ConfigurableTestSourceFactory.cpp index af8a17ebfe..7a91c22c14 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/ConfigurableTestSourceFactory.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/ConfigurableTestSourceFactory.cpp @@ -154,6 +154,11 @@ namespace AppInstaller::Repository::Microsoft { return true; } + + bool Edit(const SourceDetails&, IProgressCallback&) override final + { + return true; + } }; } diff --git a/src/AppInstallerRepositoryCore/Microsoft/PreIndexedPackageSourceFactory.cpp b/src/AppInstallerRepositoryCore/Microsoft/PreIndexedPackageSourceFactory.cpp index bbe0082cb8..cb483a7bb3 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PreIndexedPackageSourceFactory.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/PreIndexedPackageSourceFactory.cpp @@ -254,6 +254,15 @@ namespace AppInstaller::Repository::Microsoft return UpdateBase(details, true, progress); } + bool Edit(const SourceDetails& details, IProgressCallback& progress) override final + { + UNREFERENCED_PARAMETER(details); + UNREFERENCED_PARAMETER(progress); + + // Edit does not change or update the installed packages. + return true; + } + // Retrieves the currently cached version of the package. virtual std::optional GetCurrentVersion(const SourceDetails& details) = 0; diff --git a/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp b/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp index ca93275329..698d503e73 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp @@ -445,6 +445,11 @@ namespace AppInstaller::Repository::Microsoft // Similar to add, remove should never be needed. THROW_HR(E_NOTIMPL); } + + bool Edit(const SourceDetails&, IProgressCallback&) override final + { + THROW_HR(E_NOTIMPL); + } }; } diff --git a/src/AppInstallerRepositoryCore/Microsoft/PredefinedWriteableSourceFactory.cpp b/src/AppInstallerRepositoryCore/Microsoft/PredefinedWriteableSourceFactory.cpp index 53accca45c..c3beccf362 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PredefinedWriteableSourceFactory.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/PredefinedWriteableSourceFactory.cpp @@ -44,6 +44,11 @@ namespace AppInstaller::Repository::Microsoft // Similar to add, remove should never be needed. THROW_HR(E_NOTIMPL); } + + bool Edit(const SourceDetails&, IProgressCallback&) override final + { + THROW_HR(E_NOTIMPL); + } }; struct PredefinedWriteableSourceReference : public ISourceReference diff --git a/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp b/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp index 76a2311cfb..159d70df60 100644 --- a/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp +++ b/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp @@ -125,6 +125,11 @@ namespace AppInstaller::Repository THROW_HR(E_NOTIMPL); } + bool Edit(const SourceDetails&, IProgressCallback&) override final + { + THROW_HR(E_NOTIMPL); + } + bool Remove(const SourceDetails& details, IProgressCallback& progress) override final { THROW_HR_IF(E_INVALIDARG, !Utility::CaseInsensitiveEquals(details.Type, PackageTrackingCatalogSourceFactory::Type())); diff --git a/src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h b/src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h index 2acc163469..aab305e96b 100644 --- a/src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h +++ b/src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h @@ -224,6 +224,9 @@ namespace AppInstaller::Repository // Constructor for a source to be added. Source(std::string_view name, std::string_view arg, std::string_view type, SourceTrustLevel trustLevel, bool isExplicit); + // Constructor for a source to be edited. + Source(std::string_view name, bool isExplicit); + // Constructor for creating a composite source from a list of available sources. Source(const std::vector& availableSources); @@ -330,6 +333,13 @@ namespace AppInstaller::Repository // Remove source. Source remove command. bool Remove(IProgressCallback& progress); + // Edit source. Source edit command. + bool Edit(IProgressCallback& progress); + + // Determines if this source is a valid edit of otherSource. + // Returns true if this source qualifies as an edit of the other source. + bool IsEditOfSource(const Source& otherSource); + // Gets the tracking catalog for the current source. PackageTrackingCatalog GetTrackingCatalog() const; diff --git a/src/AppInstallerRepositoryCore/RepositorySource.cpp b/src/AppInstallerRepositoryCore/RepositorySource.cpp index 3ceebb110a..95b8225469 100644 --- a/src/AppInstallerRepositoryCore/RepositorySource.cpp +++ b/src/AppInstallerRepositoryCore/RepositorySource.cpp @@ -135,6 +135,12 @@ namespace AppInstaller::Repository return AddOrUpdateFromDetails(details, &ISourceFactory::Update, progress); } + bool EditSourceFromDetails(SourceDetails& details, IProgressCallback& progress) + { + auto factory = ISourceFactory::GetForType(details.Type); + return factory->Edit(details, progress); + } + AddOrUpdateResult BackgroundUpdateSourceFromDetails(SourceDetails& details, IProgressCallback& progress) { return AddOrUpdateFromDetails(details, &ISourceFactory::BackgroundUpdate, progress); @@ -485,6 +491,25 @@ namespace AppInstaller::Repository m_sourceReferences.emplace_back(CreateSourceFromDetails(details)); } + Source::Source(std::string_view name, bool isExplicit) + { + SourceList sourceList; + auto source = sourceList.GetCurrentSource(name); + if (!source) + { + AICLI_LOG(Repo, Info, << "Named source requested, but not found: " << name); + } + else + { + AICLI_LOG(Repo, Info, << "Named source requested, found: " << source->Name); + + // This is intended to support Editing a source, and at present only the Explicit + // property of SourceDetails can be edited. + source->Explicit = isExplicit; + m_sourceReferences.emplace_back(CreateSourceFromDetails(*source)); + } + } + Source::Source(const std::vector& availableSources) { std::shared_ptr compositeSource = std::make_shared("*CompositeSource"); @@ -989,6 +1014,54 @@ namespace AppInstaller::Repository return result; } + bool Source::Edit(IProgressCallback& progress) + { + THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), m_isSourceToBeAdded || m_sourceReferences.size() != 1 || m_source); + + auto& details = m_sourceReferences[0]->GetDetails(); + AICLI_LOG(Repo, Info, << "Named source to be edited, found: " << details.Name << " [" << ToString(details.Origin) << ']'); + + // This is intentionally the same policy checks as Remove. If the source cannot be removed then it cannot be edited. + EnsureSourceIsRemovable(details); + + details.LastUpdateTime = Utility::ConvertUnixEpochToSystemClock(0); + + bool result = EditSourceFromDetails(details, progress); + if (result) + { + SourceList sourceList; + sourceList.EditSource(details); + } + + return result; + } + + bool Source::IsEditOfSource(const Source& otherSource) + { + THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), m_sourceReferences.size() != 1); + + const auto& details = m_sourceReferences[0]->GetDetails(); + const auto& otherDetails = otherSource.GetDetails(); + + // Verify name matches. At this time this is the unique identifier of a source and + // is used to compare to group policy and well-known sources. If the name matches + // we will assume this is intended to be an edit of an existing source. + if (details.Name != otherDetails.Name) + { + return false; + } + + // For now the only supported editable difference is Explicit. + // If others are added, they would be checked below for changes. + bool isChanged = false; + if (details.Explicit != otherDetails.Explicit) + { + isChanged = true; + } + + return isChanged; + } + PackageTrackingCatalog Source::GetTrackingCatalog() const { // With C++20, consider removing the shared_ptr here and making the one inside PackageTrackingCatalog atomic. diff --git a/src/AppInstallerRepositoryCore/Rest/RestSourceFactory.cpp b/src/AppInstallerRepositoryCore/Rest/RestSourceFactory.cpp index b30e18d882..b8f6f56778 100644 --- a/src/AppInstallerRepositoryCore/Rest/RestSourceFactory.cpp +++ b/src/AppInstallerRepositoryCore/Rest/RestSourceFactory.cpp @@ -134,6 +134,12 @@ namespace AppInstaller::Repository::Rest THROW_HR_IF(E_INVALIDARG, !Utility::CaseInsensitiveEquals(details.Type, RestSourceFactory::Type())); return true; } + + bool Edit(const SourceDetails& details, IProgressCallback&) override final + { + THROW_HR_IF(E_INVALIDARG, !Utility::CaseInsensitiveEquals(details.Type, RestSourceFactory::Type())); + return true; + } }; } diff --git a/src/AppInstallerRepositoryCore/SourceFactory.h b/src/AppInstallerRepositoryCore/SourceFactory.h index 0445d950aa..df5dc8a472 100644 --- a/src/AppInstallerRepositoryCore/SourceFactory.h +++ b/src/AppInstallerRepositoryCore/SourceFactory.h @@ -41,6 +41,10 @@ namespace AppInstaller::Repository // Return value indicates whether the action completed. virtual bool Remove(const SourceDetails& details, IProgressCallback& progress) = 0; + // Edit the source from the given details.. + // Return value indicates whether the action completed. + virtual bool Edit(const SourceDetails& details, IProgressCallback& progress) = 0; + // Gets the factory for the given type. static std::unique_ptr GetForType(std::string_view type); }; diff --git a/src/AppInstallerRepositoryCore/SourceList.cpp b/src/AppInstallerRepositoryCore/SourceList.cpp index 560cd88db0..15c94fc080 100644 --- a/src/AppInstallerRepositoryCore/SourceList.cpp +++ b/src/AppInstallerRepositoryCore/SourceList.cpp @@ -24,6 +24,7 @@ namespace AppInstaller::Repository constexpr std::string_view s_SourcesYaml_Source_Data = "Data"sv; constexpr std::string_view s_SourcesYaml_Source_Identifier = "Identifier"sv; constexpr std::string_view s_SourcesYaml_Source_IsTombstone = "IsTombstone"sv; + constexpr std::string_view s_SourcesYaml_Source_IsOverride = "IsOverride"sv; constexpr std::string_view s_SourcesYaml_Source_Explicit = "Explicit"sv; constexpr std::string_view s_SourcesYaml_Source_TrustLevel = "TrustLevel"sv; @@ -188,6 +189,7 @@ namespace AppInstaller::Repository out << YAML::Key << s_SourcesYaml_Source_Data << YAML::Value << details.Data; out << YAML::Key << s_SourcesYaml_Source_Identifier << YAML::Value << details.Identifier; out << YAML::Key << s_SourcesYaml_Source_IsTombstone << YAML::Value << details.IsTombstone; + out << YAML::Key << s_SourcesYaml_Source_IsOverride << YAML::Value << details.IsOverride; out << YAML::Key << s_SourcesYaml_Source_Explicit << YAML::Value << details.Explicit; out << YAML::Key << s_SourcesYaml_Source_TrustLevel << YAML::Value << static_cast(details.TrustLevel); out << YAML::EndMap; @@ -236,6 +238,12 @@ namespace AppInstaller::Repository DoNotUpdateBefore = source.DoNotUpdateBefore; } + void SourceDetailsInternal::CopyOverrideFieldsFrom(const SourceDetails& overrideSource) + { + // These are the supported Override fields. + Explicit = overrideSource.Explicit; + } + std::string_view GetWellKnownSourceName(WellKnownSource source) { switch (source) @@ -445,6 +453,25 @@ namespace AppInstaller::Repository }); } + bool SourceList::TryFindSourceByOrigin(std::string_view name, SourceOrigin origin, SourceDetailsInternal& targetSourceOut, bool includeHidden) + { + auto defaultSources = GetSourcesByOrigin(origin); + auto iter = std::find_if(defaultSources.begin(), defaultSources.end(), + [name, includeHidden](const SourceDetailsInternal& sd) + { + return Utility::ICUCaseInsensitiveEquals(sd.Name, name) && + (includeHidden || !ShouldBeHidden(sd)); + }); + + if (iter == defaultSources.end()) + { + return false; + } + + targetSourceOut = (*iter); + return true; + } + SourceDetailsInternal* SourceList::GetCurrentSource(std::string_view name) { auto itr = FindSource(name); @@ -527,6 +554,14 @@ namespace AppInstaller::Repository return; } + // If this is an override of a default source, turn this into a tombstone instead of removing it. + if (target->IsOverride) + { + target->IsOverride = false; + target->IsTombstone = true; + break; + } + m_sourceList.erase(target); } break; @@ -552,6 +587,71 @@ namespace AppInstaller::Repository SaveMetadataInternal(details, true); } + void SourceList::EditSource(const SourceDetailsInternal& detailsRef) + { + // Copy the incoming details because we might destroy the referenced structure + // when reloading the source details from settings. + SourceDetailsInternal details = detailsRef; + bool sourcesSet = false; + + for (size_t i = 0; !sourcesSet && i < 10; ++i) + { + switch (details.Origin) + { + case SourceOrigin::Default: + { + auto target = FindSource(details.Name, true); + if (target == m_sourceList.end()) + { + THROW_HR_MSG(E_UNEXPECTED, "Default source not in SourceList"); + } + + if (!target->IsTombstone) + { + // Copy the original and then apply the override fields. + SourceDetailsInternal override = *target; + override.Origin = SourceOrigin::User; + override.IsOverride = true; + override.CopyOverrideFieldsFrom(details); + m_sourceList.emplace_back(std::move(override)); + } + } + break; + case SourceOrigin::User: + { + auto target = FindSource(details.Name); + if (target == m_sourceList.end()) + { + // Assumed that an update to the sources removed it first + return; + } + + // Editing a User Source is just replacing the fields that can be edited. + target->CopyOverrideFieldsFrom(details); + } + break; + case SourceOrigin::GroupPolicy: + // This should have already been blocked higher up. + AICLI_LOG(Repo, Error, << "Attempting to edit a Group Policy source: " << details.Name); + THROW_HR(E_UNEXPECTED); + default: + THROW_HR(E_UNEXPECTED); + } + + sourcesSet = SetSourcesByOrigin(SourceOrigin::User, m_sourceList); + + if (!sourcesSet) + { + OverwriteSourceList(); + OverwriteMetadata(); + } + } + + THROW_HR_IF_MSG(E_UNEXPECTED, !sourcesSet, "Too many attempts at SetSourcesByOrigin"); + + SaveMetadataInternal(details, true); + } + void SourceList::SaveMetadata(const SourceDetailsInternal& details) { SaveMetadataInternal(details); @@ -689,6 +789,7 @@ namespace AppInstaller::Repository if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_IsTombstone, details.IsTombstone)) { return false; } TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Explicit, details.Explicit, false); TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Identifier, details.Identifier, false); + TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_IsOverride, details.IsOverride, false); int64_t trustLevelValue; if (TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_TrustLevel, trustLevelValue, false)) @@ -708,6 +809,25 @@ namespace AppInstaller::Repository continue; } + // If this is an override source, we need to get the target of the override and apply the override data on top of it. + if (source.IsOverride) + { + SourceDetailsInternal override; + if (!TryFindSourceByOrigin(source.Name, SourceOrigin::Default, override)) + { + // The default source may be disabled, in which case it may not be returned in the list of default sources. + AICLI_LOG(Repo, Warning, << "User source " << source.Name << " is an override for a non-existent Default Source."); + continue; + } + + override.CopyOverrideFieldsFrom(source); + override.Origin = SourceOrigin::User; + override.IsOverride = true; + result.emplace_back(std::move(override)); + AICLI_LOG(Repo, Info, << "User source " << source.Name << " is overriding the Default source of the same name."); + continue; + } + result.emplace_back(std::move(source)); } } diff --git a/src/AppInstallerRepositoryCore/SourceList.h b/src/AppInstallerRepositoryCore/SourceList.h index b8808be3c7..5fb3052a3b 100644 --- a/src/AppInstallerRepositoryCore/SourceList.h +++ b/src/AppInstallerRepositoryCore/SourceList.h @@ -25,9 +25,16 @@ namespace AppInstaller::Repository // Copies the metadata fields from this source. This only include partial metadata. void CopyMetadataFieldsFrom(const SourceDetails& source); + // Copies the overridden fields from the target source to this source. This is only the supported override fields. + void CopyOverrideFieldsFrom(const SourceDetails& overrideSource); + // If true, this is a tombstone, marking the deletion of a source at a lower priority origin. bool IsTombstone = false; + // If true, this is an override of a source at a lower priority. An override source only defines + // changes on top of the lower priority source, otherwise uses the same as the lower priority source. + bool IsOverride = false; + // If false, this is not visible in GetCurrentSource or GetAllSources, it's only available when explicitly requested. bool IsVisible = true; @@ -55,9 +62,10 @@ namespace AppInstaller::Repository // Source includes ones in tombstone SourceDetailsInternal* GetSource(std::string_view name); - // Add/remove a current source + // Add/remove/edit a current source void AddSource(const SourceDetailsInternal& details); void RemoveSource(const SourceDetailsInternal& details); + void EditSource(const SourceDetailsInternal& details); // Save source metadata; the particular source with the metadata update is given. // The given source must already be in the internal source list. @@ -83,6 +91,9 @@ namespace AppInstaller::Repository // calls std::find_if and return the iterator. auto FindSource(std::string_view name, bool includeHidden = false); + // Tries to find a named source from the specified origin. + [[nodiscard]] bool TryFindSourceByOrigin(std::string_view name, SourceOrigin origin, SourceDetailsInternal& targetSourceOut, bool includeHidden = false); + std::vector GetSourcesByOrigin(SourceOrigin origin); // Does *NOT* set metadata; call SaveMetadataInternal afterward. [[nodiscard]] bool SetSourcesByOrigin(SourceOrigin origin, const std::vector& sources); From d72c686de7c5fc7646c2451dfc2044f1aece410c Mon Sep 17 00:00:00 2001 From: David Bennett Date: Mon, 1 Dec 2025 09:07:50 -0800 Subject: [PATCH 02/19] Add E2E tests for source edit --- src/AppInstallerCLIE2ETests/SourceCommand.cs | 78 ++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/src/AppInstallerCLIE2ETests/SourceCommand.cs b/src/AppInstallerCLIE2ETests/SourceCommand.cs index c873a6d72b..e85af37655 100644 --- a/src/AppInstallerCLIE2ETests/SourceCommand.cs +++ b/src/AppInstallerCLIE2ETests/SourceCommand.cs @@ -258,5 +258,83 @@ public void SourceForceReset() Assert.False(result.StdOut.Contains(Constants.TestSourceName)); Assert.False(result.StdOut.Contains(Constants.TestSourceUrl)); } + + /// + /// Test source add with explicit flag, edit the source to not be explicit. + /// + [Test] + public void SourceEdit() + { + // Remove the test source. + TestCommon.RunAICLICommand("source remove", Constants.TestSourceName); + + // Add source as explicit and verify it is explicit. + var addResult = TestCommon.RunAICLICommand("source add", $"SourceTest {Constants.TestSourceUrl} --trust-level trusted --explicit"); + Assert.AreEqual(Constants.ErrorCode.S_OK, addResult.ExitCode); + Assert.True(addResult.StdOut.Contains("Done")); + + var searchResult = TestCommon.RunAICLICommand("search", "TestExampleInstaller"); + Assert.AreEqual(Constants.ErrorCode.ERROR_NO_SOURCES_DEFINED, searchResult.ExitCode); + Assert.True(searchResult.StdOut.Contains("No sources defined; add one with 'source add' or reset to defaults with 'source reset'")); + + // Run the edit, this should be S_OK with "Done" as it changed the state to not-explicit. + var editResult = TestCommon.RunAICLICommand("source edit", $"SourceTest"); + Assert.AreEqual(Constants.ErrorCode.S_OK, editResult.ExitCode); + Assert.True(editResult.StdOut.Contains("Done")); + + // Run it again, this should result in S_OK with no changes and a message that the source is already in that state. + var editResult2 = TestCommon.RunAICLICommand("source edit", $"SourceTest"); + Assert.AreEqual(Constants.ErrorCode.S_OK, editResult2.ExitCode); + Assert.True(editResult2.StdOut.Contains("The source named 'SourceTest' is already in that desired state.")); + + // Now verify it is no longer explicit by running the search again without adding the source parameter. + var searchResult2 = TestCommon.RunAICLICommand("search", "TestExampleInstaller"); + Assert.AreEqual(Constants.ErrorCode.S_OK, searchResult2.ExitCode); + Assert.True(searchResult2.StdOut.Contains("TestExampleInstaller")); + Assert.True(searchResult2.StdOut.Contains("AppInstallerTest.TestExampleInstaller")); + TestCommon.RunAICLICommand("source remove", $"-n SourceTest"); + } + + /// + /// Test override of a default source via edit command. + /// + [Test] + public void SourceEditOverrideDefault() + { + // Force Reset Sources + var resetResult = TestCommon.RunAICLICommand("source reset", "--force"); + Assert.AreEqual(Constants.ErrorCode.S_OK, resetResult.ExitCode); + + // Verify it is explicit true. Explicit is the only boolean value in the output. + var listResult = TestCommon.RunAICLICommand("source list", "winget-font"); + Assert.AreEqual(Constants.ErrorCode.S_OK, listResult.ExitCode); + Assert.True(listResult.StdOut.Contains("true")); + + var editResult = TestCommon.RunAICLICommand("source edit", "winget-font"); + Assert.AreEqual(Constants.ErrorCode.S_OK, editResult.ExitCode); + Assert.True(editResult.StdOut.Contains("Done")); + + // Verify that after edit it is now explicit false. + var listResult2 = TestCommon.RunAICLICommand("source list", "winget-font"); + Assert.AreEqual(Constants.ErrorCode.S_OK, listResult2.ExitCode); + Assert.True(listResult2.StdOut.Contains("false")); + + // Remove the source. This should correctly tombstone it, even though it is overridden. + var removeResult = TestCommon.RunAICLICommand("source remove", "winget-font"); + Assert.AreEqual(Constants.ErrorCode.S_OK, removeResult.ExitCode); + Assert.True(removeResult.StdOut.Contains("Done")); + + var listResult3 = TestCommon.RunAICLICommand("source list", "winget-font"); + Assert.AreEqual(Constants.ErrorCode.ERROR_SOURCE_NAME_DOES_NOT_EXIST, listResult3.ExitCode); + + // Force Reset Sources + var resetResult2 = TestCommon.RunAICLICommand("source reset", "--force"); + Assert.AreEqual(Constants.ErrorCode.S_OK, resetResult2.ExitCode); + + // Verify it is back to being explicit true. + var listResult4 = TestCommon.RunAICLICommand("source list", "winget-font"); + Assert.AreEqual(Constants.ErrorCode.S_OK, listResult4.ExitCode); + Assert.True(listResult4.StdOut.Contains("true")); + } } } From 569e4808cb04c06829cdd7cc924057b83f15e481 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Mon, 1 Dec 2025 09:31:39 -0800 Subject: [PATCH 03/19] Spelling --- src/AppInstallerCLITests/Sources.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AppInstallerCLITests/Sources.cpp b/src/AppInstallerCLITests/Sources.cpp index b9fd72b15d..7c0a31dfeb 100644 --- a/src/AppInstallerCLITests/Sources.cpp +++ b/src/AppInstallerCLITests/Sources.cpp @@ -324,7 +324,7 @@ TEST_CASE("RepoSources_DefaultSourceOverride", "[sources]") // The override will change the index value as the Default will be replaced by the override. // User sources have higher priority so the override will be at index 0. // We expect the same count, and the Name, Arg, Data, and Type properties to all be identical. - // Only the name is defined in the override setting so all others should be propertly populated. + // Only the name is defined in the override setting so all others should be properly populated. REQUIRE(afterOverride.size() == c_DefaultSourceCount); REQUIRE(afterOverride[0].Name == beforeOverride[2].Name); REQUIRE(afterOverride[0].Arg == beforeOverride[2].Arg); From d835f0b772c1512be44e56ecf8f2fc69a61265c7 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Mon, 1 Dec 2025 09:37:24 -0800 Subject: [PATCH 04/19] The hyphen is now nonexistent --- src/AppInstallerRepositoryCore/SourceList.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AppInstallerRepositoryCore/SourceList.cpp b/src/AppInstallerRepositoryCore/SourceList.cpp index 15c94fc080..79194fc9e3 100644 --- a/src/AppInstallerRepositoryCore/SourceList.cpp +++ b/src/AppInstallerRepositoryCore/SourceList.cpp @@ -816,7 +816,7 @@ namespace AppInstaller::Repository if (!TryFindSourceByOrigin(source.Name, SourceOrigin::Default, override)) { // The default source may be disabled, in which case it may not be returned in the list of default sources. - AICLI_LOG(Repo, Warning, << "User source " << source.Name << " is an override for a non-existent Default Source."); + AICLI_LOG(Repo, Warning, << "User source " << source.Name << " is an override for a nonexistent Default Source."); continue; } From 9c3fe1293abc5d0df10c70c520ed66fc73de5a55 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Mon, 1 Dec 2025 11:11:48 -0800 Subject: [PATCH 05/19] Add SourceEdit experimental feature and release notes. --- doc/ReleaseNotes.md | 11 +++++++++++ doc/Settings.md | 10 ++++++++++ src/AppInstallerCLICore/Commands/SourceCommand.cpp | 1 + src/AppInstallerCLICore/Commands/SourceCommand.h | 2 +- src/AppInstallerCommonCore/ExperimentalFeature.cpp | 4 ++++ .../Public/winget/ExperimentalFeature.h | 1 + .../Public/winget/UserSettings.h | 2 ++ src/AppInstallerCommonCore/UserSettings.cpp | 1 + 8 files changed, 31 insertions(+), 1 deletion(-) diff --git a/doc/ReleaseNotes.md b/doc/ReleaseNotes.md index 8ec1e210c6..547c118c39 100644 --- a/doc/ReleaseNotes.md +++ b/doc/ReleaseNotes.md @@ -1,3 +1,14 @@ ## New in v1.28 +# Experimental Feature: 'sourceEdit' +New feature that adds an 'edit' subcommand to the 'source' command. This can be used to set an explicit source to be implicit and vice-versa. For example, with this feature you can make the 'winget-font' source an implicit source instead of explicit source. + +To enable this feature, add the 'sourceEdit' experimental feature to your settings. +``` +"experimentalFeatures": { + "sourceEdit": true +}, +``` +To use the feature, try `winget source edit winget-font` to set the Explicit state to the default. + diff --git a/doc/Settings.md b/doc/Settings.md index 9593346539..766688976d 100644 --- a/doc/Settings.md +++ b/doc/Settings.md @@ -364,3 +364,13 @@ This feature enables support for fonts via `winget settings`. The `winget font l "fonts": true }, ``` + +### sourceEdit + +This feature enables support for additional source command improvements via `winget settings`. The `winget source edit` command will become available with this feature. + +```json + "experimentalFeatures": { + "sourceEdit": true + }, +``` diff --git a/src/AppInstallerCLICore/Commands/SourceCommand.cpp b/src/AppInstallerCLICore/Commands/SourceCommand.cpp index 871758938f..b5fdf1d1fc 100644 --- a/src/AppInstallerCLICore/Commands/SourceCommand.cpp +++ b/src/AppInstallerCLICore/Commands/SourceCommand.cpp @@ -342,6 +342,7 @@ namespace AppInstaller::CLI void SourceEditCommand::ExecuteInternal(Context& context) const { context << + Workflow::EnsureFeatureEnabled(Settings::ExperimentalFeature::Feature::SourceEdit) << Workflow::EnsureRunningAsAdmin << Workflow::GetSourceListWithFilter << Workflow::EditSources; diff --git a/src/AppInstallerCLICore/Commands/SourceCommand.h b/src/AppInstallerCLICore/Commands/SourceCommand.h index 2385155126..6bd761c631 100644 --- a/src/AppInstallerCLICore/Commands/SourceCommand.h +++ b/src/AppInstallerCLICore/Commands/SourceCommand.h @@ -124,7 +124,7 @@ namespace AppInstaller::CLI struct SourceEditCommand final : public Command { - SourceEditCommand(std::string_view parent) : Command("edit", {}, parent, Settings::TogglePolicy::Policy::AllowedSources) {} + SourceEditCommand(std::string_view parent) : Command("edit", {}, parent, Settings::ExperimentalFeature::Feature::SourceEdit) {} std::vector GetArguments() const override; diff --git a/src/AppInstallerCommonCore/ExperimentalFeature.cpp b/src/AppInstallerCommonCore/ExperimentalFeature.cpp index caeb32d0fe..bc7a377dca 100644 --- a/src/AppInstallerCommonCore/ExperimentalFeature.cpp +++ b/src/AppInstallerCommonCore/ExperimentalFeature.cpp @@ -44,6 +44,8 @@ namespace AppInstaller::Settings return userSettings.Get(); case ExperimentalFeature::Feature::Font: return userSettings.Get(); + case ExperimentalFeature::Feature::SourceEdit: + return userSettings.Get(); default: THROW_HR(E_UNEXPECTED); } @@ -77,6 +79,8 @@ namespace AppInstaller::Settings return ExperimentalFeature{ "Resume", "resume", "https://aka.ms/winget-settings", Feature::Resume }; case Feature::Font: return ExperimentalFeature{ "Font", "Font", "https://aka.ms/winget-settings", Feature::Font }; + case Feature::SourceEdit: + return ExperimentalFeature{ "Source Editing", "sourceEdit", "https://aka.ms/winget-settings", Feature::SourceEdit }; default: THROW_HR(E_UNEXPECTED); diff --git a/src/AppInstallerCommonCore/Public/winget/ExperimentalFeature.h b/src/AppInstallerCommonCore/Public/winget/ExperimentalFeature.h index 2dc097f548..5e83bd35ea 100644 --- a/src/AppInstallerCommonCore/Public/winget/ExperimentalFeature.h +++ b/src/AppInstallerCommonCore/Public/winget/ExperimentalFeature.h @@ -25,6 +25,7 @@ namespace AppInstaller::Settings DirectMSI = 0x1, Resume = 0x2, Font = 0x4, + SourceEdit = 0x8, Max, // This MUST always be after all experimental features // Features listed after Max will not be shown with the features command diff --git a/src/AppInstallerCommonCore/Public/winget/UserSettings.h b/src/AppInstallerCommonCore/Public/winget/UserSettings.h index b06bc57170..1a40f4db30 100644 --- a/src/AppInstallerCommonCore/Public/winget/UserSettings.h +++ b/src/AppInstallerCommonCore/Public/winget/UserSettings.h @@ -76,6 +76,7 @@ namespace AppInstaller::Settings EFDirectMSI, EFResume, EFFonts, + EFSourceEdit, // Telemetry TelemetryDisable, // Install behavior @@ -159,6 +160,7 @@ namespace AppInstaller::Settings SETTINGMAPPING_SPECIALIZATION(Setting::EFDirectMSI, bool, bool, false, ".experimentalFeatures.directMSI"sv); SETTINGMAPPING_SPECIALIZATION(Setting::EFResume, bool, bool, false, ".experimentalFeatures.resume"sv); SETTINGMAPPING_SPECIALIZATION(Setting::EFFonts, bool, bool, false, ".experimentalFeatures.fonts"sv); + SETTINGMAPPING_SPECIALIZATION(Setting::EFSourceEdit, bool, bool, false, ".experimentalFeatures.sourceEdit"sv); // Telemetry SETTINGMAPPING_SPECIALIZATION(Setting::TelemetryDisable, bool, bool, false, ".telemetry.disable"sv); // Install behavior diff --git a/src/AppInstallerCommonCore/UserSettings.cpp b/src/AppInstallerCommonCore/UserSettings.cpp index 5d7b000949..67e3fa1f01 100644 --- a/src/AppInstallerCommonCore/UserSettings.cpp +++ b/src/AppInstallerCommonCore/UserSettings.cpp @@ -267,6 +267,7 @@ namespace AppInstaller::Settings WINGET_VALIDATE_PASS_THROUGH(EFDirectMSI) WINGET_VALIDATE_PASS_THROUGH(EFResume) WINGET_VALIDATE_PASS_THROUGH(EFFonts) + WINGET_VALIDATE_PASS_THROUGH(EFSourceEdit) WINGET_VALIDATE_PASS_THROUGH(AnonymizePathForDisplay) WINGET_VALIDATE_PASS_THROUGH(TelemetryDisable) WINGET_VALIDATE_PASS_THROUGH(InteractivityDisable) From 8288c229e02fa0721873cec18abc6f8fb7237e6f Mon Sep 17 00:00:00 2001 From: David Bennett Date: Mon, 1 Dec 2025 12:05:08 -0800 Subject: [PATCH 06/19] Add experimental feature enable to source tests --- src/AppInstallerCLIE2ETests/SourceCommand.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/AppInstallerCLIE2ETests/SourceCommand.cs b/src/AppInstallerCLIE2ETests/SourceCommand.cs index e85af37655..5502ac7838 100644 --- a/src/AppInstallerCLIE2ETests/SourceCommand.cs +++ b/src/AppInstallerCLIE2ETests/SourceCommand.cs @@ -14,6 +14,15 @@ namespace AppInstallerCLIE2ETests /// public class SourceCommand : BaseCommand { + /// + /// One time set up. + /// + [OneTimeSetUp] + public void OneTimeSetup() + { + WinGetSettingsHelper.ConfigureFeature("sourceEdit", true); + } + /// /// Test set up. /// From 43805585d2e6d3632d4877cbf64c4d96784101d2 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Mon, 8 Dec 2025 14:59:15 -0800 Subject: [PATCH 07/19] PR updates --- src/AppInstallerCLICore/Argument.cpp | 4 +++ .../Commands/SourceCommand.cpp | 18 +++++++++- .../Commands/SourceCommand.h | 3 +- src/AppInstallerCLICore/ExecutionArgs.h | 1 + src/AppInstallerCLICore/Resources.h | 1 + .../Workflows/SourceFlow.cpp | 28 +++++++-------- src/AppInstallerCLIE2ETests/SourceCommand.cs | 6 ++-- .../Shared/Strings/en-us/winget.resw | 7 ++-- src/AppInstallerCLITests/TestSource.cpp | 8 ++--- src/AppInstallerCLITests/TestSource.h | 6 ++-- .../ConfigurableTestSourceFactory.cpp | 2 +- .../PreIndexedPackageSourceFactory.cpp | 10 +++--- .../PredefinedInstalledSourceFactory.cpp | 2 +- .../PredefinedWriteableSourceFactory.cpp | 2 +- .../PackageTrackingCatalog.cpp | 2 +- .../Public/winget/RepositorySource.h | 15 ++++++-- .../RepositorySource.cpp | 34 ++++++++----------- .../Rest/RestSourceFactory.cpp | 2 +- .../SourceFactory.h | 5 ++- .../AppInstallerStrings.cpp | 7 ++++ .../Public/AppInstallerStrings.h | 3 ++ 21 files changed, 103 insertions(+), 63 deletions(-) diff --git a/src/AppInstallerCLICore/Argument.cpp b/src/AppInstallerCLICore/Argument.cpp index 9263b35b41..9caccb6921 100644 --- a/src/AppInstallerCLICore/Argument.cpp +++ b/src/AppInstallerCLICore/Argument.cpp @@ -121,6 +121,8 @@ namespace AppInstaller::CLI return { type, "explicit"_liv }; case Execution::Args::Type::SourceTrustLevel: return { type, "trust-level"_liv }; + case Execution::Args::Type::SourceEditExplicit: + return { type, "explicit"_liv, 'e' }; //Hash Command case Execution::Args::Type::HashFile: @@ -410,6 +412,8 @@ namespace AppInstaller::CLI return Argument{ type, Resource::String::SourceTypeArgumentDescription, ArgumentType::Positional }; case Args::Type::SourceExplicit: return Argument{ type, Resource::String::SourceExplicitArgumentDescription, ArgumentType::Flag }; + case Args::Type::SourceEditExplicit: + return Argument{ type, Resource::String::SourceEditExplicitArgumentDescription, ArgumentType::Positional }; case Args::Type::SourceTrustLevel: return Argument{ type, Resource::String::SourceTrustLevelArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help }; case Args::Type::ValidateManifest: diff --git a/src/AppInstallerCLICore/Commands/SourceCommand.cpp b/src/AppInstallerCLICore/Commands/SourceCommand.cpp index b5fdf1d1fc..89e1bf6f9f 100644 --- a/src/AppInstallerCLICore/Commands/SourceCommand.cpp +++ b/src/AppInstallerCLICore/Commands/SourceCommand.cpp @@ -320,7 +320,7 @@ namespace AppInstaller::CLI { return { Argument::ForType(Args::Type::SourceName).SetRequired(true), - Argument::ForType(Args::Type::SourceExplicit), + Argument::ForType(Args::Type::SourceEditExplicit), }; } @@ -339,6 +339,22 @@ namespace AppInstaller::CLI return s_SourceCommand_HelpLink; } + void SourceEditCommand::ValidateArgumentsInternal(Execution::Args& execArgs) const + { + if (execArgs.Contains(Execution::Args::Type::SourceEditExplicit)) + { + std::string_view explicitArg = execArgs.GetArg(Execution::Args::Type::SourceEditExplicit); + if (!Utility::CaseInsensitiveEquals(explicitArg, "true") && !Utility::CaseInsensitiveEquals(explicitArg, "false")) + { + auto validOptions = Utility::Join(", "_liv, std::vector{ + "true"_lis, + "false"_lis, + }); + throw CommandException(Resource::String::InvalidArgumentValueError(Argument::ForType(Execution::Args::Type::SourceEditExplicit).Name(), validOptions)); + } + } + } + void SourceEditCommand::ExecuteInternal(Context& context) const { context << diff --git a/src/AppInstallerCLICore/Commands/SourceCommand.h b/src/AppInstallerCLICore/Commands/SourceCommand.h index 6bd761c631..b576c61581 100644 --- a/src/AppInstallerCLICore/Commands/SourceCommand.h +++ b/src/AppInstallerCLICore/Commands/SourceCommand.h @@ -124,7 +124,7 @@ namespace AppInstaller::CLI struct SourceEditCommand final : public Command { - SourceEditCommand(std::string_view parent) : Command("edit", {}, parent, Settings::ExperimentalFeature::Feature::SourceEdit) {} + SourceEditCommand(std::string_view parent) : Command("edit", { "config" }, parent, Settings::ExperimentalFeature::Feature::SourceEdit) {} std::vector GetArguments() const override; @@ -134,6 +134,7 @@ namespace AppInstaller::CLI Utility::LocIndView HelpLink() const override; protected: + void ValidateArgumentsInternal(Execution::Args& execArgs) const override; void ExecuteInternal(Execution::Context& context) const override; }; } diff --git a/src/AppInstallerCLICore/ExecutionArgs.h b/src/AppInstallerCLICore/ExecutionArgs.h index 3d701ac73f..8031d86bf2 100644 --- a/src/AppInstallerCLICore/ExecutionArgs.h +++ b/src/AppInstallerCLICore/ExecutionArgs.h @@ -65,6 +65,7 @@ namespace AppInstaller::CLI::Execution ForceSourceReset, SourceExplicit, SourceTrustLevel, + SourceEditExplicit, //Hash Command HashFile, diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h index c81f79889c..eba343937a 100644 --- a/src/AppInstallerCLICore/Resources.h +++ b/src/AppInstallerCLICore/Resources.h @@ -689,6 +689,7 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(SourceCommandShortDescription); WINGET_DEFINE_RESOURCE_STRINGID(SourceEditCommandLongDescription); WINGET_DEFINE_RESOURCE_STRINGID(SourceEditCommandShortDescription); + WINGET_DEFINE_RESOURCE_STRINGID(SourceEditExplicitArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(SourceEditOne); WINGET_DEFINE_RESOURCE_STRINGID(SourceEditNoChanges); WINGET_DEFINE_RESOURCE_STRINGID(SourceExplicitArgumentDescription); diff --git a/src/AppInstallerCLICore/Workflows/SourceFlow.cpp b/src/AppInstallerCLICore/Workflows/SourceFlow.cpp index 13b75d5e5e..ee1cd7d1b2 100644 --- a/src/AppInstallerCLICore/Workflows/SourceFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/SourceFlow.cpp @@ -276,32 +276,30 @@ namespace AppInstaller::CLI::Workflow { // We are assuming there is only one match, as SourceName is a required parameter. const std::vector& sources = context.Get(); - bool isExplicit = context.Args.Contains(Args::Type::SourceExplicit); + for (const auto& sd : sources) { // Get the current source with this name. - Repository::Source currentSource{ sd.Name }; + Repository::Source targetSource{ sd.Name }; - // Only permitting Explicit to be edited at this time. - Repository::Source source{ sd.Name, isExplicit }; + // Default to the current explicit value unless we are overriding it. + auto isExplicit = sd.Explicit; + if (context.Args.Contains(Execution::Args::Type::SourceEditExplicit)) + { + auto explicitArg = context.Args.GetArg(Execution::Args::Type::SourceEditExplicit); + isExplicit = Utility::ConvertStringToBool(explicitArg); + } - // Is anything being changed?? - if (!source.IsEditOfSource(currentSource)) + Repository::SourceEdit edits{ std::optional{ isExplicit } }; + if (!targetSource.RequiresChanges(edits)) { context.Reporter.Info() << Resource::String::SourceEditNoChanges(Utility::LocIndView{ sd.Name }) << std::endl; continue; } context.Reporter.Info() << Resource::String::SourceEditOne(Utility::LocIndView{ sd.Name }, Utility::LocIndView{ Utility::ConvertBoolToString(isExplicit) }) << std::endl; - auto editFunction = [&](IProgressCallback& progress)->bool { return source.Edit(progress); }; - if (context.Reporter.ExecuteWithProgress(editFunction)) - { - context.Reporter.Info() << Resource::String::Done << std::endl; - } - else - { - context.Reporter.Info() << Resource::String::Cancelled << std::endl; - } + targetSource.Edit(edits); + context.Reporter.Info() << Resource::String::Done << std::endl; } } diff --git a/src/AppInstallerCLIE2ETests/SourceCommand.cs b/src/AppInstallerCLIE2ETests/SourceCommand.cs index 5502ac7838..04fabd4924 100644 --- a/src/AppInstallerCLIE2ETests/SourceCommand.cs +++ b/src/AppInstallerCLIE2ETests/SourceCommand.cs @@ -287,14 +287,14 @@ public void SourceEdit() Assert.True(searchResult.StdOut.Contains("No sources defined; add one with 'source add' or reset to defaults with 'source reset'")); // Run the edit, this should be S_OK with "Done" as it changed the state to not-explicit. - var editResult = TestCommon.RunAICLICommand("source edit", $"SourceTest"); + var editResult = TestCommon.RunAICLICommand("source edit", $"SourceTest --explicit false"); Assert.AreEqual(Constants.ErrorCode.S_OK, editResult.ExitCode); Assert.True(editResult.StdOut.Contains("Done")); // Run it again, this should result in S_OK with no changes and a message that the source is already in that state. - var editResult2 = TestCommon.RunAICLICommand("source edit", $"SourceTest"); + var editResult2 = TestCommon.RunAICLICommand("source edit", $"SourceTest --explicit false"); Assert.AreEqual(Constants.ErrorCode.S_OK, editResult2.ExitCode); - Assert.True(editResult2.StdOut.Contains("The source named 'SourceTest' is already in that desired state.")); + Assert.True(editResult2.StdOut.Contains("The source named 'SourceTest' is already in the desired state.")); // Now verify it is no longer explicit by running the search again without adding the source parameter. var searchResult2 = TestCommon.RunAICLICommand("search", "TestExampleInstaller"); diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index 8bb77f2743..57b9223255 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -546,7 +546,7 @@ They can be configured through the settings file 'winget settings'. Manage sources of packages - Edit properties of an existing source. A source provides the data for you to discover and install packages. Only add a new source if you trust it as a secure location. + Edit properties of an existing source. A source provides the data for you to discover and install packages. Edit properties of a source @@ -556,7 +556,7 @@ They can be configured through the settings file 'winget settings'. {Locked="{0}","{1}"} Message displayed to inform the user about a registered repository source that is currently being edited. {0} is a placeholder replaced by the repository source name. {1} is a placeholder replaced by the changed value. - The source named '{0}' is already in that desired state. + The source named '{0}' is already in the desired state. {Locked="{0}"} Message displayed to inform the user about a registered repository source that is currently being edited. {0} is a placeholder replaced by the repository source name. @@ -2904,6 +2904,9 @@ Please specify one of them using the --source option to proceed. Excludes a source from discovery unless specified + + Excludes a source from discovery (true or false) + Explicit diff --git a/src/AppInstallerCLITests/TestSource.cpp b/src/AppInstallerCLITests/TestSource.cpp index 0281f2726c..c602d25bd7 100644 --- a/src/AppInstallerCLITests/TestSource.cpp +++ b/src/AppInstallerCLITests/TestSource.cpp @@ -454,11 +454,11 @@ namespace TestCommon return true; } - bool TestSourceFactory::Edit(const SourceDetails& details, IProgressCallback&) + bool TestSourceFactory::Edit(SourceDetails& details, const SourceEdit& edits) { if (OnEdit) { - OnEdit(details); + OnEdit(details, edits); } return true; } @@ -475,10 +475,10 @@ namespace TestCommon return source.Add(progress); } - bool EditSource(const AppInstaller::Repository::SourceDetails& details, AppInstaller::IProgressCallback& progress) + bool EditSource(const AppInstaller::Repository::SourceDetails& details, const AppInstaller::Repository::SourceEdit& edits) { Repository::Source source{ details.Name, details.Explicit }; - return source.Edit(progress); + return source.Edit(edits); } bool UpdateSource(std::string_view name, AppInstaller::IProgressCallback& progress) diff --git a/src/AppInstallerCLITests/TestSource.h b/src/AppInstallerCLITests/TestSource.h index 4dc8a33192..e32dfaab95 100644 --- a/src/AppInstallerCLITests/TestSource.h +++ b/src/AppInstallerCLITests/TestSource.h @@ -187,7 +187,7 @@ namespace TestCommon using AddFunctor = std::function; using UpdateFunctor = std::function; using RemoveFunctor = std::function; - using EditFunctor = std::function; + using EditFunctor = std::function; TestSourceFactory(OpenFunctor open) : OnOpen(std::move(open)) {} TestSourceFactory(OpenFunctorWithCustomHeader open) : OnOpenWithCustomHeader(std::move(open)) {} @@ -198,7 +198,7 @@ namespace TestCommon bool Add(AppInstaller::Repository::SourceDetails& details, AppInstaller::IProgressCallback&) override; bool Update(const AppInstaller::Repository::SourceDetails& details, AppInstaller::IProgressCallback&) override; bool Remove(const AppInstaller::Repository::SourceDetails& details, AppInstaller::IProgressCallback&) override; - bool Edit(const AppInstaller::Repository::SourceDetails& details, AppInstaller::IProgressCallback&) override; + bool Edit(AppInstaller::Repository::SourceDetails& details, const AppInstaller::Repository::SourceEdit& edits) override; // Make copies of self when requested. operator std::function()>(); @@ -213,7 +213,7 @@ namespace TestCommon }; bool AddSource(const AppInstaller::Repository::SourceDetails& details, AppInstaller::IProgressCallback& progress); - bool EditSource(const AppInstaller::Repository::SourceDetails& details, AppInstaller::IProgressCallback& progress); + bool EditSource(AppInstaller::Repository::SourceDetails& details, const AppInstaller::Repository::SourceEdit& edits); bool UpdateSource(std::string_view name, AppInstaller::IProgressCallback& progress); bool RemoveSource(std::string_view name, AppInstaller::IProgressCallback& progress); AppInstaller::Repository::Source OpenSource(std::string_view name, AppInstaller::IProgressCallback& progress); diff --git a/src/AppInstallerRepositoryCore/Microsoft/ConfigurableTestSourceFactory.cpp b/src/AppInstallerRepositoryCore/Microsoft/ConfigurableTestSourceFactory.cpp index 7a91c22c14..35be4911e5 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/ConfigurableTestSourceFactory.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/ConfigurableTestSourceFactory.cpp @@ -155,7 +155,7 @@ namespace AppInstaller::Repository::Microsoft return true; } - bool Edit(const SourceDetails&, IProgressCallback&) override final + bool Edit(SourceDetails&, const SourceEdit&) override final { return true; } diff --git a/src/AppInstallerRepositoryCore/Microsoft/PreIndexedPackageSourceFactory.cpp b/src/AppInstallerRepositoryCore/Microsoft/PreIndexedPackageSourceFactory.cpp index cb483a7bb3..2cb137fb4f 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PreIndexedPackageSourceFactory.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/PreIndexedPackageSourceFactory.cpp @@ -254,12 +254,14 @@ namespace AppInstaller::Repository::Microsoft return UpdateBase(details, true, progress); } - bool Edit(const SourceDetails& details, IProgressCallback& progress) override final + // Applies the edits to the source. + bool Edit(SourceDetails& details, const SourceEdit& edits) override final { - UNREFERENCED_PARAMETER(details); - UNREFERENCED_PARAMETER(progress); + if (edits.Explicit.has_value()) + { + details.Explicit = edits.Explicit.value(); + } - // Edit does not change or update the installed packages. return true; } diff --git a/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp b/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp index 698d503e73..665f6ecf7d 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp @@ -446,7 +446,7 @@ namespace AppInstaller::Repository::Microsoft THROW_HR(E_NOTIMPL); } - bool Edit(const SourceDetails&, IProgressCallback&) override final + bool Edit(SourceDetails&, const SourceEdit&) override final { THROW_HR(E_NOTIMPL); } diff --git a/src/AppInstallerRepositoryCore/Microsoft/PredefinedWriteableSourceFactory.cpp b/src/AppInstallerRepositoryCore/Microsoft/PredefinedWriteableSourceFactory.cpp index c3beccf362..ba179eee2c 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PredefinedWriteableSourceFactory.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/PredefinedWriteableSourceFactory.cpp @@ -45,7 +45,7 @@ namespace AppInstaller::Repository::Microsoft THROW_HR(E_NOTIMPL); } - bool Edit(const SourceDetails&, IProgressCallback&) override final + bool Edit(SourceDetails&, const SourceEdit&) override final { THROW_HR(E_NOTIMPL); } diff --git a/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp b/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp index 159d70df60..da801554d9 100644 --- a/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp +++ b/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp @@ -125,7 +125,7 @@ namespace AppInstaller::Repository THROW_HR(E_NOTIMPL); } - bool Edit(const SourceDetails&, IProgressCallback&) override final + bool Edit(SourceDetails&, const SourceEdit&) override final { THROW_HR(E_NOTIMPL); } diff --git a/src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h b/src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h index aab305e96b..31cf7fb8ce 100644 --- a/src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h +++ b/src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h @@ -197,6 +197,15 @@ namespace AppInstaller::Repository Authentication::AuthenticationInfo Authentication; }; + // Contains information about edits to a source. + struct SourceEdit + { + SourceEdit(std::optional isExplicit); + + // The explicit property of a source. + std::optional Explicit; + }; + // Allows calling code to inquire about specific features of an ISource implementation. // The default state of any new flag is false. enum class SourceFeatureFlag @@ -225,7 +234,7 @@ namespace AppInstaller::Repository Source(std::string_view name, std::string_view arg, std::string_view type, SourceTrustLevel trustLevel, bool isExplicit); // Constructor for a source to be edited. - Source(std::string_view name, bool isExplicit); + Source(std::string_view name, std::optional isExplicit); // Constructor for creating a composite source from a list of available sources. Source(const std::vector& availableSources); @@ -334,11 +343,11 @@ namespace AppInstaller::Repository bool Remove(IProgressCallback& progress); // Edit source. Source edit command. - bool Edit(IProgressCallback& progress); + bool Edit(const SourceEdit& edits); // Determines if this source is a valid edit of otherSource. // Returns true if this source qualifies as an edit of the other source. - bool IsEditOfSource(const Source& otherSource); + bool RequiresChanges(const SourceEdit& edits); // Gets the tracking catalog for the current source. PackageTrackingCatalog GetTrackingCatalog() const; diff --git a/src/AppInstallerRepositoryCore/RepositorySource.cpp b/src/AppInstallerRepositoryCore/RepositorySource.cpp index 95b8225469..2f05ccb661 100644 --- a/src/AppInstallerRepositoryCore/RepositorySource.cpp +++ b/src/AppInstallerRepositoryCore/RepositorySource.cpp @@ -135,10 +135,10 @@ namespace AppInstaller::Repository return AddOrUpdateFromDetails(details, &ISourceFactory::Update, progress); } - bool EditSourceFromDetails(SourceDetails& details, IProgressCallback& progress) + bool EditSource(SourceDetails& details, const SourceEdit& edits) { auto factory = ISourceFactory::GetForType(details.Type); - return factory->Edit(details, progress); + return factory->Edit(details, edits); } AddOrUpdateResult BackgroundUpdateSourceFromDetails(SourceDetails& details, IProgressCallback& progress) @@ -438,6 +438,8 @@ namespace AppInstaller::Repository return CheckForWellKnownSourceMatch(sourceDetails.Name, sourceDetails.Arg, sourceDetails.Type); } + SourceEdit::SourceEdit(std::optional isExplicit) : Explicit(isExplicit) {} + Source::Source() {} Source::Source(std::string_view name) @@ -491,7 +493,7 @@ namespace AppInstaller::Repository m_sourceReferences.emplace_back(CreateSourceFromDetails(details)); } - Source::Source(std::string_view name, bool isExplicit) + Source::Source(std::string_view name, std::optional isExplicit) { SourceList sourceList; auto source = sourceList.GetCurrentSource(name); @@ -505,7 +507,11 @@ namespace AppInstaller::Repository // This is intended to support Editing a source, and at present only the Explicit // property of SourceDetails can be edited. - source->Explicit = isExplicit; + if (isExplicit.has_value()) + { + source->Explicit = isExplicit.value(); + } + m_sourceReferences.emplace_back(CreateSourceFromDetails(*source)); } } @@ -1014,7 +1020,7 @@ namespace AppInstaller::Repository return result; } - bool Source::Edit(IProgressCallback& progress) + bool Source::Edit(const SourceEdit& edits) { THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), m_isSourceToBeAdded || m_sourceReferences.size() != 1 || m_source); @@ -1024,9 +1030,8 @@ namespace AppInstaller::Repository // This is intentionally the same policy checks as Remove. If the source cannot be removed then it cannot be edited. EnsureSourceIsRemovable(details); - details.LastUpdateTime = Utility::ConvertUnixEpochToSystemClock(0); - - bool result = EditSourceFromDetails(details, progress); + // Apply the edits and update source list. + bool result = EditSource(details, edits); if (result) { SourceList sourceList; @@ -1036,25 +1041,16 @@ namespace AppInstaller::Repository return result; } - bool Source::IsEditOfSource(const Source& otherSource) + bool Source::RequiresChanges(const SourceEdit& edits) { THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), m_sourceReferences.size() != 1); const auto& details = m_sourceReferences[0]->GetDetails(); - const auto& otherDetails = otherSource.GetDetails(); - - // Verify name matches. At this time this is the unique identifier of a source and - // is used to compare to group policy and well-known sources. If the name matches - // we will assume this is intended to be an edit of an existing source. - if (details.Name != otherDetails.Name) - { - return false; - } // For now the only supported editable difference is Explicit. // If others are added, they would be checked below for changes. bool isChanged = false; - if (details.Explicit != otherDetails.Explicit) + if (edits.Explicit.has_value() && edits.Explicit.value() != details.Explicit) { isChanged = true; } diff --git a/src/AppInstallerRepositoryCore/Rest/RestSourceFactory.cpp b/src/AppInstallerRepositoryCore/Rest/RestSourceFactory.cpp index b8f6f56778..a5060ec09d 100644 --- a/src/AppInstallerRepositoryCore/Rest/RestSourceFactory.cpp +++ b/src/AppInstallerRepositoryCore/Rest/RestSourceFactory.cpp @@ -135,7 +135,7 @@ namespace AppInstaller::Repository::Rest return true; } - bool Edit(const SourceDetails& details, IProgressCallback&) override final + bool Edit(SourceDetails& details, const SourceEdit&) override final { THROW_HR_IF(E_INVALIDARG, !Utility::CaseInsensitiveEquals(details.Type, RestSourceFactory::Type())); return true; diff --git a/src/AppInstallerRepositoryCore/SourceFactory.h b/src/AppInstallerRepositoryCore/SourceFactory.h index df5dc8a472..37df62f66b 100644 --- a/src/AppInstallerRepositoryCore/SourceFactory.h +++ b/src/AppInstallerRepositoryCore/SourceFactory.h @@ -41,9 +41,8 @@ namespace AppInstaller::Repository // Return value indicates whether the action completed. virtual bool Remove(const SourceDetails& details, IProgressCallback& progress) = 0; - // Edit the source from the given details.. - // Return value indicates whether the action completed. - virtual bool Edit(const SourceDetails& details, IProgressCallback& progress) = 0; + // Edit the given source with the provided edits. + virtual bool Edit(SourceDetails& details, const SourceEdit& edits) = 0; // Gets the factory for the given type. static std::unique_ptr GetForType(std::string_view type); diff --git a/src/AppInstallerSharedLib/AppInstallerStrings.cpp b/src/AppInstallerSharedLib/AppInstallerStrings.cpp index 83e9971e6f..931ae8a108 100644 --- a/src/AppInstallerSharedLib/AppInstallerStrings.cpp +++ b/src/AppInstallerSharedLib/AppInstallerStrings.cpp @@ -939,6 +939,13 @@ namespace AppInstaller::Utility return value ? "true"sv : "false"sv; } + bool ConvertStringToBool(const std::string_view& input) + { + bool value; + std::istringstream(std::string(input)) >> std::boolalpha >> value; + return value; + } + std::string ConvertGuidToString(const GUID& value) { wchar_t buffer[40]; diff --git a/src/AppInstallerSharedLib/Public/AppInstallerStrings.h b/src/AppInstallerSharedLib/Public/AppInstallerStrings.h index 5ead01371e..055e9eb57c 100644 --- a/src/AppInstallerSharedLib/Public/AppInstallerStrings.h +++ b/src/AppInstallerSharedLib/Public/AppInstallerStrings.h @@ -290,6 +290,9 @@ namespace AppInstaller::Utility // Converts the given boolean value to a string. std::string_view ConvertBoolToString(bool value); + // Converts the given stringview into a bool. + bool ConvertStringToBool(const std::string_view& value); + // Converts the given GUID value to a string. std::string ConvertGuidToString(const GUID& value); From 3caf74bb84b903a31688f3ba39364fe682c028f1 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Mon, 8 Dec 2025 15:08:05 -0800 Subject: [PATCH 08/19] Fix spelling --- src/AppInstallerSharedLib/Public/AppInstallerStrings.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AppInstallerSharedLib/Public/AppInstallerStrings.h b/src/AppInstallerSharedLib/Public/AppInstallerStrings.h index 055e9eb57c..95c46ca90d 100644 --- a/src/AppInstallerSharedLib/Public/AppInstallerStrings.h +++ b/src/AppInstallerSharedLib/Public/AppInstallerStrings.h @@ -290,7 +290,7 @@ namespace AppInstaller::Utility // Converts the given boolean value to a string. std::string_view ConvertBoolToString(bool value); - // Converts the given stringview into a bool. + // Converts the given string view into a bool. bool ConvertStringToBool(const std::string_view& value); // Converts the given GUID value to a string. From 98d972b0dd9e8738036364e904215f2f897cae2e Mon Sep 17 00:00:00 2001 From: David Bennett Date: Tue, 9 Dec 2025 15:15:12 -0800 Subject: [PATCH 09/19] Fix E2E default override test --- src/AppInstallerCLIE2ETests/SourceCommand.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AppInstallerCLIE2ETests/SourceCommand.cs b/src/AppInstallerCLIE2ETests/SourceCommand.cs index 04fabd4924..8afca3b35a 100644 --- a/src/AppInstallerCLIE2ETests/SourceCommand.cs +++ b/src/AppInstallerCLIE2ETests/SourceCommand.cs @@ -319,7 +319,7 @@ public void SourceEditOverrideDefault() Assert.AreEqual(Constants.ErrorCode.S_OK, listResult.ExitCode); Assert.True(listResult.StdOut.Contains("true")); - var editResult = TestCommon.RunAICLICommand("source edit", "winget-font"); + var editResult = TestCommon.RunAICLICommand("source edit", "winget-font -e false"); Assert.AreEqual(Constants.ErrorCode.S_OK, editResult.ExitCode); Assert.True(editResult.StdOut.Contains("Done")); From 659be671cc8fbddf625a32ae4aa43683be6a776f Mon Sep 17 00:00:00 2001 From: David Bennett Date: Wed, 10 Dec 2025 10:19:12 -0800 Subject: [PATCH 10/19] Add COM for EditPackageCatalog --- .../Commands/SourceCommand.h | 2 +- .../Interop/PackageCatalogInterop.cs | 74 +++++++++++++++++++ ....Management.Deployment.InProc.dll.manifest | 4 + .../Factory.cpp | 5 +- .../ClassesDefinition.cs | 12 +++ .../WinGetProjectionFactory.cs | 4 +- .../ComClsids.cpp | 5 ++ .../Converters.cpp | 13 ++++ .../Converters.h | 5 ++ .../EditPackageCatalogOptions.cpp | 35 +++++++++ .../EditPackageCatalogOptions.h | 35 +++++++++ .../EditPackageCatalogResult.cpp | 25 +++++++ .../EditPackageCatalogResult.h | 27 +++++++ .../Microsoft.Management.Deployment.vcxproj | 7 +- ...soft.Management.Deployment.vcxproj.filters | 4 + .../PackageManager.cpp | 49 ++++++++++++ .../PackageManager.h | 2 + .../PackageManager.idl | 47 +++++++++++- .../Public/ComClsids.h | 3 + 19 files changed, 352 insertions(+), 6 deletions(-) create mode 100644 src/Microsoft.Management.Deployment/EditPackageCatalogOptions.cpp create mode 100644 src/Microsoft.Management.Deployment/EditPackageCatalogOptions.h create mode 100644 src/Microsoft.Management.Deployment/EditPackageCatalogResult.cpp create mode 100644 src/Microsoft.Management.Deployment/EditPackageCatalogResult.h diff --git a/src/AppInstallerCLICore/Commands/SourceCommand.h b/src/AppInstallerCLICore/Commands/SourceCommand.h index b576c61581..2e52ccf578 100644 --- a/src/AppInstallerCLICore/Commands/SourceCommand.h +++ b/src/AppInstallerCLICore/Commands/SourceCommand.h @@ -124,7 +124,7 @@ namespace AppInstaller::CLI struct SourceEditCommand final : public Command { - SourceEditCommand(std::string_view parent) : Command("edit", { "config" }, parent, Settings::ExperimentalFeature::Feature::SourceEdit) {} + SourceEditCommand(std::string_view parent) : Command("edit", { "config", "set" }, parent, Settings::ExperimentalFeature::Feature::SourceEdit) {} std::vector GetArguments() const override; diff --git a/src/AppInstallerCLIE2ETests/Interop/PackageCatalogInterop.cs b/src/AppInstallerCLIE2ETests/Interop/PackageCatalogInterop.cs index c38cb36f6b..df139b827a 100644 --- a/src/AppInstallerCLIE2ETests/Interop/PackageCatalogInterop.cs +++ b/src/AppInstallerCLIE2ETests/Interop/PackageCatalogInterop.cs @@ -254,6 +254,63 @@ public async Task RemoveNonExistingPackageCatalog() await this.RemoveAndValidatePackageCatalogAsync(removePackageCatalogOptions, RemovePackageCatalogStatus.InvalidOptions, Constants.ErrorCode.ERROR_SOURCE_NAME_DOES_NOT_EXIST); } + /// + /// Edit package catalog with invalid options. + /// + [Test] + public void EditPackageCatalogWithInvalidOptions() + { + // Edit package catalog with null options. + Assert.Throws(() => this.packageManager.EditPackageCatalog(null)); + + // Edit package catalog with empty options. + Assert.Throws(() => this.packageManager.EditPackageCatalog(this.TestFactory.CreateEditPackageCatalogOptions())); + } + + /// + /// Edit a package catalog that is not present. + /// + [Test] + public void EditNonExistingPackageCatalog() + { + EditPackageCatalogOptions editPackageCatalogOptions = this.TestFactory.CreateEditPackageCatalogOptions(); + editPackageCatalogOptions.Name = Constants.TestSourceName; + + this.EditAndValidatePackageCatalog(editPackageCatalogOptions, EditPackageCatalogStatus.InvalidOptions, Constants.ErrorCode.ERROR_SOURCE_NAME_DOES_NOT_EXIST); + } + + /// + /// Edit a package catalog that is not present. + /// + /// representing the asynchronous unit test. + [Test] + public async Task AddEditRemovePackageCatalog() + { + // Add + AddPackageCatalogOptions options = this.TestFactory.CreateAddPackageCatalogOptions(); + options.SourceUri = Constants.TestSourceUrl; + options.Name = Constants.TestSourceName; + options.TrustLevel = PackageCatalogTrustLevel.Trusted; + + await this.AddAndValidatePackageCatalogAsync(options, AddPackageCatalogStatus.Ok); + + // Edit + EditPackageCatalogOptions editOptions = this.TestFactory.CreateEditPackageCatalogOptions(); + editOptions.Name = Constants.TestSourceName; + editOptions.Explicit = "false"; + this.EditAndValidatePackageCatalog(editOptions, EditPackageCatalogStatus.Ok); + + // Remove + RemovePackageCatalogOptions removePackageCatalogOptions = this.TestFactory.CreateRemovePackageCatalogOptions(); + removePackageCatalogOptions.Name = Constants.TestSourceName; + var removeCatalogResult = await this.packageManager.RemovePackageCatalogAsync(removePackageCatalogOptions); + Assert.IsNotNull(removeCatalogResult); + Assert.AreEqual(RemovePackageCatalogStatus.Ok, removeCatalogResult.Status); + + var testSource = this.packageManager.GetPackageCatalogByName(Constants.TestSourceName); + Assert.IsNull(testSource); + } + /// /// Test class Tear down. /// @@ -324,5 +381,22 @@ private async Task RemoveAndValidatePackageCatalogAsync(RemovePackageCatalogOpti var packageCatalog = this.packageManager.GetPackageCatalogByName(removePackageCatalogOptions.Name); Assert.IsNull(packageCatalog); } + + private void EditAndValidatePackageCatalog(EditPackageCatalogOptions editPackageCatalogOptions, EditPackageCatalogStatus expectedStatus, int expectedErrorCode = 0) + { + var editCatalogResult = this.packageManager.EditPackageCatalog(editPackageCatalogOptions); + Assert.IsNotNull(editCatalogResult); + Assert.AreEqual(expectedStatus, editCatalogResult.Status); + + if (expectedStatus != EditPackageCatalogStatus.Ok && expectedErrorCode != 0) + { + Assert.AreEqual(expectedErrorCode, editCatalogResult.ExtendedErrorCode.HResult); + return; + } + + // Verify edits are correct. + var packageCatalog = this.packageManager.GetPackageCatalogByName(editPackageCatalogOptions.Name); + Assert.AreEqual(packageCatalog.Info.Explicit, bool.Parse(editPackageCatalogOptions.Explicit)); + } } } diff --git a/src/Microsoft.Management.Deployment.InProc/Microsoft.Management.Deployment.InProc.dll.manifest b/src/Microsoft.Management.Deployment.InProc/Microsoft.Management.Deployment.InProc.dll.manifest index 4e9725d652..14ca57a175 100644 --- a/src/Microsoft.Management.Deployment.InProc/Microsoft.Management.Deployment.InProc.dll.manifest +++ b/src/Microsoft.Management.Deployment.InProc/Microsoft.Management.Deployment.InProc.dll.manifest @@ -55,5 +55,9 @@ clsid="{1125D3A6-E2CE-479A-91D5-71A3F6F8B00B}" threadingModel="Both" description="RemovePackageCatalogOptions"/> + diff --git a/src/Microsoft.Management.Deployment.OutOfProc/Factory.cpp b/src/Microsoft.Management.Deployment.OutOfProc/Factory.cpp index 89bd2f4048..087210d08b 100644 --- a/src/Microsoft.Management.Deployment.OutOfProc/Factory.cpp +++ b/src/Microsoft.Management.Deployment.OutOfProc/Factory.cpp @@ -24,6 +24,7 @@ namespace Microsoft::Management::Deployment::OutOfProc constexpr CLSID CLSID_RepairOptions = { 0x0498F441, 0x3097, 0x455F, { 0x9C, 0xAF, 0x14, 0x8F, 0x28, 0x29, 0x38, 0x65 } }; //0498F441-3097-455F-9CAF-148F28293865 constexpr CLSID CLSID_AddPackageCatalogOptions = { 0xDB9D012D, 0x00D7, 0x47EE, { 0x8F, 0xB1, 0x60, 0x6E, 0x10, 0xAC, 0x4F, 0x51 } }; //DB9D012D-00D7-47EE-8FB1-606E10AC4F51 constexpr CLSID CLSID_RemovePackageCatalogOptions = { 0x032B1C58, 0xB975, 0x469B, { 0xA0, 0x13, 0xE6, 0x32, 0xB6, 0xEC, 0xE8, 0xD8 } }; //032B1C58-B975-469B-A013-E632B6ECE8D8 + constexpr CLSID CLSID_EditPackageCatalogOptions = { 0xA9F5E736, 0x68CE, 0x463C, { 0xBA, 0x6D, 0xDE, 0x96, 0x8F, 0x0C, 0xCE, 0x04 } }; //A9F5E736-68CE-463C-BA6D-DE968F0CCE04 #else constexpr CLSID CLSID_PackageManager = { 0x74CB3139, 0xB7C5, 0x4B9E, { 0x93, 0x88, 0xE6, 0x61, 0x6D, 0xEA, 0x28, 0x8C } }; //74CB3139-B7C5-4B9E-9388-E6616DEA288C constexpr CLSID CLSID_InstallOptions = { 0x44FE0580, 0x62F7, 0x44D4, { 0x9E, 0x91, 0xAA, 0x96, 0x14, 0xAB, 0x3E, 0x86 } }; //44FE0580-62F7-44D4-9E91-AA9614AB3E86 @@ -36,6 +37,7 @@ namespace Microsoft::Management::Deployment::OutOfProc constexpr CLSID CLSID_RepairOptions = { 0xE62BB1E7, 0xC7B2, 0x4AEC, { 0x9E, 0x28, 0xFB, 0x64, 0x9B, 0x30, 0xFF, 0x03 } }; //E62BB1E7-C7B2-4AEC-9E28-FB649B30FF03 constexpr CLSID CLSID_AddPackageCatalogOptions = { 0xD58C7E4C, 0x70E6, 0x476C, { 0xA5, 0xD4, 0x80, 0x34, 0x1E, 0xD8, 0x02, 0x52 } }; //D58C7E4C-70E6-476C-A5D4-80341ED80252 constexpr CLSID CLSID_RemovePackageCatalogOptions = { 0x87A96609, 0x1A39, 0x4955, { 0xBE, 0x72, 0x71, 0x74, 0xE1, 0x47, 0xB7, 0xDC } }; //87A96609-1A39-4955-BE72-7174E147B7DC + constexpr CLSID CLSID_EditPackageCatalogOptions = { 0x29B19238, 0x81AD, 0x4A8E, { 0xA2, 0xFC, 0xAD, 0xF1, 0x7C, 0x38, 0xCA, 0xEB } }; //29B19238-81AD-4A8E-A2FC-ADF17C38CAEB #endif @@ -45,7 +47,7 @@ namespace Microsoft::Management::Deployment::OutOfProc GUID CLSID; }; - constexpr std::array s_nameCLSIDPairs + constexpr std::array s_nameCLSIDPairs { NameCLSIDPair{ L"Microsoft.Management.Deployment.PackageManager"sv, CLSID_PackageManager }, NameCLSIDPair{ L"Microsoft.Management.Deployment.InstallOptions"sv, CLSID_InstallOptions }, @@ -58,6 +60,7 @@ namespace Microsoft::Management::Deployment::OutOfProc NameCLSIDPair{ L"Microsoft.Management.Deployment.RepairOptions"sv, CLSID_RepairOptions }, NameCLSIDPair{ L"Microsoft.Management.Deployment.AddPackageCatalogOptions"sv, CLSID_AddPackageCatalogOptions }, NameCLSIDPair{ L"Microsoft.Management.Deployment.RemovePackageCatalogOptions"sv, CLSID_RemovePackageCatalogOptions }, + NameCLSIDPair{ L"Microsoft.Management.Deployment.EditPackageCatalogOptions"sv, CLSID_EditPackageCatalogOptions }, }; bool IsCLSIDPresent(const GUID& clsid) diff --git a/src/Microsoft.Management.Deployment.Projection/ClassesDefinition.cs b/src/Microsoft.Management.Deployment.Projection/ClassesDefinition.cs index ed6adfb500..ecae3141cf 100644 --- a/src/Microsoft.Management.Deployment.Projection/ClassesDefinition.cs +++ b/src/Microsoft.Management.Deployment.Projection/ClassesDefinition.cs @@ -150,6 +150,18 @@ internal static class ClassesDefinition [ClsidContext.OutOfProc] = new Guid("032B1C58-B975-469B-A013-E632B6ECE8D8"), [ClsidContext.OutOfProcDev] = new Guid("87A96609-1A39-4955-BE72-7174E147B7DC"), } + }, + + [typeof(EditPackageCatalogOptions)] = new() + { + ProjectedClassType = typeof(EditPackageCatalogOptions), + InterfaceType = typeof(IEditPackageCatalogOptions), + Clsids = new Dictionary() + { + [ClsidContext.InProc] = new Guid("E8E12FE1-AB77-40C4-A562-E91FB51B4E82"), + [ClsidContext.OutOfProc] = new Guid("A9F5E736-68CE-463C-BA6D-DE968F0CCE04"), + [ClsidContext.OutOfProcDev] = new Guid("29B19238-81AD-4A8E-A2FC-ADF17C38CAEB"), + } } }; diff --git a/src/Microsoft.Management.Deployment.Projection/WinGetProjectionFactory.cs b/src/Microsoft.Management.Deployment.Projection/WinGetProjectionFactory.cs index 5201492b2f..356d1ff8b3 100644 --- a/src/Microsoft.Management.Deployment.Projection/WinGetProjectionFactory.cs +++ b/src/Microsoft.Management.Deployment.Projection/WinGetProjectionFactory.cs @@ -39,6 +39,8 @@ public WinGetProjectionFactory(IInstanceInitializer instanceInitializer) public AddPackageCatalogOptions CreateAddPackageCatalogOptions() => InstanceInitializer.CreateInstance(); - public RemovePackageCatalogOptions CreateRemovePackageCatalogOptions() => InstanceInitializer.CreateInstance(); + public RemovePackageCatalogOptions CreateRemovePackageCatalogOptions() => InstanceInitializer.CreateInstance(); + + public EditPackageCatalogOptions CreateEditPackageCatalogOptions() => InstanceInitializer.CreateInstance(); } } diff --git a/src/Microsoft.Management.Deployment/ComClsids.cpp b/src/Microsoft.Management.Deployment/ComClsids.cpp index 1cf315ba5c..24c64ec942 100644 --- a/src/Microsoft.Management.Deployment/ComClsids.cpp +++ b/src/Microsoft.Management.Deployment/ComClsids.cpp @@ -17,6 +17,7 @@ #include "RepairOptions.h" #include "AddPackageCatalogOptions.h" #include "RemovePackageCatalogOptions.h" +#include "EditPackageCatalogOptions.h" #pragma warning( pop ) namespace winrt::Microsoft::Management::Deployment @@ -70,6 +71,10 @@ namespace winrt::Microsoft::Management::Deployment else if (IsEqualCLSID(clsid, WINGET_INPROC_COM_CLSID_RemovePackageCatalogOptions)) { return __uuidof(winrt::Microsoft::Management::Deployment::implementation::RemovePackageCatalogOptions); + } + else if (IsEqualCLSID(clsid, WINGET_INPROC_COM_CLSID_EditPackageCatalogOptions)) + { + return __uuidof(winrt::Microsoft::Management::Deployment::implementation::EditPackageCatalogOptions); } else { diff --git a/src/Microsoft.Management.Deployment/Converters.cpp b/src/Microsoft.Management.Deployment/Converters.cpp index 8524ad5c3a..3c3b4f238b 100644 --- a/src/Microsoft.Management.Deployment/Converters.cpp +++ b/src/Microsoft.Management.Deployment/Converters.cpp @@ -530,6 +530,19 @@ namespace winrt::Microsoft::Management::Deployment::implementation } } + EditPackageCatalogStatus GetEditPackageCatalogOperationStatus(winrt::hresult hresult) + { + switch (hresult) + { + case APPINSTALLER_CLI_ERROR_SOURCE_NAME_DOES_NOT_EXIST: + return EditPackageCatalogStatus::InvalidOptions; + case APPINSTALLER_CLI_ERROR_INVALID_SOURCE_TYPE: + return EditPackageCatalogStatus::CatalogError; + default: + return HandleCommonCatalogOperationStatus(hresult); + } + } + ::AppInstaller::Manifest::PlatformEnum GetPlatformEnum(WindowsPlatform value) { switch (value) diff --git a/src/Microsoft.Management.Deployment/Converters.h b/src/Microsoft.Management.Deployment/Converters.h index 31db68531a..460fe4e21c 100644 --- a/src/Microsoft.Management.Deployment/Converters.h +++ b/src/Microsoft.Management.Deployment/Converters.h @@ -33,6 +33,7 @@ namespace winrt::Microsoft::Management::Deployment::implementation ::AppInstaller::Manifest::ScopeEnum GetManifestRepairScope(winrt::Microsoft::Management::Deployment::PackageRepairScope scope); winrt::Microsoft::Management::Deployment::AddPackageCatalogStatus GetAddPackageCatalogOperationStatus(winrt::hresult hresult); winrt::Microsoft::Management::Deployment::RemovePackageCatalogStatus GetRemovePackageCatalogOperationStatus(winrt::hresult hresult); + winrt::Microsoft::Management::Deployment::EditPackageCatalogStatus GetEditPackageCatalogOperationStatus(winrt::hresult hresult); ::AppInstaller::Manifest::PlatformEnum GetPlatformEnum(winrt::Microsoft::Management::Deployment::WindowsPlatform value); #define WINGET_GET_OPERATION_RESULT_STATUS(_installResultStatus_, _uninstallResultStatus_, _downloadResultStatus_, _repairResultStatus_) \ @@ -194,6 +195,10 @@ namespace winrt::Microsoft::Management::Deployment::implementation { return HandleCommonCatalogOperationStatus(hresult); } + else if constexpr (std::is_same_v) + { + return GetEditPackageCatalogOperationStatus(hresult); + } else { throw winrt::hresult_error(E_UNEXPECTED); diff --git a/src/Microsoft.Management.Deployment/EditPackageCatalogOptions.cpp b/src/Microsoft.Management.Deployment/EditPackageCatalogOptions.cpp new file mode 100644 index 0000000000..9089be6a0c --- /dev/null +++ b/src/Microsoft.Management.Deployment/EditPackageCatalogOptions.cpp @@ -0,0 +1,35 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#include "pch.h" +#pragma warning( push ) +#pragma warning ( disable : 4467 6388) +// 6388 Allow CreateInstance. +#include +// 4467 Allow use of uuid attribute for com object creation. +#include "EditPackageCatalogOptions.h" +#pragma warning( pop ) +#include "EditPackageCatalogOptions.g.cpp" +#include "Converters.h" +#include "Helpers.h" + +namespace winrt::Microsoft::Management::Deployment::implementation +{ + hstring EditPackageCatalogOptions::Name() + { + return hstring(m_name); + } + void EditPackageCatalogOptions::Name(hstring const& value) + { + m_name = value; + } + hstring EditPackageCatalogOptions::Explicit() + { + return hstring(m_explicit); + } + void EditPackageCatalogOptions::Explicit(hstring const& value) + { + m_explicit = value; + } + + CoCreatableMicrosoftManagementDeploymentClass(EditPackageCatalogOptions); +} diff --git a/src/Microsoft.Management.Deployment/EditPackageCatalogOptions.h b/src/Microsoft.Management.Deployment/EditPackageCatalogOptions.h new file mode 100644 index 0000000000..e09d00c437 --- /dev/null +++ b/src/Microsoft.Management.Deployment/EditPackageCatalogOptions.h @@ -0,0 +1,35 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#pragma once +#include "EditPackageCatalogOptions.g.h" +#include "public/ComClsids.h" + +namespace winrt::Microsoft::Management::Deployment::implementation +{ + [uuid(WINGET_OUTOFPROC_COM_CLSID_EditPackageCatalogOptions)] + struct EditPackageCatalogOptions : EditPackageCatalogOptionsT + { + EditPackageCatalogOptions() = default; + + hstring Name(); + void Name(hstring const& value); + + hstring Explicit(); + void Explicit(hstring const& value); + +#if !defined(INCLUDE_ONLY_INTERFACE_METHODS) + private: + hstring m_name = L""; + hstring m_explicit = L""; +#endif + }; +} + +#if !defined(INCLUDE_ONLY_INTERFACE_METHODS) +namespace winrt::Microsoft::Management::Deployment::factory_implementation +{ + struct EditPackageCatalogOptions : EditPackageCatalogOptionsT + { + }; +} +#endif diff --git a/src/Microsoft.Management.Deployment/EditPackageCatalogResult.cpp b/src/Microsoft.Management.Deployment/EditPackageCatalogResult.cpp new file mode 100644 index 0000000000..377d97221e --- /dev/null +++ b/src/Microsoft.Management.Deployment/EditPackageCatalogResult.cpp @@ -0,0 +1,25 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#include "pch.h" +#include "EditPackageCatalogResult.h" +#include "EditPackageCatalogResult.g.cpp" +#include + +namespace winrt::Microsoft::Management::Deployment::implementation +{ + void EditPackageCatalogResult::Initialize( + winrt::Microsoft::Management::Deployment::EditPackageCatalogStatus status, + winrt::hresult extendedErrorCode) + { + m_status = status; + m_extendedErrorCode = extendedErrorCode; + } + winrt::Microsoft::Management::Deployment::EditPackageCatalogStatus EditPackageCatalogResult::Status() + { + return m_status; + } + winrt::hresult EditPackageCatalogResult::ExtendedErrorCode() + { + return m_extendedErrorCode; + } +} diff --git a/src/Microsoft.Management.Deployment/EditPackageCatalogResult.h b/src/Microsoft.Management.Deployment/EditPackageCatalogResult.h new file mode 100644 index 0000000000..4a38543fe4 --- /dev/null +++ b/src/Microsoft.Management.Deployment/EditPackageCatalogResult.h @@ -0,0 +1,27 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#pragma once +#include "EditPackageCatalogResult.g.h" + +namespace winrt::Microsoft::Management::Deployment::implementation +{ + struct EditPackageCatalogResult : EditPackageCatalogResultT + { + EditPackageCatalogResult() = default; + +#if !defined(INCLUDE_ONLY_INTERFACE_METHODS) + void Initialize( + winrt::Microsoft::Management::Deployment::EditPackageCatalogStatus status, + winrt::hresult extendedErrorCode); +#endif + + winrt::Microsoft::Management::Deployment::EditPackageCatalogStatus Status(); + winrt::hresult ExtendedErrorCode(); + +#if !defined(INCLUDE_ONLY_INTERFACE_METHODS) + private: + winrt::Microsoft::Management::Deployment::EditPackageCatalogStatus m_status = winrt::Microsoft::Management::Deployment::EditPackageCatalogStatus::Ok; + winrt::hresult m_extendedErrorCode = S_OK; +#endif + }; +} diff --git a/src/Microsoft.Management.Deployment/Microsoft.Management.Deployment.vcxproj b/src/Microsoft.Management.Deployment/Microsoft.Management.Deployment.vcxproj index a01d0839e5..fcf50b21c8 100644 --- a/src/Microsoft.Management.Deployment/Microsoft.Management.Deployment.vcxproj +++ b/src/Microsoft.Management.Deployment/Microsoft.Management.Deployment.vcxproj @@ -169,6 +169,8 @@ + + @@ -217,6 +219,8 @@ + + @@ -275,5 +279,4 @@ - - + \ No newline at end of file diff --git a/src/Microsoft.Management.Deployment/Microsoft.Management.Deployment.vcxproj.filters b/src/Microsoft.Management.Deployment/Microsoft.Management.Deployment.vcxproj.filters index 78c593d394..58b571bd39 100644 --- a/src/Microsoft.Management.Deployment/Microsoft.Management.Deployment.vcxproj.filters +++ b/src/Microsoft.Management.Deployment/Microsoft.Management.Deployment.vcxproj.filters @@ -46,6 +46,8 @@ + + @@ -97,6 +99,8 @@ + + diff --git a/src/Microsoft.Management.Deployment/PackageManager.cpp b/src/Microsoft.Management.Deployment/PackageManager.cpp index 2697e733c8..165488b77c 100644 --- a/src/Microsoft.Management.Deployment/PackageManager.cpp +++ b/src/Microsoft.Management.Deployment/PackageManager.cpp @@ -33,6 +33,7 @@ #include "PackageVersionId.h" #include "AddPackageCatalogResult.h" #include "RemovePackageCatalogResult.h" +#include "EditPackageCatalogResult.h" #include "Converters.h" #include "Helpers.h" #include "ContextOrchestrator.h" @@ -1438,5 +1439,53 @@ namespace winrt::Microsoft::Management::Deployment::implementation return winrt::hstring{ AppInstaller::Utility::ConvertToUTF16(AppInstaller::Runtime::GetClientVersion()) }; } + winrt::Microsoft::Management::Deployment::EditPackageCatalogResult GetEditPackageCatalogResult(winrt::hresult terminationStatus) + { + winrt::Microsoft::Management::Deployment::EditPackageCatalogStatus status = GetPackageCatalogOperationStatus(terminationStatus); + auto editResult = winrt::make_self>(); + editResult->Initialize(status, terminationStatus); + return *editResult; + } + + winrt::Microsoft::Management::Deployment::EditPackageCatalogResult PackageManager::EditPackageCatalog(winrt::Microsoft::Management::Deployment::EditPackageCatalogOptions options) + { + LogStartupIfApplicable(); + + // options must be set. + THROW_HR_IF_NULL(E_POINTER, options); + THROW_HR_IF(E_INVALIDARG, options.Name().empty()); + + HRESULT terminationHR = S_OK; + try { + + // Check if running as admin/system. + // [NOTE:] For OutOfProc calls, the Windows Package Manager Service executes in the context initiated by the caller process, + // so the same admin/system validation check is applicable for both InProc and OutOfProc calls. + THROW_HR_IF(APPINSTALLER_CLI_ERROR_COMMAND_REQUIRES_ADMIN, !AppInstaller::Runtime::IsRunningAsAdminOrSystem()); + + auto matchingSource = GetMatchingSource(winrt::to_string(options.Name())); + THROW_HR_IF(APPINSTALLER_CLI_ERROR_SOURCE_NAME_DOES_NOT_EXIST, !matchingSource.has_value()); + ::AppInstaller::Repository::Source sourceToEdit = ::AppInstaller::Repository::Source{ matchingSource.value().Name }; + + auto isExplicit = matchingSource.value().Explicit; + if (!options.Explicit().empty()) + { + isExplicit = AppInstaller::Utility::ConvertStringToBool(winrt::to_string(options.Explicit())); + } + + ::AppInstaller::Repository::SourceEdit edits{ std::optional{ isExplicit } }; + if (sourceToEdit.RequiresChanges(edits)) + { + sourceToEdit.Edit(edits); + } + } + catch (...) + { + terminationHR = AppInstaller::CLI::Workflow::HandleException(nullptr, std::current_exception()); + } + + return GetEditPackageCatalogResult(terminationHR); + } + CoCreatableMicrosoftManagementDeploymentClass(PackageManager); } diff --git a/src/Microsoft.Management.Deployment/PackageManager.h b/src/Microsoft.Management.Deployment/PackageManager.h index 0969836d6f..e191ae3ce6 100644 --- a/src/Microsoft.Management.Deployment/PackageManager.h +++ b/src/Microsoft.Management.Deployment/PackageManager.h @@ -51,6 +51,8 @@ namespace winrt::Microsoft::Management::Deployment::implementation RemovePackageCatalogAsync(winrt::Microsoft::Management::Deployment::RemovePackageCatalogOptions options); // Contract 13.0 winrt::hstring Version() const; + // Contract 28.0 + winrt::Microsoft::Management::Deployment::EditPackageCatalogResult EditPackageCatalog(winrt::Microsoft::Management::Deployment::EditPackageCatalogOptions options); }; #if !defined(INCLUDE_ONLY_INTERFACE_METHODS) diff --git a/src/Microsoft.Management.Deployment/PackageManager.idl b/src/Microsoft.Management.Deployment/PackageManager.idl index d931ff6b39..87f4d2df5d 100644 --- a/src/Microsoft.Management.Deployment/PackageManager.idl +++ b/src/Microsoft.Management.Deployment/PackageManager.idl @@ -2,7 +2,7 @@ // Licensed under the MIT License. namespace Microsoft.Management.Deployment { - [contractversion(13)] // For version 1.12 + [contractversion(28)] // For version 1.28 apicontract WindowsPackageManagerContract{}; /// State of the install @@ -1541,6 +1541,45 @@ namespace Microsoft.Management.Deployment HRESULT ExtendedErrorCode { get; }; }; + /// IMPLEMENTATION NOTE: EditPackageCatalogOptions + [contract(Microsoft.Management.Deployment.WindowsPackageManagerContract, 28)] + runtimeclass EditPackageCatalogOptions + { + EditPackageCatalogOptions(); + + /// The name of the package catalog. + /// SAMPLE VALUES: For OpenWindowsCatalog "winget". + /// For contoso sample on msdn "contoso" + String Name; + + /// Editing the Explicit property has three states: true, false, and not specified (no changes). + /// SAMPLE VALUES: "true" or "false" or emptystring. + String Explicit; + }; + + /// IMPLEMENTATION NOTE: RemovePackageCatalogStatus + [contract(Microsoft.Management.Deployment.WindowsPackageManagerContract, 28)] + enum EditPackageCatalogStatus + { + Ok, + GroupPolicyError, + CatalogError, + InternalError, + AccessDenied, + InvalidOptions, + }; + + /// IMPLEMENTATION NOTE: RemovePackageCatalogResult + /// Result of editing a package catalog. + [contract(Microsoft.Management.Deployment.WindowsPackageManagerContract, 28)] + runtimeclass EditPackageCatalogResult + { + EditPackageCatalogStatus Status { get; }; + + /// Error codes + HRESULT ExtendedErrorCode { get; }; + }; + [contract(Microsoft.Management.Deployment.WindowsPackageManagerContract, 1)] runtimeclass PackageManager { @@ -1573,6 +1612,12 @@ namespace Microsoft.Management.Deployment Windows.Foundation.IAsyncOperationWithProgress RemovePackageCatalogAsync(RemovePackageCatalogOptions options); } + [contract(Microsoft.Management.Deployment.WindowsPackageManagerContract, 28)] + { + /// Edit an existing Windows Package Catalog. + EditPackageCatalogResult EditPackageCatalog(EditPackageCatalogOptions options); + } + /// Install the specified package Windows.Foundation.IAsyncOperationWithProgress InstallPackageAsync(CatalogPackage package, InstallOptions options); diff --git a/src/Microsoft.Management.Deployment/Public/ComClsids.h b/src/Microsoft.Management.Deployment/Public/ComClsids.h index 75f14afbd6..e9137d788f 100644 --- a/src/Microsoft.Management.Deployment/Public/ComClsids.h +++ b/src/Microsoft.Management.Deployment/Public/ComClsids.h @@ -17,6 +17,7 @@ #define WINGET_OUTOFPROC_COM_CLSID_RepairOptions "0498F441-3097-455F-9CAF-148F28293865" #define WINGET_OUTOFPROC_COM_CLSID_AddPackageCatalogOptions "DB9D012D-00D7-47EE-8FB1-606E10AC4F51" #define WINGET_OUTOFPROC_COM_CLSID_RemovePackageCatalogOptions "032B1C58-B975-469B-A013-E632B6ECE8D8" +#define WINGET_OUTOFPROC_COM_CLSID_EditPackageCatalogOptions "A9F5E736-68CE-463C-BA6D-DE968F0CCE04" #else #define WINGET_OUTOFPROC_COM_CLSID_PackageManager "74CB3139-B7C5-4B9E-9388-E6616DEA288C" #define WINGET_OUTOFPROC_COM_CLSID_FindPackagesOptions "1BD8FF3A-EC50-4F69-AEEE-DF4C9D3BAA96" @@ -30,6 +31,7 @@ #define WINGET_OUTOFPROC_COM_CLSID_RepairOptions "E62BB1E7-C7B2-4AEC-9E28-FB649B30FF03" #define WINGET_OUTOFPROC_COM_CLSID_AddPackageCatalogOptions "D58C7E4C-70E6-476C-A5D4-80341ED80252" #define WINGET_OUTOFPROC_COM_CLSID_RemovePackageCatalogOptions "87A96609-1A39-4955-BE72-7174E147B7DC" +#define WINGET_OUTOFPROC_COM_CLSID_EditPackageCatalogOptions "29B19238-81AD-4A8E-A2FC-ADF17C38CAEB" #endif // Clsids only used in in-proc invocation @@ -50,6 +52,7 @@ namespace winrt::Microsoft::Management::Deployment const CLSID WINGET_INPROC_COM_CLSID_RepairOptions = { 0x30c024c4, 0x852c, 0x4dd4, 0x98, 0x10, 0x13, 0x48, 0xc5, 0x1e, 0xf9, 0xbb }; // {30C024C4-852C-4DD4-9810-1348C51EF9BB} const CLSID WINGET_INPROC_COM_CLSID_AddPackageCatalogOptions = { 0x24e6f1fa, 0xe4c3, 0x4acd, 0x96, 0x5d, 0xdf, 0x21, 0x3f, 0xd5, 0x8f, 0x15 }; // {24E6F1FA-E4C3-4ACD-965D-DF213FD58F15} const CLSID WINGET_INPROC_COM_CLSID_RemovePackageCatalogOptions = { 0x1125d3a6, 0xe2ce, 0x479a, 0x91, 0xd5, 0x71, 0xa3, 0xf6, 0xf8, 0xb0, 0xb }; // {1125D3A6-E2CE-479A-91D5-71A3F6F8B00B} + const CLSID WINGET_INPROC_COM_CLSID_EditPackageCatalogOptions = { 0xe8e12fe1, 0xab77, 0x40c4, 0xa5, 0x62, 0xe9, 0x1f, 0xb5, 0x1b, 0x4e, 0x82 }; // {E8E12FE1-AB77-40C4-A562-E91FB51B4E82} CLSID GetRedirectedClsidFromInProcClsid(REFCLSID clsid); } From ba9a2a09ff41b093576732c63bcb36c3349f6381 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Wed, 10 Dec 2025 10:23:55 -0800 Subject: [PATCH 11/19] Fix spelling --- src/Microsoft.Management.Deployment/PackageManager.idl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Management.Deployment/PackageManager.idl b/src/Microsoft.Management.Deployment/PackageManager.idl index 87f4d2df5d..8a056e80bb 100644 --- a/src/Microsoft.Management.Deployment/PackageManager.idl +++ b/src/Microsoft.Management.Deployment/PackageManager.idl @@ -1553,7 +1553,7 @@ namespace Microsoft.Management.Deployment String Name; /// Editing the Explicit property has three states: true, false, and not specified (no changes). - /// SAMPLE VALUES: "true" or "false" or emptystring. + /// SAMPLE VALUES: "true" or "false" or empty string. String Explicit; }; From ceef591545764f072e6fc81ebe84cf1cd24ad002 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Wed, 10 Dec 2025 13:40:31 -0800 Subject: [PATCH 12/19] Add missing registration, policy test --- src/AppInstallerCLIE2ETests/Interop/GroupPolicyForInterop.cs | 4 ++++ src/AppInstallerCLIPackage/Package.appxmanifest | 2 ++ 2 files changed, 6 insertions(+) diff --git a/src/AppInstallerCLIE2ETests/Interop/GroupPolicyForInterop.cs b/src/AppInstallerCLIE2ETests/Interop/GroupPolicyForInterop.cs index e86e27beca..f5e86f9434 100644 --- a/src/AppInstallerCLIE2ETests/Interop/GroupPolicyForInterop.cs +++ b/src/AppInstallerCLIE2ETests/Interop/GroupPolicyForInterop.cs @@ -110,6 +110,10 @@ public void DisableWinGetPolicy() Assert.AreEqual(Constants.BlockByWinGetPolicyErrorMessage, groupPolicyException.Message); Assert.AreEqual(Constants.ErrorCode.ERROR_BLOCKED_BY_POLICY, groupPolicyException.HResult); + groupPolicyException = Assert.Catch(() => { EditPackageCatalogOptions packageManagerSettings = this.TestFactory.CreateEditPackageCatalogOptions(); }); + Assert.AreEqual(Constants.BlockByWinGetPolicyErrorMessage, groupPolicyException.Message); + Assert.AreEqual(Constants.ErrorCode.ERROR_BLOCKED_BY_POLICY, groupPolicyException.HResult); + // PackageManagerSettings is not implemented in context OutOfProcDev if (this.TestFactory.Context == ClsidContext.InProc) { diff --git a/src/AppInstallerCLIPackage/Package.appxmanifest b/src/AppInstallerCLIPackage/Package.appxmanifest index 8bcd03e112..8559da43ae 100644 --- a/src/AppInstallerCLIPackage/Package.appxmanifest +++ b/src/AppInstallerCLIPackage/Package.appxmanifest @@ -84,6 +84,8 @@ + + From 067510a0e0ebcee0bc51a9db8d428a773cdd4865 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Wed, 10 Dec 2025 17:02:39 -0800 Subject: [PATCH 13/19] Update IDL with OptionalBoolean --- .../Interop/PackageCatalogInterop.cs | 7 ++-- .../Converters.cpp | 33 +++++++++++++++++++ .../Converters.h | 2 ++ .../EditPackageCatalogOptions.cpp | 6 ++-- .../EditPackageCatalogOptions.h | 6 ++-- .../PackageManager.cpp | 4 +-- .../PackageManager.idl | 18 +++++++--- 7 files changed, 61 insertions(+), 15 deletions(-) diff --git a/src/AppInstallerCLIE2ETests/Interop/PackageCatalogInterop.cs b/src/AppInstallerCLIE2ETests/Interop/PackageCatalogInterop.cs index df139b827a..3a4e241159 100644 --- a/src/AppInstallerCLIE2ETests/Interop/PackageCatalogInterop.cs +++ b/src/AppInstallerCLIE2ETests/Interop/PackageCatalogInterop.cs @@ -297,7 +297,7 @@ public async Task AddEditRemovePackageCatalog() // Edit EditPackageCatalogOptions editOptions = this.TestFactory.CreateEditPackageCatalogOptions(); editOptions.Name = Constants.TestSourceName; - editOptions.Explicit = "false"; + editOptions.Explicit = OptionalBoolean.False; this.EditAndValidatePackageCatalog(editOptions, EditPackageCatalogStatus.Ok); // Remove @@ -396,7 +396,10 @@ private void EditAndValidatePackageCatalog(EditPackageCatalogOptions editPackage // Verify edits are correct. var packageCatalog = this.packageManager.GetPackageCatalogByName(editPackageCatalogOptions.Name); - Assert.AreEqual(packageCatalog.Info.Explicit, bool.Parse(editPackageCatalogOptions.Explicit)); + if (editPackageCatalogOptions.Explicit != OptionalBoolean.Unspecified) + { + Assert.AreEqual(packageCatalog.Info.Explicit, editPackageCatalogOptions.Explicit); + } } } } diff --git a/src/Microsoft.Management.Deployment/Converters.cpp b/src/Microsoft.Management.Deployment/Converters.cpp index 3c3b4f238b..80def812a2 100644 --- a/src/Microsoft.Management.Deployment/Converters.cpp +++ b/src/Microsoft.Management.Deployment/Converters.cpp @@ -556,4 +556,37 @@ namespace winrt::Microsoft::Management::Deployment::implementation default: return AppInstaller::Manifest::PlatformEnum::Unknown; } } + + std::optional GetOptionalBoolean(winrt::Microsoft::Management::Deployment::OptionalBoolean optionalBoolean) + { + switch (optionalBoolean) + { + case OptionalBoolean::True: + return std::optional { true }; + case OptionalBoolean::False: + return std::optional { false }; + default: + return std::nullopt; + } + } + + winrt::Microsoft::Management::Deployment::OptionalBoolean GetOptionalBoolean(std::optional optionalBoolean) + { + if (optionalBoolean.has_value()) + { + if (optionalBoolean.value()) + { + return OptionalBoolean::True; + } + else + { + return OptionalBoolean::False; + } + } + else + { + return OptionalBoolean::Unspecified; + } + } + } diff --git a/src/Microsoft.Management.Deployment/Converters.h b/src/Microsoft.Management.Deployment/Converters.h index 460fe4e21c..f2535704ab 100644 --- a/src/Microsoft.Management.Deployment/Converters.h +++ b/src/Microsoft.Management.Deployment/Converters.h @@ -35,6 +35,8 @@ namespace winrt::Microsoft::Management::Deployment::implementation winrt::Microsoft::Management::Deployment::RemovePackageCatalogStatus GetRemovePackageCatalogOperationStatus(winrt::hresult hresult); winrt::Microsoft::Management::Deployment::EditPackageCatalogStatus GetEditPackageCatalogOperationStatus(winrt::hresult hresult); ::AppInstaller::Manifest::PlatformEnum GetPlatformEnum(winrt::Microsoft::Management::Deployment::WindowsPlatform value); + std::optional GetOptionalBoolean(winrt::Microsoft::Management::Deployment::OptionalBoolean optionalBoolean); + winrt::Microsoft::Management::Deployment::OptionalBoolean GetOptionalBoolean(std::optional optionalBoolean); #define WINGET_GET_OPERATION_RESULT_STATUS(_installResultStatus_, _uninstallResultStatus_, _downloadResultStatus_, _repairResultStatus_) \ if constexpr (std::is_same_v) \ diff --git a/src/Microsoft.Management.Deployment/EditPackageCatalogOptions.cpp b/src/Microsoft.Management.Deployment/EditPackageCatalogOptions.cpp index 9089be6a0c..906106fa54 100644 --- a/src/Microsoft.Management.Deployment/EditPackageCatalogOptions.cpp +++ b/src/Microsoft.Management.Deployment/EditPackageCatalogOptions.cpp @@ -22,11 +22,11 @@ namespace winrt::Microsoft::Management::Deployment::implementation { m_name = value; } - hstring EditPackageCatalogOptions::Explicit() + OptionalBoolean EditPackageCatalogOptions::Explicit() { - return hstring(m_explicit); + return m_explicit; } - void EditPackageCatalogOptions::Explicit(hstring const& value) + void EditPackageCatalogOptions::Explicit(OptionalBoolean const& value) { m_explicit = value; } diff --git a/src/Microsoft.Management.Deployment/EditPackageCatalogOptions.h b/src/Microsoft.Management.Deployment/EditPackageCatalogOptions.h index e09d00c437..976cb02280 100644 --- a/src/Microsoft.Management.Deployment/EditPackageCatalogOptions.h +++ b/src/Microsoft.Management.Deployment/EditPackageCatalogOptions.h @@ -14,13 +14,13 @@ namespace winrt::Microsoft::Management::Deployment::implementation hstring Name(); void Name(hstring const& value); - hstring Explicit(); - void Explicit(hstring const& value); + OptionalBoolean Explicit(); + void Explicit(OptionalBoolean const& value); #if !defined(INCLUDE_ONLY_INTERFACE_METHODS) private: hstring m_name = L""; - hstring m_explicit = L""; + OptionalBoolean m_explicit = OptionalBoolean::Unspecified; #endif }; } diff --git a/src/Microsoft.Management.Deployment/PackageManager.cpp b/src/Microsoft.Management.Deployment/PackageManager.cpp index 165488b77c..fd80f5cfcb 100644 --- a/src/Microsoft.Management.Deployment/PackageManager.cpp +++ b/src/Microsoft.Management.Deployment/PackageManager.cpp @@ -1468,9 +1468,9 @@ namespace winrt::Microsoft::Management::Deployment::implementation ::AppInstaller::Repository::Source sourceToEdit = ::AppInstaller::Repository::Source{ matchingSource.value().Name }; auto isExplicit = matchingSource.value().Explicit; - if (!options.Explicit().empty()) + if (options.Explicit() != OptionalBoolean::Unspecified) { - isExplicit = AppInstaller::Utility::ConvertStringToBool(winrt::to_string(options.Explicit())); + isExplicit = options.Explicit() == OptionalBoolean::True; } ::AppInstaller::Repository::SourceEdit edits{ std::optional{ isExplicit } }; diff --git a/src/Microsoft.Management.Deployment/PackageManager.idl b/src/Microsoft.Management.Deployment/PackageManager.idl index 8a056e80bb..7d72a35758 100644 --- a/src/Microsoft.Management.Deployment/PackageManager.idl +++ b/src/Microsoft.Management.Deployment/PackageManager.idl @@ -1541,9 +1541,18 @@ namespace Microsoft.Management.Deployment HRESULT ExtendedErrorCode { get; }; }; + /// IMPLEMENTATION NOTE: OptionalBoolean + [contract(Microsoft.Management.Deployment.WindowsPackageManagerContract, 28)] + enum OptionalBoolean + { + Unspecified, + False, + True, + }; + /// IMPLEMENTATION NOTE: EditPackageCatalogOptions [contract(Microsoft.Management.Deployment.WindowsPackageManagerContract, 28)] - runtimeclass EditPackageCatalogOptions + runtimeclass EditPackageCatalogOptions { EditPackageCatalogOptions(); @@ -1553,13 +1562,12 @@ namespace Microsoft.Management.Deployment String Name; /// Editing the Explicit property has three states: true, false, and not specified (no changes). - /// SAMPLE VALUES: "true" or "false" or empty string. - String Explicit; + OptionalBoolean Explicit; }; /// IMPLEMENTATION NOTE: RemovePackageCatalogStatus [contract(Microsoft.Management.Deployment.WindowsPackageManagerContract, 28)] - enum EditPackageCatalogStatus + enum EditPackageCatalogStatus { Ok, GroupPolicyError, @@ -1572,7 +1580,7 @@ namespace Microsoft.Management.Deployment /// IMPLEMENTATION NOTE: RemovePackageCatalogResult /// Result of editing a package catalog. [contract(Microsoft.Management.Deployment.WindowsPackageManagerContract, 28)] - runtimeclass EditPackageCatalogResult + runtimeclass EditPackageCatalogResult { EditPackageCatalogStatus Status { get; }; From d708ba41f9bf4869e493201a1387dfb0fd33f77b Mon Sep 17 00:00:00 2001 From: David Bennett Date: Wed, 10 Dec 2025 22:38:26 -0800 Subject: [PATCH 14/19] Fix interop test --- src/AppInstallerCLIE2ETests/Interop/PackageCatalogInterop.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AppInstallerCLIE2ETests/Interop/PackageCatalogInterop.cs b/src/AppInstallerCLIE2ETests/Interop/PackageCatalogInterop.cs index 3a4e241159..5493f951ae 100644 --- a/src/AppInstallerCLIE2ETests/Interop/PackageCatalogInterop.cs +++ b/src/AppInstallerCLIE2ETests/Interop/PackageCatalogInterop.cs @@ -398,7 +398,7 @@ private void EditAndValidatePackageCatalog(EditPackageCatalogOptions editPackage var packageCatalog = this.packageManager.GetPackageCatalogByName(editPackageCatalogOptions.Name); if (editPackageCatalogOptions.Explicit != OptionalBoolean.Unspecified) { - Assert.AreEqual(packageCatalog.Info.Explicit, editPackageCatalogOptions.Explicit); + Assert.AreEqual(packageCatalog.Info.Explicit, editPackageCatalogOptions.Explicit == OptionalBoolean.True); } } } From d9bb591ef4ec5f9dbc76e497f33819ed879478c7 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Mon, 15 Dec 2025 09:27:41 -0800 Subject: [PATCH 15/19] Update source output --- src/AppInstallerCLICore/Resources.h | 4 +++- src/AppInstallerCLICore/Workflows/SourceFlow.cpp | 14 +++++++++----- src/AppInstallerCLIE2ETests/SourceCommand.cs | 4 ++-- .../Shared/Strings/en-us/winget.resw | 16 ++++++++++++---- 4 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h index eba343937a..71f3404ce3 100644 --- a/src/AppInstallerCLICore/Resources.h +++ b/src/AppInstallerCLICore/Resources.h @@ -690,8 +690,10 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(SourceEditCommandLongDescription); WINGET_DEFINE_RESOURCE_STRINGID(SourceEditCommandShortDescription); WINGET_DEFINE_RESOURCE_STRINGID(SourceEditExplicitArgumentDescription); - WINGET_DEFINE_RESOURCE_STRINGID(SourceEditOne); + WINGET_DEFINE_RESOURCE_STRINGID(SourceEditNewValue); WINGET_DEFINE_RESOURCE_STRINGID(SourceEditNoChanges); + WINGET_DEFINE_RESOURCE_STRINGID(SourceEditOldValue); + WINGET_DEFINE_RESOURCE_STRINGID(SourceEditOne); WINGET_DEFINE_RESOURCE_STRINGID(SourceExplicitArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(SourceExportCommandLongDescription); WINGET_DEFINE_RESOURCE_STRINGID(SourceExportCommandShortDescription); diff --git a/src/AppInstallerCLICore/Workflows/SourceFlow.cpp b/src/AppInstallerCLICore/Workflows/SourceFlow.cpp index ee1cd7d1b2..10d819bdda 100644 --- a/src/AppInstallerCLICore/Workflows/SourceFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/SourceFlow.cpp @@ -281,13 +281,13 @@ namespace AppInstaller::CLI::Workflow { // Get the current source with this name. Repository::Source targetSource{ sd.Name }; + auto oldExplicitValue = sd.Explicit; // Default to the current explicit value unless we are overriding it. - auto isExplicit = sd.Explicit; + auto isExplicit = oldExplicitValue; if (context.Args.Contains(Execution::Args::Type::SourceEditExplicit)) { - auto explicitArg = context.Args.GetArg(Execution::Args::Type::SourceEditExplicit); - isExplicit = Utility::ConvertStringToBool(explicitArg); + isExplicit = Utility::ConvertStringToBool(context.Args.GetArg(Execution::Args::Type::SourceEditExplicit)); } Repository::SourceEdit edits{ std::optional{ isExplicit } }; @@ -297,9 +297,13 @@ namespace AppInstaller::CLI::Workflow continue; } - context.Reporter.Info() << Resource::String::SourceEditOne(Utility::LocIndView{ sd.Name }, Utility::LocIndView{ Utility::ConvertBoolToString(isExplicit) }) << std::endl; + context.Reporter.Info() << Resource::String::SourceEditOne(Utility::LocIndView{ sd.Name }) << std::endl; targetSource.Edit(edits); - context.Reporter.Info() << Resource::String::Done << std::endl; + + // Output updated source information. Since only Explicit is editable, we will only list that field. The name of the source being edited is listed prior to the edits. + Execution::TableOutput<3> table(context.Reporter, { Resource::String::SourceListField, Resource::String::SourceEditOldValue, Resource::String::SourceEditNewValue }); + table.OutputLine({ Resource::LocString(Resource::String::SourceListExplicit), std::string{ Utility::ConvertBoolToString(oldExplicitValue) }, std::string{ Utility::ConvertBoolToString(isExplicit) } }); + table.Complete(); } } diff --git a/src/AppInstallerCLIE2ETests/SourceCommand.cs b/src/AppInstallerCLIE2ETests/SourceCommand.cs index 8afca3b35a..4f60b94455 100644 --- a/src/AppInstallerCLIE2ETests/SourceCommand.cs +++ b/src/AppInstallerCLIE2ETests/SourceCommand.cs @@ -289,7 +289,7 @@ public void SourceEdit() // Run the edit, this should be S_OK with "Done" as it changed the state to not-explicit. var editResult = TestCommon.RunAICLICommand("source edit", $"SourceTest --explicit false"); Assert.AreEqual(Constants.ErrorCode.S_OK, editResult.ExitCode); - Assert.True(editResult.StdOut.Contains("Done")); + Assert.True(editResult.StdOut.Contains("Explicit")); // Run it again, this should result in S_OK with no changes and a message that the source is already in that state. var editResult2 = TestCommon.RunAICLICommand("source edit", $"SourceTest --explicit false"); @@ -321,7 +321,7 @@ public void SourceEditOverrideDefault() var editResult = TestCommon.RunAICLICommand("source edit", "winget-font -e false"); Assert.AreEqual(Constants.ErrorCode.S_OK, editResult.ExitCode); - Assert.True(editResult.StdOut.Contains("Done")); + Assert.True(editResult.StdOut.Contains("Explicit")); // Verify that after edit it is now explicit false. var listResult2 = TestCommon.RunAICLICommand("source list", "winget-font"); diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index 57b9223255..0f0d74ce14 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -552,10 +552,10 @@ They can be configured through the settings file 'winget settings'. Edit properties of a source - Changing source: {0} to Explicit={1}... - {Locked="{0}","{1}"} Message displayed to inform the user about a registered repository source that is currently being edited. {0} is a placeholder replaced by the repository source name. {1} is a placeholder replaced by the changed value. + Editing source: {0} + {Locked="{0}"} Message displayed to inform the user about a registered repository source that is currently being edited. {0} is a placeholder replaced by the repository source name. - + The source named '{0}' is already in the desired state. {Locked="{0}"} Message displayed to inform the user about a registered repository source that is currently being edited. {0} is a placeholder replaced by the repository source name. @@ -3475,4 +3475,12 @@ An unlocalized JSON fragment will follow on another line. Font package is already installed. - + + Old value + Column title for listing edit changes. + + + New value + Column title for listing the new value. + + \ No newline at end of file From ed29780ef5d2ccb0a5c61e260febf79898b0b74a Mon Sep 17 00:00:00 2001 From: David Bennett Date: Mon, 15 Dec 2025 09:54:45 -0800 Subject: [PATCH 16/19] Update bool conversion and arguments --- .../Commands/SourceCommand.cpp | 3 ++- src/AppInstallerCLICore/Workflows/SourceFlow.cpp | 5 ++--- .../Shared/Strings/en-us/winget.resw | 4 ++-- src/AppInstallerSharedLib/AppInstallerStrings.cpp | 15 +++++++++++---- .../Public/AppInstallerStrings.h | 3 ++- 5 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/AppInstallerCLICore/Commands/SourceCommand.cpp b/src/AppInstallerCLICore/Commands/SourceCommand.cpp index 89e1bf6f9f..968873b757 100644 --- a/src/AppInstallerCLICore/Commands/SourceCommand.cpp +++ b/src/AppInstallerCLICore/Commands/SourceCommand.cpp @@ -344,7 +344,8 @@ namespace AppInstaller::CLI if (execArgs.Contains(Execution::Args::Type::SourceEditExplicit)) { std::string_view explicitArg = execArgs.GetArg(Execution::Args::Type::SourceEditExplicit); - if (!Utility::CaseInsensitiveEquals(explicitArg, "true") && !Utility::CaseInsensitiveEquals(explicitArg, "false")) + auto convertedArg = Utility::TryConvertStringToBool(explicitArg); + if (!convertedArg.has_value()) { auto validOptions = Utility::Join(", "_liv, std::vector{ "true"_lis, diff --git a/src/AppInstallerCLICore/Workflows/SourceFlow.cpp b/src/AppInstallerCLICore/Workflows/SourceFlow.cpp index 10d819bdda..f0122614f2 100644 --- a/src/AppInstallerCLICore/Workflows/SourceFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/SourceFlow.cpp @@ -283,11 +283,10 @@ namespace AppInstaller::CLI::Workflow Repository::Source targetSource{ sd.Name }; auto oldExplicitValue = sd.Explicit; - // Default to the current explicit value unless we are overriding it. - auto isExplicit = oldExplicitValue; + std::optional isExplicit; if (context.Args.Contains(Execution::Args::Type::SourceEditExplicit)) { - isExplicit = Utility::ConvertStringToBool(context.Args.GetArg(Execution::Args::Type::SourceEditExplicit)); + isExplicit = Utility::TryConvertStringToBool(context.Args.GetArg(Execution::Args::Type::SourceEditExplicit)); } Repository::SourceEdit edits{ std::optional{ isExplicit } }; diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index 0f0d74ce14..4566581c53 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -3476,11 +3476,11 @@ An unlocalized JSON fragment will follow on another line. Font package is already installed. - Old value + Old Value Column title for listing edit changes. - New value + New Value Column title for listing the new value. \ No newline at end of file diff --git a/src/AppInstallerSharedLib/AppInstallerStrings.cpp b/src/AppInstallerSharedLib/AppInstallerStrings.cpp index 931ae8a108..74bebeedb4 100644 --- a/src/AppInstallerSharedLib/AppInstallerStrings.cpp +++ b/src/AppInstallerSharedLib/AppInstallerStrings.cpp @@ -939,11 +939,18 @@ namespace AppInstaller::Utility return value ? "true"sv : "false"sv; } - bool ConvertStringToBool(const std::string_view& input) + std::optional TryConvertStringToBool(const std::string_view& input) { - bool value; - std::istringstream(std::string(input)) >> std::boolalpha >> value; - return value; + try + { + bool value; + std::istringstream(std::string(input)) >> std::boolalpha >> value; + return { value }; + } + catch + { + return {}; + } } std::string ConvertGuidToString(const GUID& value) diff --git a/src/AppInstallerSharedLib/Public/AppInstallerStrings.h b/src/AppInstallerSharedLib/Public/AppInstallerStrings.h index 95c46ca90d..7d5beb951e 100644 --- a/src/AppInstallerSharedLib/Public/AppInstallerStrings.h +++ b/src/AppInstallerSharedLib/Public/AppInstallerStrings.h @@ -3,6 +3,7 @@ #pragma once #include #include +#include #include #include #include @@ -291,7 +292,7 @@ namespace AppInstaller::Utility std::string_view ConvertBoolToString(bool value); // Converts the given string view into a bool. - bool ConvertStringToBool(const std::string_view& value); + std::optional TryConvertStringToBool(const std::string_view& value); // Converts the given GUID value to a string. std::string ConvertGuidToString(const GUID& value); From 9221c20363d382235ba6a6a18dadf53019fbf658 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Mon, 15 Dec 2025 10:58:03 -0800 Subject: [PATCH 17/19] Feedback update, rework string to bool --- .../Workflows/SourceFlow.cpp | 4 +- src/AppInstallerCLITests/TestSource.cpp | 15 ------- src/AppInstallerCLITests/TestSource.h | 4 -- .../ConfigurableTestSourceFactory.cpp | 5 --- .../PreIndexedPackageSourceFactory.cpp | 11 ----- .../PredefinedInstalledSourceFactory.cpp | 5 --- .../PredefinedWriteableSourceFactory.cpp | 5 --- .../PackageTrackingCatalog.cpp | 5 --- .../Public/winget/RepositorySource.h | 5 +-- .../RepositorySource.cpp | 43 ++++--------------- .../Rest/RestSourceFactory.cpp | 6 --- .../SourceFactory.h | 3 -- .../AppInstallerStrings.cpp | 16 +++++-- .../PackageManager.cpp | 8 +--- 14 files changed, 24 insertions(+), 111 deletions(-) diff --git a/src/AppInstallerCLICore/Workflows/SourceFlow.cpp b/src/AppInstallerCLICore/Workflows/SourceFlow.cpp index f0122614f2..1f34512e61 100644 --- a/src/AppInstallerCLICore/Workflows/SourceFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/SourceFlow.cpp @@ -289,7 +289,7 @@ namespace AppInstaller::CLI::Workflow isExplicit = Utility::TryConvertStringToBool(context.Args.GetArg(Execution::Args::Type::SourceEditExplicit)); } - Repository::SourceEdit edits{ std::optional{ isExplicit } }; + Repository::SourceEdit edits{ isExplicit }; if (!targetSource.RequiresChanges(edits)) { context.Reporter.Info() << Resource::String::SourceEditNoChanges(Utility::LocIndView{ sd.Name }) << std::endl; @@ -301,7 +301,7 @@ namespace AppInstaller::CLI::Workflow // Output updated source information. Since only Explicit is editable, we will only list that field. The name of the source being edited is listed prior to the edits. Execution::TableOutput<3> table(context.Reporter, { Resource::String::SourceListField, Resource::String::SourceEditOldValue, Resource::String::SourceEditNewValue }); - table.OutputLine({ Resource::LocString(Resource::String::SourceListExplicit), std::string{ Utility::ConvertBoolToString(oldExplicitValue) }, std::string{ Utility::ConvertBoolToString(isExplicit) } }); + table.OutputLine({ Resource::LocString(Resource::String::SourceListExplicit), std::string{ Utility::ConvertBoolToString(oldExplicitValue) }, std::string{ Utility::ConvertBoolToString(isExplicit.value()) } }); table.Complete(); } } diff --git a/src/AppInstallerCLITests/TestSource.cpp b/src/AppInstallerCLITests/TestSource.cpp index c602d25bd7..6544f99395 100644 --- a/src/AppInstallerCLITests/TestSource.cpp +++ b/src/AppInstallerCLITests/TestSource.cpp @@ -454,15 +454,6 @@ namespace TestCommon return true; } - bool TestSourceFactory::Edit(SourceDetails& details, const SourceEdit& edits) - { - if (OnEdit) - { - OnEdit(details, edits); - } - return true; - } - // Make copies of self when requested. TestSourceFactory::operator std::function()>() { @@ -475,12 +466,6 @@ namespace TestCommon return source.Add(progress); } - bool EditSource(const AppInstaller::Repository::SourceDetails& details, const AppInstaller::Repository::SourceEdit& edits) - { - Repository::Source source{ details.Name, details.Explicit }; - return source.Edit(edits); - } - bool UpdateSource(std::string_view name, AppInstaller::IProgressCallback& progress) { Repository::Source source{ name }; diff --git a/src/AppInstallerCLITests/TestSource.h b/src/AppInstallerCLITests/TestSource.h index e32dfaab95..bc5aca74f7 100644 --- a/src/AppInstallerCLITests/TestSource.h +++ b/src/AppInstallerCLITests/TestSource.h @@ -187,7 +187,6 @@ namespace TestCommon using AddFunctor = std::function; using UpdateFunctor = std::function; using RemoveFunctor = std::function; - using EditFunctor = std::function; TestSourceFactory(OpenFunctor open) : OnOpen(std::move(open)) {} TestSourceFactory(OpenFunctorWithCustomHeader open) : OnOpenWithCustomHeader(std::move(open)) {} @@ -198,7 +197,6 @@ namespace TestCommon bool Add(AppInstaller::Repository::SourceDetails& details, AppInstaller::IProgressCallback&) override; bool Update(const AppInstaller::Repository::SourceDetails& details, AppInstaller::IProgressCallback&) override; bool Remove(const AppInstaller::Repository::SourceDetails& details, AppInstaller::IProgressCallback&) override; - bool Edit(AppInstaller::Repository::SourceDetails& details, const AppInstaller::Repository::SourceEdit& edits) override; // Make copies of self when requested. operator std::function()>(); @@ -209,11 +207,9 @@ namespace TestCommon AddFunctor OnAdd; UpdateFunctor OnUpdate; RemoveFunctor OnRemove; - EditFunctor OnEdit; }; bool AddSource(const AppInstaller::Repository::SourceDetails& details, AppInstaller::IProgressCallback& progress); - bool EditSource(AppInstaller::Repository::SourceDetails& details, const AppInstaller::Repository::SourceEdit& edits); bool UpdateSource(std::string_view name, AppInstaller::IProgressCallback& progress); bool RemoveSource(std::string_view name, AppInstaller::IProgressCallback& progress); AppInstaller::Repository::Source OpenSource(std::string_view name, AppInstaller::IProgressCallback& progress); diff --git a/src/AppInstallerRepositoryCore/Microsoft/ConfigurableTestSourceFactory.cpp b/src/AppInstallerRepositoryCore/Microsoft/ConfigurableTestSourceFactory.cpp index 35be4911e5..af8a17ebfe 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/ConfigurableTestSourceFactory.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/ConfigurableTestSourceFactory.cpp @@ -154,11 +154,6 @@ namespace AppInstaller::Repository::Microsoft { return true; } - - bool Edit(SourceDetails&, const SourceEdit&) override final - { - return true; - } }; } diff --git a/src/AppInstallerRepositoryCore/Microsoft/PreIndexedPackageSourceFactory.cpp b/src/AppInstallerRepositoryCore/Microsoft/PreIndexedPackageSourceFactory.cpp index 2cb137fb4f..bbe0082cb8 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PreIndexedPackageSourceFactory.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/PreIndexedPackageSourceFactory.cpp @@ -254,17 +254,6 @@ namespace AppInstaller::Repository::Microsoft return UpdateBase(details, true, progress); } - // Applies the edits to the source. - bool Edit(SourceDetails& details, const SourceEdit& edits) override final - { - if (edits.Explicit.has_value()) - { - details.Explicit = edits.Explicit.value(); - } - - return true; - } - // Retrieves the currently cached version of the package. virtual std::optional GetCurrentVersion(const SourceDetails& details) = 0; diff --git a/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp b/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp index 665f6ecf7d..ca93275329 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp @@ -445,11 +445,6 @@ namespace AppInstaller::Repository::Microsoft // Similar to add, remove should never be needed. THROW_HR(E_NOTIMPL); } - - bool Edit(SourceDetails&, const SourceEdit&) override final - { - THROW_HR(E_NOTIMPL); - } }; } diff --git a/src/AppInstallerRepositoryCore/Microsoft/PredefinedWriteableSourceFactory.cpp b/src/AppInstallerRepositoryCore/Microsoft/PredefinedWriteableSourceFactory.cpp index ba179eee2c..53accca45c 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PredefinedWriteableSourceFactory.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/PredefinedWriteableSourceFactory.cpp @@ -44,11 +44,6 @@ namespace AppInstaller::Repository::Microsoft // Similar to add, remove should never be needed. THROW_HR(E_NOTIMPL); } - - bool Edit(SourceDetails&, const SourceEdit&) override final - { - THROW_HR(E_NOTIMPL); - } }; struct PredefinedWriteableSourceReference : public ISourceReference diff --git a/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp b/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp index da801554d9..76a2311cfb 100644 --- a/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp +++ b/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp @@ -125,11 +125,6 @@ namespace AppInstaller::Repository THROW_HR(E_NOTIMPL); } - bool Edit(SourceDetails&, const SourceEdit&) override final - { - THROW_HR(E_NOTIMPL); - } - bool Remove(const SourceDetails& details, IProgressCallback& progress) override final { THROW_HR_IF(E_INVALIDARG, !Utility::CaseInsensitiveEquals(details.Type, PackageTrackingCatalogSourceFactory::Type())); diff --git a/src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h b/src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h index 31cf7fb8ce..46fd89fdbd 100644 --- a/src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h +++ b/src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h @@ -233,9 +233,6 @@ namespace AppInstaller::Repository // Constructor for a source to be added. Source(std::string_view name, std::string_view arg, std::string_view type, SourceTrustLevel trustLevel, bool isExplicit); - // Constructor for a source to be edited. - Source(std::string_view name, std::optional isExplicit); - // Constructor for creating a composite source from a list of available sources. Source(const std::vector& availableSources); @@ -343,7 +340,7 @@ namespace AppInstaller::Repository bool Remove(IProgressCallback& progress); // Edit source. Source edit command. - bool Edit(const SourceEdit& edits); + void Edit(const SourceEdit& edits); // Determines if this source is a valid edit of otherSource. // Returns true if this source qualifies as an edit of the other source. diff --git a/src/AppInstallerRepositoryCore/RepositorySource.cpp b/src/AppInstallerRepositoryCore/RepositorySource.cpp index 2f05ccb661..7067b7c51c 100644 --- a/src/AppInstallerRepositoryCore/RepositorySource.cpp +++ b/src/AppInstallerRepositoryCore/RepositorySource.cpp @@ -135,12 +135,6 @@ namespace AppInstaller::Repository return AddOrUpdateFromDetails(details, &ISourceFactory::Update, progress); } - bool EditSource(SourceDetails& details, const SourceEdit& edits) - { - auto factory = ISourceFactory::GetForType(details.Type); - return factory->Edit(details, edits); - } - AddOrUpdateResult BackgroundUpdateSourceFromDetails(SourceDetails& details, IProgressCallback& progress) { return AddOrUpdateFromDetails(details, &ISourceFactory::BackgroundUpdate, progress); @@ -493,29 +487,6 @@ namespace AppInstaller::Repository m_sourceReferences.emplace_back(CreateSourceFromDetails(details)); } - Source::Source(std::string_view name, std::optional isExplicit) - { - SourceList sourceList; - auto source = sourceList.GetCurrentSource(name); - if (!source) - { - AICLI_LOG(Repo, Info, << "Named source requested, but not found: " << name); - } - else - { - AICLI_LOG(Repo, Info, << "Named source requested, found: " << source->Name); - - // This is intended to support Editing a source, and at present only the Explicit - // property of SourceDetails can be edited. - if (isExplicit.has_value()) - { - source->Explicit = isExplicit.value(); - } - - m_sourceReferences.emplace_back(CreateSourceFromDetails(*source)); - } - } - Source::Source(const std::vector& availableSources) { std::shared_ptr compositeSource = std::make_shared("*CompositeSource"); @@ -1020,7 +991,7 @@ namespace AppInstaller::Repository return result; } - bool Source::Edit(const SourceEdit& edits) + void Source::Edit(const SourceEdit& edits) { THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), m_isSourceToBeAdded || m_sourceReferences.size() != 1 || m_source); @@ -1030,15 +1001,17 @@ namespace AppInstaller::Repository // This is intentionally the same policy checks as Remove. If the source cannot be removed then it cannot be edited. EnsureSourceIsRemovable(details); - // Apply the edits and update source list. - bool result = EditSource(details, edits); - if (result) + if (RequiresChanges(edits)) { + if (edits.Explicit.has_value()) + { + details.Explicit = edits.Explicit.value(); + } + + // Apply the edits and update source list. SourceList sourceList; sourceList.EditSource(details); } - - return result; } bool Source::RequiresChanges(const SourceEdit& edits) diff --git a/src/AppInstallerRepositoryCore/Rest/RestSourceFactory.cpp b/src/AppInstallerRepositoryCore/Rest/RestSourceFactory.cpp index a5060ec09d..b30e18d882 100644 --- a/src/AppInstallerRepositoryCore/Rest/RestSourceFactory.cpp +++ b/src/AppInstallerRepositoryCore/Rest/RestSourceFactory.cpp @@ -134,12 +134,6 @@ namespace AppInstaller::Repository::Rest THROW_HR_IF(E_INVALIDARG, !Utility::CaseInsensitiveEquals(details.Type, RestSourceFactory::Type())); return true; } - - bool Edit(SourceDetails& details, const SourceEdit&) override final - { - THROW_HR_IF(E_INVALIDARG, !Utility::CaseInsensitiveEquals(details.Type, RestSourceFactory::Type())); - return true; - } }; } diff --git a/src/AppInstallerRepositoryCore/SourceFactory.h b/src/AppInstallerRepositoryCore/SourceFactory.h index 37df62f66b..0445d950aa 100644 --- a/src/AppInstallerRepositoryCore/SourceFactory.h +++ b/src/AppInstallerRepositoryCore/SourceFactory.h @@ -41,9 +41,6 @@ namespace AppInstaller::Repository // Return value indicates whether the action completed. virtual bool Remove(const SourceDetails& details, IProgressCallback& progress) = 0; - // Edit the given source with the provided edits. - virtual bool Edit(SourceDetails& details, const SourceEdit& edits) = 0; - // Gets the factory for the given type. static std::unique_ptr GetForType(std::string_view type); }; diff --git a/src/AppInstallerSharedLib/AppInstallerStrings.cpp b/src/AppInstallerSharedLib/AppInstallerStrings.cpp index 74bebeedb4..c1335f0315 100644 --- a/src/AppInstallerSharedLib/AppInstallerStrings.cpp +++ b/src/AppInstallerSharedLib/AppInstallerStrings.cpp @@ -943,11 +943,19 @@ namespace AppInstaller::Utility { try { - bool value; - std::istringstream(std::string(input)) >> std::boolalpha >> value; - return { value }; + if (ICUCaseInsensitiveEquals(input, "false"sv)) + { + return { false }; + } + + if (ICUCaseInsensitiveEquals(input, "true"sv)) + { + return { true }; + } + + return {}; } - catch + catch (...) { return {}; } diff --git a/src/Microsoft.Management.Deployment/PackageManager.cpp b/src/Microsoft.Management.Deployment/PackageManager.cpp index fd80f5cfcb..fd08134c93 100644 --- a/src/Microsoft.Management.Deployment/PackageManager.cpp +++ b/src/Microsoft.Management.Deployment/PackageManager.cpp @@ -1467,13 +1467,7 @@ namespace winrt::Microsoft::Management::Deployment::implementation THROW_HR_IF(APPINSTALLER_CLI_ERROR_SOURCE_NAME_DOES_NOT_EXIST, !matchingSource.has_value()); ::AppInstaller::Repository::Source sourceToEdit = ::AppInstaller::Repository::Source{ matchingSource.value().Name }; - auto isExplicit = matchingSource.value().Explicit; - if (options.Explicit() != OptionalBoolean::Unspecified) - { - isExplicit = options.Explicit() == OptionalBoolean::True; - } - - ::AppInstaller::Repository::SourceEdit edits{ std::optional{ isExplicit } }; + ::AppInstaller::Repository::SourceEdit edits{ GetOptionalBoolean(options.Explicit())}; if (sourceToEdit.RequiresChanges(edits)) { sourceToEdit.Edit(edits); From 5b74b3010229676a692814ad92cd0e16ad58b244 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Mon, 15 Dec 2025 11:56:45 -0800 Subject: [PATCH 18/19] Fix com class --- .../EditPackageCatalogOptions.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Management.Deployment/EditPackageCatalogOptions.h b/src/Microsoft.Management.Deployment/EditPackageCatalogOptions.h index 976cb02280..68e3372f0d 100644 --- a/src/Microsoft.Management.Deployment/EditPackageCatalogOptions.h +++ b/src/Microsoft.Management.Deployment/EditPackageCatalogOptions.h @@ -3,6 +3,7 @@ #pragma once #include "EditPackageCatalogOptions.g.h" #include "public/ComClsids.h" +#include namespace winrt::Microsoft::Management::Deployment::implementation { @@ -28,7 +29,9 @@ namespace winrt::Microsoft::Management::Deployment::implementation #if !defined(INCLUDE_ONLY_INTERFACE_METHODS) namespace winrt::Microsoft::Management::Deployment::factory_implementation { - struct EditPackageCatalogOptions : EditPackageCatalogOptionsT + struct EditPackageCatalogOptions : + EditPackageCatalogOptionsT, + AppInstaller::WinRT::ModuleCountBase { }; } From 5a78e6b0237e88c224ae74ecd23b52c7617358ef Mon Sep 17 00:00:00 2001 From: David Bennett Date: Tue, 16 Dec 2025 11:55:33 -0800 Subject: [PATCH 19/19] Use more performant string compare --- src/AppInstallerSharedLib/AppInstallerStrings.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/AppInstallerSharedLib/AppInstallerStrings.cpp b/src/AppInstallerSharedLib/AppInstallerStrings.cpp index c1335f0315..3cd67d13c9 100644 --- a/src/AppInstallerSharedLib/AppInstallerStrings.cpp +++ b/src/AppInstallerSharedLib/AppInstallerStrings.cpp @@ -943,12 +943,12 @@ namespace AppInstaller::Utility { try { - if (ICUCaseInsensitiveEquals(input, "false"sv)) + if (CaseInsensitiveEquals(input, "false"sv)) { return { false }; } - if (ICUCaseInsensitiveEquals(input, "true"sv)) + if (CaseInsensitiveEquals(input, "true"sv)) { return { true }; }