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

System.IO.Pipelines is missing doc XML from package #42479

Closed
ericstj opened this issue Sep 18, 2020 · 18 comments
Closed

System.IO.Pipelines is missing doc XML from package #42479

ericstj opened this issue Sep 18, 2020 · 18 comments

Comments

@ericstj
Copy link
Member

ericstj commented Sep 18, 2020

Examine the ref folder in the System.IO.Pipelines package. It's missing it's doc XML.

Every other package seems to have it but this one.

This appears to be a regression from 3.1. Likely has to do with the custom way that Pipelines is packaged:

<!-- We only plan to use this ref in netcoreapp. For all other netstandard compatible frameworks
we should use the lib asset instead. -->
<PackageTargetFramework>netcoreapp2.0</PackageTargetFramework>

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.IO.Pipelines untriaged New issue has not been triaged by the area owner labels Sep 18, 2020
@ericstj ericstj added this to the 5.0.0 milestone Sep 18, 2020
@ghost
Copy link

ghost commented Sep 18, 2020

Tagging subscribers to this area: @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

@ericstj
Copy link
Member Author

ericstj commented Sep 19, 2020

@carlossanlop @safern this is happening because System.IO.Pipelines.xml is missing from microsoft.private.intellisense Can we make sure that's fixed as part of #41904?

The lib files in this package were produced by the src build, so they are coming from checked in sources, which we don't want as these might be out of sync with what's in the docs repo.

@ericstj
Copy link
Member Author

ericstj commented Sep 19, 2020

@gewarren perhaps you can help on this. I'm not sure what the process is to get assemblies added to the Microsoft.Private.Intellisense package. I looked at some builds and it seems that its driven off this file https://github.com/dotnet/dotnet-api-docs/blob/master/xml/FrameworksIndex/net-5.0.xml. I couldn't find what is responsible for deciding what assemblies are listed in that file, and it appears to be derived by some tooling and then checked in by you. Do you think you can help?

@gewarren
Copy link
Contributor

gewarren commented Sep 21, 2020

@ericstj The process is that @safern creates an assembly drop folder of the ref assemblies, for example, this one for .NET 5 RC1: \\safernmain\docslayout\5.0-RC1. I then run a job that uses a tool called "mdoc" to generate/update the XML stubs. Santi will have to provide you info on how he collects the assemblies because I'm not sure.

@ericstj
Copy link
Member Author

ericstj commented Sep 21, 2020

@gewarren in that drop I see \\safernmain\docslayout\5.0-RC1\microsoft.aspnetcore.app\System.IO.Pipelines.dll and \\safernmain\docslayout\5.0-RC1\extensions\System.IO.Pipelines.dll

Why wouldn't these then show up in the microsoft.private.intellisense package?

@gewarren
Copy link
Contributor

