Introduce `OS.FileSystemComparer` / `FileSystemComparison` helpers and apply at filesystem-name comparison sites
- Dominant language
- C#
- Stars
- 2.1k
- Forks
- 579
- Avg merge
- 1d 19h
- Merged PRs (30d)
- 252
Description
## Background
While reviewing PR dotnet/android-tools#326 (AVD skin enumeration), two independent rubber-duck reviews surfaced that the skin-dedup `SortedSet` was using `StringComparer.OrdinalIgnoreCase` unconditionally, which would silently collapse legitimately distinct skin directories (e.g. `Pixel` vs `pixel`) on case-sensitive Linux/macOS filesystems. That was fixed in dotnet/android-tools#326 with an inline `OS.IsWindows ? OrdinalIgnoreCase : Ordinal` ternary.
A broader scan of the codebase found the same pattern (and the same latent bug) in several other places that compare filesystem names or paths, with no shared helper to reach for. Worth consolidating before more drift accumulates.
## Proposed helpers
In `src/Xamarin.Android.Tools.AndroidSdk/OS.cs`:
```csharp
public static readonly StringComparer FileSystemComparer =
IsWindows ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal;
public static readonly StringComparison FileSystemComparison =
IsWindows ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal;
```
**Caveat to document inline:** this is a heuristic by OS, not a true per-volume filesystem-case-sensitivity query. macOS APFS is case-insensitive by default, and Windows ReFS can be case-sensitive. The helper matches the codebase's implicit existing assumption (Windows = case-insensitive, everything else = case-sensitive) and is strictly more correct than the current state, but isn't perfect. A future iteration could query the actual filesystem (e.g. via `FILE_CASE_SENSITIVE_INFORMATION` on Windows, `pathconf(_PC_CASE_SENSITIVE)` on macOS) if it ever matters.
## Sites to migrate
| File | Line(s) | Note |
|---|---|---|
| `src/Xamarin.Android.Tools.AndroidSdk/FileUtil.cs` | 193, 195 | `IsUnderDirectory` — **pre-existing bug on Linux**: `path.StartsWith(dir + Sep, OrdinalIgnoreCase)` would say `/foo/Bar/x` is under `/foo/bar`, which is wrong. |
| `src/Xamarin.Android.Tools.AndroidSdk/AndroidVersions.cs` | 40 | `dirs.Distinct(OrdinalIgnoreCase)` on framework dir paths — **same pre-existing bug on Linux**. |
| `src/Xamarin.Android.Tools.AndroidSdk/Runners/AvdManagerRunner.cs` | 184 | Skin dedup (currently uses the inline ternary added in dotnet/android-tools#326 — replace with the helper). |
| `src/Xamarin.Android.Tools.AndroidSdk/Runners/AvdManagerRunner.cs` | 66, 99, 113 | AVD name lookups — debatable. AVD `.ini` files live on disk so are filesystem-cased, but `avdmanager` itself treats AVD names case-insensitively in commands. Probably keep as `OrdinalIgnoreCase` (tool semantics, not filesystem semantics) and add a comment explaining why. |
## Out of scope
- The many other `StringComparison.OrdinalIgnoreCase` uses in the codebase that operate on **non-filesystem** strings (license IDs, AVD names in process output, "Name:" prefixes in `avdmanager list` output, ADB serials, etc.). Those are correctly case-insensitive regardless of platform.
- Adding a per-volume case-sensitivity probe — defer until there's a concrete need.
## Acceptance
- New helpers added to `OS.cs` with an XML doc summary explaining the heuristic and its limits.
- All four sites above migrated (or explicitly excluded with a comment).
- No new `OrdinalIgnoreCase` introductions for filesystem-name comparisons elsewhere — call out in `.github/copilot-instructions.md` so future PRs reach for the helper.
- Build with `--no-incremental`: 0 warnings, 0 errors. PublicAPI files updated for the new public surface (the helpers are public).
- A regression test for `FileUtil.IsUnderDirectory` exercising the case-sensitive Linux behavior (the existing tests likely only cover Windows).
## Origin
Spotted during second-pass rubber-duck review of dotnet/android-tools#326. The inline ternary added there works for the AVD skin case but doesn't help the other sites and is itself a candidate for consolidation.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.