Skip to content

fix: Prevent bundled dependency conflicts in consuming Unity projects#1364

Merged
hatayama merged 2 commits into
v3-betafrom
fix/asmdef-leak
Jun 17, 2026
Merged

fix: Prevent bundled dependency conflicts in consuming Unity projects#1364
hatayama merged 2 commits into
v3-betafrom
fix/asmdef-leak

Conversation

@hatayama

@hatayama hatayama commented Jun 17, 2026

Copy link
Copy Markdown
Owner

Summary

  • Keep Unity CLI Loop bundled CodeAnalysis dependencies private so consuming Unity projects can resolve their own dependency versions.
  • Prevent package dependency clashes around System.Runtime.CompilerServices.Unsafe.dll while preserving dynamic code support.

User Impact

  • Projects that install packages such as MemoryPack can keep their own System.Runtime.CompilerServices.Unsafe.dll dependency without Unity CLI Loop bundled assemblies being selected accidentally.
  • Dynamic code execution keeps its required dependencies through explicit editor-only references.

Changes

  • Rename the bundled CodeAnalysis dependency assemblies to UnityCliLoop.System.* private names and update their managed assembly identities.
  • Mark the bundled dependency plugins as explicitly referenced only, and update the Execute Dynamic Code assembly definition to reference them directly.
  • Add license text next to the bundled dependency assemblies and expand package metadata tests for private identities, explicit references, stale strong-name flags, and old file removal.

Verification

  • cli/dist/darwin-arm64/uloop compile --project-path "$(git rev-parse --show-toplevel)"
  • cli/dist/darwin-arm64/uloop run-tests --project-path "$(git rev-parse --show-toplevel)" --test-mode EditMode --filter-type regex --filter-value "CodeAnalysisPluginImporterTests"
  • /Users/a12115/.codex/skills/codex-review/scripts/codex-review v3-beta

Review in cubic

hatayama added 2 commits June 17, 2026 22:07
Rename the bundled Roslyn dependency DLLs with UnityCliLoop-private assembly identities and require explicit asmdef references so consuming Unity projects do not resolve this package's System.Runtime.CompilerServices.Unsafe assembly by accident.

- Mark dependency plugins explicitly referenced only
- Guard private identities and stale strong-name flags with EditMode tests
Keep the MIT license text next to the private renamed Microsoft .NET assemblies so the package carries the dependency license with the bundled binaries.
@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 92675a27-2352-420d-8bf4-43bd75bb95a3

📥 Commits

Reviewing files that changed from the base of the PR and between 732756d and 416df76.

⛔ Files ignored due to path filters (11)
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Plugins/CodeAnalysis/LICENSE.md.meta is excluded by none and included by none
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Plugins/CodeAnalysis/System.Collections.Immutable.dll is excluded by !**/*.dll and included by none
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Plugins/CodeAnalysis/System.Reflection.Metadata.dll is excluded by !**/*.dll and included by none
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Plugins/CodeAnalysis/System.Runtime.CompilerServices.Unsafe.dll is excluded by !**/*.dll and included by none
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Plugins/CodeAnalysis/UnityCliLoop.System.Collections.Immutable.dll is excluded by !**/*.dll and included by none
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Plugins/CodeAnalysis/UnityCliLoop.System.Collections.Immutable.dll.meta is excluded by none and included by none
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Plugins/CodeAnalysis/UnityCliLoop.System.Reflection.Metadata.dll is excluded by !**/*.dll and included by none
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Plugins/CodeAnalysis/UnityCliLoop.System.Reflection.Metadata.dll.meta is excluded by none and included by none
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Plugins/CodeAnalysis/UnityCliLoop.System.Runtime.CompilerServices.Unsafe.dll is excluded by !**/*.dll and included by none
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Plugins/CodeAnalysis/UnityCliLoop.System.Runtime.CompilerServices.Unsafe.dll.meta is excluded by none and included by none
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/UnityCLILoop.FirstPartyTools.ExecuteDynamicCode.Editor.asmdef is excluded by none and included by none
📒 Files selected for processing (3)
  • Assets/Tests/Editor/DynamicCodeToolTests/CodeAnalysisPluginImporterTests.cs
  • Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Plugins/CodeAnalysis/LICENSE.md
  • Packages/src/THIRD-PARTY-NOTICES.md

📝 Walkthrough

Walkthrough

The PR refactors CodeAnalysisPluginImporterTests to use a PrivateAssembly model with computed paths, adds five test methods validating editor-only import, explicit .meta references, private assembly identity strings, absent StrongNameSigned PE/CLI flags, and removal of legacy System.* assets. A LICENSE.md is added for the CodeAnalysis plugins, and THIRD-PARTY-NOTICES.md component names are prefixed with UnityCliLoop..

Changes

Private Assembly Validation Tests and Notices

Layer / File(s) Summary
Test fixture: constants, PrivateAssembly model, and legacy path list
Assets/Tests/Editor/DynamicCodeToolTests/CodeAnalysisPluginImporterTests.cs
Introduces package-relative constants, the PrivateAssembly helper class with computed asset/meta paths, the PrivateAssemblies data set, and OldCodeAnalysisPluginPaths for legacy System.* names.
Test methods for editor-only, explicit reference, identity, strong-name, and legacy removal
Assets/Tests/Editor/DynamicCodeToolTests/CodeAnalysisPluginImporterTests.cs
Updates editor-only test to iterate PrivateAssemblies; adds tests for isExplicitlyReferenced in .meta, private identity strings in DLL bytes, absent StrongNameSigned CLI flag, and non-existence of old System.* asset paths and .meta files.
PE/CLI header parsing helpers and byte scanning utilities
Assets/Tests/Editor/DynamicCodeToolTests/CodeAnalysisPluginImporterTests.cs
Adds ContainsAsciiText, ContainsBytesAt, ReadCorHeaderFlags, ReadInt16, and ReadInt32 to support PE structure parsing and ASCII substring detection used by the new test methods.
CodeAnalysis LICENSE.md and THIRD-PARTY-NOTICES component name updates
Packages/src/Editor/.../Plugins/CodeAnalysis/LICENSE.md, Packages/src/THIRD-PARTY-NOTICES.md
Adds the full MIT-based license for private-renamed .NET assemblies and updates the three component names in the third-party notices table to include the UnityCliLoop. prefix.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • hatayama/unity-cli-loop#1229: Directly modifies CodeAnalysisPluginImporterTests.cs to verify bundled code-analysis plugin DLLs are marked editor-only, which is the same test file and same base assertion this PR refactors.
  • hatayama/unity-cli-loop#1288: Extends the same test class with explicit-reference .meta checks and PE/CLI header flag validation (clearing StrongNameSigned), matching the new test methods added in this PR.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main objective of the PR: preventing bundled dependency conflicts by making CodeAnalysis dependencies private with UnityCliLoop prefixes.
Description check ✅ Passed The description is directly related to the changeset, clearly explaining the rationale, user impact, and specific changes made to address dependency version conflicts.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/asmdef-leak

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 14 files

Re-trigger cubic

@hatayama
hatayama merged commit f195892 into v3-beta Jun 17, 2026
9 checks passed
@hatayama
hatayama deleted the fix/asmdef-leak branch June 17, 2026 13:29
@github-actions github-actions Bot mentioned this pull request Jun 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant