[FastDeploy2] fix adb push argument quoting on Windows

[jonathanpeppers]

Problem

FastDeploy2.QuoteProcessArgument() escaped every backslash:

foreach (char c in argument) {
	if (c == '"' || c == '\\') {
		sb.Append ('\\');
	}
	sb.Append (c);
}

Windows (CommandLineToArgvW/MSVCRT — and .NET's own argument parsing on Unix) only treats a run of backslashes as an escape sequence when it is immediately followed by a quote. A \\ not followed by " is two literal backslashes. So C:\dir\file.dll was delivered to adb as C:\\dir\\file.dll.

Win32 usually collapses the duplicate separators, so most pushes work by luck. But the inflated path breaks adb in two distinct ways, and it is important not to conflate them:

Mode 1 — adb: error: cannot stat '...'
The doubled path exceeds MAX_PATH (260). Independent of file size. Measured: doubled=257 OK, doubled=261 fails.

Mode 2 — adb: error: failed to read all of '...': Invalid argument
The doubled path is comfortably under 260, but the file is smaller than 64 KB. This is the mode that actually broke real builds. adb's sync protocol splits at SYNC_DATA_MAX (64 KB): the small-file path packs the filename and the data into a single buffer, so an inflated path corrupts it there, while the large-file path is unaffected.

Controlled sweep with the path length held fixed at doubled=249 (under MAX_PATH), varying only file size:

size=   8 KB  doubled=249  -> FAIL-read (Invalid argument)
size=  16 KB  doubled=249  -> FAIL-read (Invalid argument)
size=  32 KB  doubled=249  -> FAIL-read (Invalid argument)
size=  48 KB  doubled=249  -> FAIL-read (Invalid argument)
size=  63 KB  doubled=249  -> FAIL-read (Invalid argument)
size=  64 KB  doubled=249  -> OK
size=  65 KB  doubled=249  -> OK
size= 128 KB  doubled=249  -> OK
size= 873 KB  doubled=249  -> OK

The boundary is exactly 64 KB. That is why the production casualties were Microsoft.Extensions.Configuration.Abstractions.dll (28 KB) and Microsoft.Extensions.FileProviders.Abstractions.dll (15 KB) — both tiny, both well under MAX_PATH.

This is not a MAX_PATH bug, and a path-length guard would not have caught it. Correct quoting removes the inflation and eliminates both modes; no \\?\ prefixing is warranted.

Both modes are properly detected: adb exits 1, and FastDeploy2 correctly surfaced that as XA0129. The deploy failed loudly, as it should have.

This is wider than FastDeploy2

Xamarin.Android.Tools.AndroidSdk ships as netstandard2.0 only (AndroidToolsDisableMultiTargeting defaults to true in its .csproj), so ProcessUtils.CreateProcessStartInfo always takes the JoinArguments path in shipping builds — the ProcessStartInfo.ArgumentList branch is effectively test-only.

And JoinArguments previously did no escaping at all:

sb.Append ('"').Append (args [i]).Append ('"');

So every AdbRunner call site is latently affected today, not just FastDeploy2 — any argument containing a " would be mis-parsed. FastDeploy2 is simply where it first produced a visible failure, because its own hand-rolled quoting had the opposite bug (over-escaping) and it pushes many small assemblies at long absolute paths.

Fix

Rather than fix hand-rolled quoting a second time, fix the one shared helper and route FastDeploy2 through it.

• ProcessUtils.JoinArguments now implements the real Windows rules (same algorithm as the BCL's PasteArguments): a run of backslashes is doubled only when it precedes a " or the closing quote, " is escaped as \", empty/null becomes "", and arguments with no whitespace/quote are emitted bare. It moved out of #if !NET5_0_OR_GREATER so it compiles and is unit-testable on all TFMs. It stays internal, so no public API change and no PublicAPI.Unshipped.txt churn.
• FastDeploy2.QuoteProcessArgument is deleted, and RunAdbCommand now uses ProcessUtils.CreateProcessStartInfo + ProcessUtils.StartProcess following the AdbRunner pattern — replacing the hand-rolled ProcessStartInfo, ManualResetEvent stdout/stderr pumping, and cancellation registration (~55 lines).
  • ThrowIfFailed is deliberately not used: FastDeploy2 inspects the exit code and adb stderr to classify install failures and must not throw.
  • The diagnostic log line is now built from the argument list instead of psi.Arguments, which is empty on the ArgumentList path.

Multi-targeting Xamarin.Android.Build.Debugging.Tasks was considered and rejected — since the SDK itself is netstandard2.0-only in product builds, it would buy nothing.

Tests

ProcessUtilsTests gains the regression test that would have caught this (a plain Windows path must not be double-escaped), plus spaces, embedded quotes, trailing backslashes, empty/null arguments, and a round-trip through a CommandLineToArgvW parser.

Emitting arguments bare is a behavior change for existing AdbRunner call sites (previously everything was quoted). It's transparent to the child process — quotes are stripped by argument parsing either way — but the shapes AdbRunner passes are locked down explicitly: device serials, tcp:/localabstract: socket specs, getprop property names, bare verbs and flags, and a full adb shell command containing spaces and quotes.

Verification

Primary: full end-to-end deploy on a physical arm64-v8a device. Bootstrap + full solution build + -t:ConfigureLocalWorkload (skipping this leaves a stale Xamarin.Android.Build.Debugging.Tasks.dll in the pack and yields a false green) + -t:Install -c Debug of Mono.Android.NET-Tests with -p:UseMonoRuntime=false:

• 0 errors, no XA0129
• 205 assemblies in files/.__override__/arm64-v8a, 401 files staged
• instrumentation runs with tests passing
• a second incremental install completed in 12s with matching hash markers, confirming the first deploy was complete rather than partial

This covers mode 2 directly: the small Microsoft.Extensions.*.Abstractions.dll assemblies that previously failed are among the assemblies that deployed successfully.

Supporting:

1. Unit tests — 371 passed, 0 failed (-p:AndroidToolsDisableMultiTargeting=false -p:DotNetTargetFrameworkVersion=10.0).
2. Direct A/B on device at doubled=249 with an 8 KB file: old quoting → failed to read all of ...: Invalid argument, 0 files pushed, Process.ExitCode = 1; new quoting → 1 file pushed, exit 0. Plus the mode-1 case at doubled=268 → cannot stat, fixed likewise.
3. AdbRunner on device through the netstandard2.0 shipping path (asserting psi.Arguments is populated and ArgumentList empty first, so it cannot silently test the wrong branch): ListDevicesAsync, GetShellPropertyAsync, RunShellCommandAsync (including echo "remote=$(...)", the FastDeploy2 marker pattern), forward/reverse port add-list-remove — all pass.

Note on partial deploys

For the record, since it came up while diagnosing this: adb push does not abandon a batch after a failure. Measured with a 4-file batch shaped [small-FAIL, big-OK, big-OK, small-FAIL], adb pushed both good files, reported both failures (not just the first), and exited 1.

The half-deployed device state came from one level up — FastDeploy2 issues several batches, one returned non-zero, and the task raised XA0129 and stopped issuing the remaining batches. The missing files were in later batches that were never attempted. Aborting remaining work after a hard error is reasonable behavior; the only residual observation is that a failed deploy leaves the device in a mixed state that the next build has to reconcile. Not a silent failure, and not something this PR changes.

[Copilot]

Pull request overview

This PR fixes Windows process-argument quoting for adb invocations by centralizing correct CommandLineToArgvW-compatible quoting in ProcessUtils.JoinArguments, and then routing FastDeploy2’s adb execution through ProcessUtils (instead of hand-rolled quoting and stream pumping). This prevents accidental backslash doubling in Windows paths (which could push command lines past MAX_PATH and break adb push batches).

Changes:

• Implement proper Windows quoting/escaping rules in ProcessUtils.JoinArguments and make it available across TFMs for unit testing.
• Simplify FastDeploy2’s adb execution by using ProcessUtils.CreateProcessStartInfo + ProcessUtils.StartProcess, removing the custom quoting helper.
• Add comprehensive regression/unit tests covering Windows paths, spaces/quotes, trailing backslashes, empty/null args, and round-tripping via a CommandLineToArgvW-style parser.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
tests/Xamarin.Android.Tools.AndroidSdk-Tests/ProcessUtilsTests.cs	Adds regression and round-trip tests for the new JoinArguments behavior.
src/Xamarin.Android.Tools.AndroidSdk/ProcessUtils.cs	Reworks JoinArguments to follow Windows quoting rules and be testable across TFMs.
src/Xamarin.Android.Build.Debugging.Tasks/Tasks/FastDeploy2.cs	Switches adb execution to ProcessUtils helpers, simplifying process handling and relying on shared quoting logic.
src/Xamarin.Android.Build.Debugging.Tasks/Tasks/FastDeploy2.Adb.cs	Removes the previous FastDeploy2-local argument quoting helper.