Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 79 additions & 20 deletions .github/scripts/inspect-packed-nupkgs.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,37 @@
# .nuspec that matches that package's own version (not merely "some bracketed
# string"), and (per ADR-0031 Amendment 1) a deliberate tested range - not a
# bare unbounded floor, not a blanket exact pin - on each integration
# package's third-party dependency.
# package's third-party dependency, read from Directory.Packages.props itself
# (issue #122) rather than a second hardcoded literal that Dependabot can't see.
set -euo pipefail

pack_output="${1:?usage: inspect-packed-nupkgs.sh <pack-output-dir>}"
work_dir=$(mktemp -d)
trap 'rm -rf "$work_dir"' EXIT

fail=0

# Evaluates Directory.Packages.props' <PackageVersion> items via `dotnet msbuild
# -getItem` (SDK-native, already a CI/dev dependency - no new tool) and returns
# the JSON verbatim. This is the one authoritative read of repository policy;
# every assert_dependency_range call below looks up its expected range from
# this same JSON rather than a duplicated string literal, so a Dependabot bump
# to Directory.Packages.props is automatically the new expected value.
load_authoritative_versions() {
local props_file="$1"
dotnet msbuild "$props_file" -nologo -getItem:PackageVersion
}

# Looks up a single dependency's authoritative version/range by its
# Directory.Packages.props <PackageVersion Include="..."> identity, from the
# JSON produced by load_authoritative_versions. Returns empty (not an error)
# when the id isn't found - callers must check for that distinctly from a
# packed-range mismatch, since "policy lookup failed" and "packed output is
# wrong" are different classes of failure (issue #122, point 10).
get_authoritative_range() {
local authoritative_json="$1"
local dep_id="$2"
jq -r --arg id "$dep_id" \
'.Items.PackageVersion[]? | select(.Identity == $id) | .Version' \
<<<"$authoritative_json" | head -1
}

