Support renamed MakePkg.exe for MSIXVC2 capability detection

[Jason Williams (WilliamsJason)]

Why

The GDK is renaming Makepkg2.exe to MakePkg.exe — the new MakePkg.exe replaces the older tool and absorbs its MSIXVC2 capabilities. The GDK still ships makepkg2.exe alongside it, so we must keep supporting it as a fallback.

Everywhere the code decided "is makepkg2.exe available and does it support MSIXVC2?", the resolution order is now:

1. Probe MakePkg.exe supports uploadsource. Exit code 0 == supported. A legacy MakePkg.exe fails this (non-zero exit / error) — that's the discriminator.
2. Otherwise fall back to makepkg2.exe, discovered the same way and probed with the same verb.
3. If both fail, MSIXVC2 capability is unavailable.

Both binaries are discovered in this order: application directory → current directory → installed GDK → PATH. The GDK step looks in <GDKInstallPath>\bin, with the install root taken from the GameDK environment variable first, then HKLM\SOFTWARE\Microsoft\GDK\Installed Roots and its HKLM\SOFTWARE\WOW6432Node\... mirror (value GDKInstallPath). This is a single shared implementation (IToolPathResolver) that both the desktop app and the command line call, so the two hosts cannot disagree about which binary they will run.

Why one order serves both tools: the GDK ships MakePkg.exe and makepkg2.exe side by side in <GDKInstallPath>\bin. Measured on a dev box with the current GDK installed:

Binary	FileVersion	supports uploadsource
...\Microsoft GDK\bin\makepkg.exe	10.0.26100.7851	exit 2 (legacy — not capable)
...\Microsoft GDK\bin\makepkg2.exe	2604.405.14000.0	exit 0 (capable)

That is exactly the case the fallback exists for, and it means a GDK user needs no separately installed tool at all.

