diff --git a/AGENTS.md b/AGENTS.md index d5363896de..232b384c28 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -36,7 +36,8 @@ Claude Code skills are provided as `/beutl-build`, `/beutl-test`, `/beutl-format | `Beutl.ProjectSystem` | Project / document persistence | | `Beutl.Editor` | Non-UI editor logic — undo/redo, packaging, editing-pipeline services (no Avalonia) | | `Beutl.Editor.Components`, `Beutl.Controls` | Avalonia UI layer (views / controls / ViewModels) | -| `Beutl.Extensibility` | Plugin abstractions | +| `Beutl.Extensibility.Abstractions` | Lightweight base extension contracts (`Extension` / `ExtensionSettings` / `ExportAttribute`); depends only on `Beutl.Core` + `Beutl.Configuration` so a plugin can target the thin surface without the full UI/media assembly | +| `Beutl.Extensibility` | Plugin abstractions — UI / media / property-editor contracts layered on top of `Beutl.Extensibility.Abstractions` | | `Beutl.NodeGraph` | Node editor | | `Beutl.FFmpegIpc` | **MIT** IPC layer (transport: Protocol / Transport / SharedMemory). Also hosts the IPC client providers under `Providers/`, which translate frame/sample messages into `Beutl.Media` / `Beutl.Extensibility` types; that adapter role is why this project deliberately takes `ProjectReference`s to `Beutl.Engine` + `Beutl.Extensibility` rather than staying dependency-free. | | `Beutl.FFmpegWorker` | **GPL** separate process; reach it only via IPC | diff --git a/Beutl.slnx b/Beutl.slnx index f709724901..b34f1f313f 100644 --- a/Beutl.slnx +++ b/Beutl.slnx @@ -25,6 +25,7 @@ + @@ -46,6 +47,7 @@ + diff --git a/nukebuild/Build.cs b/nukebuild/Build.cs index 449e746038..744ef16be1 100644 --- a/nukebuild/Build.cs +++ b/nukebuild/Build.cs @@ -189,6 +189,7 @@ private string GetTFM() [ ("Beutl.Configuration", tfm), ("Beutl.Core", tfm), + ("Beutl.Extensibility.Abstractions", tfm), ("Beutl.Extensibility", tfm), ("Beutl.Engine", tfm), ("Beutl.Engine.SourceGenerators", "netstandard2.0"), diff --git a/sdk/Beutl.Extensibility.Sdk/Sdk/Sdk.targets b/sdk/Beutl.Extensibility.Sdk/Sdk/Sdk.targets index 8719b3df58..ee4d077900 100644 --- a/sdk/Beutl.Extensibility.Sdk/Sdk/Sdk.targets +++ b/sdk/Beutl.Extensibility.Sdk/Sdk/Sdk.targets @@ -32,6 +32,7 @@ true $(BeutlAutoReferenceAll) + $(BeutlAutoReferenceAll) $(BeutlAutoReferenceAll) $(BeutlAutoReferenceAll) $(BeutlAutoReferenceAll) @@ -40,6 +41,9 @@ + + @@ -54,6 +58,8 @@ + + diff --git a/sdk/README.md b/sdk/README.md index e8a86af346..5cd98ded6d 100644 --- a/sdk/README.md +++ b/sdk/README.md @@ -12,3 +12,47 @@ ``` + +## Auto-referenced packages + +By default the SDK references every Beutl package an extension typically needs. Each can be +opted out individually: + +| Property | Package | +|---|---| +| `BeutlAutoReferenceExtensibility` | `Beutl.Extensibility` (full UI / media / property-editor surface) | +| `BeutlAutoReferenceExtensibilityAbstractions` | `Beutl.Extensibility.Abstractions` (base contracts only) | +| `BeutlAutoReferenceProjectSystem` | `Beutl.ProjectSystem` | +| `BeutlAutoReferenceNodeGraph` | `Beutl.NodeGraph` | +| `BeutlAutoReferenceEditor` | `Beutl.Editor` | +| `BeutlAutoReferenceSourceGenerators` | `Beutl.Engine.SourceGenerators` | + +Set `BeutlAutoReferenceAll` to `false` to opt out of everything and then enable only what you need. + +### Thin (abstractions-only) plugin + +`Beutl.Extensibility.Abstractions` holds just the base extension contracts — `Extension`, +`ExtensionSettings`, and `ExportAttribute` — without pulling in Avalonia, SkiaSharp, or the rest +of the full `Beutl.Extensibility` surface. A plugin that only defines these contracts can drop the +heavy package and reference the abstractions instead. + +Disabling `BeutlAutoReferenceExtensibility` alone is not enough: `BeutlAutoReferenceProjectSystem`, +`BeutlAutoReferenceNodeGraph`, and `BeutlAutoReferenceEditor` default to `true`, and those packages +reference `Beutl.Extensibility`, so it returns transitively. For a truly thin plugin, opt out of +everything with `BeutlAutoReferenceAll` and add only the abstractions back: + +```xml + + + Beutl.Extensions.MyExtension + 1.0.0 + Your Name + + false + true + + +``` + +When `BeutlAutoReferenceExtensibility` is left enabled, the abstractions arrive transitively, so +`BeutlAutoReferenceExtensibilityAbstractions` only adds a direct reference for the thin case above. diff --git a/src/Beutl.Api/Services/CoreLibraries.cs b/src/Beutl.Api/Services/CoreLibraries.cs index c6da28d005..9973de7375 100644 --- a/src/Beutl.Api/Services/CoreLibraries.cs +++ b/src/Beutl.Api/Services/CoreLibraries.cs @@ -75,6 +75,7 @@ public static IEnumerable CollectRuntimeDependencies() case "Beutl.Embedding.MediaFoundation" when OperatingSystem.IsWindows(): case "Beutl.Extensions.AVFoundation" when OperatingSystem.IsMacOS(): case "Beutl.Engine": + case "Beutl.Extensibility.Abstractions": case "Beutl.Extensibility": case "Beutl.Language": case "Beutl.NodeGraph": diff --git a/src/Beutl.Api/Services/LocalPackage.cs b/src/Beutl.Api/Services/LocalPackage.cs index ffa46400ae..300dd951d1 100644 --- a/src/Beutl.Api/Services/LocalPackage.cs +++ b/src/Beutl.Api/Services/LocalPackage.cs @@ -38,8 +38,9 @@ public LocalPackage(NuspecReader nuspecReader) if (nearest != null) { PackageDependencyGroup depGroup = depGroups.First(v => v.TargetFramework == nearest); - PackageDependency? sdkDep = depGroup.Packages.FirstOrDefault(v => v.Id == "Beutl.Sdk") - ?? depGroup.Packages.FirstOrDefault(v => v.Id == "Beutl.Extensibility"); + PackageDependency? sdkDep = depGroup.Packages.FirstOrDefault(v => StringComparer.OrdinalIgnoreCase.Equals(v.Id, "Beutl.Sdk")) + ?? depGroup.Packages.FirstOrDefault(v => StringComparer.OrdinalIgnoreCase.Equals(v.Id, "Beutl.Extensibility")) + ?? depGroup.Packages.FirstOrDefault(v => StringComparer.OrdinalIgnoreCase.Equals(v.Id, "Beutl.Extensibility.Abstractions")); if (sdkDep != null) { TargetVersion = sdkDep.VersionRange.ToShortString(); diff --git a/src/Beutl.Api/Services/PackageManager.cs b/src/Beutl.Api/Services/PackageManager.cs index 3b93070b40..8d02e4339e 100644 --- a/src/Beutl.Api/Services/PackageManager.cs +++ b/src/Beutl.Api/Services/PackageManager.cs @@ -2,6 +2,7 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Reflection; +using System.Runtime.CompilerServices; using Avalonia; using Avalonia.Platform; using Beutl.Api.Objects; @@ -26,6 +27,11 @@ public sealed class PackageManager( { private readonly ILogger _logger = Log.CreateLogger(); private readonly ConcurrentDictionary _loadedPackages = new(); + // Captures the publisher subscribed at setup so cleanup can unsubscribe even if extension.Settings is later swapped. + private sealed record SettingsSubscription(ExtensionSettings Settings, EventHandler Handler); + + // Weak key so a leftover entry can't pin the extension's collectible AssemblyLoadContext and block unload. + private readonly ConditionalWeakTable _settingsChangedHandlers = new(); private readonly ExtensionSettingsStore _settingsStore = new(); public IEnumerable LoadedPackage => _loadedPackages.Values.Select(x => x.Package); @@ -492,10 +498,18 @@ internal void SetupExtensionSettings(Extension extension) { if (extension.Settings is { } settings) { + // Unsubscribe before Restore: it raises ConfigurationChanged, which a stale handler + // would turn into a Save of partially-restored state. + if (_settingsChangedHandlers.TryGetValue(extension, out SettingsSubscription? previous)) + { + _settingsChangedHandlers.Remove(extension); + previous.Settings.ConfigurationChanged -= previous.Handler; + } + _settingsStore.Restore(extension, settings); EventHandler handler = (_, _) => _settingsStore.Save(extension, settings); - extension.SettingsChangedHandler = handler; + _settingsChangedHandlers.AddOrUpdate(extension, new SettingsSubscription(settings, handler)); settings.ConfigurationChanged += handler; _logger.LogInformation("Settings restored for extension {ExtensionName}", extension.GetType().Name); } @@ -503,10 +517,12 @@ internal void SetupExtensionSettings(Extension extension) private void CleanupExtensionSettings(Extension extension) { - if (extension.Settings is { } settings && extension.SettingsChangedHandler is { } handler) + // Unsubscribe from the captured publisher (extension.Settings may have changed) so the + // handler stops keeping the collectible AssemblyLoadContext alive. + if (_settingsChangedHandlers.TryGetValue(extension, out SettingsSubscription? subscription)) { - settings.ConfigurationChanged -= handler; - extension.SettingsChangedHandler = null; + _settingsChangedHandlers.Remove(extension); + subscription.Settings.ConfigurationChanged -= subscription.Handler; } } } diff --git a/src/Beutl.Extensibility.Abstractions/Beutl.Extensibility.Abstractions.csproj b/src/Beutl.Extensibility.Abstractions/Beutl.Extensibility.Abstractions.csproj new file mode 100644 index 0000000000..d3103f2711 --- /dev/null +++ b/src/Beutl.Extensibility.Abstractions/Beutl.Extensibility.Abstractions.csproj @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/src/Beutl.Extensibility/ExportAttribute.cs b/src/Beutl.Extensibility.Abstractions/ExportAttribute.cs similarity index 100% rename from src/Beutl.Extensibility/ExportAttribute.cs rename to src/Beutl.Extensibility.Abstractions/ExportAttribute.cs diff --git a/src/Beutl.Extensibility/Extension.cs b/src/Beutl.Extensibility.Abstractions/Extension.cs similarity index 90% rename from src/Beutl.Extensibility/Extension.cs rename to src/Beutl.Extensibility.Abstractions/Extension.cs index 1235f11e9c..5f03cad673 100644 --- a/src/Beutl.Extensibility/Extension.cs +++ b/src/Beutl.Extensibility.Abstractions/Extension.cs @@ -1,6 +1,5 @@ namespace Beutl.Extensibility; -// 拡張機能の基本クラス public abstract class Extension { public virtual string Name => GetType().Name; @@ -9,8 +8,6 @@ public abstract class Extension public virtual ExtensionSettings? Settings { get; } - internal EventHandler? SettingsChangedHandler { get; set; } - /// /// Called once when the extension is loaded. Override to perform initialization. /// If this method throws, is called to roll back any partial diff --git a/src/Beutl.Extensibility/ExtensionSettings.cs b/src/Beutl.Extensibility.Abstractions/ExtensionSettings.cs similarity index 100% rename from src/Beutl.Extensibility/ExtensionSettings.cs rename to src/Beutl.Extensibility.Abstractions/ExtensionSettings.cs diff --git a/src/Beutl.Extensibility/Beutl.Extensibility.csproj b/src/Beutl.Extensibility/Beutl.Extensibility.csproj index ee66a2edf5..552b056cf7 100644 --- a/src/Beutl.Extensibility/Beutl.Extensibility.csproj +++ b/src/Beutl.Extensibility/Beutl.Extensibility.csproj @@ -1,6 +1,7 @@ + diff --git a/tests/Beutl.Extensibility.Abstractions.Tests/Beutl.Extensibility.Abstractions.Tests.csproj b/tests/Beutl.Extensibility.Abstractions.Tests/Beutl.Extensibility.Abstractions.Tests.csproj new file mode 100644 index 0000000000..de139d44f3 --- /dev/null +++ b/tests/Beutl.Extensibility.Abstractions.Tests/Beutl.Extensibility.Abstractions.Tests.csproj @@ -0,0 +1,23 @@ + + + + false + true + net10.0 + + + + + + + + + + + + + + + + + diff --git a/tests/Beutl.Extensibility.Abstractions.Tests/ExtensibilityAbstractionsAssemblyTests.cs b/tests/Beutl.Extensibility.Abstractions.Tests/ExtensibilityAbstractionsAssemblyTests.cs new file mode 100644 index 0000000000..709d628a5e --- /dev/null +++ b/tests/Beutl.Extensibility.Abstractions.Tests/ExtensibilityAbstractionsAssemblyTests.cs @@ -0,0 +1,102 @@ +using System.Reflection; + +using Beutl.Extensibility; + +namespace Beutl.Extensibility.Abstractions.Tests; + +[TestFixture] +public class ExtensibilityAbstractionsAssemblyTests +{ + [Test] + public void BaseExtensionContracts_LiveInAbstractionsAssembly() + { + Assert.That(typeof(Extension).Assembly.GetName().Name, Is.EqualTo("Beutl.Extensibility.Abstractions")); + Assert.That(typeof(ExtensionSettings).Assembly, Is.SameAs(typeof(Extension).Assembly)); + Assert.That(typeof(ExportAttribute).Assembly, Is.SameAs(typeof(Extension).Assembly)); + } + + [Test] + public void MinimalExtension_CanUseOnlyAbstractionsProject() + { + var extension = new MinimalExtension(); + + Assert.That(extension.Name, Is.EqualTo(nameof(MinimalExtension))); + Assert.That(extension.Settings, Is.TypeOf()); + } + + [Test] + public void AbstractionsAssembly_DoesNotPullInHeavyImplementationDependencies() + { + // Walk the full closure, not just direct references: GetReferencedAssemblies() is pruned of + // unused references, so a heavy but not-type-referenced dependency would slip a direct-only check. + HashSet closure = CollectReferencedAssemblyClosure(typeof(Extension).Assembly); + + // The thin layer must never reach the heavy UI/media layer, directly or transitively. + string[] heavyAssemblies = + [ + "Beutl.Extensibility", + "Beutl.Engine", + "Avalonia", + "FluentAvaloniaUI", + "Microsoft.CodeAnalysis.CSharp.Scripting", + "SkiaSharp", + "SkiaSharp.HarfBuzz", + "Vortice.XAudio2", + ]; + foreach (string heavy in heavyAssemblies) + { + Assert.That(closure, Does.Not.Contain(heavy), $"Abstractions must not depend on {heavy}."); + } + + // Closed allowlist so any new Beutl.* assembly entering the closure fails even when the + // blocklist above doesn't name it. + string[] allowedBeutlAssemblies = + [ + "Beutl.Extensibility.Abstractions", + "Beutl.Core", + "Beutl.Configuration", + "Beutl.Utilities", + "Beutl.Language", + ]; + string[] beutlInClosure = [.. closure.Where(x => x.StartsWith("Beutl.", StringComparison.Ordinal))]; + Assert.That(beutlInClosure, Is.SubsetOf(allowedBeutlAssemblies)); + } + + private static HashSet CollectReferencedAssemblyClosure(Assembly root) + { + var seen = new HashSet(StringComparer.Ordinal) { root.GetName().Name! }; + var queue = new Queue(); + queue.Enqueue(root); + + while (queue.Count > 0) + { + foreach (AssemblyName reference in queue.Dequeue().GetReferencedAssemblies()) + { + if (!seen.Add(reference.Name!)) + { + continue; + } + + try + { + queue.Enqueue(Assembly.Load(reference)); + } + catch + { + // Name is already recorded; an assembly absent from the test output just can't be walked further. + } + } + } + + return seen; + } + + private sealed class MinimalExtension : Extension + { + public override ExtensionSettings Settings { get; } = new MinimalSettings(); + } + + private sealed class MinimalSettings : ExtensionSettings + { + } +} diff --git a/tests/Beutl.UnitTests/Api/LocalPackageTests.cs b/tests/Beutl.UnitTests/Api/LocalPackageTests.cs new file mode 100644 index 0000000000..e3b88c2775 --- /dev/null +++ b/tests/Beutl.UnitTests/Api/LocalPackageTests.cs @@ -0,0 +1,75 @@ +using System.Text; +using Beutl.Api.Services; +using NuGet.Packaging; + +namespace Beutl.UnitTests.Api; + +[TestFixture] +public class LocalPackageTests +{ + [TestCase("Beutl.Sdk")] + [TestCase("Beutl.Extensibility")] + [TestCase("Beutl.Extensibility.Abstractions")] + public void Constructor_ReadsTargetVersionFromBeutlDependency(string dependencyId) + { + using var stream = new MemoryStream(Encoding.UTF8.GetBytes(CreateNuspec(dependencyId))); + var package = new LocalPackage(new NuspecReader(stream)); + + Assert.That(package.TargetVersion, Does.Contain("2.99.99")); + } + + [Test] + public void Constructor_PrefersHigherPrecedenceBeutlDependency() + { + // Precedence is Beutl.Sdk > Beutl.Extensibility > Beutl.Extensibility.Abstractions. + LocalPackage extensibilityOverAbstractions = CreatePackage( + ("Beutl.Extensibility", "[1.1.1, )"), + ("Beutl.Extensibility.Abstractions", "[2.2.2, )")); + LocalPackage sdkOverTheRest = CreatePackage( + ("Beutl.Extensibility", "[1.1.1, )"), + ("Beutl.Sdk", "[3.3.3, )"), + ("Beutl.Extensibility.Abstractions", "[2.2.2, )")); + + Assert.Multiple(() => + { + Assert.That(extensibilityOverAbstractions.TargetVersion, Does.Contain("1.1.1")); + Assert.That(extensibilityOverAbstractions.TargetVersion, Does.Not.Contain("2.2.2")); + Assert.That(sdkOverTheRest.TargetVersion, Does.Contain("3.3.3")); + Assert.That(sdkOverTheRest.TargetVersion, Does.Not.Contain("1.1.1")); + Assert.That(sdkOverTheRest.TargetVersion, Does.Not.Contain("2.2.2")); + }); + } + + private static LocalPackage CreatePackage(params (string Id, string Version)[] dependencies) + { + using var stream = new MemoryStream(Encoding.UTF8.GetBytes(CreateNuspec(dependencies))); + return new LocalPackage(new NuspecReader(stream)); + } + + private static string CreateNuspec(string dependencyId) => + CreateNuspec((dependencyId, "[2.99.99, )")); + + private static string CreateNuspec(params (string Id, string Version)[] dependencies) + { + string depElements = string.Join( + "\n", + dependencies.Select(d => $""" """)); + + return $$""" + + + + Sample.Extension + 1.0.0 + b-editor + Sample extension. + + + {{depElements}} + + + + + """; + } +} diff --git a/tests/Beutl.UnitTests/Api/PackageManagerExtensionLifecycleTests.cs b/tests/Beutl.UnitTests/Api/PackageManagerExtensionLifecycleTests.cs index f05efa27d8..75d5cd9303 100644 --- a/tests/Beutl.UnitTests/Api/PackageManagerExtensionLifecycleTests.cs +++ b/tests/Beutl.UnitTests/Api/PackageManagerExtensionLifecycleTests.cs @@ -1,4 +1,7 @@ -using Beutl.Api.Services; +using System.Reflection; + +using Beutl.Api.Services; +using Beutl.Configuration; using Beutl.Extensibility; namespace Beutl.UnitTests.Api; @@ -109,6 +112,60 @@ public void LoadExtensionsAndRegister_RollsBackNewExtensions_WhenPackageAlreadyT Assert.That(commandManager.GetDefinitions(typeof(SuccessfulViewExtension)), Is.Empty); } + [Test] + public void SetupExtensionSettings_SubscribesToConfigurationChanged() + { + PackageManager manager = CreatePackageManager(out _, out _); + var extension = new SettingsExtension(); + + manager.SetupExtensionSettings(extension); + + Assert.That(ConfigurationChangedSubscriberCount(extension.Settings), Is.EqualTo(1)); + } + + [Test] + public void SetupExtensionSettings_DoesNotLeaveStaleSubscription_OnRepeatedSetup() + { + PackageManager manager = CreatePackageManager(out _, out _); + var extension = new SettingsExtension(); + + manager.SetupExtensionSettings(extension); + // A second setup must unsubscribe the previous handler before adding a new one. + manager.SetupExtensionSettings(extension); + + Assert.That(ConfigurationChangedSubscriberCount(extension.Settings), Is.EqualTo(1)); + } + + [Test] + public async Task Unload_RemovesSettingsChangedSubscription() + { + PackageManager manager = CreatePackageManager(out _, out ExtensionProvider provider); + var package = new LocalPackage { Name = "WithSettings" }; + + manager.LoadExtensionsAndRegister( + activity: null, + package, + assemblies: [], + loadContext: null, + [typeof(SettingsExtension)]); + + SettingsExtension extension = provider.GetExtensions().Single(); + Assert.That(ConfigurationChangedSubscriberCount(extension.Settings), Is.EqualTo(1)); + + await manager.Unload(package); + + Assert.That(ConfigurationChangedSubscriberCount(extension.Settings), Is.EqualTo(0)); + } + + private static int ConfigurationChangedSubscriberCount(ExtensionSettings settings) + { + FieldInfo field = typeof(ConfigurationBase).GetField( + nameof(ConfigurationBase.ConfigurationChanged), + BindingFlags.Instance | BindingFlags.NonPublic)!; + var handler = (EventHandler?)field.GetValue(settings); + return handler?.GetInvocationList().Length ?? 0; + } + private static PackageManager CreatePackageManager( out ContextCommandManager commandManager, out ExtensionProvider extensionProvider) @@ -181,4 +238,14 @@ public override void Unload() UnloadCount++; } } + + [Export] + private sealed class SettingsExtension : Extension + { + public override ExtensionSettings Settings { get; } = new TrackedSettings(); + } + + private sealed class TrackedSettings : ExtensionSettings + { + } }