Feature/cpm migration by EvaTheSalmon · Pull Request #95 · Semiteq/NtoLib

EvaTheSalmon · 2026-04-25T17:33:20Z

Summary

Migrates NtoLib.csproj from old-style + packages.config to SDK-style + PackageReference (no wildcards: EnableDefault*Items=false, explicit <Compile Include> rule preserved).
Adopts Central Package Management via Directory.Packages.props at the repo root — every NuGet version lives in one file and applies to NtoLib, Tests, Installer.
Replaces the Build/tools/Merge.ps1 PowerShell wrapper with ILRepack.Lib.MSBuild.Task integrated through NtoLib/ILRepack.targets. Merge is gated on -p:RunILRepack=true so plain dotnet build (and the
test build) stay un-merged — required because [InternalsVisibleTo("Tests")] would otherwise produce CS0433 collisions.
Adds nuget.config with a single explicit feed (eliminates NU1507 across all dev machines).
Fixes dependabot scope (/NtoLib → /) so it watches Directory.Packages.props. CI paths: filter triggers on it too.
Resolves PR Bump the nuget-production-updates group with 28 updates #94's root cause: Tests sees NtoLib's transitive package graph through ProjectReference, so Tests.dll.config gets correct binding redirects auto-generated. The Tests/App.config band-aid is no
longer needed.

Why

PR #94 (28-package dependabot bump) failed CI with 115 broken tests. The trigger was a FileNotFoundException for PeterO.Numbers, Version=1.8.0.0 inside MathS static ctor — Tests.dll.config lacked a
binding redirect because the format split (NtoLib on packages.config, Tests on PackageReference) isolated the two graphs. Adding redirects to Tests/App.config would have unblocked PR #94, but the underlying
drift would recur on every future bump. CPM is the structural fix.

Test plan

dotnet build NtoLib.sln -c Release — 0 errors
dotnet test NtoLib.sln -c Release — 225 / 225 passed
Build/Package.ps1 -Configuration Release — produces 5.46 MB merged NtoLib.dll; archive zip written. Verified via ilspycmd -l class that AngouriMath/Polly/Serilog/YamlDotNet/Microsoft.Extensions.*
are internalised.
dotnet format NtoLib.sln --verify-no-changes — clean
Three rounds of automated review (comprehensive → smells → critical-only) — clean

Follow-ups (not in this PR)

Close PR Bump the nuget-production-updates group with 28 updates #94 after merge — its diff (edits to deleted packages.config + deleted <Reference HintPath=..\packages\..> blocks) is obsolete. Dependabot will re-open a small PR proposing version bumps in
Directory.Packages.props only.
Manual verification: run Build/Deploy.ps1 against a MasterSCADA target machine; confirm netreg.exe NtoLib.dll /showerror registers cleanly. Catalogued in the migration plan's Post-Completion section.
CI does not currently exercise the merged DLL (-p:RunILRepack=true not set in workflow). Adding a Cecil-based smoke test on the merged artifact is a separate enhancement.

Plan to consolidate dependency management by migrating NtoLib.csproj from packages.config to PackageReference and adopting Central Package Management via Directory.Packages.props. Tests project then sees NtoLib's transitive graph, eliminating the binding-redirect drift that caused PR #94 to fail. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Consolidate dependency management so the Tests project sees NtoLib's transitive package graph, eliminating the binding-redirect drift that caused PR #94 to fail. - Add Directory.Packages.props with the centralised PackageVersion list enabling CPM (ManagePackageVersionsCentrally) for both projects. - Convert NtoLib.csproj from packages.config to PackageReference; remove packages.config and strip versions from PackageReference entries so CPM is active end-to-end. - Replace the standalone Merge.ps1 + nuget.exe ILRepack invocation with ILRepack.Lib.MSBuild.Task driven by NtoLib/ILRepack.targets, wired into the build via PackageReference. - Drop Tests/App.config binding-redirect band-aid (no longer needed once Tests resolves the same versions as NtoLib via CPM). - Simplify Build/Package.ps1 and Build/deploy.ps1: remove Merge.ps1 invocations now that ILRepack runs as part of the build. - Simplify CI restore step in .github/workflows/ci.yml (no more packages.config so plain dotnet restore suffices). - Fix dependabot.yml directory entry to track the new layout. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- AGENTS.md / CLAUDE.md: document the new dependency-management workflow (Directory.Packages.props authoring, no per-project versions, ILRepack via MSBuild task) so contributors understand the post-migration layout. - Build/readme.md: update build pipeline description to reflect the removal of Merge.ps1 and the move to ILRepack.Lib.MSBuild.Task. - .github/instructions/csharp.instructions.md: align Copilot guidance with CPM (no inline package versions) and the new ILRepack target. - Docs/known_issues/02-dll-merge-constraints.md: refresh with the current ILRepack invocation and remove the stale Merge.ps1 references. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Adds nuget.config at the repo root with <clear /> + nuget.org. Eliminates the NU1507 warnings raised by CPM whenever the developer machine has the VS-bundled "C:\Program Files (x86)\Microsoft SDKs\NuGetPackages" fallback feed configured globally. Now every contributor and CI runner restores from a single, explicit source. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

