Enable publishing XML documentation #1062

dasMulli · 2017-03-31T17:17:22Z

Fixes #795 by publishing the XML documentation by default when documentation generation is enabled unless PublishDocumentationFile is set to false.

Edit:
Note that documentation files of project references are already included in the published output.

Update:

Property PublishDocumentationFiles can be set to false to opt out of publishing own documentation and documentation of project references. (Sets PublishDocumentationFile and PublishProjectReferenceDocumentationFiles to false)
Property PublishDocumentationFile can be set individually to opt out of publishing own documentation file.
Property PublishReferencesDocumentationFiles can be set individually to opt out of publishing documentation files of project and assembly references.

cc @eerhardt @nguerrera

nguerrera · 2017-03-31T17:22:48Z

src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.NET.Sdk.BeforeCommon.targets

@@ -137,6 +137,11 @@ Copyright (c) .NET Foundation. All rights reserved.
    <DocumentationFile />
  </PropertyGroup>

+  <PropertyGroup Condition="'$(PublishDocumentationFile)' == ''">
+    <PublishDocumentationFile Condition="'$(GenerateDocumentationFile)' == 'true'">true</PublishDocumentationFile>


I think it should be false by default regardless. Apps can opt-in and packages that depend on it (like swagger) could even carry props to turn it on. It's consistent with the v1 behavior and it's pay-for-play in terms of publish size.

The situation currently is: if you haven an app and library as ProjectReference, the documentation file of the library is published with the app, but not the app's own documentation.
That's the main reason why i chose it to be on by default.

if you haven an app and library as ProjectReference, the documentation file of the library is published with the app

Can you add a test for this? I think using the KitchenSink project would be the easiest. You could probably change your two tests below to use that TestAsset, and be able to test all these scenarios pretty easy.

The situation currently is: [...]

I see. In that case, I think we want this new opt-out to disable that behavior for project refs too.

think we want this new opt-out to disable that behavior for project refs too.

I think so. I'm thinking about renaming it to PublishDocumentationFiles (plural) to:

Control adding @(FinalDocFile) to @(ResolvedFileToPublish)