extract() {
local nupkg="$1"
local dest="$2"
Expand Down Expand Up @@ -124,18 +146,47 @@ assert_dependency_range() {
local nuspec="$1"
local pkg_name="$2"
local dep_id="$3"
local expected_range="$4"
local authoritative_json="$4"
local expected_range
expected_range=$(get_authoritative_range "$authoritative_json" "$dep_id")
if [ -z "$expected_range" ]; then
# A distinct failure class from a packed-range mismatch below (issue #122,
# point 10): this means Directory.Packages.props itself has no
# <PackageVersion Include="$dep_id">, a validator/configuration problem,
# not evidence about what got packed.
echo "FAIL: could not determine authoritative PackageVersion for $dep_id in Directory.Packages.props" >&2
fail=1
return
fi
local version
version=$(grep -o "id=\"${dep_id}\" version=\"[^\"]*\"" "$nuspec" | head -1 | sed -E 's/.*version="([^"]*)".*/\1/')
if [ "$version" = "$expected_range" ]; then
echo "OK: $pkg_name's .nuspec constrains $dep_id to the intended tested range $version"
echo "OK: $pkg_name's .nuspec constrains $dep_id to the intended tested range $version (matches Directory.Packages.props)"
else
echo "FAIL: $pkg_name's .nuspec dependency on $dep_id is '$version', expected the intended tested range '$expected_range'" >&2
echo "FAIL: $pkg_name's .nuspec dependency on $dep_id is '$version', expected the intended tested range '$expected_range' (from Directory.Packages.props)" >&2
fail=1
fi
}

for pkg in Compono Compono.XunitV3 Compono.NSubstitute Compono.Bogus Compono.TUnit Compono.TestDoubles Compono.DependencyInjection Compono.Http Compono.MSTest; do
main() {
local pack_output="${1:?usage: inspect-packed-nupkgs.sh <pack-output-dir>}"
local script_dir
script_dir=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
local packages_props="$script_dir/../../Directory.Packages.props"
# Not `local`: the EXIT trap below still needs to see this after main()
# itself returns and the script falls off the end (bash pops local scope
# on function return, before the EXIT trap runs).
work_dir=$(mktemp -d)
trap 'rm -rf "$work_dir"' EXIT

local authoritative_json
authoritative_json=$(load_authoritative_versions "$packages_props") || {
echo "FAIL: could not evaluate $packages_props via dotnet msbuild" >&2
exit 1
}

local pkg nupkg extract_dir extra_paths nuspec
for pkg in Compono Compono.XunitV3 Compono.NSubstitute Compono.Bogus Compono.TUnit Compono.TestDoubles Compono.DependencyInjection Compono.Http Compono.MSTest; do
nupkg=$(find "$pack_output" -maxdepth 1 -iname "${pkg}.[0-9]*.nupkg" | head -1)
if [ -z "$nupkg" ]; then
echo "FAIL: no .nupkg found for $pkg in $pack_output" >&2
Expand Down Expand Up @@ -173,22 +224,22 @@ for pkg in Compono Compono.XunitV3 Compono.NSubstitute Compono.Bogus Compono.TUn
Compono.XunitV3)
assert_manifest_field "$nuspec" "$pkg" "title" "Compono — xUnit v3 Integration"
assert_exact_pin_dependency "$nuspec" "$pkg" "Compono"
assert_dependency_range "$nuspec" "$pkg" "xunit.v3.extensibility.core" "[3.2.2, 5.0.0)"
assert_dependency_range "$nuspec" "$pkg" "xunit.v3.extensibility.core" "$authoritative_json"
;;
Compono.NSubstitute)
assert_manifest_field "$nuspec" "$pkg" "title" "Compono — NSubstitute Integration"
assert_exact_pin_dependency "$nuspec" "$pkg" "Compono"
assert_dependency_range "$nuspec" "$pkg" "NSubstitute" "[6.2.0, 7.0.0)"
assert_dependency_range "$nuspec" "$pkg" "NSubstitute" "$authoritative_json"
;;
Compono.Bogus)
assert_manifest_field "$nuspec" "$pkg" "title" "Compono — Bogus Integration"
assert_exact_pin_dependency "$nuspec" "$pkg" "Compono"
assert_dependency_range "$nuspec" "$pkg" "Bogus" "[35.6.5, 36.0.0)"
assert_dependency_range "$nuspec" "$pkg" "Bogus" "$authoritative_json"
;;
Compono.TUnit)
assert_manifest_field "$nuspec" "$pkg" "title" "Compono — TUnit Integration"
assert_exact_pin_dependency "$nuspec" "$pkg" "Compono"
assert_dependency_range "$nuspec" "$pkg" "TUnit.Core" "[1.65.63, 2.0.0)"
assert_dependency_range "$nuspec" "$pkg" "TUnit.Core" "$authoritative_json"
;;
Compono.TestDoubles)
assert_manifest_field "$nuspec" "$pkg" "title" "Compono — Generated Test Doubles"
Expand All @@ -211,14 +262,22 @@ for pkg in Compono Compono.XunitV3 Compono.NSubstitute Compono.Bogus Compono.TUn
Compono.MSTest)
assert_manifest_field "$nuspec" "$pkg" "title" "Compono — MSTest Integration"
assert_exact_pin_dependency "$nuspec" "$pkg" "Compono"
assert_dependency_range "$nuspec" "$pkg" "MSTest.TestFramework" "[4.0.0, 5.0.0)"
assert_dependency_range "$nuspec" "$pkg" "MSTest.TestFramework" "$authoritative_json"
;;
esac
done
done

if [ "$fail" -ne 0 ]; then
echo "One or more package-contents assertions failed." >&2
exit 1
fi
if [ "$fail" -ne 0 ]; then
echo "One or more package-contents assertions failed." >&2
exit 1
fi

echo "All package-contents assertions passed."
echo "All package-contents assertions passed."
}

# Sourced (by the regression-test script) vs. executed directly: only run main
# when this file is the actual entry point, so tests can source it to reach
# the functions above without triggering a real pack-output scan.
if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then
main "$@"
fi
142 changes: 142 additions & 0 deletions .github/scripts/inspect-packed-nupkgs.tests.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
#!/usr/bin/env bash
# Regression coverage for inspect-packed-nupkgs.sh's authoritative-range lookup
# (issue #122): proves the validator derives its expected dependency range from
# Directory.Packages.props itself, not a hardcoded literal that Dependabot
# can't update. No test framework is set up for shell scripts in this repo, so
# this is a plain, dependency-free bash script - source the real script (its
# main() only runs when executed directly, not sourced) and exercise its
# functions directly against small fixture files.
set -euo pipefail

script_dir=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
# shellcheck source=./inspect-packed-nupkgs.sh
source "$script_dir/inspect-packed-nupkgs.sh"

work_dir=$(mktemp -d)
trap 'rm -rf "$work_dir"' EXIT

tests_failed=0

