Skip to content

Conversation

@GangWang01
Copy link
Member

Fix dotnet/msbuild#12044

Add the property PublishReferenceSymbols to control whether the .pdb file of the referenced project is published to the publish folder.

Copilot AI review requested due to automatic review settings July 9, 2025 02:19
@GangWang01 GangWang01 requested review from a team as code owners July 9, 2025 02:19
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a new PublishReferencesSymbols property to control publishing of referenced project PDB files, along with default behavior and tests.

  • Added PublishReferencesSymbols property with a default value in MSBuild’s BeforeCommon targets.
  • Updated publish logic to conditionally include .pdb files based on the new property.
  • Introduced a theory test to verify symbol publishing when the property is explicitly set.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
test/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishAProjectWithDependencies.cs Added a theory test for PublishReferencesSymbols=true/false scenarios.
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.BeforeCommon.targets Defined <PublishReferencesSymbols> property with a default of true.
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets Modified the copy-local asset condition to include .pdb when enabled.
Comments suppressed due to low confidence (1)

test/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishAProjectWithDependencies.cs:280

  • Consider adding a test case for the default behavior (when PublishReferencesSymbols isn't specified) to ensure it truly defaults to true and PDB files are published by default.
        [Theory]

Comment on lines +706 to 711
Condition="(('$(PublishReferencesDocumentationFiles)' == 'true' and '%(ReferenceCopyLocalPaths.Extension)' == '.xml')
or ('$(PublishReferencesSymbols)' == 'true' and '%(ReferenceCopyLocalPaths.Extension)' == '.pdb')
or ('%(ReferenceCopyLocalPaths.Extension)' != '.xml' and '%(ReferenceCopyLocalPaths.Extension)' != '.pdb'))
and '%(ReferenceCopyLocalPaths.Private)' != 'false'">
<DestinationSubPath>%(ReferenceCopyLocalPaths.DestinationSubDirectory)%(ReferenceCopyLocalPaths.Filename)%(ReferenceCopyLocalPaths.Extension)</DestinationSubPath>
</_ResolvedCopyLocalPublishAssets>
Copy link

Copilot AI Jul 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] This multi-line Condition is quite verbose and can be hard to maintain; consider breaking it into separate ItemGroups or using intermediate properties to clarify each inclusion rule.

Suggested change
Condition="(('$(PublishReferencesDocumentationFiles)' == 'true' and '%(ReferenceCopyLocalPaths.Extension)' == '.xml')
or ('$(PublishReferencesSymbols)' == 'true' and '%(ReferenceCopyLocalPaths.Extension)' == '.pdb')
or ('%(ReferenceCopyLocalPaths.Extension)' != '.xml' and '%(ReferenceCopyLocalPaths.Extension)' != '.pdb'))
and '%(ReferenceCopyLocalPaths.Private)' != 'false'">
<DestinationSubPath>%(ReferenceCopyLocalPaths.DestinationSubDirectory)%(ReferenceCopyLocalPaths.Filename)%(ReferenceCopyLocalPaths.Extension)</DestinationSubPath>
</_ResolvedCopyLocalPublishAssets>
Condition="('$(PublishReferencesDocumentationFiles)' == 'true' and '%(ReferenceCopyLocalPaths.Extension)' == '.xml') or
('$(PublishReferencesSymbols)' == 'true' and '%(ReferenceCopyLocalPaths.Extension)' == '.pdb') or
('%(ReferenceCopyLocalPaths.Extension)' != '.xml' and '%(ReferenceCopyLocalPaths.Extension)' != '.pdb')">
</_ResolvedCopyLocalPublishAssets>
<PropertyGroup>
<IsDocumentationFile Condition="'$(PublishReferencesDocumentationFiles)' == 'true' and '%(ReferenceCopyLocalPaths.Extension)' == '.xml'">true</IsDocumentationFile>
<IsSymbolFile Condition="'$(PublishReferencesSymbols)' == 'true' and '%(ReferenceCopyLocalPaths.Extension)' == '.pdb'">true</IsSymbolFile>
<IsOtherFile Condition="'%(ReferenceCopyLocalPaths.Extension)' != '.xml' and '%(ReferenceCopyLocalPaths.Extension)' != '.pdb'">true</IsOtherFile>
<IsPrivateFile Condition="'%(ReferenceCopyLocalPaths.Private)' != 'false'">true</IsPrivateFile>
</PropertyGroup>
<ItemGroup>
<_ResolvedCopyLocalPublishAssets Include="@(ReferenceCopyLocalPaths)"
Exclude="@(_ResolvedCopyLocalBuildAssets);@(RuntimePackAsset)"
Condition="('$(IsDocumentationFile)' == 'true' or
'$(IsSymbolFile)' == 'true' or
'$(IsOtherFile)' == 'true') and
'$(IsPrivateFile)' == 'true'">
<DestinationSubPath>%(ReferenceCopyLocalPaths.DestinationSubDirectory)%(ReferenceCopyLocalPaths.Filename)%(ReferenceCopyLocalPaths.Extension)</DestinationSubPath>
</_ResolvedCopyLocalPublishAssets>
</ItemGroup>
<DestinationSubPath>%(ReferenceCopyLocalPaths.DestinationSubDirectory)%(ReferenceCopyLocalPaths.Filename)%(ReferenceCopyLocalPaths.Extension)</DestinationSubPath>
</_ResolvedCopyLocalPublishAssets>

Copilot uses AI. Check for mistakes.
@GangWang01
Copy link
Member Author

GangWang01 commented Jul 9, 2025

@rainersigwald can you please take a look?

@YuliiaKovalova
Copy link
Member

@baronfel could you please check this PR?

<PublishDocumentationFiles Condition="'$(PublishDocumentationFiles)' == ''">true</PublishDocumentationFiles>
<PublishDocumentationFile Condition="'$(PublishDocumentationFile)' == '' and '$(PublishDocumentationFiles)' == 'true'">true</PublishDocumentationFile>
<PublishReferencesDocumentationFiles Condition="'$(PublishReferencesDocumentationFiles)' == '' and '$(PublishDocumentationFiles)' == 'true'">true</PublishReferencesDocumentationFiles>
<PublishReferencesSymbols Condition="'$(PublishReferencesSymbols)' == ''">true</PublishReferencesSymbols>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name seems fine to me given the other prior art.

@baronfel
Copy link
Member

baronfel commented Oct 6, 2025

I added the document for new feature label because we'd need to ensure we add docs for this flag to dotnet/docs.

@GangWang01
Copy link
Member Author

I added the document for new feature label because we'd need to ensure we add docs for this flag to dotnet/docs.

Please review the doc pr dotnet/docs#50255.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

How to not output the .pdb of the referenced project to the publish folder?

3 participants