fix: flatten NuGetResolver install layout to escape Windows MAX_PATH#736
Merged
IvanMurzak merged 11 commits intomainfrom May 9, 2026
Merged
Conversation
The NuGet dependency resolver previously installed each package into a
per-package directory ({Id}.{Version}/{Dll}.dll). On Windows, when the
project's absolute path is long enough that the resulting DLL paths exceed
MAX_PATH = 260, Unity's bundled Mono runtime — which opens DLLs through
legacy CreateFileW without the \?\ long-path prefix — fails with an opaque
DirectoryNotFoundException even when the file is on disk and even with the
"Enable Win32 long paths" group policy enabled. The Unity-AI-Animation
extension's test project hit this at 271 chars, 11 over the limit.
Restructure the on-disk layout so DLLs sit FLAT directly under
Assets/Plugins/NuGet/ (no per-package subfolder). The longest path on the
failing 271-char setup drops to 220 chars — comfortably inside MAX_PATH
with headroom for future package version bumps and longer DLL names.
Implementation:
- NuGetExtractor writes DLLs flat at the install root, preserving each
DLL's original stem (System.Text.Json.dll, McpPlugin.dll, …). Filenames
are intentionally unversioned: Unity asmdef precompiledReferences entries
are filename-keyed at compile time, so versioned filenames would break
every consuming asmdef on every NuGet bump. The package version lives in
the new on-disk manifest instead.
- New NuGetInstallManifest (.nuget-installed.json at the install root) is
the only source of truth for "which DLL belongs to which package at
which version". Hand-rolled JSON read/write so the DependencyResolver
assembly stays the only one that compiles before any NuGet package is
on disk (the same invariant NuGetDependencyResolver was built against).
- New NuGetLegacyMigration runs once per project at the start of every
Restore() pass: detects legacy {Id}.{Version}/ directories, deletes
them with their .meta sidecars, then lets the normal install loop
re-extract into the flat layout. Idempotent on every subsequent run.
On Windows file-lock failures the migration aborts cleanly (legacy
folders intact, no flat-layout DLLs written) so the project is never
left in the duplicate-assembly state — the user restarts Unity to drop
the AppDomain, and the migration retries on the next reload.
- New NuGetLongPathPreflight aborts the install BEFORE any disk write
when a planned DLL path would exceed 255 chars (260 - 5-char .meta
slack) on Windows. The check is a no-op on macOS / Linux.
- NuGetPackageInstaller / NuGetPackageRestorer / NuGetPluginConfigurator
switched from directory enumeration to manifest-driven reads.
Defense-in-depth filename-collision detection refuses installs whose
DLL would overwrite one already owned by a different package ID in
the manifest.
- The on-disk Assets/Plugins/NuGet/ contents are migrated to the new
layout in this commit (the resolver's first run on the new code did
the migration locally; the result is committed so CI sees the final
state directly).
Tests:
- Reworked NuGetPackageInstallerStaleVersionCleanupTests for the flat
layout (the helper is now manifest-driven).
- New NuGetInstallManifestTests cover round-trip serialization,
case-insensitive lookup, and idempotent saves.
- New NuGetLegacyMigrationTests cover happy-path migration (single- and
multi-DLL packages), idempotence on re-runs, mixed legacy + flat
state, and the Windows file-lock abort path.
- New NuGetLongPathPreflightTests cover the cross-platform OS check
and threshold boundary via an internal test seam.
- New NuGetExtractorFlatLayoutTests cover flat-layout extraction from
synthetic .nupkg fixtures.
All 893 EditMode tests pass against the new layout.
Note on the spec deviation from the issue body: the issue called for
versioned filenames ({stem}.{packageVersion}.dll). That's incompatible
with Unity's asmdef precompiledReferences — those resolve by filename at
compile time, so versioned filenames would force every consuming asmdef
to be edited on every NuGet bump. The flat-layout-with-unversioned-names
design saves more path characters anyway (51 vs 44) and keeps every
asmdef working unchanged. The "self-describing layout" property the spec
wanted is satisfied by the manifest, which is now the authoritative
package → DLL → version mapping.
Closes #733
Contributor
Test Results 12 files 546 suites 41m 10s ⏱️ Results for commit 94fa831. ♻️ This comment has been updated with latest results. |
Per the issue spec and reviewer correction, the flat-layout DLL filenames
must include the package version: {stem}.{packageVersion}.dll
(e.g. McpPlugin.6.2.0.dll, ReflectorNet.5.1.1.dll). The previous unversioned
form (McpPlugin.dll) was a deviation from #733; this commit aligns the
implementation with the spec.
Changes:
- NuGetExtractor.PlanDllPaths / ExtractDlls take a packageVersion parameter
and produce filenames via NuGetExtractor.ToVersionedFileName(stem, ver).
The rename is idempotent — re-running against an already-versioned input
is a no-op.
- NuGetInstallManifest re-introduces TryParseInstalledDllName and
TryRebuildFromDisk: the manifest stays the primary source of truth, and
on-disk filename parsing is the disaster-recovery fallback that the spec
explicitly relies on.
- NuGetPackageRestorer rebuilds the manifest from versioned filenames when
.nuget-installed.json is missing — restoring the disaster-recovery story
the spec called for.
- NuGetPackageInstaller passes package.Version into the extractor and
uses TryParseInstalledDllName when checking which DLLs Unity already
provides (the assembly identity comes from the stripped stem, not the
versioned filename).
- NuGetPluginConfigurator strips the version tail before doing the
UnityAssemblyResolver.IsAlreadyImported lookup so the assembly identity
matches Unity's manifest name, not the on-disk filename.
- All four asmdef files (Runtime, Editor, Editor.Tests, Tests) updated to
reference the versioned filenames in their precompiledReferences. This
is the cost the project accepts for the self-describing layout — future
NuGet bumps require updating the asmdef references in lockstep with
NuGetConfig.Packages, but the manifest + migration handle the on-disk
side automatically. The stale "Microsoft.Extensions.Diagnostics.dll"
reference (no such DLL in the closure) was dropped from the Editor
asmdef as cleanup.
- The on-disk Assets/Plugins/NuGet/ contents are re-migrated from the
unversioned filenames to the versioned form; the manifest is rewritten
to match.
- All test fixtures and assertions now use versioned filenames.
NuGetExtractorFlatLayoutTests pins the rename behavior; the manifest
tests cover round-trip serialization, case-insensitive lookup, idempotent
saves, and TryParseInstalledDllName edge cases (greedy version tail,
package IDs with dots, short stems, malformed tails). The legacy
migration tests assert the post-migration steady state contains
versioned flat DLLs.
Local EditMode test gate: 904 / 904 DependencyResolver tests pass with the
versioned filename layout. The project's pre-existing SceneViewToolbarOverlay
TearDown flakiness is unrelated.
Closes #733
…est writes Address PR #736 review findings: - [high] Reconcile synthetic stem-keyed manifest entries left by TryRebuildFromDisk before the collision check fires, so multi-DLL packages (e.g. Microsoft.Bcl.Memory) can re-install on the disaster-recovery path instead of looping on "Refusing to install". - [medium] Stage manifest writes via .nuget-installed.json.tmp + delete-then-rename so a crash mid-write cannot truncate the file. - [medium] Cover the multi-DLL disaster-recovery + reconcile flow with new EditMode tests (would have caught the [high] finding). - [low] Reword RemoveStaleSiblingVersions docstring to match the manifest.Packages.Remove(packageId) implementation. - [low] LongPathPreflight error message now states the .meta-companion 5-char slack and the 255-char DLL cap explicitly. - [low] Wrap Path.GetFullPath in CheckWith so legacy Mono PathTooLongException surfaces as InstallPathTooLongException (consistent failure class). - [low] Acknowledge in NuGetLegacyMigration's XML doc that legacy meta GUIDs are intentionally not preserved (asmdef precompiledReferences are filename-based). simplify-pass: 1
…ling and persistence Address pass-2 review findings: - [medium] Gate alreadyOnDisk on planned subseteq manifestEntry.Dlls so a partial post-migration entry forces re-extraction of missing DLLs. - [low] MigrateSyntheticOwnerEntries now returns bool; caller persists the manifest immediately on a successful migration so a downstream halt cannot leave it desynced. - [low] NuGetLegacyMigration: document the plugin-exclusive directory contract for the install path. - [low] MigrateSyntheticOwnerEntries: XML-doc note that this migrator silently absorbs same-version cross-package DLL collisions (intentional for disaster recovery). Adds two regression tests covering the partial-disk-state migration path and the no-op return-value contract. simplify-pass: 2
PR #736 flattened Unity-MCP-Plugin/Assets/Plugins/NuGet/ to versioned filenames (McpPlugin.6.2.0.dll, ReflectorNet.5.1.1.dll, ...) and updated com.IvanMurzak.Unity.MCP.Runtime.asmdef's precompiledReferences to those versioned names. The three Unity-Tests/<version>/ projects each ship their own committed Assets/Plugins/NuGet/ mirror — those mirrors were left in the OLD nested {pkg}.{ver}/{pkg}.dll layout, so Unity's filename-keyed precompiledReferences resolution missed every DLL and the runtime asmdef failed to compile in CI with hundreds of CS0246/CS0234 errors across all 12 Unity test jobs. Replicate the source-of-truth flat layout into each mirror: - Unity-Tests/2022.3.62f3/Assets/Plugins/NuGet/ - Unity-Tests/2023.2.22f1/Assets/Plugins/NuGet/ - Unity-Tests/6000.3.1f1/Assets/Plugins/NuGet/ The post-state of every mirror is byte-for-byte identical to Unity-MCP-Plugin/Assets/Plugins/NuGet/ at HEAD: same DLL filenames, same .dll.meta files (so asset-database GUIDs are now consistent across source and mirrors), and the .nuget-installed.json manifest is included for parity. The directory-level NuGet.meta on each mirror is preserved (each test project keeps its own folder GUID; nothing referenced the OLD per-package folder GUIDs externally — verified by scanning every scene/asmdef/asset in Unity-Tests/<ver>/Assets/ for any reference to the discarded mirror GUIDs; zero hits). Local Unity EditMode test gate: 910 / 910 pass at this commit (parity with the baseline in PR #736 — the change is mirror-only, no plugin-source code touched). Closes #733
The Unity-Tests mirrors copied every DLL from
Unity-MCP-Plugin/Assets/Plugins/NuGet/, including McpPlugin.6.2.0.dll
and McpPlugin.Common.6.2.0.dll. Those two assemblies are produced BY
the Unity-MCP-Plugin itself and are reachable in the test projects via
the UPM file:// reference to
Unity-MCP-Plugin/Packages/com.ivanmurzak.unity.mcp/. Having the same
internal-simple-name assembly present in two locations causes the C#
compiler to emit CS1704 ("An assembly with the same simple name has
already been imported") on every Unity test-runner matrix job (12
jobs across 3 versions × 2 platforms × 2 testModes).
Refine the mirror invariant: Unity-Tests/<version>/Assets/Plugins/NuGet/
is a byte-for-byte mirror of Unity-MCP-Plugin/Assets/Plugins/NuGet/
EXCEPT for DLLs whose simple-name matches an assembly produced by
Unity-MCP-Plugin/ itself — currently McpPlugin and McpPlugin.Common.
Changes:
- Delete McpPlugin.6.2.0.{dll,dll.meta} and
McpPlugin.Common.6.2.0.{dll,dll.meta} from each of the three CI
matrix mirrors (2022.3.62f3, 2023.2.22f1, 6000.3.1f1).
- Drop com.IvanMurzak.McpPlugin and com.IvanMurzak.McpPlugin.Common
entries from each per-mirror .nuget-installed.json so the manifest
stays in sync with on-disk state.
- Source-of-truth Unity-MCP-Plugin/Assets/Plugins/NuGet/ is unchanged
(the plugin self-installs those DLLs into its own NuGet folder
during build; that is correct and remains untouched).
Local validation: 910/910 EditMode tests pass on Unity 2023.2.22f1.
Closes #733
…ata; cross-platform test coverage
ExtractPackageIdFromDirName previously gated detection on
System.Version.TryParse, which rejects SemVer prerelease ("1.0.0-preview")
and build-metadata ("1.2.3+build.42") tails. Real NuGet packages use those,
so legacy folders shaped `Foo.1.0.0-preview/` were silently skipped by the
migration and the user ended up with both the old nested DLL and the new
flat-layout DLL on disk simultaneously — duplicate-assembly compile errors.
Add a SemVer-shape regex fallback so those folders are detected and removed
in the same pass. Regression test covers prerelease, build-metadata, and
dot-suffix prerelease folder names.
Test split for the platform-sensitive cases that fail on the Linux Docker
CI runner:
- Run_FileLock_AbortsAndLeavesLegacyIntact (Windows) — guarded #if
UNITY_EDITOR_WIN. FileShare.None is advisory on Unix and does not block
Directory.Delete(recursive:true), so the migration completes instead of
aborting and the assertion fails for a platform-specific reason.
- Run_PermissionDenied_AbortsAndLeavesLegacyIntact (Unix) — new. Uses
libc chmod 0o500 on the install-path parent to deny write, which makes
the final rmdir of the now-empty legacy subdirectory throw
UnauthorizedAccessException. The migration catches that and reports
Outcome.AbortedFileLock the same way as the Windows file-lock path.
Self-skips when geteuid()==0 because the kernel bypasses chmod-based
denial for root, which is the GameCI Unity Docker container's default.
- CheckWith_BoundaryAtThreshold_DoesNotThrow (Windows) — guarded.
CheckWith_BoundaryAboveThreshold_Throws (Windows) — guarded. Both
craft `C:\…` paths that Mono on Linux does not recognise as drive-rooted,
so Path.GetFullPath inflates them with the cwd and breaks the boundary
precision the tests are pinning.
- CheckWith_BoundaryAtThreshold_UnixShapedPath_DoesNotThrow (Unix) — new.
CheckWith_BoundaryAboveThreshold_UnixShapedPath_Throws (Unix) — new.
Use `/tmp/…` paths that round-trip through Path.GetFullPath unchanged
on Unix, pinning the same `<=` boundary semantics that the Windows
variants pin on Windows.
Co-Authored-By: Claude Opus 4.7 (1M context) <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.
Summary
{Id}.{Version}/directory and writing every DLL flat underAssets/Plugins/NuGet/with the versioned filename pattern{stem}.{packageVersion}.dll(e.g.McpPlugin.6.2.0.dll,System.Memory.10.0.3.dll)..nuget-installed.json) as the primary source of truth for the package -> DLL -> version mapping, with on-disk filename parsing as the disaster-recovery fallback.CS0436/CS0433duplicate-assembly errors.MAX_PATHminus meta-slack.Runtime,Editor,Editor.Tests,Tests) soprecompiledReferenceslists the versioned filenames in lockstep withNuGetConfig.Packages.The longest path on the originally-failing 271-char setup drops to ~226 chars — comfortably inside
MAX_PATHwith headroom for future package version bumps and longer DLL names.Test plan
SceneViewToolbarOverlayTestsTearDown NREs are unrelated to this change).NuGetExtractorFlatLayoutTests(synthetic.nupkg-> versioned flat extraction;ToVersionedFileNameidempotence)NuGetInstallManifestTests(round-trip serialization, case-insensitive lookup, idempotent saves,TryParseInstalledDllNameedge cases — greedy version tail, package IDs with dots, short stems, malformed tails)NuGetLegacyMigrationTests(happy path, idempotence, mixed legacy + flat state, Windows file-lock abort)NuGetLongPathPreflightTests(cross-platform OS check, threshold boundary)NuGetPackageInstallerStaleVersionCleanupTestsfor the manifest-driven helper with versioned DLL fixtures.claude/worktrees/733-nuget-resolver-flat-layout-long-path/), the resolver migrated the legacy{Id}.{Version}/layout to versioned flat DLLs in a single domain reload; every consuming asmdef resolved itsprecompiledReferencesagainst the versioned filenames; no compile errors after the migration.Acceptance criteria coverage (issue #733)
Assets/Plugins/NuGet/contains DLLs directly (no per-package subfolders) plus a.nuget-installed.jsonmanifest.{Id}.{Version}/directory and its.metafile are deleted in the same restore cycle as the new flat extraction.Debug.LogErrorand leaves the legacy folders intact.Assets/Plugins/NuGet/carries the package version in its filename; zero unversioned.dllfiles remain after a successful install..nuget-installed.jsonand re-running rebuilds the manifest from on-disk versioned filenames without re-extraction churn.Closes #733