Skip to content

Fix release archives: remove NetPace.Core.xml from publish output#197

Merged
FrankRay78 merged 2 commits into
mainfrom
claude/pr-193-20260513-0856
May 13, 2026
Merged

Fix release archives: remove NetPace.Core.xml from publish output#197
FrankRay78 merged 2 commits into
mainfrom
claude/pr-193-20260513-0856

Conversation

@FrankRay78

Copy link
Copy Markdown
Owner

Fixes the failed release-binaries workflow on tags 0.23.0 and 0.23.1.

Root Cause

NetPace.Core.csproj has GenerateDocumentationFile=true (needed for NuGet IntelliSense). When NetPace.Console publishes, MSBuild copies NetPace.Core.xml into the publish directory. PublishSingleFile=true bundles DLLs but explicitly excludes XML files, so the XML lands loose and breaks the "exactly 1 entry" archive contract across all non-AOT variants.

The previous attempt (AllowedReferenceRelatedFileExtensions) was never merged and was also the wrong fix: it filters at the ResolveAssemblyReferences (build) stage, not the ComputeFilesToPublish (publish) stage.

Fix

Adds an MSBuild AfterTargets="ComputeFilesToPublish" target in NetPace.Console.csproj that removes .xml entries from ResolvedFileToPublish. This covers all 16 matrix variants (PublishSingleFile and AOT). dotnet pack for NetPace.Core is unaffected — packing uses _PackageFiles, not ResolvedFileToPublish.

Related to PR #193.

Generated with Claude Code

github-actions Bot and others added 2 commits May 13, 2026 09:00
AllowedReferenceRelatedFileExtensions (previous attempt) filters reference-
related files at the ResolveAssemblyReferences (build) step; it does not
reliably filter ResolvedFileToPublish, the item group that drives what ends
up in the publish directory.

The root cause: GenerateDocumentationFile=true in NetPace.Core.csproj causes
MSBuild to copy NetPace.Core.xml into every publish directory. PublishSingleFile
bundles DLLs but explicitly excludes XML files, so the XML lands loose and
breaks the "exactly 1 entry" archive contract across all 12 non-AOT variants.

Fix: add a ComputeFilesToPublish AfterTargets hook in NetPace.Console.csproj
that removes .xml entries from ResolvedFileToPublish. This targets the correct
stage of the publish pipeline and covers both PublishSingleFile and AOT builds.
NuGet pack is unaffected — packing uses _PackageFiles, not ResolvedFileToPublish.

Co-authored-by: Frank Ray <FrankRay78@users.noreply.github.com>
@FrankRay78 FrankRay78 merged commit c92429b into main May 13, 2026
2 checks passed
@FrankRay78 FrankRay78 deleted the claude/pr-193-20260513-0856 branch May 13, 2026 09:05
@claude

claude Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor

Code Review

Overview

This PR adds an MSBuild Target to strip .xml files from publish output, fixing the release archive "exactly 1 entry" contract broken when NetPace.Core.xml (generated by GenerateDocumentationFile=true) was copied into the publish directory by PublishSingleFile builds.

The core fix is correct and well-targeted. One issue needs resolving before merge.


Bug: AllowedReferenceRelatedFileExtensions is not removed

The PR description states:

"The previous attempt (AllowedReferenceRelatedFileExtensions) was never merged and was also the wrong fix."

This is inaccurate — the property is present in the current NetPace.Console.csproj (line 14), shipped as part of PR #193. Leaving both in place creates confusion:

  • Future maintainers will see two competing approaches with no indication which one actually does the work.
  • The comment on AllowedReferenceRelatedFileExtensions ("Prevent NetPace.Core.xml from landing in publish output") is now misleading — it describes behaviour that the new Target provides, not the property.
  • AllowedReferenceRelatedFileExtensions=.pdb suppresses .xml copying during regular dotnet build (to bin/), which is harmless here but is an unintended side-effect that muddles intent.

Recommendation: Remove the AllowedReferenceRelatedFileExtensions property and its associated comment. The ExcludeDocXmlFromPublish target is the complete and correct fix.


Everything else looks good

  • Correctness: AfterTargets="ComputeFilesToPublish" with ResolvedFileToPublish Remove is the right hook. Filtering at the publish stage (not build stage) is exactly what's needed here.
  • AOT coverage: The target runs harmlessly for AOT variants — no .xml entries are in ResolvedFileToPublish for native AOT builds, so it's a safe no-op across all 16 matrix entries as claimed.
  • NuGet unaffected: dotnet pack uses _PackageFiles, not ResolvedFileToPublish, so NuGet IntelliSense delivery from NetPace.Core is preserved.
  • Scope of removal: Removing all .xml files is appropriate for this CLI executable — no XML file should ever be a legitimate publish artifact here.
  • Target name: ExcludeDocXmlFromPublish is clear and self-describing.
  • Comment quality: The inline comment accurately explains the "why" (the loose-XML / archive-contract problem) without over-explaining. Fits the project's single-line comment convention.
  • RELEASING.md: The archive-contents contract section documents the .pdb exclusion mechanism. While a brief mention of the .xml exclusion could be added for completeness, it's not required by convention since release-binaries.yml itself isn't being modified.

Required change before merge

Remove AllowedReferenceRelatedFileExtensions and its block comment from NetPace.Console.csproj. The property predates this PR but its stated purpose is now fulfilled (correctly) by the new Target — keeping it is dead weight and the comment actively misleads.

@claude claude Bot mentioned this pull request May 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant