Skip to content

Conversation

@rainersigwald
Copy link
Member

Prior to this change, ProjectReferences to non-SDK projects did not
report a TargetFrameworks list to the referencing project because
prior to the .NET Core SDK, the TFM for a project was defined in
$(TargetFrameworkMoniker) as the long form of the moniker.

Fixes #3546 by falling back to the legacy TFM in the included-in-all-
projects copy of GetTargetFrameworks.

@rainersigwald rainersigwald changed the base branch from vs15.8 to vs15.9stg July 27, 2018 17:53
Prior to this change, ProjectReferences to non-SDK projects did not
report a `TargetFrameworks` list to the referencing project because
prior to the .NET Core SDK, the TFM for a project was defined in
`$(TargetFrameworkMoniker)` as the long form of the moniker.

Fixes dotnet#3546 by falling back to the legacy TFM in the included-in-all-
projects copy of `GetTargetFrameworks`.
@rainersigwald rainersigwald force-pushed the support-non-sdk-projects-in-tf-compat-check branch from bf910ee to c139abb Compare July 27, 2018 18:05
<_ThisProjectBuildMetadata Include="$(MSBuildProjectFullPath)">
<TargetFrameworks Condition="'$(TargetFrameworks)' != ''">$(TargetFrameworks)</TargetFrameworks>
<TargetFrameworks Condition="'$(TargetFrameworks)' == ''">$(TargetFramework)</TargetFrameworks>
<TargetFrameworks Condition="'$(TargetFrameworks)' == '' and '$(TargetFramework)' == ''">$(TargetFrameworkMoniker)</TargetFrameworks>
Copy link
Contributor

Choose a reason for hiding this comment

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

This probably needs to respect NuGetTargetMoniker for UWP at least where it is distinct from TargetFrameworkMoniker. But see weirdness with old school PCL in ReferrringTargetFrameworkMonikerForProjectReferences (not sure if I got the exact mouthful right from memory).

Copy link
Member Author

Choose a reason for hiding this comment

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

@nguerrera
Copy link
Contributor

I forget why we aren't doing this...

@rainersigwald
Copy link
Member Author

Oh, I didn't mean to close this, it auto closed when I deleted the target branch. Looks like I can't reopen it, so I'll have to open a new PR.

Current status is that I'm pretty sure this breaks legacy VS projects, but need to double-check.

@nguerrera
Copy link
Contributor

OK, I remember you said it might not be compatible, but I didn't remember a final conclusion. Makes sense now.

@rainersigwald
Copy link
Member Author

Opened #3620 so I don't forget about this.

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.

3 participants