Filter out xml files ~~returned from the ResolvePublishAssemblies task (maybe add a task parameter although it doesn't really seem to fit in there).~~ from project references

Thoughts?

Makes sense.

dasMulli · 2017-04-03T22:02:27Z

Updated based on discussion to enable opting out of publishing documentation of p2p references.

…ng all xml documentation.

eerhardt · 2017-04-04T14:29:30Z

src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.NET.Publish.targets

-      <ResolvedAssembliesToPublish Include="@(ReferenceCopyLocalPaths)" Exclude="@(ResolvedAssembliesToPublish)">
+      <ResolvedAssembliesToPublish Include="@(ReferenceCopyLocalPaths)"
+                                   Exclude="@(ResolvedAssembliesToPublish)"
+                                   Condition="'$(PublishProjectReferenceDocumentationFiles)' == 'true' or '%(Extension)' != '.xml'">


@rainersigwald @cdmihai @KirillOsenkov @jeffkl - are documentation files the only type of xml files that can get put in @(ReferenceCopyLocalPaths)? Is there some other way to distinguish what is a P2P reference's doc file?

I'm not sure who else would know this, maybe @ericstj @weshaggard @davkean ...?

Here's how the documentation file works:

It starts life as a property <DocumentationFile /> defined in an individual project

It gets added to an item group @(DocFileItem) here

@(DocFileItem) is passed to <Csc /> here

It is added an item group @(FinalDocFile) here which is a full path to where the file should be copied to

The @(DocFileItem) is copied to @(FinalDocFile) here

<ResolveAssemblyReference /> gets passed a list of related file extensions which includes .xml.

So technically, the only reason the Doc file shows up in @(ReferenceCopyLocalPaths) is because it has the same file name without extension as the output assembly. This logic takes the output file name and looks for "related" files by extensions, one of which is .xml. If people were trying to be fancy, they could have their documentation file named something else and it would show up in @(ReferenceCopyLocalPaths).

That said, it's probably safe to assume that the only related file ending in .xml is the doc file. MSBuild does not currently translate any metadata to these related files that indicate what they were originally.

dasMulli · 2017-04-04T22:05:29Z

I'm having doubts about the naming of the PublishProjectReferenceDocumentationFiles property.
For @(Reference) items, this will also suppress publishing xml files next to referenced assemblies, which is intended, but not reflected in its name.
I think it may be better to have it be PublishReferencesDocumentationFiles (better suggestions?) - I'd do the renaming + adding test cases for referencing a library tomorrow.

livarcocc · 2017-04-06T17:09:55Z

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

+
+
+        [Theory]
+        [MemberData(nameof(PublishDocumentationExpectations))]


Why use member data here and inlinedata in the other? I particularly prefer inlinedata in this case.

Rule of thumb.. 4x3 arguments was more than 2x2.
But you're right, even other tests use InlineData more. I'll change it.

…sDocumentationFiles and added tests for publishing with references to assemblies.

dasMulli · 2017-04-06T19:59:40Z

@dotnet-bot test OSX10.12 Release

eerhardt · 2017-04-06T21:03:20Z

@dasMulli - FYI we are having all sorts of issue with OSX across repos, in Jenkins and in VSO.

@mmitche - Any update?

mmitche · 2017-04-06T21:39:49Z

@eerhardt Not yet. IT updated that other groups are seeing symptoms too.

livarcocc · 2017-04-11T05:26:29Z

Thanks for the PR @dasMulli

RehanSaeed · 2017-04-11T14:04:24Z

I heard performance being mentioned as an issue regarding turning on these flags. Looks like both of these options are turned on by default which is great.

NETESudarshan · 2017-06-12T16:02:39Z

@livarcocc Hi, is this fix available in SDK 1.0.4?. I'm using 1.0.4 SDK, however, dotnet publish doesn't seem to output the XML documentation for my project.

I have <GenerateDocumentationFile>true</GenerateDocumentationFile> in my .csproj.

Am I missing something?, thanks.

/cc @dasMulli

dasMulli · 2017-06-12T16:09:49Z

@NETESudarshan this is part of the upcoming 2.0.0 tooling which is currently in preview.

NETESudarshan · 2017-06-12T16:28:22Z

@dasMulli Ok, thanks. Is there a quick way to find out the milestone or target release for an issue here?

freeranger · 2017-08-28T14:32:45Z

@dasMulli Did this make it into the 2.0 release in the end?

dasMulli · 2017-08-28T15:35:48Z

@freeranger yes.. are you experiencing problems with it? (if so and it takes a bit longer to explain better create a new issue)

freeranger · 2017-08-28T16:09:33Z

@dasMulli Thanks for the quick response - I haven't tried it yet but the related issue #2079 is still open so that confused me.
I am looking for a reason to move our codebase to 2.0 sooner rather than later, and this is a reason if it works :)

thanks

dasMulli · 2017-08-28T16:20:10Z

@freeranger this should work when building with the 2.0.0 SDK for .net core 1.0/1.1 (there are some issues with the dependency model for asp.net though)

freeranger · 2017-08-28T17:28:20Z

Thanks @dasMulli, I'll give it a go. Can you point me to some docs on the new way of doing it?

…0191031.6 (dotnet#1062) - Microsoft.AspNetCore.Analyzers - 5.0.0-alpha1.19531.6 - Microsoft.AspNetCore.Mvc.Api.Analyzers - 5.0.0-alpha1.19531.6 - Microsoft.AspNetCore.Mvc.Analyzers - 5.0.0-alpha1.19531.6 - Microsoft.AspNetCore.Components.Analyzers - 5.0.0-alpha1.19531.6

…Files) to false In `PerformanceTest`, I've been seeing this MSBuild target take time on macOS: _ComputeResolvedCopyLocalPublishAssets 183ms This is in a build with no changes, and the target doesn't actually do anything besides emit an `<ItemGroup/>`: https://github.com/dotnet/sdk/blob/955c0fc7b06e2fa34bacd076ed39f61e4fb61716/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets#L708-L726 In application projects, we should be able to set `$(PublishReferencesDocumentationFiles)` to `false`, so this target will skip most of its work. I have not found any proper documentation for `$(PublishReferencesDocumentationFiles)`, but I believe the intention is that `.xml` documentation files are copied to the build output from all dependencies. We shouldn't need this behavior for an Android application. See: dotnet/sdk#1062 I also modified `PerformanceTest` to remove some additional time allowed for .NET 6 builds.

