Make dotnetup's managed root authoritative for Doctor - #222
Open
Redth wants to merge 2 commits into
Open
Conversation
Doctor and the .NET SDK Manager could disagree about the active .NET SDK. On macOS with dotnetup opted in, Doctor reported 11.0.100-preview.5 from /usr/local/share/dotnet while the SDK Manager reported preview.6 from the dotnetup-managed root, and the resulting feature-band mismatch silently downgraded Doctor's workload check to a NuGet-only path. Two root causes: 1. Prerelease-blind ordering. SdkVersion exposed only Major/Minor/Patch, so every OrderByDescending tied across 11.0.100-preview.4/5/6 and stable 11.0.100, letting directory/feed enumeration order pick the winner. SdkVersion now implements IComparable over a full NuGetVersion and all consumers sort through the shared SdkVersion.SortDescending helper. 2. Doctor never treated dotnetup as authoritative — it scanned the system root and merely merged managed versions in by version string, keeping the system install root, architecture, manifests and workload-set files. Adds DotnetSdkSourceResolver as the single decision point for which install root wins: repo-local .dotnet, then the dotnetup-managed root whenever dotnetup manages at least one valid SDK (preferring the process architecture, then the newest SDK), then the machine scan. When the managed root wins it is the sole source of truth — DoctorService pins a LocalSdkService to it so manifests and workload sets read the same place, and ResolveDotNetExecutable invokes the matching dotnet. Doctor's ".NET SDK" check now reuses dotnetup's tracked-channel update preview when managed, so it shows exactly what the SDK Manager shows, and emits dotnetup-update-sdk:<channel> (specific channels preferred over the latest/lts/sts/preview aliases; pinned specs skipped). DoctorContext exposes UsesDotnetUpManagedSdk and the Doctor page badges the ".NET SDK Path" row. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 38513608-bbd3-48e1-8ac5-7a5a304654ca
The Mac Catalyst app head is superseded by src/MauiSherpa.MacOS (net10.0-macos, AppKit), which is what CI publishes. Remove net10.0-maccatalyst from src/MauiSherpa so it is now a Windows-only head, and no-op Build/Rebuild/Publish there when not on Windows so solution builds succeed on macOS and Linux. Building any Apple head also failed with: install_name_tool: cannot rename ... libcopilot_runtime.dylib.tmp The "InstallNameTool" task failed unexpectedly GitHub.Copilot.SDK stages its native runtime once per referencing project, and the diamond (app head -> AppInspector -> Core, plus app head -> Core) produced three _FileNativeReference items pointing at the same destination. The Apple SDK's InstallNameTool task processes items in parallel and derives its scratch file from the destination, so those items raced on the same .tmp path. build/DedupeNativeReferences.targets collapses _FileNativeReference items that share a RelativePath down to one before _ComputeDynamicLibrariesToReidentify runs. Deduping has to match on RelativePath rather than ItemSpec because the duplicates can share an identical ItemSpec and differ only in MSBuildSourceProjectFile. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 38513608-bbd3-48e1-8ac5-7a5a304654ca
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The Doctor page and the .NET SDK Manager page disagreed about .NET 11. Doctor reported
11.0.100-preview.5and offered to install a workload set for that band, while the SDK Manager correctly showed thepreview.6SDK that dotnetup had actually installed. Acting on Doctor's advice would have installed workloads into an SDK the user was not building with.What was actually wrong
Two independent bugs compounded:
SdkVersionhad no prerelease ordering. It only exposedMajor/Minor/Patch, and every sort in the Workloads library wasOrderByDescending(Major).ThenBy...(Patch). That meant11.0.100-preview.4,-preview.5,-preview.6and stable11.0.100all compared equal, so LINQ's stable sort let directory enumeration order decide the "newest" SDK.Doctor never treated the dotnetup root as authoritative. It resolved
dotnetthrough the machine install location (/etc/dotnet/install_location_arm64->/usr/local/share/dotnetfor a GUI process, which does not inherit a shellDOTNET_ROOT), then merged dotnetup's SDK list on top. The resulting feature band matched no dotnetup-managed workload target, so Doctor silently fell back to a NuGet-only workload path and reported against a different install root than the SDK Manager.Approach
DotnetSdkSourceResolveris a new pure function that is the single decision point for "which .NET install root is authoritative." It groups dotnetup's SDK list by (install root, architecture), prefers the process architecture, then the newest SDK, and falls back to the machine scan when dotnetup has nothing valid installed. Precedence is repo-local.dotnet/sdk> dotnetup-managed root > machine-discovered root.DoctorServicenow builds aLocalSdkServicerooted at that path (newinstallRootOverrideconstructor argument), so SDK enumeration, workload manifests, and workload sets all read from the same place instead of three different ones.MergeManagedSdksis gone, since there is nothing left to merge once one root wins.SdkVersionimplementsIComparable<SdkVersion>backed byNuGetVersion, and every call site uses the newSdkVersion.SortDescendinghelper.When the active SDK is dotnetup-managed, the update check reuses dotnetup's own tracked channels and the fix action becomes
dotnetup-update-sdk:<channel>(for example11.0.1xx) instead of a raw version install, so Doctor's remediation stays inside dotnetup rather than fighting it.The Doctor context section gets a small
dotnetupbadge on the .NET SDK Path row so it is obvious at a glance which root is being inspected. It reuses the existingbadge-infoclasses, so light and dark mode are already covered by the theme variables.Verification
Confirmed live in the running app: Doctor now reports SDK path
~/Library/Application Support/dotnetwith the dotnetup badge,.NET SDK 11.0.100-preview.6.26359.118"managed by dotnetup", and workload set11.0.100-preview.6.26364.2"Up to date" - matching the SDK Manager. Tests: 191 Workloads, 508 Core.Note for anyone writing tests here: running
dotnet testfrom a shell that hasDOTNET_ROOTexported masks the GUI behaviour, since the GUI process does not inherit it. A probe has to clearDOTNET_ROOT,DOTNET_ROOT_X64andDOTNET_ROOT_ARM64in-process to reproduce what the app actually sees.Second commit: dropping the Mac Catalyst head
This started as collateral. The Mac Catalyst build was already broken on
main, which blocked verifying any of the above, so it had to be dealt with.src/MauiSherpa.MacOS(net10.0-macos, AppKit) is the real macOS app and is what CI publishes, sonet10.0-maccatalystis removed fromsrc/MauiSherpa, leaving it a Windows-only head.Build/Rebuild/Publishthere are no-ops off Windows sodotnet build MauiSherpa.slnsucceeds on macOS and Linux.The underlying build failure also affected the macOS head, so it is fixed properly rather than sidestepped:
GitHub.Copilot.SDKstages its native runtime once per referencing project, and the diamond (app head -> AppInspector -> Core, plus app head -> Core directly) produced three_FileNativeReferenceitems pointing at the same destination. The Apple SDK'sInstallNameTooltask runs its items in parallel and derives its scratch file from the destination path, so those items raced on the same.tmp. Note that-m:1does not help, because the parallelism is inside the task rather than in MSBuild's scheduler.build/DedupeNativeReferences.targetscollapses items sharing aRelativePathdown to one before_ComputeDynamicLibrariesToReidentifyruns. It has to match onRelativePathrather than ItemSpec, because the duplicates can share an identical ItemSpec and differ only inMSBuildSourceProjectFile, which is why a plainDistinct()does not fix it. Any future Apple app head will need to import this too.Left alone
src/MauiSherpa/Platforms/MacCatalyst/and the Catalyst entitlements are still on disk, now unreferenced. Kept for minimal churn and easy revival; happy to delete in a follow-up.RunDoctorAsyncstill reports againstsdkVersions[0]rather thancontext.ActiveSdkVersion. That is pre-existing behaviour and was deliberately not changed here.