[FastDev] Add faster FastDeploy2#11698
Conversation
Add a new FastDeploy2 strategy that uses a local manifest to push only changed files to temporary device storage and mirrors the app override directory with shell-created symlinks. Keep legacy FastDeploy selectable while making FastDeploy2 the default. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove existing override paths before copying changed files so Copy mode can safely recover from a symlink-based override tree. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a new manifest-driven fast deployment implementation (FastDeploy2) alongside the legacy FastDeploy, and makes FastDeploy2 the default strategy for app install fast deployment.
Changes:
- Register and invoke the new
FastDeploy2MSBuild task, with_AndroidFastDevStrategydefaulting toFastDeploy2. - Add
FastDeploy2Base(core orchestration + adb/run-as helpers) plus a manifest-basedFastDeploy2implementation that pushes only changed files and updates overrides via symlink (with copy fallback). - Add MSBuild-time validation for supported strategy/transfer-mode values, and a new adb compression algorithm property default (
any).
Show a summary per file
| File | Description |
|---|---|
| src/Xamarin.Android.Build.Debugging.Tasks/Xamarin.Android.Common.Debugging.targets | Registers FastDeploy2, selects strategy, sets defaults for strategy/transfer mode/compression, and wires task invocation into _Upload. |
| src/Xamarin.Android.Build.Debugging.Tasks/Tasks/FastDeploy2.cs | Adds FastDeploy2Base task implementation: device/package orchestration, adb invocation, file staging/stat/diff/copy paths, diagnostics logging, and error handling. |
| src/Xamarin.Android.Build.Debugging.Tasks/Tasks/FastDeploy2.Manifest.cs | Implements manifest-driven incremental upload, remote “ready” marker handling, stale cleanup, and shell-symlink override updates with copy fallback. |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 8
simonrozsival
changed the title
Fix manifest reset handling, device-scoped manifest state, adb batching edge cases, symlink/copy recovery, and diagnostics logging concurrency. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use shell variables and cd to avoid repeating long staging and override paths in generated run-as symlink scripts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Drop leftover experimental staging and symlink helper methods that are no longer referenced by the manifest-driven direct-push implementation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Skip missing FastDev inputs before manifest creation and clear symlink-managed override state before Copy mode so stale symlinks cannot survive mode switches or fallback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
simonrozsival
changed the title
|
/review |
|
✅ Android PR Reviewer completed successfully! |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ Needs Changes
Thorough review of the new FastDeploy2 implementation. The architecture is well thought out — manifest-based incremental diffing, a clean strategy/mode split in the targets with condition-first <Error> validation, careful shell-argument quoting (ShellQuote single-quote escaping) and package-name sanitization (IsSafePackageNameForShell) to prevent shell injection, ADB cancellation handling with process kill + output draining, and reuse of the existing XA012x/XA013x error codes. Nice work.
A few things to address before merge:
Issues by severity
⚠️ 2 warnings- Order-dependent shell globbing (
FastDeploy2.Manifest.cs:132): the full-refreshrm -f ./*/ln -sf "$s"/* .globs also match subdirectories such as the staged{abi}/environment. Becausedirectoriesis iterated in nondeterministicHashSetorder, this can either delete real staged files through a directory symlink or silently fall back to copy. Please handle nested directories explicitly. - Dead/duplicated base implementation (
FastDeploy2.cs:549): thevirtual DeployFastDevFilesWithAdbPushand its exclusive helpers are fully superseded by the override and never run — including a duplicatedCreatePushArgsPrefixthat still uses--sync. Recommend deleting it (or making the methodabstract).
- Order-dependent shell globbing (
- 💡 2 suggestions
MarkRemoteReadyignores the adb exit code (FastDeploy2.Manifest.cs:447).ManifestEntry.LocalPathis persisted but never read (FastDeploy2.Manifest.cs:496).
See the inline comments for mechanisms and concrete fixes.
CI: license/cla passed; the Azure DevOps builds (Linux/Mac/Windows) are still in progress at review time, so green CI is not yet confirmed — please ensure the internal pipeline passes before merge.
The symlink-glob behavior is the main correctness concern; the rest are maintainability cleanups. I couldn't reproduce on-device, so if I've misread the staging layout for the glob case, a quick confirmation of how {abi}/environment is handled during a full refresh would resolve it.
Generated by Android PR Reviewer for issue #11698 · 1.4K AIC · ⌖ 48.9 AIC · ⊞ 37.8K
Comment /review to run again
jonathanpeppers
left a comment
jonathanpeppers
left a comment
There was a problem hiding this comment.
I think I want to see a .binlog of a deploy to understand what it does -- I might have time to manually test some later.
Overall, I like what it does -- introduce a private property by default you can opt out of if something goes wrong.
| static string GetSafeFileName (string value) | ||
| { | ||
| if (string.IsNullOrEmpty (value)) { | ||
| return "_"; | ||
| } | ||
|
|
||
| var builder = new StringBuilder (value.Length); | ||
| foreach (char c in value) { | ||
| builder.Append (char.IsLetterOrDigit (c) || c == '.' || c == '-' || c == '_' ? c : '_'); | ||
| } | ||
| return builder.ToString (); | ||
| } |
There was a problem hiding this comment.
Is there an existing helper somewhere that does this? I can think of maybe things related to Java names, or Android resources.
There was a problem hiding this comment.
I looked and couldn't find an existing helper for turning an arbitrary string into a filesystem-safe path segment. The Java-name and Android-resource helpers (identifier mangling / resource-designer naming) solve a different problem with different rules — they're about valid C#/Java identifiers, not reversible, collision-free directory names. Here we just need a unique cache directory per device/package/user/abi, which is what Uri.EscapeDataString gives (reversible, no path separators). Happy to switch to a shared helper if you have one in mind — leaving this thread open for your call.
|
@jonathanpeppers I captured a binlog from building a fresh https://gist.github.com/simonrozsival/c06cacff00c99ee3504c256205e24492 Command used for the successful binlog: ./dotnet-local.sh build JonMauiBinlog/JonMauiBinlog.csproj \
-f net11.0-android \
-p:TargetFrameworks=net11.0-android \
-p:SupportedOSPlatformVersion=24.0 \
-bl:JonMauiBinlog-net11.0-android.binlogThe TFM override keeps the template Android-only because I only installed |
Move FastDeploy2 diagnostic JSON helpers out of the main task, use System.Text.Json source generation for FastDeploy2 JSON, remove unused FastDeploy2 task inputs, and consolidate repeated path/grouping helpers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Flatten FastDeploy2 into a single concrete task, remove the development diagnostics property bag and JSON payload, and keep only the functional manifest serialization state. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
I think your command needs |
|
@jonathanpeppers you're right — the earlier binlog was build-only. I updated the same gist with a new https://gist.github.com/simonrozsival/c06cacff00c99ee3504c256205e24492 New file: Command used: ./dotnet-local.sh build JonMauiBinlog/JonMauiBinlog.csproj \
-t:Install \
-f net11.0-android \
--no-restore \
-m:1 \
-nr:false \
-p:TargetFrameworks=net11.0-android \
-p:SupportedOSPlatformVersion=24.0 \
-p:AdbTarget=-s%20R58Y30HZ65V \
-p:_FastDeploymentDiagnosticLogging=False \
-bl:JonMauiBinlog-net11.0-android-install-fastdeploy2.binlog \
-clp:PerformanceSummary \
-v:minimalThe console log shows |
Avoid logging empty run-as command output and buffer optional missing-file messages behind FastDeploy2 diagnostics so normal install binlogs stay readable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
There are many log messages that just say 
I think we should probably skip these messages in general unless But then I probably need to see a log where |
Stage FastDeploy2 files under /data/local/tmp instead of /tmp so Android emulators with read-only /tmp can install. Also remove existing override contents recursively before full symlink refreshes so resource/culture directories do not fail rm. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@jonathanpeppers to clarify the physical-device validation: that was a Samsung Galaxy A16 ( The emulator validation rows in the PR description are separate API 24/29/36 emulator checks. |
Add target identity to FastDeploy2 manifests and store the manifest hash on both the remote staging tree and the app override tree. Read both device-side hashes in one adb shell command before deciding whether incremental deploy state can be trusted, and expand adb diagnostics to include exact commands, exit codes, stdout, and stderr. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
FastDeploy2 (the new default fast-deployment strategy) broke several
existing on-device tests. Three deterministic, device-verified root causes:
- Missing sync log markers: tests assert `NotifySync CopyFile`/
`NotifySync SkipCopyFile` per file. FastDeploy2 only logged
`Prepared X => Y`, so IncrementalFastDeployment and
SkipFastDevAlreadyInstalledFile failed even though deployment
succeeded. Emit the markers from the manifest's changed-file set.
- Symlink mode followed subdirectory symlinks into staging: the full
rewrite used `ln -sf "$s"/* .`, which also symlinked staging
subdirectories. Processing a child directory then `cd`'d through that
symlink into the shell-owned /data/local/tmp staging area and
`rm -rf ./*` failed with "Permission denied" under run-as (XA0133 /
XA0129). This broke any app with culture/satellite subdirectories
(LocalizedAssemblies_ShouldBeFastDeployed, DotNetNewAndroidTest). Only
symlink regular files, and process parent directories before children
so a parent's `rm -rf ./*` cannot delete a freshly created child.
- Copy fallback's stat format was split by the device shell: the
`find ... -exec stat -c "%n|%s|%Y" {} +` argv reached the device shell
unquoted, so `|` was treated as a pipe (exit 127, XA0129). Build the
command as a single string with the format single-quoted, matching the
RunAsShell quoting pattern.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Self-review follow-up. The previous fix replaced `ln -sf "$s"/* .` with a files-only loop but still ran `rm -rf ./*`, which deletes child directories. The old glob accidentally masked an incremental-deploy bug by symlinking whole staging subdirectories: on an incremental deploy where a parent directory gains all-new files while a child directory keeps unchanged files, the parent's `rm -rf ./*` wiped the child's still-valid symlinks. Clear only the regular files in each directory (`for e in ./*; do [ -d "$e" ] || rm -f "$e"; done`) so subdirectories and their contents survive, and drop the now-unnecessary parents-first ordering. Verified on an emulator: the parent relinks its files, the child subdirectory and its symlink are preserved, and the staging area is untouched. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Self-review follow-up. The abi-skip `NotifySync SkipCopyFile` marker logged the local build path (file.ItemSpec) while the per-file sync markers log the device-relative target path, making the NotifySync marker family inconsistent within the task. Use GetAdbPushTargetPath for the abi-skip marker too so all markers report the same device-relative identifier. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The device-state readiness check built its probe with `printf '\noverride='` / `printf '\n'`. The `\n` escape does not survive the adb + device-shell quoting layers and arrives as a literal two-character "\n", so the device emits a single line: remote=<hash>\noverride=<hash>\n ParseDeviceManifestState splits on real newlines, so the whole tail lands in RemoteHash and OverrideHash stays empty. RemoteHash never equals the previous manifest hash, `remoteReady` is always false, and every incremental install resets the staging directory and redeploys *all* files (observed as `changed files: 329` in CI), so unchanged assemblies are reported as NotifySync CopyFile instead of SkipCopyFile. Build the probe with `echo` + command substitution instead, which emits real newlines and contains no backslash escapes to be mangled. Verified on an API 36 emulator: the device now returns two parseable lines and `remoteReady` becomes true on an unchanged reinstall. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ManifestEntry.LocalPath was written into manifest.json and folded into the device-side manifest hash, but it is host-specific (an absolute build path) and nothing reads it for change detection (GetChangedFiles compares Size and LastWriteTimeUtcTicks; the dictionary key is RelativePath). Persisting and hashing it bloats the manifest and couples the device readiness check to the build machine's paths for no benefit. Remove the field, its JSON property, and its contribution to the canonical manifest text. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
This is the cleaned-up follow-up to the experimentation PR:
This PR keeps legacy
FastDeployin place, adds a newFastDeploy2strategy, and makesFastDeploy2the default for app install fast deployment.FastDeployremains selectable as a fallback.Review follow-up
Jon's review feedback is addressed in the latest cleanup commit:
FastDeploy2Baseinheritance split.System.Text.Jsonsource-generated context./data/local/tmp/fastdeploy2instead of/tmp/fastdeploy2, because/tmpis read-only on CI emulators.A fresh
dotnet new mauiAndroid sample build binlog is available here:CI on-device test fixes
The first full CI run surfaced regressions in existing fast-deployment device tests now that
FastDeploy2is the default. Three deterministic root causes were found (each reproduced and verified on an API 36 emulator) and fixed:IncrementalFastDeploymentandSkipFastDevAlreadyInstalledFileassert per-fileNotifySync CopyFile/NotifySync SkipCopyFilemarkers. FastDeploy2 only loggedPrepared X => Y, so these tests failed even though deployment succeeded. FastDeploy2 now emits the markers from the manifest's changed-file set, and the abi-skip marker uses the same device-relative identifier for consistency.ln -sf "$s"/* ., which also symlinked staging subdirectories. Processing a child directory thencd'd through that symlink into the shell-owned/data/local/tmpstaging area, andrm -rf ./*failed with "Permission denied" underrun-as(XA0133/XA0129). This broke any app with culture/satellite subdirectories (LocalizedAssemblies_ShouldBeFastDeployed,DotNetNewAndroidTest). The full rewrite now clears and symlinks only the regular files in each directory and leaves subdirectories to their own iterations, so it no longer follows or deletes child directories.statformat split by the device shell.find ... -exec stat -c "%n|%s|%Y" {} +reached the device shell unquoted, so|was treated as a pipe (exit 127 →XA0129) and the copy fallback also failed. The command is now built as a single string with the format single-quoted (matching theRunAsShellquoting pattern).printf '\n'arrived as a literal\n. The readiness check built its probe withprintf '\noverride='/printf '\n'; the backslash escape does not survive the adb + device-shell quoting layers, so the device emittedremote=<hash>\noverride=<hash>\non a single line.ParseDeviceManifestState(splitting on real newlines) put the whole tail intoRemoteHashand leftOverrideHashempty, soremoteReadywas always false and every incremental install reset the staging directory and redeployed all files (changed files: 329in CI), reporting unchanged assemblies asCopyFile. The probe is now built withecho+ command substitution (real newlines, no backslash escapes); verified on an API 36 emulator thatremoteReadybecomes true on an unchanged reinstall.FastDeploy2 approach
FastDeploy2 uses the final direction from the experiment PR:
Build a local manifest of the deployment target and FastDev inputs:
Compare with the last successful deploy manifest, then verify that the device-side staging tree and app override tree both carry the expected manifest hash.
Push only changed files to device temp storage, without
adb push --sync:Remove stale staged files for removed inputs.
Maintain
files/.__override__as symlinks to staged files with batchedrun-as sh -ccommands. The generated shell scripts use shortd/svariables pluscdso long source/target directories are not repeated for every file.After a successful deploy, write the manifest and matching device-side manifest hash markers:
Why this shape is viable now
The existing FastDeploy v2.0 design was added when Android 11 broke the older external-storage based fast deployment model. At that time, adb compression and multi-file push/sync behavior were still new: Android SDK Platform-Tools 30.0.0 (April 2020) introduced client-side compression for
adb {push,pull,sync}when used with Android 11 devices, and Platform-Tools 30.0.5 (November 2020) later fixedadb push --syncwith multiple inputs and improved pushing many files over high-latency links.Today it is more reasonable to depend on a modern Android SDK Platform-Tools package for developer inner-loop scenarios.
adbis supplied by the Android SDK Platform-Tools package, and developers using .NET for Android / MAUI can update it independently of end-user device support. With Platform-Tools 36.0.0,adb push -z anywas also tested against API 24 and API 29 emulators that did not advertisesendrecv_v2*compression features; both accepted the command and transferred the file, apparently negotiating down to uncompressed transfer.This PR also no longer relies on
adb push --sync: the local manifest tells us exactly which files changed, so FastDeploy2 pushes only those files and avoids an adb-side scan of the full directory.What else we tried but abandoned
From the experiment PR:
FastDeployis excellent for very small changes but very slow for full payloads.adb push --syncover the whole directory is much faster for full payloads, but still scans/probes too much for tiny changes.adb pushper file is too slow because every adb invocation has ~40ms median overhead even when skipped.maui.link), so this PR does not include a native helper.How legacy FastDeploy could be improved instead
If we wanted to keep the native FastDeploy tool model, the most promising improvement would be to make
xamarin.syncbatch-aware. Instead of invokingrun-as ... xamarin.synconce per changed file, the host could open a singlerun-as <package> xamarin.sync-batchstdin stream and write all changed files one after another using a small binary protocol:The device helper would create directories, write each file to a temporary path, set mtime/permissions, rename into place, and remove stale files. This would keep the existing app-private real-file model and avoid symlink assumptions, while removing most per-file adb/run-as process overhead.
That approach is intentionally not part of this PR because it keeps the custom native tool/protocol complexity. FastDeploy2 tries the simpler path first: use adb's built-in transfer/compression support, keep the changed-file decision local, and leave legacy
FastDeployavailable as a fallback while we validate the symlink approach across more devices.Preliminary benchmark from the experiment PR
Device: physical Samsung Galaxy A16 (
SM-A165F,R58Y30HZ65V) connected by USB 3 cableProject:
samples/HelloWorld/HelloWorld/HelloWorld.DotNet.csprojValidation
Focused task builds pass in this branch:
Device/emulator validation passed:
SM-A165F,R58Y30HZ65V) over USB 3 cableSM-A165F,R58Y30HZ65V) over USB 3 cableSM-A165F,R58Y30HZ65V) over USB 3 cabled/sshell-variable symlink scriptsObserved validation details:
Additional adb compatibility probes with Platform-Tools 36.0.0:
adb push -z anycmdonly, nosendrecv_v2*cmdonly, nosendrecv_v2*sendrecv_v2,brotli,lz4,zstdRisks
FastDeployis still present and selectable.files/.__override__to/data/local/tmp/fastdeploy2/...worked on the tested physical Samsung device over USB.adb push -z any; older devices can still fall back to uncompressed transfer when driven by a modern adb.Future work
FastDeployimplementation once we are confident the new approach is better across a broader device matrix.FastDeployno longer needs them.xamarin.syncprotocol as the app-private real-file fallback design.