microsoft / microsoft/winappCli

find-api: reuse winapp's existing NuGet packages folder and Windows SDK root resolution

Open
#831 0 comments 0 reactions 1 assignee Claimed by @Jaylyn-Barbee View on GitHub
bug cli
Dominant language
C#
Stars
1.3k
Forks
80
Avg merge
3d 6h
Merged PRs (30d)
51

Description

## Context

`winapp find-api` landed in #744 (the port of the `winmd` tool, tracked by #652). One of #652's goals was to avoid standing up a second copy of logic `winapp` already has. The port did that where it mattered most — `Services/ApiSearch/` uses the shared `PathSafety` helper in 14 places, and #744 extended that helper rather than forking it.

Two places were missed. Both are in new `find-api` code only, neither is a regression, and `find-api` has not shipped in a release yet — so this is cleanup, not a fix for a broken user contract. Filed as follow-up so #744 could merge on its existing review.

---

## 1. `find-api` resolves the NuGet global packages folder itself, and gets it wrong

**What is wrong:** `NuGetResolver.GetNuGetPackagesDir()` (`src/winapp-CLI/WinApp.Cli/Services/ApiSearch/NuGetResolver.cs:1597`) looks only at the `NUGET_PACKAGES` environment variable, then falls back to `%USERPROFILE%\.nuget\packages`. It ignores the `globalPackagesFolder` setting in `nuget.config`.

`winapp` already resolves this correctly. `INugetService.GetNuGetGlobalPackagesDir()` (`Services/INugetService.cs:52`) is implemented with `SettingsUtility.GetGlobalPackagesFolder` (`Services/NugetService.cs:82`), which honors the environment variable **and** the `nuget.config` setting.

**Show me:** in a repo whose `nuget.config` sets ``:

- `winapp find-api "app notification"` on a **packages.config** project → the package directories are looked up under `%USERPROFILE%\.nuget\packages`, are not there, and the APIs report as not found (`FindPackagesFromConfig`, `NuGetResolver.cs:1199`).
- On any project → Windows SDK projection XML docs are not found, so results come back without descriptions (`DiscoverSdkXmlDocs`, `NuGetResolver.cs:416`).

Expected in both cases: the packages are found in the configured folder.

Modern SDK-style projects are **not** affected on the main path — `FindPackagesFromAssets` reads the `packageFolders` recorded in `project.assets.json` instead of guessing.

**Why it matters:** `find-api` exists to answer from the project's real metadata. On a machine with a redirected package folder it silently answers "does not exist" for APIs the project can actually compile against, which is the exact failure mode the command is meant to prevent.

**Smallest fix:** have the two call sites use `INugetService.GetNuGetGlobalPackagesDir()` instead of the private helper, and delete the helper.

**This is not a drop-in swap — there is a design question first.** `NugetSourceProvider` loads its settings rooted at `_configRoot ?? currentDirectory` (`Services/NugetSourceProvider.cs:74-75`), but `find-api` indexes whatever directory `--project` points at, which need not be the working directory. Reusing the existing service as-is would read the wrong `nuget.config` hierarchy for an out-of-tree project. Decide whether to:

- construct/scope the settings root to the project directory being indexed (preferred — it matches what the rest of `find-api` already does), or
- accept cwd-rooted settings and document the limitation.

Wiring cost, once that is settled: inject `INugetService` into `ApiMetadataService` (`Services/ApiSearch/ApiMetadataService.cs:65-69`, which currently takes four unrelated services), pass the resolved directory through the static `ApiCacheBuilder` (`ApiCacheBuilder.cs:22`, called at `:76`) into `NuGetResolver.FindPackagesWithWinMd` (`NuGetResolver.cs:21`), and down to the two call sites.

---

## 2. `find-api` hardcodes the Windows Kits install path

**What is wrong:** `NuGetResolver.FindWindowsSdkWinMd` (`NuGetResolver.cs:1263`) builds its search root as `%ProgramFiles(x86)%\Windows Kits\10\UnionMetadata` (`NuGetResolver.cs:1265`) and returns nothing when that directory is absent.

`winapp` elsewhere resolves the kit root properly: `CsWinRTMetadataShimService` (`Services/CsWinRTMetadataShimService.cs:196-197`) reads `HKLM\SOFTWARE\Microsoft\Windows Kits\Installed Roots\KitsRoot10`, mirroring what `cswinrt` itself does.

**Show me:** on a machine with the Windows SDK installed to a non-default root (for example `D:\Kits\10`), `winapp find-api "app notification" --project sdk` finds no Windows SDK metadata and reports version `unknown`, while `winapp`'s cswinrt path on the same machine locates the kit fine. Expected: `find-api` finds the same SDK.

**Why it matters:** `--project sdk` is documented as the way to query before a project exists. On a non-default SDK install it silently has nothing to query.

**Smallest fix:** extract the `KitsRoot10` registry resolution out of `CsWinRTMetadataShimService` into a small shared helper (registry first, `%ProgramFiles(x86)%` fallback) and call it from both. Do **not** add a third inline copy inside `NuGetResolver` — that would defeat the point of this issue.

---

## Deliberately out of scope

Listed so they are not re-litigated:

- **`Helpers/ProjectAssetsFileReader.cs` vs `NuGetResolver.FindPackagesFromAssets`** — both read `project.assets.json`, but produce different things: the helper returns a `dotnet package list`–shaped package list, the resolver returns per-package `.winmd` and XML-doc paths. Merging them would mean one function serving two unrelated outputs.
- **`Helpers/NuGetVersionHelper.Compare` vs `NuGetResolver.MajorMinor`/`ReleaseSortKey`** — the helper implements full SemVer precedence; the resolver matches WinAppSDK release channels on major.minor. Different grammars for different jobs; sharing would muddy both.
- **`Services/WinmdService.cs` vs `Services/ApiSearch/WinMdParser.cs`** — #652 asked to reconcile these, but they turned out to do different work: `IWinmdService` enumerates activatable classes for manifest generation, `WinMdParser` extracts the full type/member surface for search. Keeping them separate is the right call; noting it here so the open question in #652 is closed rather than left dangling.
- **`NuGetResolver.cs` file size (~1,700 lines)** — it is long because it handles five discovery sources (assets file, packages.config, `winmds.lock.json`, project references, SDK/runtime probing), not because it repeats itself. There is no dedupe fix here; if it is worth revisiting it is a separate cohesion question.
- **`NuGetResolver.FindPackagesFromWinmdsLockfile` vs `IWinmdsLockfileService.TryReadAsync`** — the resolver already shares `WinmdsLockfileService.LockfileName` and `WinmdsLockfileJsonContext`; only the deserialize-plus-schema-check is duplicated. Reusing the service would mean threading a DI dependency into a static path for a handful of lines. Cost is roughly equal to the benefit; skip unless item 1 makes that plumbing available anyway.

## Acceptance

- `find-api` honors `globalPackagesFolder` from `nuget.config`, with a test covering the packages.config path.
- `find-api` locates a Windows SDK installed to a non-default root, sharing one `KitsRoot10` resolution with `CsWinRTMetadataShimService`.
- No new duplicate of either resolution is introduced.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.