This PR migrates the repo to Central Package Management (CPM) and modernizes the NuGet/build pipeline by converting NtoLib to SDK-style PackageReference while integrating ILRepack as an MSBuild task (gated via RunILRepack=true) to preserve test build behavior.

Changes:

Adopt CPM with Directory.Packages.props, removing per-project NuGet version pins and deleting NtoLib/packages.config.
Convert NtoLib to SDK-style (net48) with explicit item includes (default globs disabled) and move dependency merging to ILRepack.Lib.MSBuild.Task via NtoLib/ILRepack.targets.
Simplify CI restore (remove nuget restore), add nuget.config, and update Dependabot scope to repo root.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
`nuget.config`	Adds explicit NuGet feed configuration (single source).
`Directory.Packages.props`	Introduces CPM as the single source of NuGet package versions.
`Tests/Tests.csproj`	Removes `Version=` attributes from `PackageReference` to comply with CPM.
`NtoLib/NtoLib.csproj`	Migrates to SDK-style `net48`, disables default item globs, switches to `PackageReference`, wires ILRepack hook property.
`NtoLib/ILRepack.targets`	Adds gated MSBuild-driven ILRepack merge target (`RunILRepack=true`).
`NtoLib/app.config`	Updates a binding redirect entry (System.Resources.Extensions).
`NtoLib/packages.config`	Deletes legacy packages.config-based dependency list.
`Build/Package.ps1`	Replaces Merge.ps1 invocation with `dotnet build -p:RunILRepack=true` step.
`Build/deploy.ps1`	Replaces Merge.ps1 invocation with `dotnet build -p:RunILRepack=true` step.
`Build/tools/Merge.ps1`	Removes legacy ILRepack PowerShell wrapper.
`Build/readme.md`	Updates build documentation to reflect MSBuild-based merge and new restore model.
`Docs/plans/completed/20260425-cpm-migration.md`	Adds detailed completed migration plan/record.
`Docs/known_issues/02-dll-merge-constraints.md`	Documents `RunILRepack` gate rationale with InternalsVisibleTo constraint.
`.github/workflows/ci.yml`	Simplifies restore to `dotnet restore` and adds CPM file to path filters.
`.github/dependabot.yml`	Updates NuGet ecosystem scan scope to repo root for CPM.
`.github/instructions/csharp.instructions.md`	Updates repo guidance for SDK-style + CPM + gated ILRepack behavior.
`CLAUDE.md`	Updates repo conventions and build instructions for SDK-style + CPM + gated ILRepack.
`AGENTS.md`	Mirrors updated conventions and build instructions for SDK-style + CPM + gated ILRepack.

Copilot · 2026-04-25T17:40:03Z

+      <ILRepackInputAssemblies Include="$(OutputPath)NtoLib.dll" />
+      <ILRepackInputAssemblies Include="$(OutputPath)*.dll"
+        Exclude="$(OutputPath)NtoLib.dll;
+                 $(OutputPath)System.Resources.Extensions.dll;
+                 $(OutputPath)System.Text.Json.dll;
+                 $(OutputPath)System.Text.Encodings.Web.dll" />


ILRepackInputAssemblies currently excludes only a few framework DLLs. The previous Build/tools/Merge.ps1 also excluded vendor/host assemblies (FB/InSAT*/MasterSCADA*/ICSharpCode*/OpcUaClient/Opc.Ua.Core/etc.). Relying solely on <Private>False</Private> means stale files in bin/ (from older builds or manual copies) could still be picked up by the *.dll glob and merged accidentally. To make the merge robust, consider explicitly excluding the vendor/host DLL name patterns here (or cleaning the output directory before running ILRepack).

Copilot · 2026-04-25T17:40:03Z