…Files) to false In `PerformanceTest`, I've been seeing the `_ComputeResolvedCopyLocalPublishAssets` MSBuild target from the dotnet/sdk take time on our CI on macOS: _ComputeResolvedCopyLocalPublishAssets 183ms This is in a build with no changes, and the target doesn't actually do anything besides emit an `<ItemGroup/>`: https://github.com/dotnet/sdk/blob/955c0fc7b06e2fa34bacd076ed39f61e4fb61716/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets#L708-L726 In application projects, we should be able to set `$(PublishReferencesDocumentationFiles)` to `false`, so this target will skip most of its work. I have not found any proper documentation for `$(PublishReferencesDocumentationFiles)`, but I believe the intention is that `.xml` documentation files are copied to the build output from all dependencies. We shouldn't need this behavior for an Android application. See: dotnet/sdk#1062 `_ComputeResolvedCopyLocalPublishAssets` now shows 0ms, so this seems to save ~183ms for all .NET 6 app builds.

In `PerformanceTest`, I've been seeing the `_ComputeResolvedCopyLocalPublishAssets` MSBuild target from the dotnet/sdk take time on our CI on macOS: _ComputeResolvedCopyLocalPublishAssets 183ms This is in a build with no changes, and the target doesn't actually do anything besides emit an `<ItemGroup/>`: https://github.com/dotnet/sdk/blob/955c0fc7b06e2fa34bacd076ed39f61e4fb61716/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets#L708-L726 In application projects, we should be able to set `$(PublishReferencesDocumentationFiles)` to `false`, so this target will skip most of its work. I have not found any proper documentation for `$(PublishReferencesDocumentationFiles)`, but I believe the intention is that `.xml` documentation files are copied to the build output from all dependencies. We shouldn't need this behavior for an Android application. See: dotnet/sdk#1062 `_ComputeResolvedCopyLocalPublishAssets` now shows 0ms, so this seems to save ~183ms for all .NET 6 app builds.

