Conversation
|
@Wtiben Before I review this in the context of moon, is it possible to create a separate proto tool so that .NET can be installed within proto as well? |
|
@milesj I'll look into it when i have time |
aa9be30 to
f6199a5
Compare
There was a problem hiding this comment.
@Wtiben Thanks for working on proto tool! Just left 1 comment really, the rest looks good. I'll take a look at the toolchain portion in a bit.
Also this is just for core right? Not runtime? I don't know .NET. I'm assuming this is a JDK vs JRE kind of situation? Do we need to support both?
920af85 to
03419fe
Compare
|
@milesj Thanks for taking a look. Yes, dotnet SDK only, it bundles the runtime. .NET splits the runtime side into a few separate downloads (base runtime, ASP.NET Core, and Windows Desktop on Windows), so it isn't a single artifact. For moon the SDK is probably the only useful one, since you can't build with a runtime alone. Runtime only installs would mainly help proto users running prebuilt apps. The release metadata lists those downloads as well, so adding them later isn't much work if there's demand for it. I'm still testing/ironing out some issues regarding the tool and toolchain, I'll convert this PR into draft temporarily. I'll let you know when both are ready. On structure: I can split the tool and toolchain into 2 PR's or keep it as one. Just let me know which one you prefer |
0f5d675 to
faa69d7
Compare
|
@Wtiben Is the toolchain code ready for review? |
|
@Wtiben Bump |
3f9a77b to
f6a5322
Compare
|
@milesj Sorry for the delay, i've been busy. Yes, ready for review. |
| mod dotnet_tool { | ||
| use super::*; | ||
| use ::dotnet_tool::DotnetToolConfig; | ||
|
|
There was a problem hiding this comment.
You need to call generate_download_install_tests!() to verify it actually installs.
| @@ -0,0 +1,342 @@ | |||
| //! Which task ids are already claimed by moon's inherited task files. | |||
There was a problem hiding this comment.
IMO this kind of logic should not exist in the plugin. Your duplicating core moon logic that can easily drift.
| "Provides .NET SDK project-graph extraction, dependency install (dotnet restore), and Docker support for SDK-style C#/F#/VB projects.".into(), | ||
| ), | ||
| plugin_version: env!("CARGO_PKG_VERSION").into(), | ||
| language: Some(LanguageType::CSharp), |
| /// Reading the file is the only way to know at graph-build time, and it decides | ||
| /// whether an unresolvable SDK is a terminal misconfiguration or simply an SDK | ||
| /// that has not been installed yet. | ||
| pub fn sdk_install_configured(workspace_root: &VirtualPath) -> bool { |
| } | ||
|
|
||
| #[plugin_fn] | ||
| pub fn hash_task_contents( |
There was a problem hiding this comment.
This seems very heavy, as this runs for every task. Especially the lockfile part.
Can you explain what this is trying to achieve exactly.
|
|
||
| // Degrade silently like hash_task_contents: a missing dotnet must not | ||
| // fail moon's install fingerprinting. | ||
| if !command_exists(env, "dotnet") { |
There was a problem hiding this comment.
How often would this actually happen? If you have the dotnet toolchain installed, then this bin should exist.
| let manifest_dir = input | ||
| .path | ||
| .parent() | ||
| .unwrap_or_else(|| input.context.workspace_root.clone()); |
There was a problem hiding this comment.
This else isn't entirely accurate.
| // the repository root, no per-project lock files. That means nothing is ever | ||
| // restored, and the inferred `build --no-restore` fails on the missing assets | ||
| // file. | ||
| output.members = Some(vec!["**".into()]); |
| }; | ||
|
|
||
| for (id, source) in &input.project_sources { | ||
| let project_root = input.context.workspace_root.join(source); |
There was a problem hiding this comment.
input.context.get_project_root_from_source
| // machine. Erroring here would fail the whole-workspace graph, for every | ||
| // toolchain, before moon ever gets to install the SDK it was told to | ||
| // install. | ||
| if !command_exists(get_host_environment()?, "dotnet") { |
There was a problem hiding this comment.
Same here, shouldn't need this.
| language: Some(LanguageType::CSharp), | ||
| exe_names: vec!["dotnet".into()], | ||
| config_file_globs: vec![ | ||
| "*.{csproj,fsproj,vbproj}".into(), |
There was a problem hiding this comment.
Simple .proj extension shall also be supported, which is a feature of dotnet
| use starbase_utils::fs; | ||
|
|
||
| /// Project file extensions this toolchain understands. | ||
| pub const PROJECT_EXTENSIONS: &[&str] = &["csproj", "fsproj", "vbproj"]; |
There was a problem hiding this comment.
Simple .proj extension shall also be supported, which is a feature of dotnet
Installs .NET SDKs through proto, so the SDK can be managed the same way every other language runtime in this repository is. Versions come from Microsoft's release metadata (`releases-index.json`, then each channel's `releases.json`) and not from git tags. Tags are what both existing third-party .NET proto plugins resolve from, and they are wrong in two ways: published SDKs have no tag at all (8.0.125, 8.0.201 and 9.0.101 among others), and some tags that do exist 404 on download. The metadata also lists every feature band under `sdks[]`, which the headline `sdk` field omits, and carries the archive URL and its SHA512 per platform, so a download and its verification need one lookup and no checksum sidecar request. Loading versions costs one request per channel, EOL ones included, because an old SDK is still a legitimate thing to pin. proto caches the result, so it is paid once per cache expiry rather than per command. A channel that cannot be fetched is skipped with a warning rather than failing the whole listing, since most of them are EOL and one missing `releases.json` must not take out the channels people actually install from. Only an entirely empty result is an error, which means the metadata host itself moved. An SDK archive is self-contained: muxer, `host/fxr`, matching `shared/` runtimes, one `sdk/<band>`, packs and templates. It unpacks into proto's per-version product directory and works there, so no `archive_prefix` is stripped and installs stay isolated. Confirmed against an old release, a musl build and a preview. `no_bin` is set on the executable, and that is load-bearing: the muxer resolves `host/fxr` and `shared/` relative to its own path on disk, so it only runs from inside its install directory. proto's `bin` entries are symlinks, and a symlinked muxer fails with "host/fxr does not exist". Shims are fine, since they execute the real path. There is a test guarding this. `minimum_proto_version` is 0.60, not 0.61. moon 2.5 embeds proto_core 0.60.4 and runs proto 0.60.2, and this plugin is loaded as a moon toolchain as well as a proto tool, so declaring 0.61 (as the proto-only swift tool does) makes moon refuse it outright. Nothing here needs a 0.61 API. Two .NET specifics worth knowing: `global.json` is parsed structurally rather than read as a bare version, so `sdk.rollForward` is honored alongside `sdk.version`. Feature bands are the hundreds component of the patch, and they are parallel product lines rather than a sequence: 8.0.404 and 8.0.130 are both current and neither is newer. A band needs two bounds rather than one, which a compound requirement expresses fine, so every mode maps exactly: | `rollForward` | Requirement | | ----------------------------- | ----------------------- | | `disable` | `8.0.404` | | `patch`, `latestPatch`, unset | `>=8.0.404 && <8.0.500` | | `feature`, `latestFeature` | `~8.0.404` | | `minor`, `latestMinor` | `^8.0.404` | | `major`, `latestMajor` | `>=8.0.404` | The property that matters across all of them is that `rollForward` only ever rolls *forward*, so the pinned patch survives into the spec and no mode can resolve an SDK older than the pin. Versions predating feature bands (a two-digit patch, as in 2.1.14) fall back to the major.minor. Unrecognized modes are treated as the default, since new ones get added over time. `allowPrerelease` is not expressed: ranges exclude pre-releases by semver convention and a pinned pre-release resolves to itself, which is close enough to the host's own behavior. The compound form is covered by resolution tests against the live metadata, including a case in a lower band, since an upper bound that was dropped would resolve to the channel's latest instead. `pin_version`/`unpin_version` write `sdk.version` into `global.json`, the counterpart to already reading it for detection. Following bun, node and node-depman, an existing file is edited and never created: `global.json` also carries `msbuild-sdks` and `projects`, so conjuring one would be writing a config the repository never asked for. Unpinning removes only `version`, and drops the `sdk` object solely when nothing else is left in it, because `rollForward` and `allowPrerelease` are not ours to discard. `activate_environment` exports `DOTNET_ROOT`, mirroring how the java plugin exports `JAVA_HOME`. It is for `proto activate` alone; nothing in moon 2.5 references it, and the muxer itself does not need it since it resolves `host/fxr` and `shared/` from its own path. Everything else does: MSBuild, the SDK resolvers, and any `dotnet` reached other than through this install. `resolve_version` answers aliases and nothing else. Every alias this plugin publishes names a channel, and the index already carries each channel's `latest-sdk`, so `lts` costs the one index request instead of the fourteen a full listing would. Returning a version is what makes proto skip `load_versions` entirely. A channel counts as pre-release in two phases, not one. `preview` is the obvious one; `go-live` is the release-candidate phase, which carries a production-use licence but still ships an `-rc` SDK as its `latest-sdk`. Reading `go-live` as generally available would hand an RC to anyone asking for `latest`, and would drop the `preview` alias the moment a channel moved from one phase to the other, which is what .NET 11 did. Exact versions are deliberately left alone there. proto already short-circuits them itself on the paths that matter, and where it does not — `proto install` — it is asking for the version to be validated against the real list before anything is downloaded, which is not worth overriding to save requests on a command people run rarely. What `resolve_version` still will not do is answer a requirement, since that has to be matched against the list. It only warns about the one that cannot say what it means: a channel-only pin such as `8.0` selects the highest band when the repository may want a lower one. No metadata lookup for that warning, since naming the exact bands is not worth a round trip. The tool is registered as unstable, as every recently added tool here is, and starts at 0.1.0 to match them. The release metadata writes `null` for values it has no entry for, and `#[serde(default)]` only covers a *missing* key, so every deserialized field carries a null-tolerant deserializer. An empty checksum warns rather than silently installing unverified, since it is the only integrity check in the flow; `dist-url` drops verification by design, because the metadata's hashes describe Microsoft's archives and not a mirror's.
f6a5322 to
132c3d5
Compare
Tiers 1 through 3 for SDK-style C#, F# and VB projects. The graph comes from real MSBuild evaluation rather than parsing project XML. Reading the XML gets a partial and sometimes wrong answer: references that arrive through an implicit `Directory.Build.*` import are invisible, Central Package Management moves versions out of the project file entirely, and a reference written as `$(SolutionDir)Common/Foo/Foo.csproj` is not a path that resolves to anything until MSBuild evaluates it. So the plugin asks MSBuild instead, through `-getProperty`/`-getItem` JSON output, and everything resolves the way the SDK resolves it. Evaluation is a process start, so the whole workspace is evaluated in one batched invocation: a generated traversal project fans out to every project with parallel in-process nodes. Results are cached under `.moon/cache` for `hash_task_contents` to reuse, and anything missing from the batch falls back to per-project evaluation, so one unloadable csproj cannot sink the graph. - tier1: `register_toolchain`, `define_toolchain_config`, `initialize_toolchain`, `define_docker_metadata`, `prune_docker`. - tier2: `locate_dependencies_root`, `install_dependencies` (`dotnet restore`, `--locked-mode` when a NuGet lock file exists), `setup_environment` (`dotnet tool restore` for repo-local tools), `extend_task_command` (DOTNET_ROOT and PATH), `extend_project_graph` (dependency and task inference, plus `AssemblyName` as a project alias), `parse_lock`, `parse_manifest`, `hash_task_contents`. - tier3: re-exports the proto tool, which is how every paired plugin here is wired. The toolchain takes the tool as a path dependency and chains its `wasm` feature, so one wasm binary exports both `register_toolchain` and the proto surface, and moon drives installation itself. This is why none of rust, go or node define `setup_toolchain` either. Task inference gives every project a `build`, test projects a `test`, and executables `run` and `publish`. `build` passes `--no-dependencies` and depends on the parent scope's `build`, so moon owns the graph rather than MSBuild. Anything you define wins: a task whose id matches in a project's `moon.yml` replaces the inferred one, and ids coming from an applicable inherited task file are skipped. That last rule is load-bearing and fails toward safety, because moon *merges* an inferred task over an inherited one by appending args, which yields a broken command line rather than an override. A file whose `inheritedBy` this plugin cannot model therefore still has its ids reserved. Discovery is moon's own: since 2.5 a project glob may end in a file name, so a single `**/*.csproj` glob is the whole setup and no `moon.yml` is needed. Note that moon derives each id from the leaf directory name, so repositories with repeated directory names (sample and template trees) need a narrower glob or an `id:`-only `moon.yml` there. A directory holding several project files can only own one task per id, and the extras are reported rather than silently dropped. SDK discovery has to serve two callers that must agree, or the graph gets evaluated by one SDK while tasks run under another. Order is an existing `DOTNET_ROOT`, then proto's own installs newest first, then `~/.dotnet` when it holds a real SDK layout. The proto step exists because graph building cannot see the task PATH: moon puts the resolved tool directory on a task's PATH, but `extend_project_graph` shells out to `dotnet` from the host PATH and is handed no tool directory, so without it an SDK proto installed is invisible to inference. proto's store is reachable because moon's plugin registry adds proto's virtual paths to every plugin, and its inventory layout is fixed by proto core rather than chosen by a plugin. Going through the virtual mapping rather than a hardcoded real path is what makes this follow a relocated `PROTO_HOME`. Both discovered candidates are held to the same rule: a root counts only when it holds the host executable and an SDK satisfying the workspace's `global.json` pin. The pin is enforced whenever something else can take the rejected candidate's place, which is either a later candidate or a `dotnet` on PATH. Version ordering is numeric, and prefers a release over its own pre-release, because sorting directory names as strings ranks 8.0.9 above 8.0.10 and leaves a release tied with its own release candidate. Resolution is deliberately not memoized in a plugin var. The answer is not stable for the lifetime of the process: the project graph is built before `SetupToolchain`, so graph building legitimately resolves one root and proto then installs the SDK that should outrank it. `global.json` is parsed by both crates on purpose. The proto plugin needs an unresolved version spec to resolve against; the toolchain needs the requirement itself to judge SDKs already on disk and to detect the test runner. Merging them would couple test-runner detection to proto's version model. `activate_environment` is for `proto activate` alone. Nothing in moon 2.5 references it, so moon-side DOTNET_ROOT injection stays the toolchain's job via `extend_task_command`. MSBuild returns property values exactly as composed, without normalizing them, so a `Directory.Build.props` composing a parent-relative artifacts path hands back a literal `..`. moon rejects any path containing `..`, so output paths are resolved lexically before use; one that genuinely escapes the project disables caching rather than failing inference for the whole project. Project references map onto moon ids by exact real path first, then by the longest matching workspace-relative suffix. Longest, not first: one key can end with several indexed suffixes, and the shorter one belongs to a different project, so first-in-map-order is a wrong edge. That fallback matches on the tail alone and is deliberately not anchored to the workspace root. The key comes from MSBuild and the index from the paths the host hands the plugin, and the two need not agree lexically — a Windows runner whose temp directory prints in its long form against an index built from the short form is routine. Requiring the key to start with the workspace root makes every lookup there fail, which takes out every dependency edge on that platform. The price of staying loose is that a reference to a project outside the workspace can match a same-named project inside it at the same relative depth; that is the cheaper of the two failures, and the matching logic is unit tested for both. Verified against six public repositories on moon 2.5.2, chosen to spread across sizes and configuration styles. Between them they cover Central Package Management, both test runners in use today (Microsoft.Testing.Platform and classic VSTest, whose `dotnet test` command lines are mutually incompatible), multi-targeted projects, custom project SDKs, and `global.json` pins across every roll-forward mode. Graph built clean on all six, with the moon project count matching the project files on disk: | Repository | Projects | Edges | Tasks | Cold | Warm | | --- | --- | --- | --- | --- | --- | | serilog/serilog | 6 | 6 | 6 build, 3 test, 1 run, 1 publish | 5s | 1s | | dotnet/eShop | 28 | 55 | 28 build, 9 test, 12 run, 10 publish | 5s | 1s | | jellyfin/jellyfin | 42 | 134 | 42 build, 16 test, 3 run, 3 publish | 6s | 1s | | OrchardCMS/OrchardCore | 244 | 1517 | 244 build, 4 test, 8 run, 3 publish | 11s | 1s | | ThreeMammals/Ocelot | 21 | 30 | 21 build, 3 test, 15 run | 5s | 1s | | abpframework/abp | 560 | 1633 | 560 build, 149 test, 26 run, 26 publish | 26s | 1s | Ocelot needed eight `id:`-only `moon.yml` files for its samples collisions; abp was globbed at its framework and modules directories rather than give 80 template projects id overrides. The other four ran from a bare glob with no configuration at all. Building a project ran for real in serilog and hit the cache on a second run, and touching one source file marks all 6 projects affected through deep downstream traversal on inferred edges alone. Known limits: multi-targeted projects are evaluated as the outer build across all frameworks, where MSBuild leaves the current framework empty, so anything gated on one specific framework is invisible to inference (unconditional references and packages resolve fine). SDK-style projects only, not the pre-2017 format. The integration tests evaluate their fixtures with a real `dotnet msbuild`, since command execution is not mocked in the sandbox, so this adds `actions/setup-dotnet` to CI. SDK 8 or newer is required, since that is where MSBuild gained the JSON output this relies on.
132c3d5 to
593a1bf
Compare
I saw moonrepo/moon#2447 and your note there that someone could contribute a .NET toolchain to this repo, so I put one together. Before the details: is this something you'd want in-tree? Happy to rework whatever you'd like, and fine either way if you'd rather it stayed outside.
I needed this for a .NET monorepo at work that shares a moon workspace with a pnpm frontend. I've tested it there and against the six public repos further down.
Some .NET background
You mentioned in #2447 that you're not a C#/.NET dev, so here's the bit the design decisions hang off. Skip if you already know it.
.csprojfile (or.fsproj,.vbproj). The filename is arbitrary and it usually sits one or two directories below anything you'd glob for.Directory.Build.propsandDirectory.Build.targetsare implicitly imported into every project underneath the directory they live in. So a project file on its own doesn't tell you what that project references; the imports do too.Directory.Packages.propsis Central Package Management: package versions are declared centrally and project files then reference packages without a version. Similar idea to a pnpm catalog.global.jsonpins which SDK version a directory tree must build with, a bit like.prototoolsbut for the SDK, and it can also select the test runner.<TargetFrameworks>net8.0;net9.0</TargetFrameworks>).$(SolutionDir)Common\Foo\Foo.csproj. The literal text in the file isn't a path you can resolve without evaluating it.The repo I built this for has all six at once, which is where most of the design pressure came from.
Why MSBuild evaluation instead of parsing XML
So reading the project XML gets you a partial and sometimes wrong answer. You miss references that came from an import, you miss which packages a project actually has under Central Package Management, and property-based references come out as literal
$(SolutionDir)...strings that match nothing.The plugin asks MSBuild instead, through
-getPropertyand-getItem, which emit a project's evaluated properties and items as JSON. Everything resolves the way the SDK resolves it, and there's no XML parser in here to keep in sync with MSBuild.The downside is that every evaluation is a process start, which isn't cheap. So the whole workspace is evaluated in one batched invocation instead, a generated traversal project that fans out to every project with parallel in-process MSBuild nodes. 238 projects takes 18 seconds cold. The results are cached under
.moon/cachesohash_task_contentsreuses them rather than evaluating again. Anything missing from the batch falls back to evaluating on its own, so one unloadable csproj can't sink the graph.What it does
Tiers 1 through 3, for SDK-style projects.
register_toolchain(the file types above),define_toolchain_config,initialize_toolchain,define_docker_metadata,prune_docker.locate_dependencies_root,install_dependencies(dotnet restore, with--locked-modewhen a NuGet lock file exists),setup_environment(dotnet tool restorefor repo-local CLI tools),extend_task_command(DOTNET_ROOT and PATH),extend_project_graph(dependency and task inference, plusAssemblyNameas a project alias),parse_lock,parse_manifest,hash_task_contents.setup_toolchain, installing the SDK via the official dotnet-install scripts whenversionis set.Task inference gives every project a
build, test projects atest, and executablesrunandpublish.buildpasses--no-dependenciesand depends on^:build, so moon owns the graph instead of MSBuild. Anything you define wins: a task with the same id in a project'smoon.ymlreplaces the inferred one, and ids coming from an applicable inherited task file are skipped entirely.Repos I tested against
All large public .NET codebases, picked to spread across sizes and configuration styles: serilog is the de facto .NET logging library, Ocelot an API gateway, jellyfin a media server, OrchardCore a CMS, abp an application framework, and dotnet/eShop is Microsoft's own reference application. I cloned each one unmodified and generated a project map for it. The graph built clean on all six, with the moon project count matching the project files on disk.
Between them they cover Central Package Management, both test runners in use today (Microsoft.Testing.Platform and classic VSTest, whose
dotnet testcommand lines are mutually incompatible), multi-targeted projects, custom project SDKs, andglobal.jsonpins across every roll-forward mode.moon run <project>:buildran for real in serilog, eShop, jellyfin and OrchardCore, and hit the cache on a second run. In serilog, touching one file undersrc/Serilogmarks all 6 projects affected through--downstream deepoff inferred edges alone. That repo has nomoon.ymland nodependsOnanywhere.Tests
109, one ignored because it downloads a full SDK over the network. The command to run it is in a comment above it.
The integration tests evaluate their fixtures with a real
dotnet msbuild, sinceexec_commandisn't mocked in the sandbox. That's the one new CI requirement, so this addsactions/setup-dotnettoci.yml. It needs SDK 8 or newer, since that's where MSBuild gained the JSON output this relies on.Caveats
Project discovery is the real one. moon only creates projects declared in
workspace.yml, plugins can't contribute projects, andprojects.globswon't match.csprojfiles, so every repo above needed a generatedprojects.sourcesmap, 671 entries for abp. Opened moonrepo/moon#2640 for that.Multi-targeted projects get evaluated once, as the outer build across all their frameworks. MSBuild leaves the current framework empty there, so anything gated on one specific framework is invisible to inference. Unconditional references and packages resolve fine. It's also SDK-style projects only, not the pre-2017 format. Fuller notes and the remaining .NET specifics are in the README of the repo I developed it in: https://github.com/Wtiben/moon-dotnet-plugin
Notes
tier3 shells out to dotnet-install rather than going through a proto tool plugin. SDKs install side by side under one root that
DOTNET_ROOTpoints at, and proto's per-version inventory doesn't model that. Can change it if you'd rather.Tagged 0.1.0. I can send the
unstable_dotnetlocator entry fortoolchains_config_ext.rsas a separate PR.