+    <CentralPackageTransitivePinningEnabled>true</CentralPackageTransitivePinningEnabled>
+  </PropertyGroup>
+  <ItemGroup>
+    <!-- NtoLib runtime dependencies (currently in NtoLib/packages.config) -->


This comment refers to NtoLib/packages.config, but that file is deleted as part of this migration. Suggest rewording to avoid pointing readers at a non-existent file (e.g., “NtoLib runtime dependencies”).

Suggested change

Copilot · 2026-04-25T17:40:04Z

+dotnet build NtoLib.sln                        # un-merged DLL (used by tests)
+dotnet build NtoLib.sln -p:RunILRepack=true    # merged DLL (used for deployment)
+Build/Package.ps1   # build + test + ILRepack merge + archive
 Build/Deploy.ps1    # build + merge + copy to target machine


Docs reference Build/Deploy.ps1, but the repository file is Build/deploy.ps1 (lowercase). On case-sensitive setups this path won’t resolve; consider updating the docs to match the actual filename.

Suggested change

Build/Deploy.ps1 # build + merge + copy to target machine

Build/deploy.ps1 # build + merge + copy to target machine

Copilot · 2026-04-25T17:40:04Z

+dotnet build NtoLib.sln                        # un-merged DLL (used by tests)
+dotnet build NtoLib.sln -p:RunILRepack=true    # merged DLL (used for deployment)
+Build/Package.ps1   # build + test + ILRepack merge + archive
 Build/Deploy.ps1    # build + merge + copy to target machine


Docs reference Build/Deploy.ps1, but the repository file is Build/deploy.ps1 (lowercase). On case-sensitive setups this path won’t resolve; consider updating the docs to match the actual filename.

Suggested change

Build/Deploy.ps1 # build + merge + copy to target machine

Build/deploy.ps1 # build + merge + copy to target machine

Copilot · 2026-04-25T17:40:04Z

+- **Build:** `dotnet build` produces an un-merged `NtoLib.dll` for unit tests. Merging
+  is gated on `/p:RunILRepack=true` (because `[InternalsVisibleTo("Tests")]` would
+  otherwise cause CS0433 ambiguity in the test project) and runs only via
+  `Build/Package.ps1` / `Build/Deploy.ps1` after the test step. `netreg.exe` registers


This document references Build/Deploy.ps1, but the repository file is Build/deploy.ps1 (lowercase). Consider updating the path so it matches the actual script name.

Suggested change

`Build/Package.ps1` / `Build/Deploy.ps1` after the test step. `netreg.exe` registers

`Build/Package.ps1` / `Build/deploy.ps1` after the test step. `netreg.exe` registers

Copilot · 2026-04-25T17:40:04Z

+        System.Text.Json + System.Text.Encodings.Web (kept external; bundled
+        separately by Archive.ps1 / Copy.ps1).


The comment says System.Text.Json / System.Text.Encodings.Web are “bundled separately by Archive.ps1 / Copy.ps1”, but those scripts currently only copy NtoLib.dll (+ optionally System.Resources.Extensions.dll). If these assemblies are expected to be present at runtime, consider updating the packaging/deploy pipeline to include them; otherwise, update this comment to reflect the real source (e.g., provided by the host environment).

Suggested change

System.Text.Json + System.Text.Encodings.Web (kept external; bundled

separately by Archive.ps1 / Copy.ps1).

System.Text.Json + System.Text.Encodings.Web (kept external and expected

to be supplied by the host/runtime environment or another deployment step).

Copilot · 2026-04-25T17:40:05Z

+      ILRepack-merged DLL surface. Currently no entry below is transitive-only,
+      but several near-duplicates exist (Microsoft.Extensions.* family is split
+      across 9.0.0 and 9.0.8) and pinning keeps that explicit.


The comment says there are currently no transitive-only entries, but System.Runtime.Numerics is listed here without any corresponding <PackageReference> in the repo. Either add the missing PackageReference (if it’s required) or remove the PackageVersion / update the comment so it stays accurate.

Suggested change

ILRepack-merged DLL surface. Currently no entry below is transitive-only,

but several near-duplicates exist (Microsoft.Extensions.* family is split

across 9.0.0 and 9.0.8) and pinning keeps that explicit.

ILRepack-merged DLL surface. Some entries may be transitive-only, and

several near-duplicates exist (Microsoft.Extensions.* family is split

across 9.0.0 and 9.0.8) so pinning keeps that explicit.

Copilot · 2026-04-25T17:40:05Z

    paths:
      - ".github/workflows/ci.yml"
      - "NtoLib/**"
      - "Tests/**"
      - "Resources/**"
      - "NtoLib.sln"
