Clean-up pkgproj leftovers in libs and apply fixes #56899

ViktorHofer · 2021-08-05T10:51:58Z

Remove any pkgproj infrastructure that was only used by src/libraries
(which is the majority). Delete the packageindex YAY.
Update the documentation that covered the packageindex and the pkgprojs.

Avoid any incremental builds during packaging by removing
libraries-packages.proj and use src.proj for packaging instead. Make use
of the GeneratePackageOnBuild property to build package during the
allconfigurations without requiring a different entry target.

Fix the addition of the DocumentationFile during packaging when
GeneratePackageOnBuid is used by hooking onto the
DocumentationProjectOutputGroup that NuGet uses and replacing the
generated documentation file with the one that comes via the
intellisense package.
Also introduce a property to choose the generated documentation file
over the one from the intellisense package:
false

Removing a few leftover PackageDescription properties from the projects'
Directory.Build.props file.

Cleaning up properties in Directory.Build.props & Directory.Build.targets
files.

ghost · 2021-08-05T10:52:03Z

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

Issue Details

Remove any pkgproj infrastructure that was only used by src/libraries
(which is the majority). Delete the packageindex YAY.
Update the documentation that covered the packageindex and the pkgprojs.

Avoid any incremental builds during packaging by removing
libraries-packages.proj and use src.proj for packaging instead. Make use
of the GeneratePackageOnBuild property to build package during the
allconfigurations without requiring a different entry target.

Remove the runtime.native.System.IO.Ports s390x variant as that one
isn't built in official builds and causes the restore of the
meta package runtime.native.System.IO.Ports to fail because of its
missing dependency.

Fix the addition of the DocumentationFile during packaging when
GeneratePackageOnBuid is used by hooking onto the
DocumentationProjectOutputGroup that NuGet uses and replacing the
generated documentation file with the one that comes via the
intellisense package.
Also introduce a property to choose the generated documentation file
over the one from the intellisense package:
false

Removing a few leftover PackageDescription properties from the projects'
Directory.Build.props file.

Cleaning up properties in Directory.Build.props & Directory.Build.targets
files.

Author:	ViktorHofer
Assignees:	ViktorHofer
Labels:	`area-Infrastructure-libraries`
Milestone:	-

am11 · 2021-08-05T12:11:48Z

Nice cleanup! 🙂

as that one isn't built in official builds and causes the restore of the meta package runtime.native.System.IO.Ports to fail because of its missing dependency.

I think we just need to distinguish a bit more between official and community supported platforms in config files. e.g. in coreclr all community-maintained platforms are also getting OfficialBuildRID. I tried to clean that up here https://github.com/dotnet/runtime/pull/55279/files#diff-8ff7afaf3544b3026538912211057b70e70c5957b1581d3623ad34d4eba2fc59L69 but it's a bit tricky to pass the official build validation (which runs on vertical matrix) without first testing it on local machine. (I will get back to it when i get some cycles) 😅

ViktorHofer · 2021-08-05T12:57:33Z

FYI I moved the revert out into #56906.

I think we just need to distinguish a bit more between official and community supported platforms in config files

Are you saying that the meta-package should ignore community supported build configurations like x390x and not have them as a dependency in the nuspec? If so, would that mean that s390x needs to be source-built to get such a package?

ViktorHofer · 2021-08-05T13:02:36Z

e.g. in coreclr all community-maintained platforms are also getting OfficialBuildRID. I tried to clean that up here https://github.com/dotnet/runtime/pull/55279/files#diff-8ff7afaf3544b3026538912211057b70e70c5957b1581d3623ad34d4eba2fc59L69 but it's a bit tricky to pass the official build validation (which runs on vertical matrix) without first testing it on local machine. (I will get back to it when i get some cycles) 😅

Do you mean all compatible rids that that we (Microsoft) build in official builds are in the OfficialBuildRID item and everything else should be filtered out in meta-packages? Having such a list in a common place would definitely be great and tbh necessary.

am11 · 2021-08-05T13:54:13Z

Are you saying that the meta-package should ignore community supported build configurations like x390x and not have them as a dependency in the nuspec? If so, would that mean that s390x needs to be source-built to get such a package?

Currently, it is unclear what does it mean to be a community supported platform, where do we draw the line and what is the right way to handle such new platforms. source-build sounds like a step in right direction. One viable option I think could be that community supported packages are source-built and published to NuGet by responsible maintainer of that platform. In the package description, a note to the effect "it is a drop-in replacement of S.IO.P for XYZ platform" would help discoverability for the consumers. If we think of a platform as "OS+Arch+LibC+TFM", then it is a very similar thing as, e.g., System.Data.Linq is not supported by Microsoft on new TFMs, but Mindbox.Data.Linq provides a drop-in replacement on those platforms (TFMs) for users who are requiring it.

docs/project/library-servicing.md

safern · 2021-08-05T18:23:36Z

eng/Subsets.props

@@ -63,7 +63,7 @@
    <DefaultLibrariesSubsets Condition="'$(BuildTargetFramework)' == '$(NetCoreAppCurrent)' or
                                        '$(BuildTargetFramework)' == '' or
                                        '$(BuildAllConfigurations)' == 'true'">libs.native+</DefaultLibrariesSubsets>
