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

Replace harvested package assets with live built configurations #47530

Closed
25 tasks done
ViktorHofer opened this issue Jan 27, 2021 · 6 comments
Closed
25 tasks done

Replace harvested package assets with live built configurations #47530

ViktorHofer opened this issue Jan 27, 2021 · 6 comments

Comments

@ViktorHofer
Copy link
Member

ViktorHofer commented Jan 27, 2021

The runtime repository currently produces 98 packages from libraries and 25 of those contain prebuilts (assets which are not built in the master/main branch anymore). Such prebuilt assets are redistributed into packages via a mechanism called “harvesting”. Without going into detail, the harvesting infrastructure downloads the latest available assets of the “current band” and places them into a specified subfolder inside the package.

In contrast to simplification of the build graph, harvesting has numerous downsides:

  • Increased cost of servicing as code fixes need to be applied in the repository’s branch in which the code lives, which is not necessarily the same place where the package is produced and shipped from.
  • Dependency between servicing releases, especially during MSRCs which forces schedules to be aligned.
  • Source link and newer compiler features (IL size reduction / optimization) are not applicable to harvested assets which causes NuGet Package Explorer to list the whole package as unhealthy.
  • Package validation costs
  • Fat packages
  • Harvesting build validation causes builds to fail in PRs every time new assets are shipped to indicate that harvesting defines should be updated.
  • Maintenance of code bases which are not tested anymore.

Based on these disadvantages, we would like to remove harvesting in dotnet/runtime entirely and only ship what is being built live.

Implementation

For the following libraries, build configurations need to be added and/or harvesting configurations in the pkgproj need to be removed.

Add a build warning wherever functionality would regress.

cc @danmosemsft @ericstj @terrajobst @tommcdon

@ViktorHofer ViktorHofer added this to the 6.0.0 milestone Jan 27, 2021
@ViktorHofer ViktorHofer self-assigned this Jan 27, 2021
@ghost
Copy link

ghost commented Jan 27, 2021

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

Issue Details

The runtime repository currently produces 98 packages from libraries and 25 of those contain prebuilts (assets which are not built in the master/main branch anymore). Such prebuilt assets are redistributed into packages via a mechanism called “harvesting”. Without going into detail, the harvesting infrastructure downloads the latest available assets of the “current band” and places them into a specified subfolder inside the package.

In contrast to simplification of the build graph, harvesting has numerous downsides:

  • Increased cost of servicing as code fixes need to be applied in the repository’s branch in which the code lives, which is not necessarily the same place where the package is produced and shipped from.
  • Dependency between servicing releases, especially during MSRCs which forces schedules to be aligned.
  • Source link and newer compiler features (IL size reduction / optimization) are not applicable to harvested assets which causes NuGet Package Explorer to list the whole package as unhealthy.
  • Package validation costs
  • Fat packages
  • Harvesting build validation causes builds to fail in PRs every time new assets are shipped to indicate that harvesting defines should be updated.
  • Maintenance of code bases which are not tested anymore.

Based on these disadvantages, we would like to remove harvesting in dotnet/runtime entirely and only ship what is being built live.

Implementation

For the following libraries, build configurations need to be added and/or harvesting configurations in the pkgproj need to be removed.

  • Microsoft.Extensions.DependencyModel (netstandard1.6)
  • Microosft.Win32.Registry (netstandard1.3)
  • Microsoft.Win32.Registry.AccessControl
  • System.ComponentModel.Annotations (net461, netstandard1.4, netstandard2.0)
  • System.Composition.AttributedModel
  • System.Composition.Convention
  • System.Composition.Hosting
  • System.Composition.Runtime
  • System.Composition.TypedParts
  • System.Drawing.Common (net461, netstandard2.0)
  • System.IO.FileSystem.AccessControl
  • System.IO.Pipelines
  • System.IO.Pipes.AccessControl (net461, net5.0, netstandard2.0, netcoreapp3.1)
  • System.Net.Http.WinHttpHandler
  • System.Reflection.Context
  • System.Security.AccessControl
  • System.Security.Cryptography.Cng (remove package)
  • System.Security.Cryptography.OpenSsl (remove package)
  • System.Security.Cryptography.Pkcs
  • System.Security.Cryptography.ProtectedData
  • System.Security.Principal.Windows (netstandard1.3)
  • System.ServiceProcess.ServiceController
  • System.Text.Encoding.CodePages (netstandard1.3)
  • System.Text.Encodings.Web
  • System.Threading.AccessControl

cc @danmosemsft @ericstj @terrajobst @tommcdon

Author: ViktorHofer
Assignees: ViktorHofer
Labels:

area-Infrastructure-libraries

Milestone: 6.0.0

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Jan 27, 2021
@ViktorHofer ViktorHofer removed the untriaged New issue has not been triaged by the area owner label Jan 27, 2021
ViktorHofer added a commit to ViktorHofer/runtime that referenced this issue Apr 21, 2021
The netstandard1.6 configuration of Microsoft.Extensions.DependencyModel
isn't built anymore. Instead the already built matching binary from the
latest available package version is redistributed when packaging the
DependencyModel library.

Also dropping the netstandard1.3 asset and the net451 one as the
minimum supported set of platforms are ones that support netstandard2.0.

In addition to the harvesting removal, cleaning up the src project which
had an unnecessary condition and property set.

Contributes to dotnet#47530
ViktorHofer added a commit to ViktorHofer/runtime that referenced this issue Apr 21, 2021
The net46 and netstandard1.3 configuration of Microsoft.Win32.Registry
and Microsoft.Win32.Registry.AccessControl isn't built anymore. Instead
the already built matching binary from the latest available package
version is redistributed when packaging these libraries.

Dropping the netstandard1.3 asset and the net46 one as the
minimum supported set of platforms are ones that support netstandard2.0.
For .NET Framework, there's still a net461 configuration to avoid
binding redirect issues.

Contributes to dotnet#47530
ViktorHofer added a commit that referenced this issue Apr 22, 2021
* Remove harvesting of M.E.DependencyModel

The netstandard1.6 configuration of Microsoft.Extensions.DependencyModel
isn't built anymore. Instead the already built matching binary from the
latest available package version is redistributed when packaging the
DependencyModel library.

Also dropping the netstandard1.3 asset and the net451 one as the
minimum supported set of platforms are ones that support netstandard2.0.

In addition to the harvesting removal, cleaning up the src project which
had an unnecessary condition and property set.

Contributes to #47530
ViktorHofer added a commit to ViktorHofer/runtime that referenced this issue Apr 22, 2021
The netstandard1.0 and portable* configurations of the
System.Composition.* packages aren't built anymore. Instead
the already built matching binary from the latest available package
version is redistributed when packaging these libraries.

Dropping the netstandard1.0 asset and the portable* one as the
minimum supported set of platforms are ones that support netstandard2.0.
For .NET Framework, there's still a net461 configuration to avoid
binding redirect issues.

Contributes to dotnet#47530
ViktorHofer added a commit to ViktorHofer/runtime that referenced this issue Apr 22, 2021
The removed configurations (few netstandard1.x, portable*, netcore50)
of the touched packages aren't built anymore. Instead
the already built matching binaries from the latest available packages
are redistributed when packaging these libraries.

Dropping these assets as the minimum supported set of platforms are ones
that support netstandard2.0. For .NET Framework, there's still a net461
configuration present to avoid binding redirect issues.

Contributes to dotnet#47530
ViktorHofer added a commit that referenced this issue Apr 22, 2021
The netstandard1.0 and portable* configurations of the
System.Composition.* packages aren't built anymore. Instead
the already built matching binary from the latest available package
version is redistributed when packaging these libraries.

Dropping the netstandard1.0 asset and the portable* one as the
minimum supported set of platforms are ones that support netstandard2.0.
For .NET Framework, there's still a net461 configuration to avoid
binding redirect issues.

Contributes to #47530
@danmoseley
Copy link
Member

Are there any updates to the plan (since this was opened) that we need to share here?

@ViktorHofer
Copy link
Member Author

ViktorHofer commented Apr 22, 2021

Only minor adjustments, two configurations which are dropped instead of brought back. I'm in regular sync with @terrajobst about this. I do have an excel spreadsheet which has more details which I will share with you again offline.

@danmoseley
Copy link
Member

@ViktorHofer I don't have concerns that I'm not up to date. I just wanted to make sure that community was up to date, presumably here or possibly in some linked design.

ViktorHofer added a commit that referenced this issue Apr 23, 2021
* Stop harvesting old frameworks in some libraries

The removed configurations (few netstandard1.x, portable*, netcore50)
of the touched packages aren't built anymore. Instead
the already built matching binaries from the latest available packages
are redistributed when packaging these libraries.

Dropping these assets as the minimum supported set of platforms are ones
that support netstandard2.0. For .NET Framework, there's still a net461
configuration present to avoid binding redirect issues.

Contributes to #47530
ViktorHofer added a commit to ViktorHofer/runtime that referenced this issue Apr 26, 2021
To reduce complexity of the cryptography cng library, dead ending its
package as most of the configurations are partial facades anyway and
adding Cng to the targeting pack. It's already part of the runtime and
exposed in aspnetcore's targeting pack.

.NET Standard libraries can continue to use the latest available package
which harvests for .NET Standard configurations.

Contributes to dotnet#47530
ViktorHofer added a commit to ViktorHofer/runtime that referenced this issue Apr 26, 2021
The netstandard1.3 and net46 configurations of the
System.Security.Cryptography.ProtectedData packages aren't built
anymore. Instead the already built matching binary from the latest
available package version is redistributed when packaging these
libraries.

Dropping the netstandard1.3 asset and the net46 one as the
minimum supported set of platforms are ones that support netstandard2.0.
For .NET Framework, there's still a net461 configuration to avoid
binding redirect issues.

Contributes to dotnet#47530
@ViktorHofer
Copy link
Member Author

Replied offline.

ViktorHofer added a commit that referenced this issue Apr 26, 2021
The netstandard1.3 and net46 configurations of the
System.Security.Cryptography.ProtectedData packages aren't built
anymore. Instead the already built matching binary from the latest
available package version is redistributed when packaging these
libraries.

Dropping the netstandard1.3 asset and the net46 one as the
minimum supported set of platforms are ones that support netstandard2.0.
For .NET Framework, there's still a net461 configuration to avoid
binding redirect issues.

Contributes to #47530
ViktorHofer added a commit that referenced this issue Apr 26, 2021
* Dead end S.Security.Cryptography.OpenSsl package

To reduce complexity of the cryptography OpenSsl library, dead ending
its package as most of the configurations are partial facades anyway and
adding OpenSsl to the targeting pack. It's already part of the runtime
and exposed in aspnetcore's targeting pack.

.NET Standard libraries can continue to use the latest available package
which harvests for .NET Standard configurations.

Contributes to #47530
ViktorHofer added a commit to ViktorHofer/runtime that referenced this issue Apr 26, 2021
Nearly all configurations in the System.ComponentModel.Annotations
package weren't built live anymore and just redistributed from older
packages. Except for the netstandard2.0 configuration, the package
didn't receive any changes. As the library is exposed as part of the
shared framework, a reference to the package on .NETCoreApp isn't
necessary and will be ignored by the SDK.

Based on these reasons, dead-ending the package to remove the burden
of maintaining the harvesting of old configurations.

Contributes to dotnet#47530
ViktorHofer added a commit that referenced this issue Apr 26, 2021
* Dead end S.Security.Cryptography.Cng package

To reduce complexity of the cryptography cng library, dead ending its
package as most of the configurations are partial facades anyway and
adding Cng to the targeting pack. It's already part of the runtime and
exposed in aspnetcore's targeting pack.

.NET Standard libraries can continue to use the latest available package
which harvests for .NET Standard configurations.

Contributes to #47530
ViktorHofer added a commit to ViktorHofer/runtime that referenced this issue Apr 26, 2021
The removed configurations (few netstandard1.x, portable*, netcore50,
net46) of the touched packages aren't built anymore. Instead
the already built matching binaries from the latest available packages
are redistributed when packaging these libraries.

Dropping these assets as the minimum supported set of platforms are ones
that support netstandard2.0. For .NET Framework, there's still a net461
configuration present to avoid binding redirect issues.

Contributes to dotnet#47530
ViktorHofer added a commit to ViktorHofer/runtime that referenced this issue Apr 27, 2021
Removing the netcoreapp2.0, netstandard2.0 and net461 configurations
of System.Drawing.Common as these aren't built live anymore (harvested).
Replace the netstandard2.0 and net461 harvested asset with live builds
and stop supporting netcoreapp2.x.

Bringing these assets back as the minimum supported set of platforms are
ones that support netstandard2.0 and are still in support.

Contributes to dotnet#47530
ViktorHofer added a commit that referenced this issue Apr 27, 2021
* Stop harvesting old frameworks in more libraries

The removed configurations (few netstandard1.x, portable*, netcore50,
net46) of the touched packages aren't built anymore. Instead
the already built matching binaries from the latest available packages
are redistributed when packaging these libraries.

Dropping these assets as the minimum supported set of platforms are ones
that support netstandard2.0. For .NET Framework, there's still a net461
configuration present to avoid binding redirect issues.

Contributes to #47530
ViktorHofer added a commit that referenced this issue Apr 27, 2021
Nearly all configurations in the System.ComponentModel.Annotations
package weren't built live anymore and just redistributed from older
packages. Except for the netstandard2.0 configuration, the package
didn't receive any changes. As the library is exposed as part of the
shared framework, a reference to the package on .NETCoreApp isn't
necessary and will be ignored by the SDK.

