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

Add Quic back to aspnetcore transport pack #57338

Merged

Conversation

ViktorHofer
Copy link
Member

@ViktorHofer ViktorHofer commented Aug 13, 2021

  1. System.Net.Quic was accidentally dropped from aspnetcore transport
    package with 4684a1d. Reverting that.

  2. Make sure that the documentation file is placed next to the
    reference assembly if such is included as part of a ProjectReference.

  3. Don't include pdbs for reference assemblies as those aren't needed
    in the transport package.

  4. Move the IsAspNetCore property into packaging.targets as it's not
    needed by the binplacing system anymore.

Also make sure that the documentation file is placed next to the
reference assembly if such is included as part of a ProjectReference.

Also move the IsAspNetCore property into packaging.targets as it's not
needed by the binplacing system anymore.
@ghost
Copy link

ghost commented Aug 13, 2021

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

Issue Details

Also make sure that the documentation file is placed next to the
reference assembly if such is included as part of a ProjectReference.

Also move the IsAspNetCore property into packaging.targets as it's not
needed by the binplacing system anymore.

Author: ViktorHofer
Assignees: ViktorHofer
Labels:

area-Infrastructure-libraries

Milestone: -

ViktorHofer referenced this pull request Aug 13, 2021
…7158)

* automatically update the assembly version

* address feedback

* move IsAspNetCoreApp to packaging.targets

* make assemblyintef pack private, update header and instructions on building the package in servicing

* fixes the allconfig leg

* update the docs

* Update docs/project/library-servicing.md

Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>

Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>
@ViktorHofer
Copy link
Member Author

Failure is #57221.

@ViktorHofer ViktorHofer merged commit fad530f into dotnet:main Aug 13, 2021
@ViktorHofer ViktorHofer deleted the QuicTransportPackAndDocumentation branch August 13, 2021 10:19
@@ -181,6 +181,7 @@
Microsoft.Extensions.Primitives;
System.Diagnostics.EventLog;
System.IO.Pipelines;
System.Net.Quic;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ViktorHofer I dont this as a part of the aspnetcore ref pack(preview 7) ? whats the reason behind including this ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was part of the transport package so it should continue to be part of it. Check the transport package before the pkgproj to csproj migration: https://dnceng.visualstudio.com/public/_packaging?_a=package&feed=dotnet6-transport%40Local&package=Microsoft.AspNetCore.Internal.Transport&protocolType=NuGet&version=6.0.0-rc.1.21401.1&view=overview.

We carry this along in the transport package and aspnetcore then excludes the assembly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But here we are using this list to not update the assembly versions for the things which are part of the ref pack during servicing. As system.Net.Quic is not a part of the ref packs we should be able to update the assembly verison.
We could just manually add a project reference to the transport package to have this assembly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

System.Net.Quic is already part of the Microsoft.NETCore.App shared framework:

. It doesn't matter if it's added to the package via a direct ProjectReference or via an entry in the AspNetCoreAppLibrary property.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only concern left here is that having it in the AspNetCoreAppLibrary property implies that its the part of the aspnetcore shared framework which it is not.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which definitely is a valid concern. Just don't want to again move things around before branching off.

@ghost ghost locked as resolved and limited conversation to collaborators Sep 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants