From daa036898a8efd1ed109d64fd224fe280d52ec99 Mon Sep 17 00:00:00 2001 From: line0 Date: Sun, 16 Aug 2026 21:54:47 +0200 Subject: [PATCH 1/4] fix(ModuleLoader): exception instead of proper error message when a required module failed install --- DependencyControl.json | 13 ++++++++----- modules/l0/DependencyControl.moon | 2 +- modules/l0/DependencyControl/ModuleLoader.moon | 6 +++--- .../l0/DependencyControl/test/ModuleLoader.moon | 17 ++++++++--------- 4 files changed, 20 insertions(+), 18 deletions(-) diff --git a/DependencyControl.json b/DependencyControl.json index bec095d..c1010d3 100644 --- a/DependencyControl.json +++ b/DependencyControl.json @@ -121,14 +121,14 @@ "url": "@{baseUrl}#@{namespace}", "channels": { "main": { - "version": "0.8.1", - "released": "2026-07-24", + "version": "0.8.2", + "released": null, "default": true, "files": [ { "name": ".moon", "url": "@{fileBaseUrl}", - "sha1": "87CDA007933A971BAEFC91A044CF7FC941535D0D" + "sha1": "29CCC772940BFB3F7A92FD93EA0306905A05897F" }, { "name": "/Constants.moon", @@ -229,7 +229,7 @@ { "name": "/ModuleLoader.moon", "url": "@{fileBaseUrl}", - "sha1": "F1ACB8570A361F9006A6E6BA74FFF84D1F058F3A" + "sha1": "42DE5BE3BED98011ABE0A0420C2C5FB7201C77AA" }, { "name": "/ModuleProvider.moon", @@ -488,7 +488,7 @@ { "name": "/ModuleLoader.moon", "url": "@{fileBaseUrl}", - "sha1": "8FE0E4ED90D3ED719F7DE80877591EF2F30014AA", + "sha1": "9615E3A053EEB1A7D57A17FDDF06F5AB422D5533", "type": "test" }, { @@ -779,6 +779,9 @@ ], "0.8.1": [ "fix: Reading the channel list of a package that declares no channels returns an empty list instead of throwing an error." + ], + "0.8.2": [ + "fix: A required module that can't be installed or updated once again reports why. The error path that turns an updater status into a readable reason was itself throwing, so every such failure surfaced as an unrelated \"attempt to call field 'getUpdaterErrorMsg'\" error instead of naming the module and the cause." ] } }, diff --git a/modules/l0/DependencyControl.moon b/modules/l0/DependencyControl.moon index 2b5d2a8..d92e6d6 100644 --- a/modules/l0/DependencyControl.moon +++ b/modules/l0/DependencyControl.moon @@ -108,7 +108,7 @@ Accessors.install DependencyControl rec = DependencyControl{ name: "DependencyControl", - version: "0.8.1", -- @{l0.DependencyControl:version} + version: "0.8.2", -- @{l0.DependencyControl:version} description: "Provides script management and auto-updating for Aegisub macros and modules.", author: "line0", url: "http://github.com/TypesettingTools/DependencyControl", diff --git a/modules/l0/DependencyControl/ModuleLoader.moon b/modules/l0/DependencyControl/ModuleLoader.moon index 579dbee..48dcdf4 100644 --- a/modules/l0/DependencyControl/ModuleLoader.moon +++ b/modules/l0/DependencyControl/ModuleLoader.moon @@ -130,6 +130,7 @@ class ModuleLoader ---@return boolean success ---@return string err Combined error message (empty on success). @loadModules = (modules, addFeeds = {@feed}, skip = @moduleName and {[@moduleName]: true} or {}) => + UpdateTask or= require "l0.DependencyControl.UpdateTask" for mdl in *modules continue if skip[mdl.moduleName] with mdl @@ -147,9 +148,8 @@ class ModuleLoader if ._ref ._updated, ._missing = true, false else - UpdateTask or= require "l0.DependencyControl.UpdateTask" unless code == UpdateTask.UpdateStatus.SkippedOptional - ._reason = @@updater.__class.getUpdaterErrorMsg code, .name or .moduleName, domain.ScriptType.Module, true, extErr + ._reason = UpdateTask.getUpdaterErrorMsg code, .name or .moduleName, domain.ScriptType.Module, true, extErr -- nuke dummy reference for circular dependencies LOADED_MODULES[.moduleName] = nil @@ -171,7 +171,7 @@ class ModuleLoader ._ref = ref elseif not .optional ._outdated = true - ._reason = @@updater.__class.getUpdaterErrorMsg code, .name or .moduleName, domain.ScriptType.Module, false, extErr + ._reason = UpdateTask.getUpdaterErrorMsg code, .name or .moduleName, domain.ScriptType.Module, false, extErr missing, outdated, moduleError = {}, {}, {} for mdl in *modules diff --git a/modules/l0/DependencyControl/test/ModuleLoader.moon b/modules/l0/DependencyControl/test/ModuleLoader.moon index fc63124..d25e8e9 100644 --- a/modules/l0/DependencyControl/test/ModuleLoader.moon +++ b/modules/l0/DependencyControl/test/ModuleLoader.moon @@ -6,6 +6,7 @@ ModuleLoader = require "l0.DependencyControl.ModuleLoader" ModuleProvider = require "l0.DependencyControl.ModuleProvider" SemanticVersion = require "l0.DependencyControl.SemanticVersion" + UpdateTask = require "l0.DependencyControl.UpdateTask" DEPCTRL_DUMMY_MODULE_MARKER = "#{constants.DEPCTRL_PRIVATE_GLOBAL_VAR_PREFIX}Dummy" @@ -211,8 +212,7 @@ -- dummy ref is cleared. loadModules_missingRequiredFails: (ut) -> ns = "test.ModuleLoader.missingFail" - updaterClass = {getUpdaterErrorMsg: (code, name) -> "fetch failed: #{name}"} - updater = {require: ((...) => return nil, -6, "no feed"), __class: updaterClass} + updater = {require: ((...) => return nil, UpdateTask.UpdateStatus.NoSuitablePackage, "no feed")} recClass = setmetatable {ScriptType: domain.ScriptType, __name: "DependencyControl", :updater}, {__call: (cls, args) -> {}} rec = {feed: nil, moduleName: "host.Module", name: "host", __class: recClass} @@ -223,13 +223,13 @@ success, err = ModuleLoader.loadModules rec, {mdl} ut\assertFalse success ut\assertContains err, ns + ut\assertContains err, "no feed" -- the updater's detail reaches the user through the real formatter ut\assertNil LOADED_MODULES[ns] -- dummy ref nuked -- loadModules: a missing *optional* module the updater skips is left missing without an error -- reason and doesn't fail the overall load; the circular-dependency dummy ref is still cleared. loadModules_missingOptionalSkipped: (ut) -> ns = "test.ModuleLoader.missingOptionalSkip" - UpdateTask = require "l0.DependencyControl.UpdateTask" updater = {require: ((...) => return nil, UpdateTask.UpdateStatus.SkippedOptional)} recClass = setmetatable {ScriptType: domain.ScriptType, __name: "DependencyControl", :updater}, {__call: (cls, args) -> {}} @@ -245,13 +245,12 @@ ut\assertNil LOADED_MODULES[ns] -- dummy ref nuked -- loadModules: a required module that fails because one of ITS OWN requirements couldn't be satisfied - -- surfaces the nested reason (which sub-requirement failed, and why) in the error — using the real - -- getUpdaterErrorMsg so the RequirementsUnmet template's detail isn't dropped on the way to the UI. + -- surfaces the nested reason (which sub-requirement failed, and why) in the error, so the + -- RequirementsUnmet template's detail isn't dropped on the way to the UI. loadModules_requirementsUnmetSurfacesNestedReason: (ut) -> ns = "l0.ASSFoundation" - UpdateTask = require "l0.DependencyControl.UpdateTask" innerReason = "— SubInspector.Inspector (v0.7.2)\n—— Reason: no build for your platform (Linux-x64)" - updater = {require: ((...) => return nil, UpdateTask.UpdateStatus.RequirementsUnmet, innerReason), __class: UpdateTask} + updater = {require: ((...) => return nil, UpdateTask.UpdateStatus.RequirementsUnmet, innerReason)} recClass = setmetatable {ScriptType: domain.ScriptType, __name: "DependencyControl", :updater}, {__call: (cls, args) -> {}} rec = {feed: nil, moduleName: "host.Module", name: "Vector Gradient", __class: recClass} @@ -286,8 +285,7 @@ loadModules_outdatedRequiredFails: (ut) -> ns = "test.ModuleLoader.outdatedFail" loadedRef = {version: {version: 65793, checkVersion: ((target) => false)}} - updaterClass = {getUpdaterErrorMsg: (code, name) -> "too old: #{name}"} - updater = {require: ((...) => return nil, -6, "no newer version"), __class: updaterClass} + updater = {require: ((...) => return nil, UpdateTask.UpdateStatus.NoSuitablePackage, "no newer version")} recClass = setmetatable {ScriptType: domain.ScriptType, __name: "DependencyControl", :updater}, {__call: (cls, args) -> {}} rec = {feed: nil, moduleName: "host.Module", name: "host", __class: recClass} @@ -297,6 +295,7 @@ success, err = ModuleLoader.loadModules rec, {mdl} ut\assertFalse success ut\assertContains err, ns + ut\assertContains err, "no newer version" -- the updater's detail reaches the user through the real formatter -- checkOptionalModules: mock self with requiredModules From a4e60f490fa9b388e6219dd8d8ac7539b56b24cf Mon Sep 17 00:00:00 2001 From: line0 Date: Sun, 2 Aug 2026 19:50:46 +0200 Subject: [PATCH 2/4] fix(utils): deep copy breaking on circular references --- DependencyControl.json | 3 +- modules/l0/DependencyControl/test/utils.moon | 36 +++++++++++++++++++- modules/l0/DependencyControl/utils.moon | 18 ++++++++-- 3 files changed, 52 insertions(+), 5 deletions(-) diff --git a/DependencyControl.json b/DependencyControl.json index c1010d3..963c246 100644 --- a/DependencyControl.json +++ b/DependencyControl.json @@ -781,7 +781,8 @@ "fix: Reading the channel list of a package that declares no channels returns an empty list instead of throwing an error." ], "0.8.2": [ - "fix: A required module that can't be installed or updated once again reports why. The error path that turns an updater status into a readable reason was itself throwing, so every such failure surfaced as an unrelated \"attempt to call field 'getUpdaterErrorMsg'\" error instead of naming the module and the cause." + "fix: A required module that can't be installed or updated once again reports why. The error path that turns an updater status into a readable reason was itself throwing, so every such failure surfaced as an unrelated \"attempt to call field 'getUpdaterErrorMsg'\" error instead of naming the module and the cause.", + "fix(Utils): `deepCopy()` no longer recurses forever on a table that references itself. Circular references are reproduced as cycles in the copy, and a table reached through several keys is copied once." ] } }, diff --git a/modules/l0/DependencyControl/test/utils.moon b/modules/l0/DependencyControl/test/utils.moon index 042f9b3..bff7ff9 100644 --- a/modules/l0/DependencyControl/test/utils.moon +++ b/modules/l0/DependencyControl/test/utils.moon @@ -1,4 +1,4 @@ --- Utils tests: flattening, list membership, deep equality, item equality, and pattern escaping. +-- Utils tests: flattening, list membership, deep copying, deep equality, item equality, and pattern escaping. -- Called from test.moon as: (controls\requireTest "utils")! -> utils = require "l0.DependencyControl.utils" @@ -55,6 +55,39 @@ ut\assertFalse utils.listIncludes {"a", "b"}, "z" ut\assertFalse utils.listIncludes {}, "a" + -- deepCopy + + deepCopy_nestedTablesDetached: (ut) -> + keyTbl = {} + source = {n: 1, nested: {deep: {"x"}}, [keyTbl]: "keyed"} + copy = utils.deepCopy source + ut\assertEquals copy, source + ut\assertIsNot copy.nested, source.nested + ut\assertIsNot copy.nested.deep, source.nested.deep + ut\assertEquals copy[keyTbl], "keyed" -- table keys are carried over by identity, not copied + source.nested.deep[1] = "mutated" + ut\assertEquals copy.nested.deep[1], "x" + + -- a table reaching back into one already being copied resolves to that copy, so the walk terminates + -- and the result is a cycle of the same shape rather than a chain of ever-deeper copies + deepCopy_cyclicRefsPreserved: (ut) -> + source = {name: "root"} + source.self = source + source.child = {parent: source} + + copy = utils.deepCopy source + ut\assertIsNot copy, source + ut\assertIs copy.self, copy + ut\assertIs copy.child.parent, copy + ut\assertEquals copy.name, "root" + + -- one source table reached through two keys stays one table in the copy + deepCopy_sharedTableCopiedOnce: (ut) -> + shared = {"x"} + copy = utils.deepCopy {first: shared, second: shared} + ut\assertIsNot copy.first, shared + ut\assertIs copy.first, copy.second + -- equals: a cyclic value is treated as matched at that key, not as making the whole tables equal equals_cyclicRefs: (ut) -> cyc = (x) -> @@ -105,6 +138,7 @@ "flatten_depth2Array", "flatten_depth1StopsEarly", "flatten_depth0NoFlatten", "flatten_scalar", "flatten_returnsCount", "flatten_toArrayTable", "listIncludes_found", "listIncludes_notFoundAndEmpty", + "deepCopy_nestedTablesDetached", "deepCopy_cyclicRefsPreserved", "deepCopy_sharedTableCopiedOnce", "equals_cyclicRefs", "itemsEqual_duplicateScalars", "escapePattern_matchesLiterally", "escapePattern_plainStringUnchanged", "seedRandom_divergesAcrossSimultaneousStates" diff --git a/modules/l0/DependencyControl/utils.moon b/modules/l0/DependencyControl/utils.moon index 9350592..1250496 100644 --- a/modules/l0/DependencyControl/utils.moon +++ b/modules/l0/DependencyControl/utils.moon @@ -140,7 +140,19 @@ flatten = (value, depth = 1, toArrayTable) -> recurse value, depth return flattened, f -deepCopy = (tbl) -> {k, (type(v) == "table" and deepCopy(v) or v) for k, v in pairs tbl} +deepCopy = (tbl) -> + seen = {} + recurse = (value) -> + return value if type(value) != "table" + return seen[value] if seen[value] + + -- create the copy and register it before recursing into its values, so circular references resolve to the copy + copy = {} + seen[value] = copy + copy[k] = recurse v for k, v in pairs value + return copy + + return recurse tbl mergeSearchPath = (pathStr, add, remove) -> removed = remove and {p, true for p in *remove} or {} @@ -203,9 +215,9 @@ Utils = { ---@return table copy The copied table. copy: (tbl) -> {k, v for k, v in pairs tbl} - ---Deep-copies a table recursively (no metatables). + ---Deep-copies a table recursively (no metatables). Circular references are preserved as cycles in the copy. ---@param tbl table The table to deep-copy. - ---@return table copy The deep-copied table. + ---@return table copy The deep-copied table. Keys are carried over as-is; only values are copied. deepCopy: deepCopy ---Builds (or extends) a set from an array's values: each value becomes a key mapped to `value`. From 285e607c10357c75d30d4b333798b6ebea2e19ac Mon Sep 17 00:00:00 2001 From: line0 Date: Sun, 2 Aug 2026 20:01:22 +0200 Subject: [PATCH 3/4] fix(CLI): file delete flags in feeds being erroneously ignored by several CLI commands --- DependencyControl.json | 4 +++- depctrl.lua | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/DependencyControl.json b/DependencyControl.json index 963c246..41c9a2c 100644 --- a/DependencyControl.json +++ b/DependencyControl.json @@ -782,7 +782,9 @@ ], "0.8.2": [ "fix: A required module that can't be installed or updated once again reports why. The error path that turns an updater status into a readable reason was itself throwing, so every such failure surfaced as an unrelated \"attempt to call field 'getUpdaterErrorMsg'\" error instead of naming the module and the cause.", - "fix(Utils): `deepCopy()` no longer recurses forever on a table that references itself. Circular references are reproduced as cycles in the copy, and a table reached through several keys is copied once." + "fix(Utils): `deepCopy()` no longer recurses forever on a table that references itself. Circular references are reproduced as cycles in the copy, and a table reached through several keys is copied once.", + "fix(CLI): `generate-types` and `generate-docs` skip files a feed marks deleted instead of reporting each one as an unreadable source", + "fix(CLI): a deleted file's entry can no longer shadow the source that replaced it when a test suite resolves its modules from the feed." ] } }, diff --git a/depctrl.lua b/depctrl.lua index 9b4ea4d..0a7b026 100644 --- a/depctrl.lua +++ b/depctrl.lua @@ -259,7 +259,7 @@ local function registerFeedSearcher(feed) local sourceById = {} for file, _, pkg in feed:walkFiles() do local src = file.localFilePath - if src then + if src and not file.delete then local base = file.type == "test" and (pkg.namespace .. ".test") or pkg.namespace local id = base .. leafSuffix(file.name) sourceById[id] = sourceById[id] or src -- first channel wins; sources are channel-agnostic @@ -281,8 +281,8 @@ local function registerFeedSearcher(feed) end -- Collects the selected module packages' non-test .moon sources from a feed, keyed by require --- id, for annotation extraction. Vendored .lua files have no annotations and are skipped; a --- warning is printed for any unreadable source. +-- id, for annotation extraction. Vendored .lua files have no annotations and are skipped, as are +-- files the feed marks deleted; a warning is printed for any unreadable source. local function collectModuleSources(feed, filter) local domain = require "l0.DependencyControl.domain" local FileOps = require "l0.DependencyControl.file-ops" @@ -299,7 +299,7 @@ local function collectModuleSources(feed, filter) local sources, seen = {}, {} for file, _, pkg in feed:walkFiles() do local src = file.localFilePath - if selected[pkg.namespace] and src and file.type ~= "test" and file.name:match("%.moon$") then + if selected[pkg.namespace] and src and not file.delete and file.type ~= "test" and file.name:match("%.moon$") then local requireId = pkg.namespace .. leafSuffix(file.name) if not seen[requireId] then seen[requireId] = true From b95be5640b5713780a1644ee335145e355b3ad11 Mon Sep 17 00:00:00 2001 From: line0 Date: Mon, 17 Aug 2026 02:20:33 +0200 Subject: [PATCH 4/4] fix(Updater): modules with circular deps failing to update --- DependencyControl.json | 17 ++++--- .../l0/DependencyControl/ModuleLoader.moon | 2 +- modules/l0/DependencyControl/UpdateTask.moon | 20 +++++++- .../DependencyControl/test/ModuleLoader.moon | 19 +++++++ .../l0/DependencyControl/test/UpdateTask.moon | 51 +++++++++++++++++++ .../l0/DependencyControl/test/Updater.moon | 30 ++++++++++- 6 files changed, 127 insertions(+), 12 deletions(-) diff --git a/DependencyControl.json b/DependencyControl.json index 41c9a2c..848192f 100644 --- a/DependencyControl.json +++ b/DependencyControl.json @@ -229,7 +229,7 @@ { "name": "/ModuleLoader.moon", "url": "@{fileBaseUrl}", - "sha1": "42DE5BE3BED98011ABE0A0420C2C5FB7201C77AA" + "sha1": "059526819899F46484C78AF6C1D6A92F2C354EB2" }, { "name": "/ModuleProvider.moon", @@ -284,7 +284,7 @@ { "name": "/UpdateTask.moon", "url": "@{fileBaseUrl}", - "sha1": "D7296244D4B9DAEAD0463CF6FE699AAF5CDF39A8" + "sha1": "4C1B8BFBE29120014A182F37F1D9E7AA0E456C07" }, { "name": "/Updater.moon", @@ -369,7 +369,7 @@ { "name": "/utils.moon", "url": "@{fileBaseUrl}", - "sha1": "FA8307BC15AD2FEC1535F9B862E1D92C2A3A1AB9" + "sha1": "35B197A1947BA389AB1F522F1EE6F351EEC54949" }, { "name": ".moon", @@ -488,7 +488,7 @@ { "name": "/ModuleLoader.moon", "url": "@{fileBaseUrl}", - "sha1": "9615E3A053EEB1A7D57A17FDDF06F5AB422D5533", + "sha1": "0B1EF3A8E4C79310DD20511F7F6BBEAF8F219CB0", "type": "test" }, { @@ -554,13 +554,13 @@ { "name": "/UpdateTask.moon", "url": "@{fileBaseUrl}", - "sha1": "5CA6FC899341677F702788F543373C8F3EDCFAA0", + "sha1": "D1C04F53DE0609804FE0E9B63AF5DD1EEA2D1D5B", "type": "test" }, { "name": "/Updater.moon", "url": "@{fileBaseUrl}", - "sha1": "7945FCE5A2DC11257301DE5A2B2423E56A0AC4CB", + "sha1": "46B1E9BD099B831E194F514971BB26667DA470EC", "type": "test" }, { @@ -638,7 +638,7 @@ { "name": "/utils.moon", "url": "@{fileBaseUrl}", - "sha1": "F78238492CB05F9F1568C14DE363A83CE1CAB6D8", + "sha1": "431E1940BB43BB2FE1C3C38A55F57574D278A3DF", "type": "test" }, { @@ -784,7 +784,8 @@ "fix: A required module that can't be installed or updated once again reports why. The error path that turns an updater status into a readable reason was itself throwing, so every such failure surfaced as an unrelated \"attempt to call field 'getUpdaterErrorMsg'\" error instead of naming the module and the cause.", "fix(Utils): `deepCopy()` no longer recurses forever on a table that references itself. Circular references are reproduced as cycles in the copy, and a table reached through several keys is copied once.", "fix(CLI): `generate-types` and `generate-docs` skip files a feed marks deleted instead of reporting each one as an unreadable source", - "fix(CLI): a deleted file's entry can no longer shadow the source that replaced it when a test suite resolves its modules from the feed." + "fix(CLI): a deleted file's entry can no longer shadow the source that replaced it when a test suite resolves its modules from the feed.", + "fix(Updater): Updates no longer fail with 'the update task is already running' for modules that require one another (such as Aegisub-Motion's 'Tags' and 'Transform')." ] } }, diff --git a/modules/l0/DependencyControl/ModuleLoader.moon b/modules/l0/DependencyControl/ModuleLoader.moon index 48dcdf4..febf2a5 100644 --- a/modules/l0/DependencyControl/ModuleLoader.moon +++ b/modules/l0/DependencyControl/ModuleLoader.moon @@ -169,7 +169,7 @@ class ModuleLoader ref, code, extErr = @@updater\require record, .version, addFeeds if ref ._ref = ref - elseif not .optional + elseif code != UpdateTask.UpdateStatus.UpdateInProgress and not .optional ._outdated = true ._reason = UpdateTask.getUpdaterErrorMsg code, .name or .moduleName, domain.ScriptType.Module, false, extErr diff --git a/modules/l0/DependencyControl/UpdateTask.moon b/modules/l0/DependencyControl/UpdateTask.moon index 00fef9c..6e0dc2d 100644 --- a/modules/l0/DependencyControl/UpdateTask.moon +++ b/modules/l0/DependencyControl/UpdateTask.moon @@ -139,6 +139,7 @@ SourceFeedKind = Enum "SourceFeedKind", { ---| 1 # Installed: the install or update succeeded ---| 2 # AlreadyUpdated: another in-flight update already brought the package to the target version ---| 3 # SkippedOptional: an optional dependency couldn't be satisfied and was skipped +---| 4 # UpdateInProgress: this package's update is already under way and will install a satisfying version ---| -1 # UpdaterDisabled: the updater is disabled in the config ---| -2 # InvalidNamespace: the record's namespace doesn't conform to the rules ---| -3 # Unmanaged: the record is virtual or unmanaged, so it isn't updated @@ -168,6 +169,7 @@ UpdateStatus = Enum "UpdateStatus", { Installed: 1 AlreadyUpdated: 2 SkippedOptional: 3 + UpdateInProgress: 4 UpdaterDisabled: -1 InvalidNamespace: -2 Unmanaged: -3 @@ -297,6 +299,7 @@ msgs = { ---Mutable execution state for one install/update operation. ---@class UpdateTask +---@field private __installingVersion? string The version this task's running update will install. Set only while it resolves its requirements and used to break module dependency cycles. class UpdateTask ---@private @__downloader = Downloader! @@ -623,6 +626,14 @@ class UpdateTask with @record do @logger\log msgs.run.starting, getInstallTerm(@record), domain.terms.scriptType.singular[.scriptType], .name + -- The field is only set while this task's own update resolves its requirements, so re-entering here + -- means we must be dealing with a dependency cycle. This short-circuits the requirement check if the + -- requested version is satisfied by the version being installed, so the cycle can be broken. + if @__installingVersion + satisfied = not @targetVersion or SemanticVersion\check @__installingVersion, @targetVersion + return UpdateStatus.UpdateInProgress, @__installingVersion if satisfied + return @__logUpdateError UpdateStatus.TaskAlreadyRunning + -- don't perform update of a script when another one is already running for the same script return @__logUpdateError UpdateStatus.TaskAlreadyRunning if @running @@ -908,8 +919,15 @@ class UpdateTask if reqs and #reqs > 0 @logger\log msgs.performUpdate.updateReqs @logger.indent += 1 - success, err = ModuleLoader.loadModules @record, reqs, {@record.feed} + -- Remember the version being installed to break dependency cycles, but prevent it from becoming stale, e.g., + -- in case of an update error, as it could otherwise corrupt subsequent update runs in an unpredictable way. + @__installingVersion = update.version + loaded, success, err = pcall ModuleLoader.loadModules, @record, reqs, {@record.feed} + @__installingVersion = nil @logger.indent -= 1 + unless loaded + @running = false + error success, 0 unless success @logger.indent += 1 @logger\log err diff --git a/modules/l0/DependencyControl/test/ModuleLoader.moon b/modules/l0/DependencyControl/test/ModuleLoader.moon index d25e8e9..7e51819 100644 --- a/modules/l0/DependencyControl/test/ModuleLoader.moon +++ b/modules/l0/DependencyControl/test/ModuleLoader.moon @@ -297,6 +297,24 @@ ut\assertContains err, ns ut\assertContains err, "no newer version" -- the updater's detail reaches the user through the real formatter + -- loadModules: a requirement caught in a dependency cycle doesn't fail the load — the module already + -- loaded keeps its place and isn't reported as outdated + loadModules_updateInProgressAcceptsLoadedRef: (ut) -> + ns = "test.ModuleLoader.inFlight" + loadedRef = {version: {version: 65793, checkVersion: ((target) => false)}} + updater = {require: ((...) => return nil, UpdateTask.UpdateStatus.UpdateInProgress, "2.0.0")} + recClass = setmetatable {ScriptType: domain.ScriptType, __name: "DependencyControl", :updater}, + {__call: (cls, args) -> {}} + rec = {feed: nil, moduleName: "host.Module", name: "host", __class: recClass} + mdl = {moduleName: ns, name: ns, version: SemanticVersion\toPacked "2.0.0", optional: false} + (ut\stub ModuleLoader, "loadModule")\calls (self, m, usePrivate) -> m._ref = loadedRef unless usePrivate + ut\stub(ModuleProvider, "isDepCtrlVersionRecord")\returns true + success, err = ModuleLoader.loadModules rec, {mdl} + ut\assertTrue success + ut\assertEquals err, "" + ut\assertEquals mdl._ref, loadedRef + ut\assertNil mdl._outdated + -- checkOptionalModules: mock self with requiredModules checkOptionalModules_noneOptional: (ut) -> @@ -335,6 +353,7 @@ "loadModules_missingFetchedViaUpdater", "loadModules_missingRequiredFails", "loadModules_missingOptionalSkipped", "loadModules_requirementsUnmetSurfacesNestedReason", "loadModules_outdatedForcesUpdate", "loadModules_outdatedRequiredFails", + "loadModules_updateInProgressAcceptsLoadedRef", "checkOptionalModules_noneOptional", "checkOptionalModules_missingOptional" } } diff --git a/modules/l0/DependencyControl/test/UpdateTask.moon b/modules/l0/DependencyControl/test/UpdateTask.moon index f7ae518..064091b 100644 --- a/modules/l0/DependencyControl/test/UpdateTask.moon +++ b/modules/l0/DependencyControl/test/UpdateTask.moon @@ -833,6 +833,28 @@ ut\assertNil task.calls.performUpdate -- no install performed ut\assertNotNil task.calls.persisted -- but the source choice is still recorded + -- run(): a dependency cycle re-enters the task whose update covers the requirement, so it reports the + -- update in progress and leaves the install to the run already under way + run_reentrantInFlightSatisfiesRequirement: (ut) -> + task = makeRunTask targetVersion: SemanticVersion\toPacked "1.3.0" + task.__installingVersion = "1.3.6" + code, detail = UpdateTask.run task + ut\assertEquals code, UpdateStatus.UpdateInProgress + ut\assertEquals detail, "1.3.6" + ut\assertNil task.calls.resolved + + -- run(): the same cycle, but the version being installed sits below the requirement, which finishing + -- that update won't change, so it stays an error + run_reentrantInFlightBelowRequirementFails: (ut) -> + task = makeRunTask targetVersion: SemanticVersion\toPacked "2.0.0" + task.__installingVersion = "1.3.6" + ut\assertEquals UpdateTask.run(task), UpdateStatus.TaskAlreadyRunning + + -- run(): with no version recorded there is no cycle to break, so a task still flagged running is rejected + run_reentrantWithoutClaimStillGuardsRunning: (ut) -> + task = makeRunTask running: true + ut\assertEquals UpdateTask.run(task), UpdateStatus.TaskAlreadyRunning + -- run(): a terminal resolution (no install required) returns its status without dispatching run_terminalResolutionReturnsStatus: (ut) -> task = makeRunTask { @@ -876,6 +898,32 @@ ut\assertTrue cap.optional ut\assertEquals cap.reason, UpdateReason.UserRequested + -- performUpdate: an error thrown while resolving requirements leaves no version behind and clears the + -- running flag, so it can't corrupt a later update run + performUpdate_clearsInFlightClaimOnThrow: (ut) -> + task = makePerformTask! + (ut\stub ModuleLoader, "loadModules")\calls -> error "requirement boom", 0 + update = {version: "1.3.6", files: {}, requiredModules: {{moduleName: "l0.dep"}}} + completed, err = pcall UpdateTask.performUpdate, task, update + ut\assertFalse completed + ut\assertEquals err, "requirement boom" + ut\assertNil task.__installingVersion + ut\assertFalse task.running + + -- performUpdate: the version being installed is readable while the requirements resolve, which is what + -- a cycle breaks against, and gone once they finish + performUpdate_publishesInFlightVersionWhileResolving: (ut) -> + task = makePerformTask! + seen = {} + (ut\stub ModuleLoader, "loadModules")\calls -> + seen.claim = task.__installingVersion + return false, "unmet" + update = {version: "1.3.6", files: {}, requiredModules: {{moduleName: "l0.dep"}}} + code = UpdateTask.performUpdate task, update + ut\assertEquals code, UpdateStatus.RequirementsUnmet + ut\assertEquals seen.claim, "1.3.6" + ut\assertNil task.__installingVersion -- and it doesn't outlive the resolution + -- performUpdate: a temp-directory creation failure aborts with -30 performUpdate_tempDirFailure: (ut) -> ut\stub(fileOps, "getTempDir")\returns "tmp" @@ -1077,8 +1125,11 @@ "resolve_autoNeverPrompts", "resolve_offerAllSourcesPromptsOnMultiple", "resolve_blockedFeedSkipped", "resolve_userFeedUsedExclusively", "run_dispatchesDirectInstall", "run_dispatchesProviderInstall", "run_upToDateShortCircuits", + "run_reentrantInFlightSatisfiesRequirement", "run_reentrantInFlightBelowRequirementFails", + "run_reentrantWithoutClaimStillGuardsRunning", "run_terminalResolutionReturnsStatus", "run_noInternetGuard", "installProvider_constructsRecordAndRequires", + "performUpdate_clearsInFlightClaimOnThrow", "performUpdate_publishesInFlightVersionWhileResolving", "performUpdate_tempDirFailure", "performUpdate_rejectsPathTraversal", "performUpdate_rejectsBadSha1", "performUpdate_reportsFailedDownloads", "performUpdate_reportsMoveFailures", "performUpdate_reloadsModuleAndRefreshesRecord", "performUpdate_recoversManagedRecordFromRegistry", diff --git a/modules/l0/DependencyControl/test/Updater.moon b/modules/l0/DependencyControl/test/Updater.moon index d79cf8b..23449ea 100644 --- a/modules/l0/DependencyControl/test/Updater.moon +++ b/modules/l0/DependencyControl/test/Updater.moon @@ -52,6 +52,17 @@ ut\assertEquals ref, task.ref ut\assertEquals code, UpdateStatus.Installed + -- a cycle's in-progress update has no ref to hand back yet, but its status isn't a failure and names + -- the version being installed + require_updateInProgressReturnsCode: (ut) -> + task = {updated: false, record: {namespace: "l0.dep", name: "Dep"}, + run: ((wait) => UpdateStatus.UpdateInProgress, "1.3.6")} + record = {scriptType: domain.ScriptType.Module, name: "Dep", namespace: "l0.dep", virtual: false} + ref, code, detail = Updater.require makeRequireUpdater(task), record, 0 + ut\assertNil ref + ut\assertEquals code, UpdateStatus.UpdateInProgress + ut\assertEquals detail, "1.3.6" + -- a skipped optional dependency yields no ref but still carries its status code require_skippedOptionalReturnsCode: (ut) -> task = {updated: false, record: {namespace: "l0.dep", name: "Dep"}, run: ((wait) => UpdateStatus.SkippedOptional)} @@ -187,6 +198,19 @@ ut\assertEquals existing.targetVersion, SemanticVersion\toPacked "2.0.0" ut\assertTrue existing.optional + -- Breaking a dependency cycle depends on the re-entrant lookup reaching the very task doing the update, + -- which the namespace key gives us for either record the loader passes — the loaded module's own on the + -- outdated path, a fresh virtual one on the missing path. Updating that task in place must not wipe the + -- version it is installing. + addTask_reentrantResolutionReachesRunningTask: (ut) -> + running = {targetVersion: 0, __installingVersion: "1.3.6"} + updater = stubSelf Updater, {tasks: {[domain.ScriptType.Module]: {["a-mo.Tags"]: running}}} + outdated = {__class: DependencyControl, scriptType: domain.ScriptType.Module, namespace: "a-mo.Tags"} + virtual = {__class: DependencyControl, scriptType: domain.ScriptType.Module, namespace: "a-mo.Tags", virtual: true} + ut\assertIs (Updater.addTask updater, outdated, "1.3.6"), running + ut\assertIs (Updater.addTask updater, virtual, "1.3.6"), running + ut\assertEquals running.__installingVersion, "1.3.6" + -- a record with no queued task gets a fresh UpdateTask, which is cached under its scriptType/namespace addTask_createsNewTask: (ut) -> record = {__class: DependencyControl, scriptType: domain.ScriptType.Module, namespace: "l0.new", validateNamespace: => true} @@ -259,7 +283,8 @@ _order: { "require_upToDateLoadsModule", "require_successReturnsRef", "require_errorPropagates" - "require_skippedOptionalReturnsCode", "require_upToDateLoadFailureReturnsCode" + "require_skippedOptionalReturnsCode", "require_updateInProgressReturnsCode" + "require_upToDateLoadFailureReturnsCode" "scheduleUpdate_disabledRejected", "scheduleUpdate_belowAutoUpdateModeRejected" "scheduleUpdate_virtualRejected" "scheduleUpdate_withinIntervalSkips", "scheduleUpdate_protectedInstallRejected" @@ -267,7 +292,8 @@ "acquireLock_returnsTrueWhenAlreadyHeld", "acquireLock_acquiresAndSetsHasLock" "acquireLock_failsWhenHeldByOther", "releaseLock_releasesWhenHeld", "releaseLock_noopWhenNotHeld" "renewLock_renewsWhenHeld" - "addTask_versionParseErrorReturns", "addTask_updatesExistingTask", "addTask_createsNewTask" + "addTask_versionParseErrorReturns", "addTask_updatesExistingTask" + "addTask_reentrantResolutionReachesRunningTask", "addTask_createsNewTask" "addTask_disabledUpdaterRejects", "addTask_modeGatesByReason", "addTask_invalidNamespaceRejects" "feedTrust_isSharedSingleton", "isEnabledFor_defaultsToAllContexts", "isEnabledFor_modeGatesByContext" }