Skip to content

Commit 793c89b

Browse files
committed
Also check the pack from a clean output directory
The loader check added alongside the packaging change runs on the nupkg CI publishes, and CI builds before it packs. So `$(OutputPath)` is already populated by then, and a return to gathering the closure with a wildcard over that directory would still produce a complete package and still pass. Measured: with the wildcard restored, packing the built tree yields all ten assemblies and the check passes, while packing into an empty output directory yields two and the plugin fails to load. The check could not catch the regression it exists to catch. Pack again into an output directory that is guaranteed empty, and run the same loader check on that package. Relocating `$(OutputPath)` rather than reordering the CI steps means the check keeps testing the cold condition however the job is rearranged later. Verified by mutation: with the wildcard restored, the new check fails with the same `ReflectionTypeLoadException` a consumer would see, while the check on the published artefact still passes - so the two are testing different things and both are worth having. Found by Codex.
1 parent 2fcb08a commit 793c89b

2 files changed

Lines changed: 44 additions & 0 deletions

File tree

.github/workflows/ci.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,11 @@ jobs:
171171
# package has to carry the plugin's dependency closure. Nothing else here would notice if it
172172
# stopped doing so; the consumer's build would be the first to find out.
173173
run: nix develop --command ./scripts/check-packed-plugin-loads.sh WoofWare.Myriad.Plugins/bin/Release/WoofWare.Myriad.Plugins.*.nupkg
174+
- name: Check the plugin also packs from a clean output directory
175+
# The step above cannot catch a return to gathering the closure with a wildcard over
176+
# $(OutputPath), because the Build step above has already filled that directory. This one
177+
# packs into an empty one, which is the condition that used to produce a broken package.
178+
run: nix develop --command ./scripts/check-cold-pack-loads.sh
174179
- name: Upload NuGet artifact (plugin)
175180
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
176181
with:

scripts/check-cold-pack-loads.sh

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
#!/bin/sh
2+
3+
# The plugin's dependency closure must reach the package by a route that does not depend on the
4+
# build directory already being populated. It used not to: the closure was gathered by a wildcard
5+
# over $(OutputPath), which MSBuild expands when the project is evaluated, before the build that
6+
# fills that directory. A package built that way carries only the plugin itself and fails to load.
7+
#
8+
# Checking the nupkg that CI publishes cannot catch a regression to that, because CI builds before
9+
# it packs, so $(OutputPath) is populated by then and even the wildcard would find everything. So
10+
# pack again into an output directory that is guaranteed empty, and check that package instead.
11+
# Relocating $(OutputPath) rather than reordering the CI steps keeps this honest no matter what
12+
# else the job has done first.
13+
14+
set -eu
15+
16+
if [ "$#" -ne 0 ]; then
17+
echo "usage: $0" >&2
18+
exit 1
19+
fi
20+
21+
repo=$(CDPATH='' cd -- "$(dirname -- "$0")/.." && pwd)
22+
23+
workdir=$(mktemp -d)
24+
# shellcheck disable=SC2064 # expand workdir now, while it is still set
25+
trap "rm -rf '$workdir'" EXIT
26+
27+
dotnet pack "$repo/WoofWare.Myriad.Plugins/WoofWare.Myriad.Plugins.fsproj" \
28+
--configuration Release \
29+
-p:BaseOutputPath="$workdir/bin/" \
30+
--output "$workdir/nupkg"
31+
32+
# shellcheck disable=SC2086 # deliberate: expand the glob into the argument list to count matches
33+
set -- "$workdir"/nupkg/*.nupkg
34+
if [ "$#" -ne 1 ] || [ ! -f "$1" ]; then
35+
echo "expected exactly one nupkg from the clean-output pack, got: $*" >&2
36+
exit 1
37+
fi
38+
39+
"$repo/scripts/check-packed-plugin-loads.sh" "$1"

0 commit comments

Comments
 (0)