Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing xml documentation for targeting packs #26073

Closed
omajid opened this issue Sep 18, 2020 · 15 comments
Closed

Missing xml documentation for targeting packs #26073

omajid opened this issue Sep 18, 2020 · 15 comments
Assignees
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework Done This issue has been fixed
Milestone

Comments

@omajid
Copy link
Member

omajid commented Sep 18, 2020

The targeting packs in .NET 5 include the assemblies (.dll files) and documentation (.xml) files. For example, the Microsoft.NETCore.App.Ref targeting pack includes System.Collections.dll as well as System.Collections.xml (the documentation).

$ tar tf dotnet-microsoft-built-sdk-5.0.100-rc.1.20452.10.tar.gz | grep 'Microsoft.NETCore.App.Ref.*System.Collections.[a-z]'
./packs/Microsoft.NETCore.App.Ref/5.0.0-rc.1.20451.14/ref/net5.0/System.Collections.dll
./packs/Microsoft.NETCore.App.Ref/5.0.0-rc.1.20451.14/ref/net5.0/System.Collections.xml

But this information is missing for some packages, such as System.IO.Pipelines

$ tar tf dotnet-microsoft-built-sdk-5.0.100-rc.1.20452.10.tar.gz | grep System.IO.Pipelines
./packs/Microsoft.NETCore.App.Ref/5.0.0-rc.1.20451.14/ref/net5.0/System.IO.Pipelines.dll
./shared/Microsoft.NETCore.App/5.0.0-rc.1.20451.14/System.IO.Pipelines.dll

Originally reported at dotnet/source-build#1686

@omajid omajid changed the title Missing xml documentation for target packs Missing xml documentation for targeting packs Sep 18, 2020
@dagood
Copy link
Member

dagood commented Sep 18, 2020

This belongs in dotnet/runtime--I confirmed System.IO.Pipelines.xml also doesn't exist in https://www.nuget.org/packages/Microsoft.NETCore.App.Ref/5.0.0-rc.1.20451.14, which dotnet/sdk has no hand in. (Targeting packs are produced from the same repo as the runtime pack + runtime that matches it.)

@dagood dagood transferred this issue from dotnet/sdk Sep 18, 2020
@ghost
Copy link

ghost commented Sep 18, 2020

Tagging subscribers to this area: @buyaa-n, @krwq, @jeffhandley
See info in area-owners.md if you want to be subscribed.

@BrennanConroy
Copy link
Member

Pipelines isn't in NetCore.App anymore, but this is an issue in AspNetCore.App
image

@BrennanConroy BrennanConroy transferred this issue from dotnet/runtime Sep 18, 2020
@BrennanConroy BrennanConroy added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Sep 18, 2020
@BrennanConroy BrennanConroy added this to the 5.0.0-rc2 milestone Sep 18, 2020
@BrennanConroy
Copy link
Member

cc @Pilchie This might be something we want to do for RTM

@omajid
Copy link
Member Author

omajid commented Sep 18, 2020

@BrennanConroy are you sure about aspnet vs runtime? Preview 7 (your screenshot) says this is in AspNetCore.App, but in RC1, this is in NETCore.App. Did it move again after RC1?

@Pilchie
Copy link
Member

Pilchie commented Sep 18, 2020

Yes, it moved back to AspNetCore.App after RC1.

@dotnet/aspnet-build can someone take a look?

@dougbu
Copy link
Member

dougbu commented Sep 18, 2020

  1. See from the 3.0.1 and 3.1.8 copies of Microsoft.AspNetCore.App.Ref that this is not a regression
  2. Same problem applies to all ref/ assemblies that we do not get from the Microsoft.Extensions.Internal.Transport bundle (those are fine)
  3. System.IO.Pipelines package (for example) does not contain an XML file in the same folder as the ref/ assembly image

@ericstj should number 3 be false i.e. should the doc file be in the same folder as the ref/ assembly❔ If not, the project creating the bundles for our targeting packs may be able to hack something using the implementation assemblies but I'm not positive all are visible in that project.

@ericstj
Copy link
Member

ericstj commented Sep 18, 2020

This appears to be a regression from 3.1 in the case of System.IO.Pipelines. In 3.1 we included (and should include) reference XML next to reference assembly. Are you seeing the same for all packages, or just Pipelines? I checked System.Diagnositcs.EventLog and Registry and they looked OK to me. I opened a bug to track the Pipelines issue: dotnet/runtime#42479

I think you should be fine to pull the XML from the nuget package for the others, and we should fix this for pipelines.

@dagood
Copy link
Member

dagood commented Sep 18, 2020

  • See from the 3.0.1 and 3.1.8 copies of Microsoft.AspNetCore.App.Ref that this is not a regression

I filed #26068 about this for 3.1.8, it seems like a bug. We also saw it in 3.1.3 but didn't quite report it here in aspnetcore at the time.

@dougbu
Copy link
Member

dougbu commented Sep 19, 2020

@ericstj I didn't check the other packages. Thanks. So, System.IO.Pipelines is a separate case that'll require a dotnet/runtime fix to enable the scenario to be patched up.

The larger issue is: We never had the correct msbuild gobbledygook to get the XML files from the packages where we get "loose" ref/ assemblies. But, we can add that code in a generic way (I hope) and pick up the documentation from all ~12 packages we reference. Shouldn't be too difficult since we include their ref/ assemblies in Microsoft.AspNetCore.App.Ref and the XML files will (soon) all sit beside those assemblies.

@dougbu
Copy link
Member

dougbu commented Sep 19, 2020

@dagood back-porting the fix to 3.1 for #26068 shouldn't be horrendous.

@dougbu
Copy link
Member

dougbu commented Sep 19, 2020

@Pilchie I'm not going to get to this 'til Monday and doubt other @dotnet/aspnet-build members are hankering for a weekend project. That puts this into 3.1.10 and 5.0 RTM. Am I correct bot fixes should meet the bar given the missing documentation files worsen the ASP.NET Core development experience in every IDE on every platform❔

@wtgodbe
Copy link
Member

wtgodbe commented Sep 21, 2020

Looking at this today

@dougbu
Copy link
Member

dougbu commented Sep 23, 2020

@wtgodbe I believe this issue can be closed. We can use #26068 to track your 3.1 change. Correct❔

@wtgodbe wtgodbe closed this as completed Sep 23, 2020
@wtgodbe
Copy link
Member

wtgodbe commented Sep 23, 2020

Yes

@wtgodbe wtgodbe added the Done This issue has been fixed label Sep 23, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Oct 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework Done This issue has been fixed
Projects
None yet
Development

No branches or pull requests

7 participants