-    <DefaultLibrariesSubsets>$(DefaultLibrariesSubsets)libs.ref+libs.src+libs.packages</DefaultLibrariesSubsets>
+    <DefaultLibrariesSubsets>$(DefaultLibrariesSubsets)libs.ref+libs.src</DefaultLibrariesSubsets>


what is the workflow to just build packages no? build.cmd libs -pack?

build.cmd libs.src -pack is probably the quickest and easiest one after this change is in. But as before build.cmd libs -allconfigurations also builds the packages. Another alternative is invoking the source project directly: dotnet pack src\libraries\src.proj.

Should we add an entry on the docs regarding this?

We definitely want to have a dedicated doc page about packaging to also document the new switches that I introduced to i.e. add placeholder files for .NETFramework/Xamarin, framework assembly references, the docs switch, etc. I would like to focus on documentation in a follow-up PR though.

Ok sounds good. Just FYI, we already have docs dedicated to packaging. https://github.com/dotnet/runtime/blob/main/docs/coding-guidelines/libraries-packaging.md, that's why I thought it might make sense to update on this PR.

That document is actually pretty well written :) I will add some parts to it in a follow-up :)

safern · 2021-08-05T18:25:56Z

eng/packaging.targets

+    <GeneratePackageOnBuild Condition="'$(GeneratePackageOnBuild)' == '' and '$(BuildAllConfigurations)' == 'true'">true</GeneratePackageOnBuild>
+    <!-- Search for the documentation file in the intellisense package and otherwise pick up the generated one. -->
+    <LibIntellisenseDocumentationFilePath>$(XmlDocFileRoot)1033\$(AssemblyName).xml</LibIntellisenseDocumentationFilePath>
+    <UseIntellisenseDocumentationFile Condition="'$(UseIntellisenseDocumentationFile)' == '' and Exists('$(LibIntellisenseDocumentationFilePath)')">true</UseIntellisenseDocumentationFile>


Thanks for adding support for this! does this work for shared framework assemblies?

cc: @carlossanlop @jeffhandley we can now annotate the 3 projects that ported their comments to triple slash.

Hmm don't know from the top of my head, will need to give it a try. Could be that binplacing uses a different extension point for the doc files.

src/libraries/Directory.Build.targets

safern · 2021-08-05T18:28:46Z

src/libraries/Microsoft.NETCore.Platforms/src/Microsoft.NETCore.Platforms.csproj

@@ -9,6 +9,8 @@

    <IsSourceProject>false</IsSourceProject>
    <IsPackable>true</IsPackable>
+    <!-- TODO: Enable after AvoidRestoreCycleOnSelfReference hack is removed. -->


Is there an issue to remove that hack?

Don't think so. This was introduced with 75206ed. @ericstj do we have an issue for that?

I tried fixing this myself but that required pinning quite a lot dependent packages on preview versions which would break sourcebuild.

safern · 2021-08-05T18:29:32Z

src/libraries/System.Runtime.Experimental/ref/System.Runtime.Experimental.csproj

@@ -19,6 +19,8 @@
    <IsPackable>true</IsPackable>
    <PackageDescription>Exposes new experimental APIs from System.Runtime</PackageDescription>
    <PackageId>$(MSBuildProjectName)</PackageId>
+    <!-- TODO: Remove when the package shipped with NET6. -->


I believe this package will be removed in .NET 7 anyways. cc: @tannergooding

Yes, assuming it doesn't get delayed by something else

Let's keep the comment as-is until we know for sure.

src/libraries/src.proj

Remove any pkgproj infrastructure that was only used by src/libraries (which is the majority). Delete the packageindex *YAY*. Update the documentation that covered the packageindex and the pkgprojs. Avoid any incremental builds during packaging by removing libraries-packages.proj and use src.proj for packaging instead. Make use of the `GeneratePackageOnBuild` property to build package during the allconfigurations without requiring a different entry target. Fix the addition of the DocumentationFile during packaging when `GeneratePackageOnBuid` is used by hooking onto the `DocumentationProjectOutputGroup` that NuGet uses and replacing the generated documentation file with the one that comes via the intellisense package. Also introduce a property to choose the generated documentation file over the one from the intellisense package: <UseIntellisenseDocumentationFile>false</UseIntellisenseDocumentationFile> Removing a few leftover PackageDescription properties from the projects' Directory.Build.props file. Cleaning up properties in Directory.Build.props & Directory.Build.targets files.

ViktorHofer · 2021-08-05T18:32:07Z

I just rebased upon latest main, nothing changed in the code.

ViktorHofer · 2021-08-05T18:51:04Z

This PR makes the allconfiguration leg 5 min faster :)

safern · 2021-08-05T19:39:23Z

This PR makes the allconfiguration leg 5 min faster :)

Nice!!! Hopefully that is not cause we are accidentally loosing test coverage 😆

safern · 2021-08-05T19:41:17Z

It seems to be because the packages tests are not running 😢 :

System.Xml -> D:\workspace\_work\1\s\artifacts\bin\manual.System.Xml\net6.0-Debug\System.Xml.dll
  ApiCompat -> 
  Trimming win-x64 runtime pack assemblies with ILLinker...
  Trimming win-x64 OOB assemblies with ILLinker...
  externals -> 
  testPackages -> 
  Determining projects to restore...
  Restored D:\workspace\_work\1\s\artifacts\bin\testPackages\dirs.proj (in 51 ms).

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:18:37.35
Finishing: Restore and Build Product

ViktorHofer · 2021-08-05T19:44:49Z

It seems to be because the packages tests are not running 😢 :

f*** :D

am11 · 2021-08-06T10:05:18Z

This PR makes the allconfiguration leg 5 min faster :)

@ViktorHofer, was there any +/- impact when pkg validation was turned on?

ViktorHofer · 2021-08-06T10:10:13Z

Not 5 but 3 minutes. So we are back at where we were before with pkgprojs before we migrated to csprojs which still is quite a good result :) (nuget does more)

ViktorHofer added the area-Infrastructure-libraries label Aug 5, 2021

ViktorHofer requested review from Anipik and safern August 5, 2021 10:51

ViktorHofer self-assigned this Aug 5, 2021

ViktorHofer mentioned this pull request Aug 5, 2021

Build runtime.native.System.IO.Ports on linux-s390x #56424

Merged

safern reviewed Aug 5, 2021

View reviewed changes

docs/project/library-servicing.md Outdated Show resolved Hide resolved

safern reviewed Aug 5, 2021

View reviewed changes

docs/project/library-servicing.md Outdated Show resolved Hide resolved

safern reviewed Aug 5, 2021

View reviewed changes

src/libraries/Directory.Build.targets Show resolved Hide resolved

safern reviewed Aug 5, 2021

View reviewed changes

src/libraries/src.proj Show resolved Hide resolved

ViktorHofer force-pushed the PkgProjCleanup branch from c18667c to 3d10eb1 Compare August 5, 2021 18:31

safern approved these changes Aug 5, 2021

View reviewed changes

ViktorHofer added 3 commits August 5, 2021 22:07

Actually run packaging during the allconfigurations build

610e1ac

Update docs

5dec364

make runtime specific pkgs non packable

a4a8b08

io.ports native pkg fixes

7225599

ViktorHofer merged commit 22cee85 into dotnet:main Aug 6, 2021

ViktorHofer deleted the PkgProjCleanup branch August 6, 2021 10:01

ghost locked as resolved and limited conversation to collaborators Sep 7, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean-up pkgproj leftovers in libs and apply fixes #56899

Clean-up pkgproj leftovers in libs and apply fixes #56899

ViktorHofer commented Aug 5, 2021 •

edited

Loading

ghost commented Aug 5, 2021

am11 commented Aug 5, 2021

ViktorHofer commented Aug 5, 2021 •

edited

Loading

ViktorHofer commented Aug 5, 2021 •

edited

Loading

am11 commented Aug 5, 2021 •

edited

Loading

safern Aug 5, 2021

ViktorHofer Aug 5, 2021

safern Aug 5, 2021

ViktorHofer Aug 5, 2021

safern Aug 5, 2021

ViktorHofer Aug 5, 2021

safern Aug 5, 2021

ViktorHofer Aug 5, 2021

safern Aug 5, 2021

ViktorHofer Aug 5, 2021

safern Aug 5, 2021

tannergooding Aug 5, 2021

ViktorHofer Aug 5, 2021

ViktorHofer commented Aug 5, 2021

ViktorHofer commented Aug 5, 2021

safern commented Aug 5, 2021

safern commented Aug 5, 2021 •

edited

Loading

ViktorHofer commented Aug 5, 2021

am11 commented Aug 6, 2021

ViktorHofer commented Aug 6, 2021 •

edited

Loading

Clean-up pkgproj leftovers in libs and apply fixes #56899

Clean-up pkgproj leftovers in libs and apply fixes #56899

Conversation

ViktorHofer commented Aug 5, 2021 • edited Loading

ghost commented Aug 5, 2021

am11 commented Aug 5, 2021

ViktorHofer commented Aug 5, 2021 • edited Loading

ViktorHofer commented Aug 5, 2021 • edited Loading

am11 commented Aug 5, 2021 • 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

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

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

Choose a reason for hiding this comment

ViktorHofer commented Aug 5, 2021

ViktorHofer commented Aug 5, 2021

safern commented Aug 5, 2021

safern commented Aug 5, 2021 • edited Loading

ViktorHofer commented Aug 5, 2021

am11 commented Aug 6, 2021

ViktorHofer commented Aug 6, 2021 • edited Loading

ViktorHofer commented Aug 5, 2021 •

edited

Loading

ViktorHofer commented Aug 5, 2021 •

edited

Loading

ViktorHofer commented Aug 5, 2021 •

edited

Loading

am11 commented Aug 5, 2021 •

edited

Loading

safern commented Aug 5, 2021 •

edited

Loading

ViktorHofer commented Aug 6, 2021 •

edited

Loading