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 test which reproduces but in complex framework resolution #71959

Merged
merged 1 commit into from
Jul 12, 2022

Conversation

vitek-karas
Copy link
Member

The problem is if there is a high-level framework which lists its dependencies as for example NETCore.App first and then ASPNET.App. When we reorder the frameworks during resolution we try to put the lowest level last, but the reorder doesn't work if there's a high-level framework which depends on middle one last - as the last resolved framework in that scenario will be the middle one (ASPNET.App) and it will end up last in the list.

The code assumes the last framework is the root (NETCore.App) and thus we look for hostpolicy there.

This test is a repro of #71027
I'm adding this as "failing" (the failing part is commented out) so that we have a in-house, simpler repro of the problem.

The problem is if there is a high-level framework which lists its dependencies as for example NETCore.App first and then ASPNET.App. When we reorder the frameworks during resolution we try to put the lowest level last, but the reorder doesn't work if there's a high-level framework which depends on middle one last - as the last resolved framework in that scenario will be the middle one (ASPNET.App) and it will end up last in the list.

The code assumes the last framework is the root (NETCore.App) and thus we look for hostpolicy there.
@vitek-karas vitek-karas added test-enhancement Improvements of test source code area-Host labels Jul 11, 2022
@vitek-karas vitek-karas added this to the 7.0.0 milestone Jul 11, 2022
@vitek-karas vitek-karas requested a review from elinor-fung July 11, 2022 18:13
@vitek-karas vitek-karas self-assigned this Jul 11, 2022
@ghost
Copy link

ghost commented Jul 11, 2022

Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov
See info in area-owners.md if you want to be subscribed.

Issue Details

The problem is if there is a high-level framework which lists its dependencies as for example NETCore.App first and then ASPNET.App. When we reorder the frameworks during resolution we try to put the lowest level last, but the reorder doesn't work if there's a high-level framework which depends on middle one last - as the last resolved framework in that scenario will be the middle one (ASPNET.App) and it will end up last in the list.

The code assumes the last framework is the root (NETCore.App) and thus we look for hostpolicy there.

This test is a repro of #71027
I'm adding this as "failing" (the failing part is commented out) so that we have a in-house, simpler repro of the problem.

Author: vitek-karas
Assignees: vitek-karas
Labels:

test-enhancement, area-Host

Milestone: 7.0.0

}

[Fact]
public void TwoAppFrameworksOnTopOfMiddleWare()
Copy link
Member

@elinor-fung elinor-fung Jul 12, 2022

Choose a reason for hiding this comment

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

It would still be an issue with just one framework on top of MiddleWare, as long as it lists MiddleWare last instead of NETCore.App, right? Or does is need the two both doing that?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes - it probably would fail with just one... I added two as it's closer to the original customer reported repro.

@vitek-karas
Copy link
Member Author

The CI failure is #70969

@vitek-karas vitek-karas merged commit 42c321a into dotnet:main Jul 12, 2022
@vitek-karas vitek-karas deleted the AddFrameworkResolutionTest branch July 12, 2022 15:40
@AraHaan
Copy link
Member

AraHaan commented Jul 13, 2022

@vitek-karas can this be backported to release/6.0 as well?

@vitek-karas
Copy link
Member Author

As noted in #72080 I personally don't think the gain/risk tradeoff is there for this to be ported to .NET 6 (the test doesn't matter, the future product change).

@ghost ghost locked as resolved and limited conversation to collaborators Aug 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Host test-enhancement Improvements of test source code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants