fix: pack content at clean paths on every platform, and guard it in CI - #57
Conversation
PackagePath separators are platform-dependent in a way that hides the
defect on Windows. A trailing backslash in PackagePath="trellis\" is
recognized as a directory marker on Windows, so the doc packs to
trellis/<name>.md. On Linux the backslash is not a separator: it
normalizes to "trellis/" and NuGet then appends its own, so the doc packs
to the malformed "trellis//<name>.md".
That malformed path still satisfies the trellis/*.md glob the copy logic
uses, so documentation is still delivered and every functional test stays
green. Nothing asserted the exact entry path, so the defect reached
published packages: Trellis.Microservices.Abstractions 0.1.0-alpha.72 on
nuget.org carries trellis// on both of its references.
Converts every PackagePath in the repository to forward slashes. This
also covers the root markers PackagePath="\" and PackagePath="\README.md"
used for icon.png and the NuGet readme. Those forms are not actually
broken today, but a single rule - no backslashes in PackagePath - is
easier to hold than "no trailing backslash, except the root one", and it
matches the sibling Trellis.ResourceNaming repository.
Adds build/test-package-layout.ps1 with two checks, because neither alone
is sufficient:
1. Declarations - no PackagePath contains a backslash. This is
platform-independent, so it holds the line on a developer's Windows
machine where check 2 cannot fail.
2. Packed entries - every entry is a clean relative path and every
reference sits at trellis/<name>.md. This is the real behaviour, but
it can only fail on Linux.
The declaration check parses the XML rather than scanning text. A
line-based regex missed two of three planted violations - the
single-quoted attribute form and the <PackagePath> metadata element - and
would also have flagged the comments that quote the malformed value on
purpose to explain the defect. XDocument handles all of these and
supplies line numbers through IXmlLineInfo.
Not every package ships a reference, so an empty set for one package is
legitimate and is not an error. The vacuous case, where nothing anywhere
ships a reference and every per-package assertion is trivially true, is
caught once after the loop.
The CI step is deliberately unconditional even though the preceding Pack
step is gated on `!github.base_ref`. On pull requests Pack is skipped, so
the packed check reports SKIP while the declaration check still runs; on
pushes to main both run against the real Linux-packed bytes.
Verified: the script exits 1 against the defective published packages and
lists their trellis// entries, exits 0 against newly packed fixed
packages, and exits 0 with SKIP when artifacts/ is absent. Packed output
still places README and icon at the package root. 352/352 tests pass.
There was a problem hiding this comment.
Pull request overview
This PR fixes a cross-platform NuGet packing defect where backslashes in PackagePath can produce malformed trellis//<name>.md entries on Linux, and adds a CI guard to prevent regressions across platforms.
Changes:
- Normalize all
PackagePathvalues to forward slashes in project/props/targets files to ensure consistent packed paths across Windows and Linux. - Add
build/test-package-layout.ps1to validate (1)PackagePathdeclarations and (2) the actual packed.nupkgentry paths/placement. - Run the package-layout verification step in CI unconditionally (declaration check always; packed-bytes check runs when
artifacts/exists).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Trellis.Yarp/src/Trellis.Yarp.csproj | Switch NuGet README PackagePath from \ to / for platform-stable root packing. |
| Trellis.Microservices.AspNetCore/src/Trellis.Microservices.AspNetCore.csproj | Switch NuGet README PackagePath from \ to / for platform-stable root packing. |
| Trellis.Microservices.Abstractions/src/Trellis.Microservices.Abstractions.csproj | Normalize README PackagePath to forward slashes. |
| Directory.Build.targets | Normalize packed doc/targets PackagePath values to forward slashes; document the Linux trellis// failure mode. |
| Directory.Build.props | Normalize packed icon PackagePath to /. |
| build/test-package-layout.ps1 | New script validating PackagePath declarations and packed entry layout. |
| .github/workflows/build.yml | Add unconditional “Verify package layout” step to enforce the guard in CI. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| @@ -0,0 +1,131 @@ | |||
| #!/usr/bin/env pwsh | |||
The file is UTF-8 with BOM per repository convention, so the kernel's binfmt_script handler never sees '#!' as the first two bytes and the shebang cannot work on Linux or macOS. It implied the script was directly executable when it is not. CI invokes it as 'pwsh build/test-package-layout.ps1', so nothing depended on it. Matches lint-api-reference.ps1 in this repository, which is BOM-encoded with no shebang. The comment-based help immediately below still documents the script. Raised in review on PR #57.
|
Good catch, and confirmed — Linux's Auditing every
The framework one was in scope because that PR already edits the file. |
Companion to xavierjohn/Trellis#713, which fixes the same defect in the framework.
PackagePathseparators are platform-dependent in a way that hides the defect on Windows. A trailing backslash inPackagePath="trellis\"is recognized as a directory marker on Windows, so the doc packs totrellis/<name>.md. On Linux the backslash is not a separator: it normalizes totrellis/and NuGet then appends its own, so the doc packs to the malformedtrellis//<name>.md.That malformed path still satisfies the
trellis/*.mdglob the copy logic uses, so documentation is still delivered and every functional test stays green. Nothing asserted the exact entry path, so the defect reached published packages —Trellis.Microservices.Abstractions0.1.0-alpha.72 on nuget.org carriestrellis//on both of its references.The fix
Converts every
PackagePathto forward slashes, including the root markersPackagePath="\"andPackagePath="\README.md". Those forms are not actually broken today — verified from published bytes thatREADME.mdandicon.pngland at the package root — but a single rule ("no backslashes inPackagePath") is easier to hold than "no trailing backslash, except the root one", and it matches the siblingTrellis.ResourceNamingrepo.The guard
New
build/test-package-layout.ps1, with two checks because neither alone is sufficient:PackagePathcontains a backslash. Platform-independent, so it holds the line on a developer's Windows machine where check 2 cannot fail.trellis/<name>.md. The real behaviour, but it can only fail on Linux.The declaration check parses the XML rather than scanning text. A line-based regex missed two of three planted violations —
PackagePath='trellis\'and the<PackagePath>trellis\</PackagePath>metadata element — and would also have flagged the comments that quote the malformed value on purpose.XDocumenthandles all of these and supplies line numbers viaIXmlLineInfo.Not every package ships a reference, so an empty set for one package is legitimate and is not an error. The vacuous case — nothing anywhere ships a reference, leaving every per-package assertion trivially true — is caught once after the loop.
CI placement
The "Verify package layout" step is deliberately unconditional, even though the preceding Pack step is gated on
!github.base_ref:SKIPwhile the declaration check still runs.artifacts/exists and both checks run against the real Linux-packed bytes — the only placetrellis//can actually appear.Validation
trellis//entriesartifacts/absent<!-- -->quoting the bad patternPacked output re-verified:
README.mdandicon.pngstill at package root;build/,buildTransitive/,trellis/all correct. 352/352 tests pass.Note
This changes packed output, so the fix reaches nuget.org only on the next publish. Already-published packages keep
trellis//, which is harmless since delivery works either way.