-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add location-gated AGENTS.md guidance for key src and test subtrees #55198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
88ead30
6650da0
ffdafca
6af3bde
47b008a
e2a0e02
31eeca6
85c8a0b
0458134
c69ae67
3d0d43b
c590fd6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| # CLI Agent Instructions | ||
|
|
||
| Guidance for changes under `src/Cli`. | ||
|
|
||
| ## Three-project split | ||
|
|
||
| A `dotnet` command or option spans three cooperating projects: | ||
|
|
||
| - **`Microsoft.DotNet.Cli.Definitions`** — the AOT-safe command tree. Shared | ||
| option factories live in `Common/CommonOptions.cs`. | ||
| - **`src/Cli/dotnet`** — the managed implementation: handlers, validation, | ||
| MSBuild/NuGet integration, runtime messages. | ||
| - **`src/Cli/dotnet-aot`** + **`src/Cli/dn`** — the NativeAOT bridge (see | ||
| `src/Cli/dotnet-aot/DESIGN.md`). | ||
|
|
||
| The same definition tree is parsed by both the managed and AOT hosts, so | ||
| parser/option/description changes flow to AOT and `--help` automatically. Keep heavy | ||
| deps out of `Definitions` and behind `#if !CLI_AOT` / `[RequiresDynamicCode]`. | ||
|
|
||
| ## Where things live | ||
|
|
||
| `src/Cli` is a set of projects, not one app. The three above carry commands; the | ||
| rest are supporting libraries: | ||
|
|
||
| | Project | Role | | ||
| |---------|------| | ||
| | `dotnet` | Primary managed executable — every command's handler lives here under `Commands/`. | | ||
| | `Microsoft.DotNet.Cli.Definitions` | AOT-safe command tree (parsed by both hosts). | | ||
| | `dotnet-aot` + `dn` | NativeAOT shared library + native host exe. | | ||
| | `Microsoft.DotNet.Cli.Utils` | MSBuild/NuGet/process/system abstractions used across the CLI. | | ||
| | `Microsoft.DotNet.Cli.CoreUtils` | Low-level version/file/env-variable parsing. | | ||
| | `Microsoft.DotNet.Cli.CommandLine` | Local extensions over `System.CommandLine`. | | ||
| | `Microsoft.DotNet.Configurer` | First-run experience and NuGet/config setup. | | ||
| | `Microsoft.DotNet.InternalAbstractions` | File-system/env abstractions for testability. | | ||
| | `Microsoft.DotNet.FileBasedPrograms` | Support for file-based programs. | | ||
| | `Microsoft.TemplateEngine.Cli` | `dotnet new` integration layer. | | ||
|
|
||
| ### Inside `src/Cli/dotnet` | ||
|
|
||
| - `Program.cs` / `Parser.cs` — entry point and parser construction. | ||
| - `Commands/` — one folder per command (Build, Restore, New, Tool, Workload, …), | ||
| plus `CliCommandStrings.resx` and `xlf/`. | ||
| - `CommandFactory/` — command resolution strategies. | ||
| - `BuildServer/` — MSBuild / VBCSCompiler / Razor build-server providers. | ||
| - `ToolPackage/`, `ToolManifest/`, `ShellShim/`, `NugetPackageDownloader/`, | ||
| `NugetSearch/` — `dotnet tool` install/run plumbing. | ||
|
|
||
| ### Inside `Microsoft.DotNet.Cli.Definitions` | ||
|
|
||
| - `Commands/` — one definition class per command; `DotNetCommandDefinition.cs` is the | ||
| registry that imports them all. | ||
| - `Common/` — shared option/argument factories. | ||
| - `Help/` — help builder and localization. | ||
|
|
||
| ## Verify (approval) snapshot tests | ||
|
|
||
| Many CLI tests use Verify (`[UsesVerify]` / VerifyMSTest): | ||
|
|
||
| - The expected output is checked in as | ||
| `<Test>.<Method>[.<OS>].verified.txt`. | ||
| - On mismatch the runner writes a git-ignored `*.received.txt`. **Never commit | ||
| `*.received.txt`.** | ||
| - When you intentionally change CLI output, promote the new `*.received.txt` over the | ||
| matching `*.verified.txt`. | ||
| - Volatile lines (paths, timings, versions) are scrubbed via | ||
| `settings.ScrubLinesContaining(...)` — scrub rather than hard-code them. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| # Containers Agent Instructions | ||
|
|
||
| Guidance for changes under `src/Containers` — the `dotnet publish` container-image | ||
| feature (`Microsoft.NET.Build.Containers`). | ||
|
|
||
| ## Where things live | ||
|
|
||
| | Path | Role | | ||
| |------|------| | ||
| | `Microsoft.NET.Build.Containers/` | The library + MSBuild tasks: `Tasks/` (task entry points), `Registry/` (OCI registry client), `LocalDaemons/` (Docker/Podman/tarball outputs). | | ||
| | `packaging/` | Wire the tasks into publish. | | ||
| | `containerize/` | Standalone CLI wrapper around the same library. | | ||
|
|
||
| ## Conventions & gotchas | ||
|
|
||
| - **User-facing config is `Container*` MSBuild properties** | ||
| - **Diagnostics use `CONTAINER####` codes** — a self-contained scheme, *not* the | ||
| `NETSDK####` sequence from `src/Tasks`. | ||
| - **Registry behavior is tuned via `DOTNET_CONTAINER_*` env vars**, each with a legacy | ||
| `SDK_CONTAINER_*` alias — keep both when adding one. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: This is not universally true. For example, the push/pull credential variables and @baronfel, what is the expectation for new environment variables? References: push/pull credential variables, insecure registries variable
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 0458134 -- reworded to describe the current state (newer vars like the push/pull credentials and DOTNET_CONTAINER_INSECURE_REGISTRIES are DOTNET_CONTAINER_* only). If the intended convention is to always add an SDK_CONTAINER_* alias for new vars, I'm happy to re-add that as forward guidance (@baronfel). |
||
|
|
||
| ## Tests | ||
|
|
||
| - Unit: `test/Microsoft.NET.Build.Containers.UnitTests` (MSTest) — run everywhere. | ||
| - Integration: `test/Microsoft.NET.Build.Containers.IntegrationTests` **require a | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not all tests in the integration project require a daemon. suggestion: distinguish between integration tests that do and do not require a container runtime instead of describing the entire project as requiring Docker/Podman. References:
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in c69ae67 |
||
| container daemon**; they skip when Docker/Podman is absent, so don't | ||
| rely on them running in every CI leg. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| # dotnet watch Agent Instructions | ||
|
|
||
| Guidance for changes under `src/Dotnet.Watch` (the `dotnet watch` tool and Hot | ||
| Reload). | ||
|
|
||
| ## Where things live | ||
|
|
||
| | Path | Role | | ||
| |---------|------| | ||
| | `dotnet-watch` | The tool executable and CLI surface. Its command/options are defined `CommandLine/DotnetWatchCommandDefinition.cs`. | | ||
|
mthalman marked this conversation as resolved.
Outdated
|
||
| | `Watch` (`Microsoft.DotNet.HotReload.Watch`) | Core watcher library: file-set computation, process launching, Hot Reload, app models. | | ||
| | `DotNetWatchTasks` | MSBuild task bundled into the tool for design-time file collection. | | ||
| | `DotNetDeltaApplier`, `Web.Middleware`, `BrowserRefresh` | Assemblies **injected into the running app** via `DOTNET_STARTUP_HOOKS`. | | ||
| | `HotReloadAgent.*`, `HotReloadClient`, `AspireService` | Shared code consumed via `.projitems`.| | ||
|
|
||
| ## Conventions & gotchas | ||
|
|
||
| - **Shared source via `.projitems`.** Several folders share code through | ||
| `*.projitems` imported into multiple projects (not NuGet packages). Before | ||
| refactoring shared files, check every importer. | ||
| - **`Watch/RuntimeDependencies.props` controls tool output layout** (the | ||
| `hotreload/<tfm>/…` paths). It must stay in sync with `GetStartupHookPath` in | ||
| `Watch/AppModels/HotReloadAppModel.cs` — a mismatch makes the agent silently fail | ||
| to load and breaks tests. | ||
| - **Hot Reload protocol differs per app model.** .NET Core apps use a binary | ||
| named-pipe protocol; Blazor WASM uses JSON over WebSocket. Each `*AppModel` has its | ||
| own `IHotReloadClient`; a new app model needs its own protocol implementation. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: There is no
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 85c8a0b |
||
| - **`CompilationHandler` drives Roslyn** via | ||
| `Microsoft.CodeAnalysis.ExternalAccess.HotReload`; unsupported edits fall | ||
| back to a full rebuild + restart. | ||
|
|
||
| ## Tests | ||
|
|
||
| - `test/dotnet-watch.Tests. Parallelism is **ClassLevel by design** — | ||
|
mthalman marked this conversation as resolved.
Outdated
|
||
| don't switch to method-level; these are heavy process-spawning tests and it causes | ||
| Helix timeouts. | ||
| - `InProcTestWatcher` runs the watcher in-process with a mocked process launcher. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| # Layout Agent Instructions | ||
|
|
||
| Guidance for changes under `src/Layout`. | ||
|
|
||
| `src/Layout` **assembles and packages the shippable SDK**. It consumes | ||
| already-built components (the CLI, templates, SDKs, workload manifests, runtimes) and | ||
| lays them out into the redist directory and OS installers. It rarely implements | ||
| product behavior — most changes here are about *what gets bundled* and *how it's | ||
| packaged*. | ||
|
|
||
| ## Where things live | ||
|
|
||
| | Path | Role | | ||
| |------|------| | ||
| | `redist/` | Composes the SDK layout: `redist.csproj` + the `targets/` that copy components into the redist. Also hosts the `dnx` launcher scripts. | | ||
| | `pkg/{deb,osx,windows}` | Native OS installer authoring (Debian/RPM, macOS `.pkg`, Windows MSI/bundle inputs). | | ||
| | `finalizer/` | Native Windows executable run during MSI/bundle install/uninstall that maintains the SDK installation registry records. | | ||
| | `VS.Redist.Common.*` | Visual Studio redist authoring projects — package SDK components for the VS installer. | | ||
|
|
||
| ### Inside `redist/targets` | ||
|
|
||
| Two families of targets: | ||
|
|
||
| - **`Bundled*.targets` — *what* ships inside the SDK.** Each | ||
| declares the components to bundle as MSBuild items. | ||
| - **`Generate*.targets` — *how* it's laid out and packaged.** | ||
|
|
||
| ## Conventions & invariants | ||
|
|
||
| - **Bundled-component versions flow in from `eng/Version.Details.{xml,props}`** | ||
| (managed by dependency flow / darc). To bundle or bump a component, set its version | ||
| there and reference the generated `$(<Name>PackageVersion)` property from the | ||
| matching `Bundled*.targets` — **never hardcode a version** in a Layout target. (For | ||
| example, a bundled template is a `<BundledTemplate Include="..." | ||
| PackageVersion="$(...)"/>` item whose version is defined in `Version.Details`.) | ||
| - Producing the laid-out SDK requires the **full repo build** (so the components exist | ||
| to copy), not just this project — see the root build/dogfood instructions. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| # NetAnalyzers Agent Instructions | ||
|
|
||
| Guidance for changes under `src/Microsoft.CodeAnalysis.NetAnalyzers` — the .NET code | ||
| analyzers (the `CA####` rules). | ||
|
|
||
| ## Where things live | ||
|
|
||
| | Path | Role | | ||
| |------|------| | ||
| | `src/Microsoft.CodeAnalysis.NetAnalyzers` (+ `CSharp`, `VisualBasic`) | The analyzer assemblies. Rules live under here grouped into `Microsoft.CodeQuality.Analyzers`, `Microsoft.NetCore.Analyzers`, `Microsoft.NetFramework.Analyzers`. | | ||
| | `src/Utilities/`| Shared analyzer/flow-analysis helpers linked into the analyzers. | | ||
| | `tests/` | Tests and the verifier harness. | | ||
| | `tools/GenerateDocumentationAndConfigFiles` | Generates rule docs, rulesets, editorconfig, and SARIF. | | ||
|
|
||
| ## Conventions & gotchas | ||
|
|
||
| - **Release tracking is mandatory (not `PublicAPI.txt`).** Any new/changed/removed | ||
| diagnostic ID **must** be recorded in the project's `AnalyzerReleases.Unshipped.md` | ||
| (it moves to `AnalyzerReleases.Shipped.md` at release). The `RS2000`/`RS2001` | ||
| analyzers fail the build if you skip this. | ||
| - **Analyzer file pattern**: `XxxAnalyzer.cs` + a **co-located** `Xxx.Fixer.cs` | ||
| + a test under `tests/…` mirroring theanalyzer's folder. | ||
|
mthalman marked this conversation as resolved.
Outdated
|
||
| - **Diagnostic IDs are allocated centrally** in | ||
| `src/Utilities/Compiler/DiagnosticCategoryAndIdRanges.txt` — take the next free | ||
| `CA####` in the category's range and update that file. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| # Resolvers Agent Instructions | ||
|
|
||
| Guidance for changes under `src/Resolvers` — the MSBuild **SDK resolvers** and the | ||
| workload manifest reader. | ||
|
|
||
| ## Where things live | ||
|
|
||
| | Project | Role | | ||
| |---------|------| | ||
| | `Microsoft.DotNet.MSBuildSdkResolver` | The `SdkResolver` MSBuild loads to find the .NET SDK. | | ||
| | `Microsoft.DotNet.SdkResolver` | Core resolution logic (`NETCoreSdkResolver`). | | ||
| | `Microsoft.DotNet.NativeWrapper` | P/Invoke wrapper over **hostfxr**. | | ||
| | `Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver` | The `SdkResolver` for workload SDKs. | | ||
| | `Microsoft.NET.Sdk.WorkloadManifestReader` | Parses/indexes workload manifest JSON; shared by the resolvers **and** the CLI. | | ||
|
|
||
| ## Conventions & gotchas | ||
|
|
||
| - **Shared code is *linked*, not referenced.** `MSBuildSdkResolver` pulls the | ||
| `NativeWrapper`, `SdkResolver`, and `WorkloadManifestReader` sources in via links | ||
| and compiles them **into itself** (to minimize DLLs loaded into MSBuild). | ||
| - **Two target frameworks, two hosts.** net472 loads into VS/`MSBuild.exe`; | ||
| `$(SdkTargetFramework)` loads into the .NET MSBuild. The **resolver projects' | ||
| (`MSBuildSdkResolver`, `SdkResolver`) net472 build is gated to `DotNetBuildPass == 2`** | ||
| (it depends on other verticals). Exercise both paths. | ||
| - **hostfxr interop is a runtime contract.** `NativeWrapper` uses `LibraryImport` | ||
| under `#if NET` and `DllImport` with manual x86/x64/arm64 preload on net472. | ||
| Changing P/Invoke signatures must stay back-compatible and coordinate with | ||
| dotnet/runtime. | ||
| - **Dependencies are frozen to MSBuild's binding redirects.** `AssemblyVersion` is | ||
| pinned and a `VerifyDependencies` build step checks the exact expected package | ||
| versions — **coordinate with the MSBuild team before bumping/adding any dependency**. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| # Tasks Agent Instructions | ||
|
|
||
| Guidance for changes under `src/Tasks` (MSBuild tasks, targets, and SDK build messages). | ||
|
|
||
| ## Where things live | ||
|
|
||
| Four projects, all multi-targeted (`net472` + current .NET) so tasks load in both | ||
| full-framework and .NET Core MSBuild: | ||
|
|
||
| | Project | Role | | ||
| |---------|------| | ||
| | `Microsoft.NET.Build.Tasks` | The core `Microsoft.NET.Sdk` task assembly **and** the targets that compose `dotnet build`. Ships in the SDK. | | ||
| | `Common` | Common shared source **linked into** both task assemblies — not its own package. | | ||
| | `Microsoft.NET.Build.Extensions.Tasks` | Tasks/targets for desktop/.NET Framework projects built **outside** the SDK. | | ||
| | `sdk-tasks` | Build-time-only tasks for **this repo's own** build/packaging — never shipped to users. | | ||
|
mthalman marked this conversation as resolved.
Outdated
|
||
|
|
||
| ### Inside `Microsoft.NET.Build.Tasks` | ||
|
|
||
| - Root `*.cs` — the MSBuild `Task` classes. | ||
| - `targets/` — the shipping `.targets`/`.props` that drive a build; | ||
| `Microsoft.NET.Sdk.props` / `Microsoft.NET.Sdk.targets` are the top-level entry | ||
| points, and `Microsoft.NET.Sdk.Common.targets` registers the diagnostic tasks. | ||
| - `sdk/` — `Sdk.props` / `Sdk.targets`, the entry points when the SDK is referenced | ||
| via the `<Sdk>` attribute. | ||
| - `FrameworkPackages/` — per-TFM runtime framework version data. | ||
|
|
||
| ## Build diagnostics (NETSDK errors / warnings / info) | ||
|
|
||
| These are conventions that aren't visible from the code alone — getting them wrong | ||
| breaks localization or silently collides with another PR. | ||
|
|
||
| - **Diagnostics are raised from targets, never as literal text.** Use | ||
| `<NETSdkError/>`, `<NETSdkWarning/>`, or `<NETSdkInformation/>` (registered in | ||
| `Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.Common.targets`) and reference | ||
| the message **by `ResourceName`** from the shared `Common/Resources/Strings.resx`: | ||
|
|
||
| ```xml | ||
| <NETSdkError Condition="'$(TargetFramework)' == ''" | ||
| ResourceName="TargetFrameworkEmpty" | ||
| FormatArguments="$([MSBuild]::Escape('$(SomeValue)'))" /> | ||
| ``` | ||
|
|
||
| - **The NETSDK code lives in the value, not in metadata.** In `Strings.resx` the | ||
| `<value>` begins with the code (`NETSDK1238: The property '{0}' ...`), and the | ||
| `<comment>` carries the localization directives — `{StrBegins="NETSDK1238: "}` plus | ||
| `{Locked="{0}"}` per placeholder (or `{Locked="--option"}` for literal flags). | ||
| - **Message entries are append-only.** Add new strings at the **end** of the resx with | ||
| the **next available** NETSDK number, and update the trailing | ||
| `<!-- The latest message added is <Name>. -->` guard comment — it exists so two PRs | ||
| adding a message conflict in git instead of silently reusing a code. (The root | ||
| instructions carry the one-line version of this rule.) | ||
| - **`.xlf` files are generated**, never hand-edited — regenerate via `/t:UpdateXlf`. | ||
|
|
||
| Diagnostics are covered by tests under `test/Microsoft.NET.Build.Tests/`. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| # Test Agent Instructions | ||
|
|
||
| Guidance for changes under `test/`. | ||
|
|
||
| ## Where things live | ||
|
|
||
| - **`Microsoft.NET.TestFramework.MSTest`** is the shared harness. Common namespaces are | ||
| exposed as global usings from `test/Directory.Build.targets`. | ||
|
mthalman marked this conversation as resolved.
Outdated
|
||
| - Test projects are grouped by area. | ||
| - **`test/TestAssets/`** holds inputs, not tests. | ||
|
|
||
| ## Conventions & gotchas | ||
|
|
||
| - **Don't raise parallelism.** MSTest is repo-defaulted to `None` in | ||
| `test/Directory.Build.props` because of concurrency flakiness; a few projects opt | ||
| into `ClassLevel`. Cranking it up causes Helix over-subscription/timeouts. | ||
| - **Skips must point to a tracking issue URL** — `[Ignore("https://github.com/dotnet/sdk/issues/N")]`. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: does this work for both MSTest and Xunit projects? Or only one?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is MSTest-specific ( |
||
| - **Verify (approval) snapshots**: `*.verified.*` is checked in; the runner writes a | ||
| git-ignored `*.received.*` on mismatch — promote received → verified when you change | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: References: root ignore rule, MTP snapshot directory
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 31eeca6 |
||
| output intentionally, and never commit `*.received.*`. (See `src/Cli/AGENTS.md` for | ||
| the CLI-specific detail.) | ||
| - Helix work-item partitioning is driven by `test/UnitTests.proj` (per-project method | ||
| limits/multipliers) — relevant if a project's tests are unusually slow or numerous. | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#if !CLI_AOTand[RequiresDynamicCode]are not interchangeable ways to keep dependencies out of the AOT build. The bridge selects shared source explicitly and usesCLI_AOTfor conditional paths;[RequiresDynamicCode]only annotates a managed API and does not exclude its dependencies from compilation.References: AOT source sharing and conditional compilation,
PackCommandannotationThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 3d0d43b