MakePkg.exe version checks (PackageCreationViewModel's _supportsSubValAutoUpdate / _supportsCustomSubValPath via FileVersionInfo) are deliberately unchanged — those are version gates, not capability probes.

New shared component

The resolution + probe logic lives in PackageUploader.ClientApi (the only project referenced by both PackageUploader.UI and PackageUploader.Application) so that a follow-up PR adding MSIXVC2 upload support to PackageUploader.exe can consume it directly. All new files are confined to a new src/PackageUploader.ClientApi/Tools/ folder; no existing ClientApi file was renamed or restructured, and no .csproj change was required.

Public surface

namespace PackageUploader.ClientApi.Tools;

/// Shared tool discovery, used by both hosts so they can never disagree
/// about which copy of a tool they will run.
public interface IToolPathResolver
{
    /// app dir -> CWD -> GDK (<root>\bin) -> PATH.
    /// Returns null when the file was not found anywhere; never throws.
    string Find(string fileName);
}

public sealed class ToolPathResolver : IToolPathResolver
{
    public ToolPathResolver();   // zero-DI, works from a console app
}

/// <summary>The MSIXVC2-capable packaging tool that was resolved.</summary>
public sealed record Msixvc2Tool(string ExecutablePath, bool IsMakePkg2Fallback);

public interface IMsixvc2ToolResolver
{
    /// Full self-discovery: app dir -> CWD -> GDK (<root>\bin) -> PATH.
    /// Returns null when no MSIXVC2-capable tool is available; never throws.
    Msixvc2Tool Resolve();

    /// Caller-supplied hints. A NON-NULL argument (including string.Empty) is
    /// authoritative and disables self-discovery for that tool; only null triggers discovery.
    /// Returns null when no MSIXVC2-capable tool is available; never throws.
    Msixvc2Tool Resolve(string makePkgPath, string makePkg2Path);

    bool IsMsixvc2Supported();
    bool IsMsixvc2Supported(string makePkgPath, string makePkg2Path);
}

/// Process abstraction so the probe can be faked in unit tests.
public readonly record struct ToolProbeResult(bool Completed, int ExitCode)
{
    public static ToolProbeResult Failed { get; }
    public bool Succeeded { get; }   // Completed && ExitCode == 0
}

public interface IToolProbeRunner
{
    ToolProbeResult Run(string executablePath, string arguments, TimeSpan timeout);
}

/// Default runner: UseShellExecute=false, CreateNoWindow=true, WaitForExit(timeout),
/// Kill(entireProcessTree: true) on timeout. Never throws.
public sealed class ProcessToolProbeRunner : IToolProbeRunner { }

public sealed class Msixvc2ToolResolver : IMsixvc2ToolResolver
{
    public Msixvc2ToolResolver();                                    // zero-DI, works from a console app
    public Msixvc2ToolResolver(ILogger<Msixvc2ToolResolver> logger);
    public Msixvc2ToolResolver(ILogger<Msixvc2ToolResolver> logger, IToolProbeRunner probeRunner, TimeSpan? probeTimeout);
}

public static class Msixvc2ToolResolverExtensions
{
    // TryAddSingleton for both IToolProbeRunner and IMsixvc2ToolResolver.
    public static IServiceCollection AddMsixvc2ToolResolver(this IServiceCollection services);
}

Internal surface (not public API)

The GDK lookup is seamed so the tests never require an installed GDK. It stays internal (visible to PackageUploader.ClientApi.Test via the existing InternalsVisibleTo) and is not part of the consumable API — only the path-resolution contract is public, not the environment and registry sources behind it:

internal interface IGdkRootLocator
{
    /// GameDK env var, then the two Installed Roots registry keys. Deduped, never throws.
    IReadOnlyList<string> GetGdkRoots();
}

internal sealed class GdkRootLocator : IGdkRootLocator { }

public sealed class ToolPathResolver
{
    // Second, internal ctor - the only way to substitute the GDK lookup.
    internal ToolPathResolver(IGdkRootLocator gdkRootLocator);
}

public sealed class Msixvc2ToolResolver
{
    // Fourth, internal ctor - the only way to substitute discovery.
    internal Msixvc2ToolResolver(
        ILogger<Msixvc2ToolResolver> logger,
        IToolProbeRunner probeRunner,
        TimeSpan? probeTimeout,
        IToolPathResolver toolPathResolver);
}

IMsixvc2ToolResolver and Msixvc2ToolResolver's public surface is unchanged — same four interface members, same three public constructors. IMsixvc2ToolResolver.cs is byte-identical to its previous revision, and the diff on Msixvc2ToolResolver.cs touches no line containing public. Only the internal fourth constructor changed, swapping IGdkRootLocator for IToolPathResolver. Anything already binding to this interface needs no adjustment.

Notes for consumers:

• null is the "no capable tool" result, and it is a documented contract rather than a compiler-enforced one. PackageUploader.ClientApi does not enable nullable reference types (no project in the repo uses file-level #nullable directives), so these signatures carry no ? annotations. Every member that can return null says so in its XML docs, together with the guarantee that it never throws — callers branch on the return value and do not need a try/catch around resolution.
• No caching by design. The original UI comment noted it re-probes on every upload so an in-place binary update is picked up; that semantic is preserved. The resolver is stateless and therefore thread-safe.
• 5s bounded probe timeout (matching the previous inline UI probe), CreateNoWindow = true, UseShellExecute = false. Exceptions are logged and swallowed, never thrown.
• GDK discovery lives inside ClientApi, so a host that supplies no hints still finds a GDK-installed tool. This was necessary rather than cosmetic: hint-passing works for the UI, which resolves paths itself, but PackageUploader.exe calls Resolve() with no hints and would otherwise have found nothing unless the tool happened to be on PATH.
  • Microsoft.Win32.Registry is in the plain net10.0 shared framework, so this needs no PackageReference and no -windows target.
  • Registry access is guarded by a runtime OperatingSystem.IsWindows() check rather than #if, which keeps CA1416 satisfied in the single cross-platform IsAotCompatible assembly. On non-Windows only the environment variable is consulted.
  • Each source is independently guarded, so a denied or malformed key cannot mask a good one or escape into resolution.
• new Msixvc2ToolResolver() works with zero DI, so a console app can construct it directly, or call services.AddMsixvc2ToolResolver().

UI changes

All MSIXVC2 availability/capability decisions now route through the resolver instead of a bare File.Exists(MakePkg2Path):

• MainPageViewModel — takes IMsixvc2ToolResolver and IToolPathResolver; IsMakePkg2Enabled → IsMsixvc2Enabled, MakePkg2UnavailableErrorMessage → Msixvc2UnavailableErrorMessage; logs which tool provided support. ResolveFilePath, FindFileInPath, and ResolveMakePkg2Path are all removed — the first two are replaced by the shared IToolPathResolver, and the third collapsed to a bare lookup once its duplicate cache scan was gone. The now-dead using Microsoft.Win32 goes with them.
• Msixvc2UploadViewModel — the ~40-line inline SupportsUploadSourceFlag() process probe is replaced by ResolveMsixvc2Tool() delegating to the resolver.
• PackageUploadViewModel — MakePkg2UnavailableMessage → Msixvc2UnavailableMessage; MSIXVC2 detection and StartMsixvc2Upload() use the resolver.
• PackageCreationViewModel — IsMakePkg2Available → IsMsixvc2Available; the UseMsixvc2 pack branch resolves the tool rather than assuming MakePkg2Path. FileVersionInfo version checks untouched.
• Msixvc2UploadingViewModel, PackageModel — MakePkg2Path → Msixvc2ToolPath.
• App.xaml.cs — services.AddMsixvc2ToolResolver().
• XAML bindings updated to match the renamed properties.

New files: src/PackageUploader.ClientApi/Tools/ — IMsixvc2ToolResolver.cs, Msixvc2Tool.cs, Msixvc2ToolResolver.cs, IToolProbeRunner.cs, Msixvc2ToolResolverExtensions.cs, GdkRootLocator.cs, IToolPathResolver.cs, ToolPathResolver.cs; and tests src/PackageUploader.ClientApi.Test/Msixvc2ToolResolverGdkDiscoveryTest.cs, Msixvc2ToolResolverExtensionsTest.cs, ToolPathResolverTest.cs.

One discovery implementation, shared by both hosts

MainPageViewModel.ResolveFilePath and Msixvc2ToolResolver.Discover were two copies of the same search, and keeping them in sync was a standing hazard. They had already drifted: the UI consulted only the registry for a GDK install, while ClientApi also honoured the GameDK environment variable. Two hosts could therefore run different binaries on the same machine — the exact class of bug this work item exists to remove.

The discovery layer is now a single public IToolPathResolver that both hosts call.

• Why discovery is the shared component, and not Msixvc2ToolResolver. The UI also resolves SubmissionValidator.dll, which is not an MSIXVC2 tool and never will be. Consolidating onto an Msixvc2*-named type would have made that a permanent special case and misnamed the type on day one.
• Why public rather than InternalsVisibleTo. ClientApi is not shipped as a NuGet package — CI only dotnet publishes PackageUploader.Application.csproj — so there are no external consumers and no semver obligation. The repo's three existing InternalsVisibleTo grants are all production→its own test project; this would have been the first production→production grant.
• ⚠️ User-visible behaviour change: the desktop app now honours the GameDK environment variable, which it previously ignored. A developer who has explicitly pointed GameDK at a GDK should get that GDK from both hosts, and one implementation cannot preserve both orders. On a stock install both routes resolve to the same place, so this is only observable where the env var and the registry disagree.
• Miss semantics. Find reports "not found" as null, matching Resolve and the rest of the namespace, rather than the empty string ResolveFilePath returned. Because a non-null argument to Resolve is authoritative and suppresses discovery, the view model coalesces to string.Empty when forwarding a path — preserving the existing behaviour of not searching twice for a tool it has already located.

Probes do not block the UI thread

The capability check launches a child process, so unlike the File.Exists test it replaced it can take seconds. Every probe that runs before the user has asked for anything now runs off the UI thread via a new BaseViewModel.RunOnUiThread helper, with the observable property initialised to a safe default and updated when the probe completes:

• MainPageViewModel and PackageCreationViewModel constructors — otherwise app start and packaging-page construction could each freeze for up to the full two-probe timeout.
• PackageUploadViewModel.ProcessSelectedPackage() — same hazard immediately after the user picks a file.

Because IsUploadReady() gates the Upload command on Msixvc2UnavailableMessage, that property's setter re-raises CanExecuteChanged, so the button's enabled state cannot go stale relative to an async probe. A stale-result guard prevents a slow probe from publishing against a package the user has since changed.

Probes that run at the moment of acting (StartMakePackageProcess, StartMsixvc2Upload, Msixvc2UploadViewModel.StartPackAndUploadAsync) remain synchronous — they must have a definitive answer before launching the tool. Clicking Upload while a background probe is still in flight is safe: StartMsixvc2Upload() re-resolves synchronously and, when the tool is missing, raises the error page and returns before setting any tool path or navigating.

Resource strings

MainPage.resx / .Designer.cs — the key MakePkg2NotFoundErrorMsg is kept and only its value reworded:

MSIXVC2 requires a packaging tool that supports it. Install the latest GDK to get MakePkg.exe with MSIXVC2 support.

This single key now backs five call sites: MainPageViewModel, PackageUploadViewModel (×2), Msixvc2UploadViewModel, and PackageCreationViewModel via LayoutParseError. Two of those were hardcoded literals on main and are now resx-backed. Three render as an error dialog on the "tool missing" path, which is the most likely place this text is ever read, so the wording is written to stand alone rather than only as inline UI text.

The localized MainPage.ja-JP / .ko-KR / .zh-CN resx files do not carry this key — it is English-only, so there is no localized resource to update.

Tests

The existing .bat-file fake-tool style in Msixvc2UploadViewModelTest.cs still works. New/updated coverage:

Resolution order (Msixvc2UploadViewModelTest.cs): new MakePkg.exe supports the verb → used, no fallback; legacy MakePkg.exe fails but makepkg2.exe succeeds → fallback used; both fail → unavailable; both missing → unavailable; MakePkg.exe hangs → timeout, falls back; MakePkg.exe path missing → falls back; MakePkg.exe path is a directory → unavailable; upload arguments include uploadsource.

GDK discovery (Msixvc2ToolResolverGdkDiscoveryTest.cs, new): MakePkg.exe from GDK bin; makepkg2.exe fallback from GDK bin; GDK preferred over PATH when both would satisfy the probe; GDK-bin miss falls through to PATH; no GDK and nothing on PATH → clean null; GameDK beats registry; registry fallback; WOW6432Node fallback; no source → empty; throwing sources → empty, nothing escapes; real (default) readers do not throw, covering the non-Windows registry skip; and an assembly-wide assertion that no Tools constant names an internal package feed. PATH is replaced with a controlled temp directory per test and restored, and the fixture reports inconclusive rather than passing misleadingly if a real tool sits in the app or current directory and would pre-empt GDK discovery.

DI from a non-UI host (Msixvc2ToolResolverExtensionsTest.cs, new): AddMsixvc2ToolResolver() resolves from a bare ServiceCollection with no logging and no UI services registered — strictly harsher than the HostApplicationBuilder the console app uses.

Off-thread probes: regression guards asserting the MainPageViewModel / PackageCreationViewModel constructors and the package-selection path all return well inside the probe duration, plus a test that clicking Upload while a probe is in flight produces a clean error with no navigation and no tool launch.

Existing view-model tests were updated for the new ctor parameter.

Additionally, resolution was verified end-to-end against a real GDK install from a plain HostApplicationBuilder with no UI services — the exact shape the follow-up console-app PR uses — producing the exit codes in the table above and resolving to the GDK copy of makepkg2.exe.

Validation

dotnet build src\PackageUploader.sln -v q
  -> Build succeeded. 0 Warning(s)  0 Error(s)

src\PackageUploader.UI.Test\bin\Debug\net10.0-windows\win-x64\PackageUploader.UI.Test.exe
  -> total: 378  failed: 0  succeeded: 378  skipped: 0

src\PackageUploader.ClientApi.Test\bin\Debug\net10.0\PackageUploader.ClientApi.Test.exe
  -> total:  99  failed: 0  succeeded:  99  skipped: 0

src\PackageUploader.Application.Test\bin\Debug\net10.0\PackageUploader.Application.Test.exe
  -> total:  66  failed: 0  succeeded:  66  skipped: 0

src\PackageUploader.IntegrationTest\bin\Debug\net10.0\PackageUploader.IntegrationTest.exe
  -> total:   1  failed: 0  succeeded:   1  skipped: 0

  544 tests, 0 failed, 0 skipped

The test executables are invoked directly because the repo's global.json opts into Microsoft.Testing.Platform. Note that --nologo is a VSTest option that Microsoft.Testing.Platform does not accept — passing it to dotnet test here fails the run rather than being ignored.

No global.json, packages.lock.json, or .csproj changes are included in this PR.

Scope

No changes to src/PackageUploader.Application or src/PackageUploader.UI/Model/Xvc/XvcFile.cs — those belong to the concurrent MSIXVC2-in-PackageUploader.exe PR, which will consume IMsixvc2ToolResolver from this change.

[elahmed-microsoft]

Maybe it should search the GDK location first before checking the current working directory. Don't know how possible it is, but what if the current working directory has a matching executable name (MakePkg.exe) and that gets found and executed instead of the GDK

[elahmed-microsoft]

For this try-catch block, if one of them throw an exception it looks like it'll exit altogether and go into the catch statement. However, what I think this is intending to do is fall back to the next option. If true, might be worth seeing if theres a way to scope each exception to allow it to search the next best candidate, and if all fail throw a catch-all error