Based on these reasons, dead-ending the package to remove the burden
of maintaining the harvesting of old configurations.

Contributes to #47530
ViktorHofer added a commit that referenced this issue Apr 29, 2021
* Drawing: Replace harvested assets with live config

Removing the netcoreapp2.0, netstandard2.0 and net461 configurations
of System.Drawing.Common as these aren't built live anymore (harvested).
Replace the netstandard2.0 and net461 harvested asset with live builds
and stop supporting netcoreapp2.x.

Bringing these assets back as the minimum supported set of platforms are
ones that support netstandard2.0 and are still in support.

Contributes to #47530
ViktorHofer added a commit to ViktorHofer/runtime that referenced this issue May 3, 2021
The removed configurations (netstandard1.3, netcoreapp2.1, net46)
of the touched packages aren't built anymore. Instead
the already built matching binaries from the latest available packages
are redistributed when packaging these libraries.

Dropping these assets as the minimum supported set of platforms are ones
that support netstandard2.0 and are still in-support. As an example,
also dropping the netcoreapp2.1 asset which isn't supported anymore and
adding a target to prevent the package being used for netcoreapp2.x.
For .NET Framework, there's still a net461
configuration present to avoid binding redirect issues.

Contributes to dotnet#47530
ViktorHofer added a commit that referenced this issue May 4, 2021
* Stop harvesting S.Sec.Cryptography.Pkcs

The removed configurations (netstandard1.3, netcoreapp2.1, net46)
of the touched packages aren't built anymore. Instead
the already built matching binaries from the latest available packages
are redistributed when packaging these libraries.

Dropping these assets as the minimum supported set of platforms are ones
that support netstandard2.0 and are still in-support. As an example,
also dropping the netcoreapp2.1 asset which isn't supported anymore and
adding a target to prevent the package being used for netcoreapp2.x.
For .NET Framework, there's still a net461
configuration present to avoid binding redirect issues.

Contributes to #47530

* Fix the build

* Remove workaround and mark Xamarin supported
ViktorHofer added a commit to ViktorHofer/runtime that referenced this issue May 5, 2021
The removed configurations (netstandard1.3, netcoreapp2.1, net46) aren't
built anymore. Instead the already built matching binaries from the
latest available packages are redistributed when packaging these
libraries.

Dropping these assets as the minimum supported set of platforms are ones
that support netstandard2.0 and are still in-support. As an example,
also dropping the netcoreapp2.1 asset which isn't supported anymore and
upgrading it to netcoreapp3.1.. For .NET Framework, there's still a
net461 configuration present to avoid binding redirect issues.

The ugly part of this change is that System.IO.Pipes.AccessControl
depends on an internal API of System.IO.Pipes for all .NETCoreApp
configurations. Because of that, for configurations that don't apply
to the current band (netcoreapp3.1, net5.0), the runtime packs need
to be downloaded so that runtime assembly can be referenced. Ideally
we will come up with a better solution to not require runtime packs
to be fetched as that adds 60MB of download size to the repo and breaks
source build.

Contributes to dotnet#47530
ViktorHofer added a commit that referenced this issue May 11, 2021
* Stop harvesting S.IO.Pipes.AccessControl

The removed configurations (netstandard1.3, netcoreapp2.1, net46) aren't
built anymore. Instead the already built matching binaries from the
latest available packages are redistributed when packaging these
libraries.

Dropping these assets as the minimum supported set of platforms are ones
that support netstandard2.0 and are still in-support. As an example,
also dropping the netcoreapp2.1 asset which isn't supported anymore and
upgrading it to netcoreapp3.1.. For .NET Framework, there's still a
net461 configuration present to avoid binding redirect issues.

The ugly part of this change is that System.IO.Pipes.AccessControl
depends on an internal API of System.IO.Pipes for all .NETCoreApp
configurations. Because of that, for configurations that don't apply
to the current band (netcoreapp3.1, net5.0), the runtime packs need
to be downloaded so that runtime assembly can be referenced. Ideally
we will come up with a better solution to not require runtime packs
to be fetched as that adds 60MB of download size to the repo and breaks
source build.

Contributes to #47530

* Use non shipping refs for Pipes typeforwards
@ViktorHofer
Copy link
Member Author

Closing as the tracked work is completed.

@ghost ghost locked as resolved and limited conversation to collaborators Jul 9, 2021
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

2 participants