Add AGENTS.md and per-TFM MSBuild loader skills#381
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Test class is QueryInstancesTests (plural); the singular form did not substring-match in dotnet test --filter. - Unregister() is a documented no-op kept only for back-compat; calling the section 'register/unregister' suggested behavior that does not exist. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The AssemblyLoadContext-vs-AppDomain handler split, the ResolveDotnetPathCandidates probe order, the hostfxr API names, the VS Setup COM details, and the net46-vs-netcore gating for VisualStudioLocationHelper are already covered in the two loader skill files. Keeping them in AGENTS.md as well burns the always-loaded context budget for work that only touches one TFM. Replace the per-TFM lines with pointers to the relevant skill while keeping the multi-targeting heads-up (TFM list + conditional compilation constants) so agents still know to check both forks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| - No `AssemblyLoadContext`, `hostfxr`, or `.NET SDK` discovery — those are | ||
| `#if NETCOREAPP` paths (see the `msbuild-loader-netcore` skill). | ||
| - `ApplyDotNetSdkEnvironmentVariables(...)` exists in the file, but both | ||
| `RegisterMSBuildPath(...)` call sites that invoke it are `#if NETCOREAPP`. Do |
There was a problem hiding this comment.
Actually we have three ApplyDotNetSdkEnvironmentVariables calls, and this third one not mentioned here is not if-gated but is guarded at runtime by DiscoveryType == DotNetSdk
| --- | ||
|
|
||
| # .NET / .NET Core (`net8.0` / `NETCOREAPP`) MSBuild loader | ||
|
|
There was a problem hiding this comment.
Would be great to have here non-window specific logic listed, how the flow differs for windows vs other OS
| @@ -0,0 +1,31 @@ | |||
| # Microsoft.Build.Locator — Copilot instructions | |||
There was a problem hiding this comment.
Is an agent smart enough to know what if we change logic described in skills agent should change skill as well?
|
|
||
| ## SDK discovery | ||
| - Lives in `DotNetSdkLocationHelper`; instances are | ||
| `VisualStudioInstance(name: ".NET Core SDK", ..., DiscoveryType.DotNetSdk)`. |
There was a problem hiding this comment.
I am confused, why this is called VisualStudioInstance but we have here DiscoveryType.DotNetSdk?
Does VisualStudioInstance really represent installed visual studio or installed msbuild?
| - `VisualStudioLocationHelper.cs` — Visual Studio Setup (COM) discovery; see the `msbuild-loader-netframework` skill. | ||
| - `VisualStudioInstance.cs`, `VisualStudioInstanceQueryOptions.cs`, `DiscoveryType.cs` — result/option types. | ||
| - `Utils/SemanticVersion*.cs`, `VersionComparer.cs` — internal SemVer parse/compare to order instances. | ||
| - Props/targets shipped from `src/MSBuildLocator/build/` (packed to `build/` + `buildTransitive/`). `EnsureMSBuildAssembliesNotCopied` emits error **MSBL001** when a consumer references `Microsoft.Build*` packages without `PrivateAssets="all"` / `ExcludeAssets="runtime"` (copying those assemblies locally breaks redirection). Keep the target's package list in sync with MSBuild's layout. |
There was a problem hiding this comment.
This point seems a little confusing if a human will read this. (at least for me)
I suggest to write it in simple words and into two different bullet points:
- The core rule of MSBuildLocator: never ship MSBuild's DLLs with the app.
The app should ship without Microsoft.Build.dll . At runtime, Locator's registered handler redirects those loads to the real MSBuild install (VS or SDK).
• Why a local copy breaks it. The resolve handler only fires on a miss — when the loader can't find an assembly. If a copy of Microsoft.Build.dll sits in the bin /output folder, the loader finds that one and the handler never runs.
• MSBL001 safeguard. The Locator NuGet package ships an MSBuild .targets file that runs inside the consuming build. The EnsureMSBuildAssembliesNotCopied target scans the resolved packages for any MSBuild package about to be copied locally. If it finds one, the build fails with error MSBL001, which names the offending package and asks for two attributes.
• The fix — two attributes, on the flagged in the consuming project's .csproj . Add ExcludeAssets="runtime" (stops the DLL from being copied to output) and PrivateAssets="all" (keeps the dependency from flowing to downstream consumers):
- Keep the target's package list in sync.
The target checks a hardcoded list of package IDs ( Microsoft.Build , .Framework , .Utilities.Core , .Tasks.Core , …) — see Microsoft.Build.Locator.targets#L3-L14 . If MSBuild ever ships a new redistributable assembly that also shouldn't be copied, it must be added here or the guard won't catch it.
| `Path.Combine(msbuildPath, assemblyName.Name + ".dll")` and load with | ||
| `Assembly.LoadFrom(targetAssembly)`. | ||
|
|
||
| ## SDK environment variables (NETCOREAPP only) |
There was a problem hiding this comment.
Would be great to have here some context why this env variables are important to msbuildLocator
| it exists. | ||
|
|
||
| ## Pre-17.1 MSBuild.exe bitness fix | ||
| - net46 reads `FileVersionInfo` from the first registered path containing |
3 participants
Why
MSBuildLocator's implementation forks heavily between
net46(Visual Studio) andnet8.0(.NET SDK). This PR adds Copilot agent instructions and splits the documentation into an always-loaded base file and two on-demand skills to prevent context bloat and confusion between the frameworks.What's added
AGENTS.md: Core repository context, build/test commands, conventions, and the cross-TFM register-before-load contract.msbuild-loader-netcoreskill:net8.0/NETCOREAPPspecifics (AssemblyLoadContext, hostfxr SDK discovery, probe order, SDK environment variables).msbuild-loader-netframeworkskill:net46specifics (AppDomain.AssemblyResolve, Developer Console / VS Setup COM discovery, pre-17.1 MSBuild.exe bitness fix).Notes
AGENTS.mdpretty manually but the skills are generated.