Skip to content

Commit ad5cf59

Browse files
[build] skip DownloadOneFileWithRetry on no-op builds (#11693)
A no-op build binlog showed `DownloadOneFileWithRetry` running 17 times for a total of ~20s on a 77s no-op build: every invocation re-hashed an already-cached file (e.g. xamarin-android-toolchain-L_18.1.6-8.0.0-1.7z, build-tools_r36_macosx.zip) just to confirm the SHA-256 still matches. The Hash MSBuild task showed maxDurationMs=2005 per call across 32 invocations. Nothing changes on a no-op build, so this work was pure waste. Add a per-file stamp file (`<cache>/<file>.<SHA>.stamp`, or `.stamp` when no hash is supplied) that gets touched once the download and SHA verification succeed, and gate the target with: Inputs=`$(MSBuildThisFileFullPath)` Outputs=`$(_DownloadPath);$(_DownloadStamp)` so MSBuild''s incremental engine skips the target - and the expensive re-hash - on no-op builds. Self-heal is preserved: * Cached file deleted -> output missing -> target re-runs. * `_DownloadSha256` updated in a caller -> stamp file name changes -> new stamp output missing -> target re-runs and re-verifies. * This .targets file itself modified -> Inputs newer -> target re-runs. The Touch lands after the SHA Error gate so a mismatch still fails the build with the existing message and never persists a stale stamp. ### [build] only stamp file in Outputs; add self-heal helper Listing the cached file in `Outputs` alongside the stamp defeats the no-op gate when cache files restored from CI caches keep their original old timestamps: Inputs.max (this targets file) > Outputs.min (the cache file from a year ago) -> target re-runs and re-hashes every build. Drop the cache file from `Outputs` and add `_PrepareDownloadOneFileWithRetry` as `DependsOnTargets`. It always runs (no Inputs/Outputs of its own) and deletes the stamp when the cached file is gone, so MSBuild re-runs DownloadOneFileWithRetry with a now-incomplete output set. Verified end-to-end against `Xamarin.Android.sln`: BEFORE: 18 invocations, 22.53s of GetFileHash work on a no-op build (matches the user's reference msbuild.binlog at 20.08s) AFTER : 18 invocations, 1ms total - ALL skipped via Inputs/Outputs. Build wall: 101.94s -> 62.39s (~40s saved). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 313da6b commit ad5cf59

1 file changed

Lines changed: 46 additions & 5 deletions

File tree

build-tools/scripts/DownloadFileWithRetry.targets

Lines changed: 46 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,23 @@
4949
whose contents drove the expected hash)
5050
so the next build re-fetches them.
5151
52+
Incremental no-op gate:
53+
A per-file stamp file next to the cached download
54+
(`$(_DownloadFolder)\$(_DownloadFileName).$(_DownloadSha256).stamp`,
55+
or `.stamp` when no hash is supplied) is touched once the download
56+
and SHA verification succeed. The target's `Inputs`/`Outputs` then
57+
lets MSBuild skip re-running it - and re-hashing the cached file -
58+
on no-op builds. The expected SHA is encoded in the stamp file
59+
name so changing `_DownloadSha256` in a caller invalidates the
60+
cached stamp and forces re-verification. Self-heal for a missing
61+
cached file is preserved by `_PrepareDownloadOneFileWithRetry`,
62+
which always runs and drops the stamp if the cached file is gone -
63+
so this target's `Outputs` becomes incomplete and MSBuild forces
64+
it to re-run. Only the stamp is listed in `Outputs`: including the
65+
cached file there would defeat the gate, because cache files
66+
restored from CI caches keep their original old timestamps and
67+
would always look out of date relative to this targets file.
68+
5269
Usage pattern. The `_DownloadFile` item group must be built inside the
5370
caller's target body (so the items are visible to `<MSBuild>`). Each
5471
item carries its per-file parameters as `AdditionalProperties` metadata,
@@ -81,13 +98,20 @@
8198
<DownloadFileWithRetryFile>$(MSBuildThisFileFullPath)</DownloadFileWithRetryFile>
8299
<_DownloadRetries>5</_DownloadRetries>
83100
<_DownloadRetryDelayMilliseconds>5000</_DownloadRetryDelayMilliseconds>
101+
<!-- Computed up-front (vs inside the target body) so the per-file
102+
destination and stamp paths are visible to the target's
103+
Inputs/Outputs incremental gate below. Each `<MSBuild>` build
104+
request that calls into this file gets its own evaluated
105+
scope with these AdditionalProperties already set. -->
106+
<_DownloadPath>$(_DownloadFolder)\$(_DownloadFileName)</_DownloadPath>
107+
<_DownloadStamp Condition=" '$(_DownloadSha256)' != '' ">$(_DownloadPath).$(_DownloadSha256).stamp</_DownloadStamp>
108+
<_DownloadStamp Condition=" '$(_DownloadSha256)' == '' ">$(_DownloadPath).stamp</_DownloadStamp>
84109
</PropertyGroup>
85110

86-
<Target Name="DownloadOneFileWithRetry">
87-
<PropertyGroup>
88-
<_DownloadPath>$(_DownloadFolder)\$(_DownloadFileName)</_DownloadPath>
89-
</PropertyGroup>
90-
111+
<Target Name="DownloadOneFileWithRetry"
112+
DependsOnTargets="_PrepareDownloadOneFileWithRetry"
113+
Inputs="$(MSBuildThisFileFullPath)"
114+
Outputs="$(_DownloadStamp)">
91115
<!-- Self-heal: if a cached file already exists AND we know the expected
92116
hash, verify it up-front. On mismatch, delete (along with any
93117
CleanupOnMismatch siblings) so the download attempts below actually
@@ -146,6 +170,23 @@
146170
Condition=" '$(_DownloadSha256)' != '' and '$(_DownloadActualSha256)' != '$(_DownloadSha256)' "
147171
Text="SHA256 mismatch for $(_DownloadFileName). Expected: $(_DownloadSha256) Actual: $(_DownloadActualSha256)"
148172
/>
173+
174+
<!-- Record success so subsequent no-op builds can skip this target via
175+
the Inputs/Outputs gate above (and avoid re-hashing the cached
176+
file). Reached only when the SHA check above passed - or when no
177+
hash was supplied to verify against. -->
178+
<Touch Files="$(_DownloadStamp)" AlwaysCreate="True" />
179+
</Target>
180+
181+
<!-- Self-heal helper for the Inputs/Outputs gate on DownloadOneFileWithRetry.
182+
Always runs (no Inputs/Outputs of its own) and is cheap: a single Exists
183+
check per file. If the stamp claims a previous successful validation but
184+
the cached file is gone, we drop the stamp so the parent target sees its
185+
Outputs as incomplete and re-runs to re-fetch. -->
186+
<Target Name="_PrepareDownloadOneFileWithRetry">
187+
<Delete Condition=" Exists('$(_DownloadStamp)') and !Exists('$(_DownloadPath)') "
188+
Files="$(_DownloadStamp)"
189+
/>
149190
</Target>
150191

151192
</Project>

0 commit comments

Comments
 (0)