make_props() {
local path="$1"
local dep_id="$2"
local range="$3"
cat >"$path" <<EOF
<Project>
<ItemGroup>
<PackageVersion Include="$dep_id" Version="$range" />
</ItemGroup>
</Project>
EOF
}

make_nuspec() {
local path="$1"
local dep_id="$2"
local range="$3"
cat >"$path" <<EOF
<?xml version="1.0"?>
<package>
<metadata>
<dependencies>
<group>
<dependency id="$dep_id" version="$range" exclude="Build,Analyzers" />
</group>
</dependencies>
</metadata>
</package>
EOF
}

expect_pass() {
local description="$1"
fail=0
"${@:2}"
if [ "$fail" -eq 0 ]; then
echo "PASS: $description"
else
echo "TEST FAILURE: expected $description to pass, but it failed" >&2
tests_failed=1
fi
}

expect_fail() {
local description="$1"
fail=0
"${@:2}" >/dev/null 2>&1 || true
if [ "$fail" -ne 0 ]; then
echo "PASS: $description"
else
echo "TEST FAILURE: expected $description to fail, but it passed" >&2
tests_failed=1
fi
}

# 1. Passing case: the packed nuspec matches Directory.Packages.props exactly.
props_current="$work_dir/props-current.props"
nuspec_matching="$work_dir/matching.nuspec"
make_props "$props_current" "TUnit.Core" "[1.65.63, 2.0.0)"
make_nuspec "$nuspec_matching" "TUnit.Core" "[1.65.63, 2.0.0)"
json_current=$(load_authoritative_versions "$props_current")
expect_pass "packed range matching current authoritative range" \
assert_dependency_range "$nuspec_matching" "Compono.TUnit" "TUnit.Core" "$json_current"

# 2. Failing case: the packed nuspec disagrees with the authoritative range.
nuspec_stale="$work_dir/stale.nuspec"
make_nuspec "$nuspec_stale" "TUnit.Core" "[1.65.38, 2.0.0)"
expect_fail "packed range disagreeing with authoritative range" \
assert_dependency_range "$nuspec_stale" "Compono.TUnit" "TUnit.Core" "$json_current"

# 3. The original bug class (issue #122): bump ONLY the authoritative props
# file (as Dependabot would) and re-pack with the SAME new range, with no
# validator literal touched anywhere - validation must still pass.
props_bumped="$work_dir/props-bumped.props"
nuspec_bumped="$work_dir/bumped.nuspec"
make_props "$props_bumped" "TUnit.Core" "[1.65.99, 2.0.0)"
make_nuspec "$nuspec_bumped" "TUnit.Core" "[1.65.99, 2.0.0)"
json_bumped=$(load_authoritative_versions "$props_bumped")
expect_pass "Dependabot-style bump: nuspec and Directory.Packages.props move together" \
assert_dependency_range "$nuspec_bumped" "Compono.TUnit" "TUnit.Core" "$json_bumped"

# 4. A bump to the authoritative range that the packed nuspec does NOT reflect
# must still fail - this is not "always pass," the validator still compares
# independently against packed output.
nuspec_not_rebumped="$work_dir/not-rebumped.nuspec"
make_nuspec "$nuspec_not_rebumped" "TUnit.Core" "[1.65.63, 2.0.0)"
expect_fail "authoritative range bumped but packed nuspec left behind" \
assert_dependency_range "$nuspec_not_rebumped" "Compono.TUnit" "TUnit.Core" "$json_bumped"

# 5. Distinct diagnostic: the authoritative lookup itself fails (no matching
# PackageVersion in Directory.Packages.props) - a validator/configuration
# error, not a "packed range mismatch".
props_missing="$work_dir/props-missing.props"
make_props "$props_missing" "SomeOtherPackage" "[1.0.0, 2.0.0)"
json_missing=$(load_authoritative_versions "$props_missing")
missing_output=$(fail=0; assert_dependency_range "$nuspec_matching" "Compono.TUnit" "TUnit.Core" "$json_missing" 2>&1 || true)
if echo "$missing_output" | grep -q "could not determine authoritative PackageVersion for TUnit.Core"; then
echo "PASS: missing authoritative entry produces a distinct diagnostic"
else
echo "TEST FAILURE: expected a distinct 'could not determine authoritative PackageVersion' message, got:" >&2
echo "$missing_output" >&2
tests_failed=1
fi

