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

TargetFrameworkVersion gets lost on PCL targets #492

Closed
clairernovotny opened this issue Dec 14, 2016 · 3 comments
Closed

TargetFrameworkVersion gets lost on PCL targets #492

clairernovotny opened this issue Dec 14, 2016 · 3 comments
Assignees
Milestone

Comments

@clairernovotny
Copy link
Member

@clairernovotny clairernovotny commented Dec 14, 2016

Splitting from #477

There are a few issues around using profile-based PCL's in the SDK build system that all seem be traced back to the TargetFrameworkVersion segment of the NuGetTargetMoniker getting "lost."

@emgarten @yishaigalatzer @rrelyea The .NETPortable v0.0 thing is more pervasive than just the above as it appears to affect at least the lock file generation and package generation.

In order to support portable-win81+wpa81 for example, you need the following. Note the explicit NuGetTargetMoniker is required with v0.0 in it because that's how the lock file was generated.

  <PropertyGroup Condition="'$(TargetFramework)' == 'portable-win81+wpa81'">
    <TargetFrameworkIdentifier>.NETPortable</TargetFrameworkIdentifier>
    <TargetFrameworkVersion>v4.6</TargetFrameworkVersion>
    <TargetFrameworkProfile>Profile32</TargetFrameworkProfile>
    <DefaultLanguage>en-US</DefaultLanguage>
    <NugetTargetMoniker>.NETPortable,Version=v0.0,Profile=Profile32</NugetTargetMoniker>
    <LanguageTargets>$(MSBuildExtensionsPath32)\Microsoft\Portable\$(TargetFrameworkVersion)\Microsoft.Portable.CSharp.targets</LanguageTargets>
  </PropertyGroup>

Later, when calling the pack target, the generated nuspec has the following

      <group targetFramework=".NETPortable0.0-Profile32">
        <dependency id="System.Reactive" version="3.1.1" exclude="Build,Analyzers" />
        <dependency id="NETStandard.Library" version="1.6.1" exclude="Build,Analyzers" />
      </group>

The pack target is generating it wrong too (I believe it should be .NETPortable4.6-Profile32)

@emgarten
Copy link
Member

@emgarten emgarten commented Dec 14, 2016

This sounds painful.

NuGet ignores the different versions of portable, so when used within a nupkg or nuspec it is fine to have .NETPortable0.0.

For the build task consuming the assets file I would expect this behavior:

  1. If a target graph with an exact match on the TFM exists use it
  2. Fallback to using .NETPortable,Version=*,Profile=Profile32

It would be really unexpected if 2 had more than one match, failing with an error would be reasonable there.

NuGet has never used the portable version number in the past, so requiring it now could break users passing in TFMs as portable-*

@clairernovotny
Copy link
Member Author

@clairernovotny clairernovotny commented Dec 14, 2016

Fair enough, using 2 is ok long as the "best match" is evaluated correctly for p2p refs. Key thing here is just ensuring that the NugetTargetMoniker hack isn't required.

@livarcocc
Copy link
Contributor

@livarcocc livarcocc commented Mar 30, 2018

Seems like the Fallback is the right escape hatch here. Closing this issue.

@livarcocc livarcocc closed this Mar 30, 2018
@clairernovotny clairernovotny mentioned this issue Jan 31, 2020
2 of 9 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.