We don't copy anything from the microsoft.aspnetcore.app folder (usually). Assemblies in the "extensions" folder in @safern's drop are copied to a "dotnet-plat-ext-5" folder instead of "dotnet-5" (which is where the assemblies from Santi's "microsoft.netcore.app" and "microsoft.windowsdesktop.app" folders are copied). So System.IO.Pipelines shows up in this file: https://github.com/dotnet/dotnet-api-docs/blob/master/xml/FrameworksIndex/dotnet-plat-ext-5.0.xml. And you can see that it shows up in docs, for example here: https://docs.microsoft.com/en-us/dotnet/api/system.io.pipelines.pipe?view=dotnet-plat-ext-5.0. As to what is included in IntelliSense, that I don't know. @TianqiZhang might be able to explain.

@safern
Copy link
Member

safern commented Sep 22, 2020

Can we generate the intellisense package using both, extensions and microsoft.netcoreapp.app?

@gewarren
Copy link
Contributor

@joelmartinez Can you help with this issue, perhaps?

@joelmartinez
Copy link

@gewarren ... after reading through the thread, I think I have an understanding of what the issue is, but just wanted to clarify; is the ask here to include the dotnet-plat-ext-5 in the intellisense job, in addition to the "regular" .net frameworks?

@safern
Copy link
Member

safern commented Sep 22, 2020

@gewarren ... after reading through the thread, I think I have an understanding of what the issue is, but just wanted to clarify; is the ask here to include the dotnet-plat-ext-5 in the intellisense job, in addition to the "regular" .net frameworks?

I think it would make sense to include all monikers for the current tfm version.

@joelmartinez
Copy link

@gewarren @safern I've created a devops item here for @TianqiZhang to look at.
https://ceapex.visualstudio.com/Engineering/_workitems/edit/306560

Please feel free to add any additional details or context there.

@safern
Copy link
Member

safern commented Sep 22, 2020

Thanks @joelmartinez.

@TianqiZhang it would be great if we could get this sorted out the earliest as we have a deadline to update the package before Thursday September 24th at 10am PST in order to be able to include this as part of the 5.0.0 release.

@ericstj
Copy link
Member Author

ericstj commented Sep 22, 2020

I took a closer look at the latest package (I was previously looking at 3.1.0-preview-20200129.1 because it had higher version, but then realized that we're actually using 3.0.0-preview-20200715.1. This has the XML docs, but their in a different folder: IntellisenseFiles\dotnet-plat-ext instead of IntellisenseFiles\net as defined here

<XmlDocFileRoot>$([MSBuild]::NormalizeDirectory('$(NuGetPackageRoot)', '$(MicrosoftPrivateIntellisensePackage)', '$(MicrosoftPrivateIntellisenseVersion)', 'IntellisenseFiles', 'net'))</XmlDocFileRoot>

I think we can make a small workaround to fix this by probing for the XML file in both locations.

@ericstj
Copy link
Member Author

ericstj commented Sep 22, 2020

It looks like this change observed the new layout, but didn't account for the different directories: #38109
And we didn't have anything in the build that failed when the source of the doc files changed.

@ericstj
Copy link
Member Author

ericstj commented Sep 22, 2020

I did a quick check and these are the packages impacted. The following all have XML in dotnet-plat-ext which isn’t used:

  • Microsoft.Extensions.DependencyModel
  • System.ComponentModel.Composition
  • System.ComponentModel.Composition.Registration
  • System.Composition.AttributedModel
  • System.Composition.Convention
  • System.Composition.Hosting
  • System.Composition.Runtime
  • System.Composition.TypedParts
  • System.Data.Odbc
  • System.Data.OleDb
  • System.DirectoryServices.AccountManagement
  • System.DirectoryServices.Protocols
  • System.IO.Pipelines
  • System.IO.Ports
  • System.Management
  • System.Net.Http.WinHttpHandler
  • System.Net.WebSockets.WebSocketProtocol
  • System.Reflection.Context
  • System.Reflection.MetadataLoadContext
  • System.Runtime.Caching
  • System.ServiceModel.Syndication
  • System.ServiceProcess.ServiceController

The following are all missing XML docs from the package entirely:

  • Microsoft.Bcl.AsyncInterfaces
  • Microsoft.Extensions.Caching.Abstractions
  • Microsoft.Extensions.Caching.Memory
  • Microsoft.Extensions.Configuration
  • Microsoft.Extensions.Configuration.Abstractions
  • Microsoft.Extensions.Configuration.Binder
  • Microsoft.Extensions.Configuration.CommandLine
  • Microsoft.Extensions.Configuration.EnvironmentVariables
  • Microsoft.Extensions.Configuration.FileExtensions
  • Microsoft.Extensions.Configuration.Ini
  • Microsoft.Extensions.Configuration.Json
  • Microsoft.Extensions.Configuration.UserSecrets
  • Microsoft.Extensions.Configuration.Xml
  • Microsoft.Extensions.DependencyInjection
  • Microsoft.Extensions.DependencyInjection.Abstractions
  • Microsoft.Extensions.FileProviders.Abstractions
  • Microsoft.Extensions.FileProviders.Composite
  • Microsoft.Extensions.FileProviders.Physical
  • Microsoft.Extensions.FileSystemGlobbing
  • Microsoft.Extensions.HostFactoryResolver
  • Microsoft.Extensions.Hosting
  • Microsoft.Extensions.Hosting.Abstractions
  • Microsoft.Extensions.Http
  • Microsoft.Extensions.Logging
  • Microsoft.Extensions.Logging.Abstractions
  • Microsoft.Extensions.Logging.Configuration
  • Microsoft.Extensions.Logging.Console
  • Microsoft.Extensions.Logging.Debug
  • Microsoft.Extensions.Logging.EventLog
  • Microsoft.Extensions.Logging.EventSource
  • Microsoft.Extensions.Logging.TraceSource
  • Microsoft.Extensions.Options
  • Microsoft.Extensions.Options.ConfigurationExtensions
  • Microsoft.Extensions.Options.DataAnnotations
  • Microsoft.Extensions.Primitives
  • System.Formats.Cbor
  • System.Security.Cryptography.OpenSsl

The latter category is more concerning, as I see authored docs for these in https://github.com/dotnet/dotnet-api-docs/tree/master/xml but they are not making it to dotnet/runtime.

@safern
Copy link
Member

safern commented Sep 22, 2020

The latter category is more concerning, as I see authored docs for these in https://github.com/dotnet/dotnet-api-docs/tree/master/xml but they are not making it to dotnet/runtime.

These were not included in the latest package we're using because these were not included in extensions at all... The Microsoft.Extensions.* packages were only included in the dotnet-aspnet moniker.

System.Formats.Cbor
System.Security.Cryptography.OpenSsl

Were not listed in: https://github.com/dotnet/api-catalog-infra/blob/master/eng/droplayout.props -- I will fix that to include it on the next drop I'm sharing today.

@jeffhandley
Copy link
Member

@safern Did #42221 include the changes that would address this, or is this still open?

@safern
Copy link
Member

safern commented Sep 24, 2020

I just did some extra validation on some of the packages that were missing the .xml file, and they are fixed and in the expected shape now.

@safern safern closed this as completed Sep 24, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants