Found while reviewing #2528 (the upgrade-darling.ps1 work for #2525). Filed rather than fixed there, because the fix has a failure mode that needs designing rather than bolting on.
What
An in-place upgrade is an overlay, not a replacement. Expand-Archive -Force (and the Copy-Item -Recurse -Force path for a folder source) overwrite what the new build ships and delete nothing else. So any file the old version had and the new one dropped stays in the install tree forever:
- a dependency removed between versions
- a renamed assembly (both names now present, the old one never loaded — or worse, loaded)
- a satellite-resource directory for a culture our dependencies no longer localize into
- anything a hotfix once dropped in by hand
Nothing reports it either. DarlingInstallDirectoryReport (#2185, and now #2525) walks top-level directories, so a stale DLL sitting in the install root, or inside viewer\ / wwwroot\ / runtimes\, is structurally invisible to it — those are directories the product owns, and the report never looks inside them.
This is not new to upgrade-darling.ps1; it is what the hand-run procedure has always done. The script just makes it worth naming, because every other partial-state case in that path is now accounted for (half-extracted trees, future-dated backups, mixed subdirectories after a failed copy) and this one is the remaining hole.
Why it is not obviously trivial
The obvious repair — diff the new build's file manifest against the install root and report the remainder — has to know about every file that legitimately lives there and was never in a zip:
darling.json (the zip ships only darling.sample.json)- the DPAPI credential blobs, and the
.bak-* config backups
- the
_rollback_manual_* directories the deploy script itself creates
pg-runtime\, extracted on first run, plus pg-runtime-prev\pg-runtime.zip- whatever an operator legitimately put there
Get that list wrong in the loud direction and it warns about darling.json on every single upgrade — which is #2525 again with a new subject, and the same guard-stops-guarding-by-being-too-loud outcome. Get it wrong in the quiet direction and it is a check nobody can trust.
Options
- Report only, from the manifest. After the copy, list files under the install root that the new build did not ship and that are not on the known-legitimate list. Cheap; the whole risk is the allowlist.
- Report only, from the previous zip. Diff old zip against new zip and name what was removed between the two versions — a much smaller, much more precise set, and it needs no allowlist at all because it only ever names files that came from one of our own zips. Requires the operator to still have (or the script to fetch) the previous zip.
- Clean-room extract. Extract the new build to a sibling directory, move the preserved files across (
darling.json, credentials, pg-runtime\), swap directories. Correct by construction, and by far the biggest change — it also has a commit point, which is what makes DarlingStoreUpgrade's equivalent hard.
- Accept and document. Where it stands today, in a comment at the copy in
upgrade-darling.ps1.
Option 2 looks the strongest per unit of risk: it cannot produce a false positive, because everything it can name provably came out of one of our zips.
What should decide it
Whether this has actually happened. It is measurable rather than arguable: take the file list from two consecutive release zips and diff them. If no release in the last year removed a shipped file, this is a documented gap and nothing more. If several did, option 2 earns itself. Worth running that diff before writing any code.
Where it is documented now
Darling/tools/upgrade-darling.ps1, in a comment immediately above the copy step, so the next reader finds it at the point it matters rather than in an issue they would have to know to look for.
Found while reviewing #2528 (the
upgrade-darling.ps1work for #2525). Filed rather than fixed there, because the fix has a failure mode that needs designing rather than bolting on.What
An in-place upgrade is an overlay, not a replacement.
Expand-Archive -Force(and theCopy-Item -Recurse -Forcepath for a folder source) overwrite what the new build ships and delete nothing else. So any file the old version had and the new one dropped stays in the install tree forever:Nothing reports it either.
DarlingInstallDirectoryReport(#2185, and now #2525) walks top-level directories, so a stale DLL sitting in the install root, or insideviewer\/wwwroot\/runtimes\, is structurally invisible to it — those are directories the product owns, and the report never looks inside them.This is not new to
upgrade-darling.ps1; it is what the hand-run procedure has always done. The script just makes it worth naming, because every other partial-state case in that path is now accounted for (half-extracted trees, future-dated backups, mixed subdirectories after a failed copy) and this one is the remaining hole.Why it is not obviously trivial
The obvious repair — diff the new build's file manifest against the install root and report the remainder — has to know about every file that legitimately lives there and was never in a zip:
darling.json(the zip ships onlydarling.sample.json).bak-*config backups_rollback_manual_*directories the deploy script itself createspg-runtime\, extracted on first run, pluspg-runtime-prev\pg-runtime.zipGet that list wrong in the loud direction and it warns about
darling.jsonon every single upgrade — which is #2525 again with a new subject, and the same guard-stops-guarding-by-being-too-loud outcome. Get it wrong in the quiet direction and it is a check nobody can trust.Options
darling.json, credentials,pg-runtime\), swap directories. Correct by construction, and by far the biggest change — it also has a commit point, which is what makesDarlingStoreUpgrade's equivalent hard.upgrade-darling.ps1.Option 2 looks the strongest per unit of risk: it cannot produce a false positive, because everything it can name provably came out of one of our zips.
What should decide it
Whether this has actually happened. It is measurable rather than arguable: take the file list from two consecutive release zips and diff them. If no release in the last year removed a shipped file, this is a documented gap and nothing more. If several did, option 2 earns itself. Worth running that diff before writing any code.
Where it is documented now
Darling/tools/upgrade-darling.ps1, in a comment immediately above the copy step, so the next reader finds it at the point it matters rather than in an issue they would have to know to look for.