# 6. Sanity check against the real repository policy file, so this test suite
# breaks if Directory.Packages.props' shape (Identity/Version JSON) ever stops
# being what the validator expects - independent of any specific package.
repo_root="$script_dir/../.."
real_json=$(load_authoritative_versions "$repo_root/Directory.Packages.props")
real_range=$(get_authoritative_range "$real_json" "NSubstitute")
if [ -n "$real_range" ]; then
echo "PASS: authoritative lookup resolves a real range for NSubstitute from the repository's own Directory.Packages.props ($real_range)"
else
echo "TEST FAILURE: could not resolve NSubstitute's range from the real Directory.Packages.props" >&2
tests_failed=1
fi

if [ "$tests_failed" -ne 0 ]; then
echo "One or more inspect-packed-nupkgs.sh regression tests failed." >&2
exit 1
fi

echo "All inspect-packed-nupkgs.sh regression tests passed."
6 changes: 6 additions & 0 deletions .github/workflows/package-validation.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,12 @@ jobs:
dotnet build "$csproj" -c Release -p:WarningsAsErrors=CS1591
done

- name: Test inspect-packed-nupkgs.sh itself
# Regression coverage for issue #122 (dependency-range-literal drift) -
# runs before the real inspection below so a broken validator fails
# fast with its own diagnostic, not a confusing downstream mismatch.
run: .github/scripts/inspect-packed-nupkgs.tests.sh

- name: Inspect packed .nupkg contents
run: .github/scripts/inspect-packed-nupkgs.sh "$PACK_OUTPUT"

Expand Down
43 changes: 43 additions & 0 deletions docs/adr/0031-public-preview-release-and-versioning-policy.md
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,49 @@ which TFM gets added/dropped going forward at the *newest* end
starts two releases further back than the original two-TFM window assumed,
per ADR-0038's own reasoning for why that trade-off was accepted.

## Amendment 4 (2026-09-02): dependency-range check derives its expected value from `Directory.Packages.props`, not a duplicated literal

Amendment 1 above established the tested-range policy for each integration
package's third-party dependency (`[6.2.0, 7.0.0)` for `NSubstitute`, etc.).
Its enforcement, `.github/scripts/inspect-packed-nupkgs.sh`'s
`assert_dependency_range`, originally took that expected range as a second
hardcoded string literal in the script itself, duplicating the
`Directory.Packages.props` `PackageVersion` that already states the same
policy for MSBuild's own restore/pack purposes.

This surfaced as [issue #122](https://github.com/LayeredCraft/compono/issues/122):
Dependabot PR #121 bumped `TUnit.Core` from `[1.65.38, 2.0.0)` to
`[1.65.63, 2.0.0)` in `Directory.Packages.props`; `Compono.TUnit` packed the
new, correct range; the validator's stale duplicate literal still expected
the old range and failed a correctly-packed package. Commit `5cfe446`
corrected that one occurrence, but the structural bug — two independently-
maintained copies of the same range, one of which Dependabot cannot see —
remained for all five dependency-range checks (`xunit.v3.extensibility.core`,
`NSubstitute`, `Bogus`, `TUnit.Core`, `MSTest.TestFramework`).

**The fix:** `assert_dependency_range` now reads its expected range directly
from `Directory.Packages.props`, evaluated via `dotnet msbuild
Directory.Packages.props -getItem:PackageVersion` (SDK-native, already a CI
dependency — no new tool) and looked up by the dependency's package ID
(`jq`, already used elsewhere in `package-validation.yaml`). The explicit
package → third-party-dependency-ID mapping in the script's `case`
statement is unchanged — only the *range* is no longer duplicated. The
validator still independently compares this authoritative value against the
actually-packed `.nuspec` (extracted from the real `.nupkg`, not trusted
as-is) — a Dependabot bump to `Directory.Packages.props` is now
automatically the new expected value, but an incorrectly-packed `.nuspec`
that doesn't reflect that value still fails, exactly as before. A missing
`Directory.Packages.props` entry for a checked dependency now fails with a
distinct diagnostic ("could not determine authoritative PackageVersion for
...") rather than being silently mistaken for a packed-range mismatch.
`.github/scripts/inspect-packed-nupkgs.tests.sh` (new) regression-tests both
the passing and failing cases directly against the script's functions,
including the exact bump-only-the-props-file scenario above.

This is an implementation fix for how Amendment 1's policy is *enforced*,
not a change to the policy itself — the tested-range semantics, exclusive
upper bounds, and package/dependency mapping are all unchanged.

## Links

- [ADR-0001](0001-source-generation-first.md) — source-generation-first
Expand Down
Loading