Skip to content

Commit 0165384

Browse files
IvanMurzakclaude
andcommitted
refactor: reuse TryParseInstalledDllName in stale-DLL filesystem sweep
Replace per-stem Regex compile inside the foreach loop with a single call into NuGetInstallManifest.TryParseInstalledDllName, which already defines the version-tail grammar. Drops the System.Text.RegularExpressions import, eliminates ~85 lines of duplicated XML doc / inline comment that narrated WHAT the code already shows, and centralises the version-tail shape in one place. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 4b8928a commit 0165384

1 file changed

Lines changed: 32 additions & 86 deletions

File tree

Unity-MCP-Plugin/Packages/com.ivanmurzak.unity.mcp/Editor/DependencyResolver/NuGetPackageInstaller.cs

Lines changed: 32 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
using System.Collections.Generic;
1414
using System.IO;
1515
using System.Linq;
16-
using System.Text.RegularExpressions;
1716
using UnityEngine;
1817

1918
namespace com.IvanMurzak.Unity.MCP.Editor.DependencyResolver
@@ -127,19 +126,8 @@ internal static bool Install(NuGetPackage package, string installPath, HashSet<s
127126
// operate on the same filenames the collision check would compare against.
128127
var planned = NuGetExtractor.PlanDllPaths(nupkgPath, installPath, package.Version);
129128

130-
// Filesystem-based stale-version sweep. The manifest-driven
131-
// RemoveStaleSiblingVersions above only catches stale DLLs the
132-
// manifest knows about — if the manifest was deleted, never
133-
// existed, or got out of sync with disk, a stale
134-
// <c>{stem}.&lt;olderVersion&gt;.dll</c> would survive and Unity
135-
// would load it alongside the freshly extracted current-version
136-
// copy (duplicate-assembly errors). For each DLL stem this
137-
// package ships, scan the install root for any
138-
// <c>{stem}.dll</c> or <c>{stem}.&lt;numericVersion&gt;.dll</c>
139-
// whose version doesn't match the current package version, and
140-
// remove it (along with its .meta sidecar). The current-version
141-
// canonical filename is preserved — it will be overwritten by
142-
// extraction or already up to date.
129+
// Filesystem fallback for stale DLLs the manifest doesn't know about
130+
// (manifest deleted, partial-restore failure, AV quarantine).
143131
if (RemoveStaleVersionDllsByStem(installPath, planned, package.Version, package.Id))
144132
anyInstalled = true;
145133

@@ -367,96 +355,54 @@ internal static bool RemoveStaleSiblingVersions(string installPath, string packa
367355
}
368356

369357
/// <summary>
370-
/// Filesystem-driven complement to <see cref="RemoveStaleSiblingVersions"/>.
371-
/// For every DLL stem the package ships (read off the .nupkg via
372-
/// <paramref name="planned"/>), scans <paramref name="installPath"/>
373-
/// for any sibling that matches <c>{stem}.dll</c> or
374-
/// <c>{stem}.&lt;numericVersion&gt;.dll</c> at a version OTHER than
375-
/// <paramref name="keepVersion"/>, and removes it together with its
376-
/// <c>.meta</c> sidecar. The canonical
377-
/// <c>{stem}.{keepVersion}.dll</c> filename is intentionally
378-
/// preserved — extraction below will overwrite it (or short-circuit
379-
/// via the <c>alreadyOnDisk</c> gate when nothing changed).
380-
///
381-
/// <para>
382-
/// Why we need both this and the manifest-driven scan: when the
383-
/// manifest is missing, corrupted, or has drifted out of sync with
384-
/// disk (manual deletion, partial-restore failure, AV quarantine),
385-
/// the manifest-driven cleanup has nothing to act on and a stale
386-
/// <c>{stem}.&lt;olderVersion&gt;.dll</c> from a prior install
387-
/// survives. Unity then sees both the stale and the freshly
388-
/// extracted current-version copy, registers the same assembly
389-
/// manifest name twice, and the project breaks with CS0436 /
390-
/// CS0433 duplicate-assembly errors. The filesystem sweep is the
391-
/// authoritative pass that catches that case independently of any
392-
/// manifest state.
393-
/// </para>
394-
///
395-
/// <para>
396-
/// Cross-stem safety: the regex anchors on the DLL stem followed
397-
/// either by <c>.dll</c> directly or by a strictly numeric version
398-
/// tail (<c>\d+(\.\d+){0,3}</c>). A package shipping <c>Foo.dll</c>
399-
/// cannot accidentally match <c>Foo.Bar.10.0.0.dll</c> because
400-
/// <c>Bar.10.0.0</c> isn't numeric-only. A package shipping
401-
/// <c>Foo.dll</c> at version 1.0.0 also won't delete a different
402-
/// package's <c>Foo.dll</c> at 2.0.0 if both somehow ended up in
403-
/// the same install path — but that scenario means two packages
404-
/// claiming the same assembly stem at incompatible versions, which
405-
/// would already be a project-breaking duplicate even without this
406-
/// sweep, and the loud delete log makes the conflict visible.
407-
/// </para>
408-
///
409-
/// Returns true when at least one file was removed.
358+
/// Filesystem-driven complement to <see cref="RemoveStaleSiblingVersions"/>:
359+
/// removes any <c>{stem}.dll</c> or <c>{stem}.{anyOtherVersion}.dll</c> on
360+
/// disk for the DLL stems this package ships, leaving only the canonical
361+
/// <c>{stem}.{keepVersion}.dll</c>. Catches stale DLLs the manifest
362+
/// doesn't know about (manifest deleted, partial-restore failure, AV
363+
/// quarantine) — without this, the freshly extracted current-version
364+
/// copy and the orphan stale copy coexist and Unity errors with CS0436.
410365
/// </summary>
411366
internal static bool RemoveStaleVersionDllsByStem(string installPath, IReadOnlyList<PlannedDll> planned, string keepVersion, string packageId)
412367
{
413368
if (!Directory.Exists(installPath) || planned.Count == 0)
414369
return false;
415370

416-
// Original DLL stems shipped by the package — keyed off the .nupkg
417-
// entry path, not the planned canonical filename, so the regex
418-
// matches whatever the user has on disk regardless of how it got
419-
// there.
420-
var originalStems = planned
421-
.Select(p => Path.GetFileNameWithoutExtension(Path.GetFileName(p.EntryFullName)))
422-
.Where(s => !string.IsNullOrEmpty(s))
423-
.Distinct(StringComparer.OrdinalIgnoreCase)
424-
.ToList();
371+
var originalStems = new HashSet<string>(
372+
planned
373+
.Select(p => Path.GetFileNameWithoutExtension(Path.GetFileName(p.EntryFullName)))
374+
.Where(s => !string.IsNullOrEmpty(s)),
375+
StringComparer.OrdinalIgnoreCase);
425376

426377
if (originalStems.Count == 0)
427378
return false;
428379

429-
// Filenames that match the canonical current-version shape — these
430-
// must NOT be deleted. Compared by exact filename (case-insensitive)
431-
// so a stale uppercase variant on Windows still gets cleaned up.
432380
var canonicalNames = new HashSet<string>(
433381
planned.Select(p => p.FileName),
434382
StringComparer.OrdinalIgnoreCase);
435383

436384
var anyRemoved = false;
437-
var existingFiles = Directory.GetFiles(installPath, "*.dll", SearchOption.TopDirectoryOnly);
438-
439-
foreach (var stem in originalStems)
385+
foreach (var dllPath in Directory.GetFiles(installPath, "*.dll", SearchOption.TopDirectoryOnly))
440386
{
441-
// {stem}.dll or {stem}.<numeric-version>.dll (1-4 segments,
442-
// each segment 1-9 digits — System.Version's ceiling).
443-
var pattern = new Regex(
444-
@"^" + Regex.Escape(stem) + @"(\.\d+(?:\.\d+){0,3})?\.dll$",
445-
RegexOptions.IgnoreCase | RegexOptions.CultureInvariant);
387+
var fileName = Path.GetFileName(dllPath);
388+
if (canonicalNames.Contains(fileName))
389+
continue;
446390

447-
foreach (var dllPath in existingFiles)
448-
{
449-
var fileName = Path.GetFileName(dllPath);
450-
if (canonicalNames.Contains(fileName))
451-
continue;
452-
if (!pattern.IsMatch(fileName))
453-
continue;
391+
// Reuse the manifest's filename parser so the version-tail
392+
// grammar stays defined in one place. Unversioned legacy
393+
// {stem}.dll files (e.g. McpPlugin.dll dropped in by hand)
394+
// fall through to GetFileNameWithoutExtension.
395+
var stem = NuGetInstallManifest.TryParseInstalledDllName(fileName, out var parsedStem, out _) && parsedStem != null
396+
? parsedStem
397+
: Path.GetFileNameWithoutExtension(fileName);
454398

455-
Debug.Log($"{Tag} Removing stale '{fileName}' from install path — superseded by {packageId} {keepVersion}.");
456-
TryDeleteFile(dllPath);
457-
TryDeleteFile(dllPath + ".meta");
458-
anyRemoved = true;
459-
}
399+
if (!originalStems.Contains(stem))
400+
continue;
401+
402+
Debug.Log($"{Tag} Removing stale '{fileName}' from install path — superseded by {packageId} {keepVersion}.");
403+
TryDeleteFile(dllPath);
404+
TryDeleteFile(dllPath + ".meta");
405+
anyRemoved = true;
460406
}
461407

462408
return anyRemoved;

0 commit comments

Comments
 (0)