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 restore support for C++/CLI .NET Core projects #4076

Merged
merged 13 commits into from
Jun 8, 2021

Conversation

nkolev92
Copy link
Member

@nkolev92 nkolev92 commented May 26, 2021

Bug

Fixes: NuGet/Home#10194

Regression? Last working version:

Description

Design at: NuGet/Home#10299

C++/CLI .NET Core projects are projects that can install both native and managed packages.
This PR introduces a new dual compatibility framework that allows us to represent these types of projects.

I'll add some comments inline to aide the review.

PR Checklist

  • PR has a meaningful title

  • PR has a linked issue.

  • Described changes

  • Tests

    • Automated tests added
    • OR
    • Test exception
    • OR
    • N/A
  • Documentation

    • Documentation PR or issue filled
    • OR
    • N/A

@nkolev92 nkolev92 force-pushed the dev-nkolev92-cppCliFrameworkSupport branch from fcc7f80 to ada28c0 Compare May 27, 2021 18:31
@nkolev92 nkolev92 marked this pull request as ready for review May 28, 2021 20:11
@nkolev92 nkolev92 requested a review from a team as a code owner May 28, 2021 20:11

return MSBuildProjectFrameworkUtility.GetProjectFramework(
projectFullPath,
targetFrameworkMoniker,
targetPlatformMoniker,
targetPlatformMinVersion);
targetPlatformMinVersion,
clrSupport);
Copy link
Member Author

Choose a reason for hiding this comment

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

This is nomination code that handles the VS side.

@@ -465,6 +465,11 @@ public class SourceRepositoryDependencyProvider : IRemoteDependencyProvider
targetFramework,
item => item.TargetFramework);

if (dependencyGroup == null && DeconstructFallbackFrameworks(targetFramework) is DualCompatibilityFramework dualCompatibilityFramework)
Copy link
Member Author

Choose a reason for hiding this comment

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

This handles the dependencies selection with dual compat framework.

@@ -152,6 +152,11 @@ internal static List<List<SelectionCriteria>> CreateOrderedCriteriaSets(RestoreT
{
// Add the root project framework first.
orderedCriteriaSets.Add(CreateCriteria(targetGraph, assetTargetFallback.RootFramework));
// Add the secondary framework if dual compatibility framework.
if (assetTargetFallback.RootFramework is DualCompatibilityFramework dualCompatibilityFramework)
Copy link
Member Author

Choose a reason for hiding this comment

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

This ensures the package assets are selected with dual compatibility in mind.

@nkolev92 nkolev92 requested a review from kartheekp-ms June 3, 2021 01:54
@nkolev92 nkolev92 force-pushed the dev-nkolev92-cppCliFrameworkSupport branch from 86edc41 to 4b13481 Compare June 3, 2021 09:51
Copy link
Member

@zivkan zivkan left a comment

Choose a reason for hiding this comment

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

I don't feel motivated to read the test code right now, but I have some comments on the product code.

@nkolev92 nkolev92 requested a review from zivkan June 7, 2021 23:37
@nkolev92
Copy link
Member Author

nkolev92 commented Jun 7, 2021

addressed your feedback @kartheekp-ms @zivkan PTAL

@nkolev92 nkolev92 force-pushed the dev-nkolev92-cppCliFrameworkSupport branch from cdff7da to 89d6b55 Compare June 8, 2021 18:07
@nkolev92 nkolev92 merged commit 702e250 into dev Jun 8, 2021
@nkolev92 nkolev92 deleted the dev-nkolev92-cppCliFrameworkSupport branch June 8, 2021 22:49
@@ -142,7 +142,7 @@ internal static List<List<SelectionCriteria>> CreateOrderedCriteriaSets(RestoreT
// Create an ordered list of selection criteria. Each will be applied, if the result is empty
// fallback frameworks from "imports" will be tried.
// These are only used for framework/RID combinations where content model handles everything.
// AssetTargetFallback frameworks will provide multiple criteria since all assets need to be
// AssetTargetFallback and DualCompatbiility frameworks will provide multiple criteria since all assets need to be

Choose a reason for hiding this comment

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

Typo, DualCompatbiility

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NuGet effective target framework inference support for C++/CLI projects
4 participants