Multiple fixes for broken master#26
Open
line0 wants to merge 122 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to restore a working “master” by reintroducing missing version-handling functionality, fixing several regressions in module loading and update/feed handling, and adding initial unit-test and line-ending consistency infrastructure.
Changes:
- Added
SemanticVersioningand migrated version parsing/formatting to use it across updater/feed paths. - Fixed/adjusted updater + module-loader behaviors related to initialization and update flows, and added a minimal DepCtrl unit test.
- Updated the project update feed metadata, bumped versions, and enforced LF + final newline consistency.
Reviewed changes
Copilot reviewed 12 out of 15 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/DependencyControl/Updater.moon | Migrates updater version handling to SemanticVersioning and adjusts update logging/changelog display. |
| modules/DependencyControl/UpdateFeed.moon | Adds legacy scriptType compatibility + new invalid scriptType messaging; moves DownloadManager init earlier. |
| modules/DependencyControl/UnitTestSuite.moon | No functional change (newline normalization). |
| modules/DependencyControl/Tests.moon | Adds a first unit test for Common.terms.capitalize. |
| modules/DependencyControl/SemanticVersioning.moon | Introduces semantic version parsing/formatting/comparison utilities. |
| modules/DependencyControl/Record.moon | Switches record version parsing to SemanticVersioning and adds compatibility helpers. |
| modules/DependencyControl/ModuleLoader.moon | Updates init-hook guard logic and switches version formatting/parsing to SemanticVersioning. |
| modules/DependencyControl/FileOps.moon | No functional change (newline normalization). |
| modules/DependencyControl/ConfigHandler.moon | No functional change (newline normalization). |
| modules/DependencyControl/Common.moon | Fixes capitalize() implementation to avoid unsupported string indexing. |
| modules/DependencyControl.moon | Bumps DepCtrl version, exposes SemanticVersioning, and adds a Moonscript minimum-version guard. |
| macros/l0.DependencyControl.Toolbox.moon | Bumps toolbox version; updates version formatting and script-type handling; registers DepCtrl test suite. |
| DependencyControl.json | Updates feed versions/hashes and adds new files (including tests) to the module feed. |
| .vscode/settings.json | Enforces final newline on save in VS Code. |
| .gitattributes | Enforces LF line endings in git. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
7f7ad69 to
9fee44a
Compare
9fee44a to
ad4a5ec
Compare
Comment on lines
57
to
64
| maxVer = SemanticVersioning\toNumber @version | ||
| minVer = SemanticVersioning\toNumber minVer | ||
|
|
||
| changelog = {} | ||
| for ver, entry in pairs @changelog | ||
| ver = DependencyControl\parseVersion ver | ||
| verStr = DependencyControl\getVersionString ver | ||
| ver = SemanticVersioning\toNumber ver | ||
| verStr = SemanticVersioning\toString ver | ||
| if ver >= minVer and ver <= maxVer |
number segments over 255 and non-dot separate values
…r string is passed
ad4a5ec to
5bc3f45
Compare
nothing inside DepCtrl is supplying this parameter, so the only effect this bug would have had, is removing the default guard against modules trying to load themselves
- assert now passes through all varargs on success (allows chaining) - new assertNotNil method - fails only on nil, unlike assert which also fails on false - dump/dumpToString methods accept a maxDepth to cap recursive table output - new static Logger.describeType method - returns Lua type name but renders MoonScript class instances as "ClassName object" - fix: guard log/format calls against non-string message templates
…e up-to-date path require's up-to-date-but-not-updated branch called ModuleLoader.loadModule with task.record.namespace as the module-spec argument. loadModule is a fat-arrow static, so the dot-call binds that string as `mdl` and immediately does `with mdl` -> indexing a string -> crash. Pass task.record as the spec, matching the other load sites, and assert the call arguments in the test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
performUpdate gated its unmanaged-record handling on @record.updateRecordType, a field that exists nowhere (the field is recordType). The check was always false, so an unmanaged module never got its dummy ref + version registered before dependency loading, and finish never removed the dummy ref on failure. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
An empty or separator-only segment made the flatten callback return nil, which
left a hole in the flattened array; the ipairs walk then stopped at the hole, so
joinPath("a", "", "b") returned "a". Return an empty array for such segments so
they contribute nothing without breaking iteration. Also wires the three
pre-existing joinPath_segments* tests into _order (they were defined but never
ran) and adds an empty-segment regression.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
remove passed its (nil) recurse straight to rmdir, whose own default is true, so `remove somePath` on a path that was unexpectedly a directory silently deleted the whole tree — the opposite of the documented default. Default remove's recurse to false; rmdir keeps its own true default for the callers that rely on it (e.g. temp-dir cleanup). A directory passed without recurse now fails cleanly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The BFS seeded every config-derived feed and fetched it unconditionally; the block/policy gate applied only to advertised children. So a blocked feed that was also a configured root (an extraFeed or a package-declared feed) got fetched on every crawl, and untrusted roots were fetched even under fetchUntrustedFeeds = never. Gate each root through feedTrust.shouldFetch, exactly as children are: a blocked root is never fetched and an untrusted root honors the policy, while the root still appears in the inventory as recorded-but-not-followed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
io.open's result was used unchecked, so a missing log dir, a permissions error, or a full disk turned every file-logging call into an 'index a nil value' crash — and nothing created ?user/log on a fresh Aegisub profile. Create the log dir best-effort before opening, and on open failure disable file logging (noting it once in the window) instead of crashing. The message still reaches the window. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…d of crashing getChangelog ran each changelog key through SemanticVersioning.toNumber, which returns false (not nil) for an invalid version; toString(false) and false >= minVer then raised. Changelog keys come from unvalidated feed data, so one bad key crashed getChangelog — called right after a successful install. Skip keys that don't parse. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A key whose value cycled back to an ancestor made _equals return true for the
whole comparison, so two cyclic tables were declared equal even when a sibling
key differed (e.g. {x=1, self=self} vs {x=2, self=self}) — and the early return
also skipped the depth decrement, corrupting later sibling comparisons. Treat a
cyclic key as matched and continue checking the remaining keys instead.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The item tracker was a boolean set, so a value appearing more than once was
recorded once and matched once: itemsEqual({1,1}, {1,1}) returned false, and
assertItemsEqual reported spurious failures on arrays with repeated scalars.
Track occurrences as a multiset and match by decrementing the count.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e dir The temp download path was built with table.concat and no separator, gluing the downloadPath directory onto the file name so the file landed in ?temp/ itself rather than the ..._feedCache subdir that trimFiles scans — so temp feed downloads accumulated there unbounded. Build the path with FileOps.joinPath so it lands inside downloadPath and the maxFiles-20 trim actually applies. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@get scanned @@handlers comparing the raw filePath, but entries are keyed by the validated path, so an unnormalized spelling (a path token, mixed separators, a /./ segment) missed the cache and built a second handler for the same file — leaving two live handlers with divergent in-memory config. Validate first, then do a direct lookup by the canonical path (also replacing the linear scan). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
setFile reassigned @__configHandler but never added the view to the handler's view set, so the handler's whole-file refreshes (view\refresh! for view in views) skipped it — after another view triggered a load that replaced the hive tables, the setFile'd view pointed at a dead table and its writes were lost. Register the view with the new handler and detach it from the old; apply the same to unsetFile. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…stance GC Collecting one handle to a named semaphore called sem_unlink immediately, so an ephemeral handle being GC'd while another holder still owned the name removed the name from the kernel — a later sem_open then created a *separate* semaphore (value 1), letting a fresh handle acquire the 'same' lock while the first holder still held it (split-brain). Register names for unlink and defer it to a single module-level canary that runs at Lua-state teardown (≈ process exit), anchored on the class so it isn't collected — and fire the unlink — early. Windows is unaffected (kernel refcount self-heals). Verified on POSIX (WSL): the added regression test reproduces the split-brain on the old code and passes on the fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…itively block stored opts.matchMode verbatim (a bogus value only coerced to prefix later, on read) and deduped by a case-sensitive URL compare while matchesBlockEntry matches case-insensitively — so https://Bad/ and https://bad/ stored as two equivalent blocks. Validate the mode at the mutation boundary (unknown -> prefix) and compare URLs case-insensitively when deduping. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
__loadOfficial cached its result even when DepCtrl's own feed failed to load, so an offline first run stuck the official trust/block lists at the minimal fallback for the whole session — never picking them up even after the feed became reachable (e.g. once the updater fetched it into the cache). Cache only on a successful load; a failure returns the fallback uncached so a later call retries. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Enum.describe: doc param order matched to the signature (values, pattern, join) — a doc-following caller passed a separator where a formatter function goes. - Updater.scheduleUpdate: return is always an UpdateStatus (never boolean), plus the ProtectedInstall entryPath second value now documented. - scriptType annotations: integer -> ScriptType across ScriptTargetFilter, ScriptUpdateRecord and UpdateFeed (left over from the string-Enum migration). - ConfigView.getSectionHandler: doc says the hive path is file-root-relative (every caller relies on that), not relative to the view. - UpdateTask.refreshRecord: drop the ---@Private (it's called cross-class from Updater) and document it. - AGENTS.md: ScriptType/RecordType are Enums now, not bare tables. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…grate old configs @depConf.scriptFields still listed the pre-0.7 boolean `unmanaged` (with a lone -- REMOVE marker) and omitted `recordType`, so an unmanaged record's type was never written to config — it round-tripped back as managed. Persist/import `recordType`, and migrate a pre-0.7 config that stored only `unmanaged` so those records don't silently become managed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Per-test durations used os.clock (CPU time) but are reported as ms wall-time, so tests that sleep or wait (downloads, timers) under-reported. Use the monotonic Timer clock instead.
…asing it to the caller (releaseLock), or it's held until orphanTimeout.
The one-time config migration now rewrites each record's legacy boolean `unmanaged` flag into its recordType (and drops the flag), rather than Record.loadConfig re-deriving recordType from the stale flag on every load. Because migration runs once and persists, the obsolete key is cleared from the config file instead of lingering indefinitely. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ction to skip tests;
Mechanical rename of the module, its test suite, and all ~180 references ahead of making the class instantiable as a version value. No behavior change. Safe because the class was only split out during the (unreleased) 0.7.0 refactor, so there is no external API to keep compatible. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… bumping Construct a version from a string or numeric components (or via fromPacked); instances compare with </<=/==, stringify, bump immutably, and satisfy ranges. The static helpers accept an instance anywhere they take a version, and the static formerly named toNumber is renamed toPacked to match the instance method. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A new primitive for declaring get/set computed properties on a class:
Accessors.property{get, set} marks a property in the class body and
Accessors.install(class) — called once after the body — rewrites the
instance metatable so reads/writes of those keys dispatch to the getter/
setter, falling through to normal method/field lookup otherwise. A
setter-less property is read-only (a write raises); a subclass that runs
install inherits its parent's properties; every property is recorded on
class.__accessors for inspection tooling. Centralizes the metatable
manipulation in one audited place.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A record's canonical version is now a SemanticVersion value object stored as @semanticVersion, with record.version a packed-integer accessor over it, so existing packed-int reads/writes/comparisons are unchanged. The version is persisted to the config as a "major.minor.patch" string (readable at a glance), migrated from the old packed-integer form on first load; @semanticVersion is initialized up front so a record exposed mid-construction never reads a nil. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
install adds a __pairs metamethod so a readable computed property appears in pairs(instance) with its getter value, like a raw field (under 52compat). Since version then shows up as a packed int while the config stores a string, it's kept out of Record's generic scriptFields copy and serialized explicitly. Inline comments across the rework are tightened per the guidelines. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contribution conventions for DependencyControl: annotation/comment style, file naming, MoonScript gotchas, the public-API and test-exposure patterns, computed properties via Accessors, module-private markers, and changelog rules. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The "build macro and module lists" and "create and run the update tasks" comments only restated the immediately following buildDlgList / buildInstalledDlgList / runUpdaterTask calls, which the variable names already convey. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
cceac44 to
9c245f0
Compare
Convert getProgress -> progress (Downloader), getState -> state (Lock), getFailures -> failures (UnitTestSuite) and isPrivate (Host) from methods into read-only computed properties via the Accessors module, updating their call sites: the DownloadManager shim, the depctrl CLI runner, and the tests. Host.isPrivate memoizes into a backing field so repeated reads (and pairs) stay cheap after the one-time resolution. Add LockState/LockScope LuaCATS aliases so the enum-typed field and returns carry named types instead of a bare integer/string with a prose hint, and refresh the now-stale getFailures example in AGENTS.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes several issues, largely caused by the incomplete refactoring some 10 years ago:
__depCtrlInithooks were called again on - already-initialized modules, causing errors in modules that mutate their exported state on - first call (e.g., BadMutex).,true/falsescript type arguments were - no longer accepted after theScriptTypeenum migration.,DownloadManagerwas instantiated too late, causing the - update process to fail.capitalize()function no longer uses unsupported string indexing and now works - for the first time ever. Before the refactor, this was previously masked by it not actually - being called anywhere.Additionally, i have added a unit test suite for DepCtrl (only 1 tiny test for now) and made all files use LF + EOF Newline to ensure consistent file hashes.