ci(win): prove the Store package loads before shipping it - #323
Merged
Conversation
Two Windows releases in a row shipped a dependency that could not be resolved on
the target machine, and both times someone outside the project found it. 1.9.0's
compositor addon was reached through PATH, which MSIX ignores, and the editor
opened with a blank preview while audio played. 1.9.1's capture helper needed the
Visual C++ Redistributable, which is not part of Windows, and Store certification
rejected it with recording unusable.
Each fix arrived with a guard aimed at the failure already understood. The
colocation check would never have caught the redistributable; the import-table
check would never have caught the PATH bug. Both are worth keeping and neither
generalises, which is the actual problem: a static check only ever knows about
the mistakes already made.
So ask the Windows loader instead. verify-appx-native.ps1 registers a built appx
and, from inside the package, calls LoadLibraryEx with LOAD_WITH_ALTERED_SEARCH_PATH
on every .dll/.node — the same call Node makes for an addon — and starts each
helper executable with no arguments. Whatever the next unresolvable dependency
turns out to be, this fails on it. The Windows Store job runs it on every build,
enabling Developer Mode on the runner it is about to discard.
It deliberately records nothing. A real capture needs a GPU and a desktop session
that a runner does not usefully have, and a flaky gate gets switched off. The
loader is what broke both times and it can be tested with neither: a helper the
loader rejects produces NO output and exits 0xC0000135, while one that reaches
main() prints its usage. That distinction needs no hardware.
Verified both ways on the 1.9.1 package. All 17 binaries load and all three
helpers reach main() under package identity. With avutil-60.dll removed from the
package, 7 of them fail with ERROR_MOD_NOT_FOUND, compositor_view.node among
them — the negative case matters more than the positive one here, and it caught a
real defect in the first version of this script: `@(... | ConvertFrom-Json)` looks
like it produces an array and does not, because Windows PowerShell writes the
whole deserialized array to the pipeline as one object. `Where-Object { -not
$_.ok }` was evaluating `-not` against an array of booleans, which is always
false. It reported "All 1 native binaries load" for seventeen of them and would
have reported success no matter what the probe found.
The documentation gains the part none of this covers. Every failure here shares
one shape — the machines we test on have more installed than the machines we ship
to — so "it works here" is not evidence about anything. Three layers, in order of
preference: remove the dependency at the source, prove what remains automatically,
and keep a virtual machine whose whole value is what is not installed in it. That
last one is the only thing that catches a failure nobody has thought of yet, and
this change does not replace it.
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Two Windows releases in a row shipped a dependency that could not be resolved on the target machine, and both times someone outside the project found it:
PATH, which MSIX ignoresEach fix arrived with a guard aimed at the failure already understood. The colocation check would never have caught the redistributable. The import-table check would never have caught the
PATHbug. Both are worth keeping, and neither generalises — a static check only knows about the mistakes already made.What
Ask the Windows loader instead of asserting a pattern.
scripts/verify-appx-native.ps1registers a built.appxand, from inside the package, callsLoadLibraryExwithLOAD_WITH_ALTERED_SEARCH_PATHon every.dll/.node— the same call Node makes for an addon — and starts each helper executable with no arguments. Whatever the next unresolvable dependency turns out to be, this fails on it. TheWindows Store packagejob runs it on every build.It deliberately records nothing. A real capture needs a GPU and a desktop session a runner does not usefully have, and a flaky gate gets switched off. The loader is what broke both times, and it can be tested with neither: a helper the loader rejects produces no output and exits
0xC0000135, while one that reachesmain()prints its usage. That distinction needs no hardware.Verified both ways on the 1.9.1 package
Positive — all 17 binaries, all three helpers reaching
main()under package identity:Negative — the same package with
avutil-60.dllremoved:The negative case matters more, and it earned its keep immediately: it caught a real defect in the first version of this script.
@(... | ConvertFrom-Json)looks like it produces an array and does not — Windows PowerShell writes the whole deserialized array to the pipeline as a single object, soWhere-Object { -not $_.ok }was evaluating-notagainst an array of booleans, which is always false. It printedAll 1 native binaries loadfor seventeen of them and would have reported success no matter what the probe found. Fixed, and the count is now asserted as well.Documentation
The new section says the part none of this covers. Every failure here shares one shape — the machines we test on have more installed than the machines we ship to — so "it works here" is not evidence about anything, however many times it is repeated. Three layers, in order of preference:
This PR is layer 2. It does not replace layer 3, and the documentation says so, along with how to keep such a VM (snapshot it before anything is installed; never put a toolchain in it).