…se (#5380) In `PerformanceTest`, I've been seeing the `_ComputeResolvedCopyLocalPublishAssets` MSBuild target from the dotnet/sdk take time on our CI on macOS: _ComputeResolvedCopyLocalPublishAssets 183ms This is in a build with no changes, and the target doesn't actually do anything besides emit an `<ItemGroup/>`: https://github.com/dotnet/sdk/blob/955c0fc7b06e2fa34bacd076ed39f61e4fb61716/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets#L708-L726 In application projects, we should be able to set `$(PublishReferencesDocumentationFiles)` to `false`, so this target will skip most of its work. I have not found any proper documentation for `$(PublishReferencesDocumentationFiles)`, but I believe the intention is that `.xml` documentation files are copied to the build output from all dependencies. We shouldn't need this behavior for an Android application. See: dotnet/sdk#1062 `_ComputeResolvedCopyLocalPublishAssets` now shows 0ms, so this seems to save ~183ms for all .NET 6 app builds.

ref dotnet/sdk#1062

dnfclas added the cla-already-signed label Mar 31, 2017

nguerrera reviewed Mar 31, 2017

View reviewed changes

dasMulli force-pushed the feature/publish-documentation branch from 58f512c to 3c6e14a Compare April 3, 2017 21:58

dasMulli force-pushed the feature/publish-documentation branch from 3c6e14a to f7b2702 Compare April 4, 2017 04:27

Publish XML documentaiton by default and enable opting out of publisi…

1afbf31

…ng all xml documentation.

dasMulli force-pushed the feature/publish-documentation branch from f7b2702 to 1afbf31 Compare April 4, 2017 04:53

eerhardt reviewed Apr 4, 2017

View reviewed changes

eerhardt approved these changes Apr 4, 2017

View reviewed changes

nguerrera approved these changes Apr 4, 2017

View reviewed changes

livarcocc reviewed Apr 6, 2017

View reviewed changes

Renamed PublishProjectReferenceDocumentationFiles to PublishReference…

56d9419

…sDocumentationFiles and added tests for publishing with references to assemblies.

dasMulli force-pushed the feature/publish-documentation branch from efdbc49 to 56d9419 Compare April 6, 2017 17:43

eerhardt approved these changes Apr 6, 2017

View reviewed changes

livarcocc merged commit 3e7c5cc into dotnet:master Apr 11, 2017

natemcmaster mentioned this pull request Jun 29, 2017

Including Xml documentation files from Nuget packages to the build output directory dotnet/aspnetcore#2079

Closed

helloitszak mentioned this pull request Nov 28, 2017

PublishReferencesDocumentationFiles not working as expected. #1773

Closed

dasMulli mentioned this pull request Jun 22, 2018

dotnet publish does not copy the documentation file with csproj #795

Closed

jonathanpeppers mentioned this pull request Dec 7, 2020

[One .NET] apps default $(PublishReferencesDocumentationFiles) to false dotnet/android#5380

Merged

database64128 added a commit to database64128/shadowsocks-uri-generator that referenced this pull request Aug 3, 2021

📕 Disable publishing unnecessary XML docs

7a10784

ref dotnet/sdk#1062

dsplaisted mentioned this pull request Aug 31, 2022

Add output DebugSymbolsFiles to ResolvePackageAssets #27580

Merged

MichalStrehovsky mentioned this pull request Apr 29, 2024

Publishing a native AOT library shouldn't publish managed symbols files of ProjectReferences #40488

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable publishing XML documentation #1062

Enable publishing XML documentation #1062

dasMulli commented Mar 31, 2017 •

edited

Loading

nguerrera Mar 31, 2017

dasMulli Mar 31, 2017

eerhardt Apr 3, 2017

nguerrera Apr 3, 2017

dasMulli Apr 3, 2017 •

edited

Loading

nguerrera Apr 3, 2017

dasMulli commented Apr 3, 2017

eerhardt Apr 4, 2017

jeffkl Apr 4, 2017

dasMulli commented Apr 4, 2017

livarcocc Apr 6, 2017

dasMulli Apr 6, 2017

dasMulli commented Apr 6, 2017

eerhardt commented Apr 6, 2017

mmitche commented Apr 6, 2017

livarcocc commented Apr 11, 2017

RehanSaeed commented Apr 11, 2017

NETESudarshan commented Jun 12, 2017 •

edited

Loading

dasMulli commented Jun 12, 2017

NETESudarshan commented Jun 12, 2017 •

edited

Loading

freeranger commented Aug 28, 2017

dasMulli commented Aug 28, 2017

freeranger commented Aug 28, 2017

dasMulli commented Aug 28, 2017

freeranger commented Aug 28, 2017



		[Theory]
		[MemberData(nameof(PublishDocumentationExpectations))]

Enable publishing XML documentation #1062

Enable publishing XML documentation #1062

Conversation

dasMulli commented Mar 31, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dasMulli Apr 3, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dasMulli commented Apr 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dasMulli commented Apr 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dasMulli commented Apr 6, 2017

eerhardt commented Apr 6, 2017

mmitche commented Apr 6, 2017

livarcocc commented Apr 11, 2017

RehanSaeed commented Apr 11, 2017

NETESudarshan commented Jun 12, 2017 • edited Loading

dasMulli commented Jun 12, 2017

NETESudarshan commented Jun 12, 2017 • edited Loading

freeranger commented Aug 28, 2017

dasMulli commented Aug 28, 2017

freeranger commented Aug 28, 2017

dasMulli commented Aug 28, 2017

freeranger commented Aug 28, 2017

dasMulli commented Mar 31, 2017 •

edited

Loading

dasMulli Apr 3, 2017 •

edited

Loading

NETESudarshan commented Jun 12, 2017 •

edited

Loading

NETESudarshan commented Jun 12, 2017 •

edited

Loading