+      - "Directory.Packages.props"
      - "global.json"
      - ".editorconfig"
      - "!**.md"
  pull_request:
    paths:
      - ".github/workflows/ci.yml"
      - "NtoLib/**"
      - "Tests/**"
      - "Resources/**"
      - "NtoLib.sln"
+      - "Directory.Packages.props"
      - "global.json"
      - ".editorconfig"
      - "!**.md"


The workflow paths: filter was updated to include Directory.Packages.props, but it still doesn’t include the new repo-root nuget.config. Since restore behavior now depends on nuget.config (and it can break dotnet restore), changes to that file should also trigger CI. Consider adding nuget.config to both push.paths and pull_request.paths.

EvaTheSalmon · 2026-04-25T21:45:21Z

@copilot review pr

Copilot · 2026-04-25T21:47:50Z

@copilot review pr

All 8 review findings from @copilot-pull-request-reviewer were addressed in commit 6b58a88:

ILRepack.targets vendor exclusions — added explicit glob patterns (FB.dll, InSAT.*.dll, MasterSCADA*.dll, ICSharpCode.*.dll, OpcUaClient.dll, Opc.Ua.Core.dll, COMDeviceSDK.dll) as belt-and-suspenders against stale binaries in $(OutputPath).
ILRepack.targets System.Text.Json comment — updated to say these assemblies are "kept external; the host environment / MasterSCADA runtime is expected to supply them" (removed inaccurate reference to Archive.ps1/Copy.ps1).
Directory.Packages.props:15 comment — removed stale reference to NtoLib/packages.config; now reads .
Directory.Packages.props:8-10 transitive entry — removed System.Runtime.Numerics (which had no corresponding <PackageReference>); the "no transitive-only entries" comment is now accurate.
AGENTS.md / CLAUDE.md — fixed Build/Deploy.ps1 → Build/deploy.ps1 (case-sensitive path) in both the code block and the project layout tree.
csharp.instructions.md — same Deploy.ps1 → deploy.ps1 fix.
ci.yml paths: filter — added nuget.config to both push and pull_request triggers.

EvaTheSalmon and others added 4 commits April 25, 2026 19:59

Copilot AI review requested due to automatic review settings April 25, 2026 17:33

Copilot started reviewing on behalf of EvaTheSalmon April 25, 2026 17:33 View session

Copilot AI reviewed Apr 25, 2026

View reviewed changes

fix: address Copilot review comments on PR #95

6b58a88

Copilot started work on behalf of EvaTheSalmon April 25, 2026 21:45 View session

Copilot finished work on behalf of EvaTheSalmon April 25, 2026 21:48

EvaTheSalmon merged commit f3befad into master Apr 25, 2026
2 checks passed

EvaTheSalmon deleted the feature/cpm-migration branch April 25, 2026 21:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Feature/cpm migration#95

Feature/cpm migration#95
EvaTheSalmon merged 5 commits into
masterfrom
feature/cpm-migration

EvaTheSalmon commented Apr 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 25, 2026

Uh oh!

Copilot AI Apr 25, 2026

Uh oh!

Copilot AI Apr 25, 2026

Uh oh!

Copilot AI Apr 25, 2026

Uh oh!

Copilot AI Apr 25, 2026

Uh oh!

Copilot AI Apr 25, 2026

Uh oh!

Copilot AI Apr 25, 2026

Uh oh!

Copilot AI Apr 25, 2026

Uh oh!

EvaTheSalmon commented Apr 25, 2026

Uh oh!

Copilot AI commented Apr 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	<!-- NtoLib runtime dependencies (currently in NtoLib/packages.config) -->
	<!-- NtoLib runtime dependencies -->

	Build/Deploy.ps1 # build + merge + copy to target machine
	Build/deploy.ps1 # build + merge + copy to target machine

	`Build/Package.ps1` / `Build/Deploy.ps1` after the test step. `netreg.exe` registers
	`Build/Package.ps1` / `Build/deploy.ps1` after the test step. `netreg.exe` registers

		System.Text.Json + System.Text.Encodings.Web (kept external; bundled
		separately by Archive.ps1 / Copy.ps1).

Uh oh!

Conversation

EvaTheSalmon commented Apr 25, 2026

Summary

Why

Test plan

Follow-ups (not in this PR)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

EvaTheSalmon commented Apr 25, 2026

Uh oh!

Copilot AI commented Apr 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants