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

NewSdk and MultiTargeting and Conditions #2596

Closed
0x53A opened this issue Aug 10, 2017 · 19 comments
Closed

NewSdk and MultiTargeting and Conditions #2596

0x53A opened this issue Aug 10, 2017 · 19 comments

Comments

@0x53A
Copy link
Contributor

0x53A commented Aug 10, 2017

I need to multitarget <TargetFrameworks>netstandard1.6;net40;net35</TargetFrameworks>.

For net35 / net40, I need to reference FSharp.Core 3.1.
For ns16 I need to reference a higher version, currently I was thinking about just taking 4.2.2.

I tried to use conditions for this:

group NetFx
	nuget FSharp.Core <= 3.1 redirects:force, content:None, CONDITION: NETFX

group NetStandard

	nuget FSharp.Core 4.2.2 redirects:force, content:None, Condition: NETSTANDARD
group NetStandard

	FSharp.Core

group NetFx

	FSharp.Core
<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    
    <TargetFrameworks>netstandard1.6;net40;net35</TargetFrameworks>

    <NetStandard Condition=" '$(TargetFramework)' == 'netstandard1.6' ">True</NetStandard>
    <NetFx Condition=" '$(TargetFramework)' == 'net35' ">True</NetFx>
    <NetFx Condition=" '$(TargetFramework)' == 'net40' ">True</NetFx>

  </PropertyGroup>

dotnet restore now fails with:

image

Q: Is what I want even possible?

@0x53A 0x53A changed the title NewSdk and MultiTargeting and Conditions [Question] NewSdk and MultiTargeting and Conditions Aug 10, 2017
@matthid
Copy link
Member

matthid commented Aug 10, 2017

First: You need to add a condition to your group.
Second: We don't support conditions on netcore yet

@0x53A
Copy link
Contributor Author

0x53A commented Aug 10, 2017

Second: We don't support conditions on netcore yet

Hm, do you know of another way to solve this? Or should I just use nuget?

@forki
Copy link
Member

forki commented Aug 10, 2017

@matthid why don't we support such conditions? What is missing?

@forki
Copy link
Member

forki commented Aug 10, 2017

This is seriously a fucked up situation with fsharp.core.
Apart from the question why we don't have restrictions for netcore yet, why does argu need net35 and net40? It will propagate the issue downstream.

@0x53A
Copy link
Contributor Author

0x53A commented Aug 10, 2017

eirik had two requirements for any change: need to target net35 and need to target FSharp.Core <= 3.1

Unfortunately, you can't build against netstandard with FSharp.Core 3.1.

I was trying to resurrect fsprojects/Argu#74 (comment)

@forki
Copy link
Member

forki commented Aug 10, 2017 via email

@0x53A
Copy link
Contributor Author

0x53A commented Aug 10, 2017

Hm, I don't think this is the fault of fsc.

FSharp.Core 3.1 just doesn't contain a valid version for netstandard.

@forki
Copy link
Member

forki commented Aug 10, 2017 via email

@0x53A
Copy link
Contributor Author

0x53A commented Aug 10, 2017

One temporary workaround could be to use two project files and build/pack
them both. Afterwards you can use nuget's merge task to merge the nupkgs.
But we should really make the framework restricted work.

Yes. afaik that's how it is currently done.

But then it may be easier to just use nuget directly and <PackageReference Condition="..." .

@forki
Copy link
Member

forki commented Aug 10, 2017

Maybe you could try to help with making the restrictions work by adding some tests? . I don't think it's too much missing.

@matthid
Copy link
Member

matthid commented Aug 10, 2017

I was talking about conditions not restrictions which is a entirely different feature.

Problem wirh restrictions are that we never resolve different versions... We cannot really detect that a package no longer supports a particular framework. so you can never resolve a different fsharp.core without help of the user

@KevinRansom
Copy link

So ... I expected this to work:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFrameworks>netstandard1.6;net40;net35</TargetFrameworks>
    <FSharpCoreImplicitPackageVersion>4.1.17</FSharpCoreImplicitPackageVersion>
    <DisableImplicitSystemValueTupleReference>true</DisableImplicitSystemValueTupleReference>
  </PropertyGroup>

  <ItemGroup>
    <Compile Include="Program.fs" />
  </ItemGroup>

</Project>

However it fails with this output:

c:\temp\aproj\aproj.fsproj : error NU1202: Package System.ValueTuple 4.3.0 is not compatible with net35 (.NETFramework,Version=v3.5). Package System.ValueTuple 4.3.0 supports:\r
c:\temp\aproj\aproj.fsproj : error NU1202:   - netstandard1.0 (.NETStandard,Version=v1.0)\r
c:\temp\aproj\aproj.fsproj : error NU1202:   - portable-net40+sl4+win8+wp8 (.NETPortable,Version=v0.0,Profile=Profile36)
c:\temp\aproj\aproj.fsproj : error NU1202: Package System.ValueTuple 4.3.0 is not compatible with net35 (.NETFramework,Version=v3.5) / win7-x86. Package System.ValueTuple 4.3.0 supports:\r
c:\temp\aproj\aproj.fsproj : error NU1202:   - netstandard1.0 (.NETStandard,Version=v1.0)\r
c:\temp\aproj\aproj.fsproj : error NU1202:   - portable-net40+sl4+win8+wp8 (.NETPortable,Version=v0.0,Profile=Profile36)

As you can see I specifically disabled the automagic package reference to System.ValueTuple even if it did that should find a much higher version than 4.3.0. I hypothesize that nuget restore is verifying:
this from the 4.1.17 nuspec against the net35 target

      <group>
        <dependency id="System.ValueTuple" version="4.3.0" />
      </group>

We don't have that in the 4.1.x nuget package that we build in our build. I haven't deployed a 4.1.18 nuget package but I could if the community is happy that we make this change.

It will break builds who are reliant on referencing fsharp.core 4.1.17 pulling down the system.valuetuple nuget package.

So ... I tried it out with a local copy of the 4.1.18 version of the package and it worked fine:

c:\temp\aproj>dotnet restore
  Restoring packages for c:\temp\aproj\aproj.fsproj...
  Installing FSharp.Core 4.1.18.
  Generating MSBuild file c:\temp\aproj\obj\aproj.fsproj.nuget.g.props.
  Restore completed in 1.6 sec for c:\temp\aproj\aproj.fsproj.

Should I publish 4.1.18 without the dependence on System.ValueTuple.dll?

Kevin

@0x53A
Copy link
Contributor Author

0x53A commented Aug 11, 2017

@KevinRansom Thanks!

So ... I tried it out with a local copy of the 4.1.18 version of the package and it worked fine:

Can you please try if dotnet pack also succeeds?

Should I publish 4.1.18 without the dependence on System.ValueTuple.dll?

Yes, I think it would be good to have a version of FSharp.Core that can be used to target everything from netfx3.5 to netstandard2.0


Edit:

this works (ref dotnet/msbuild#1333 (comment))

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Library</OutputType>
    <TargetFrameworks>net35;netstandard1.6</TargetFrameworks>
    <DisableImplicitFSharpCoreReference>true</DisableImplicitFSharpCoreReference>
    <DisableImplicitSystemValueTupleReference>true</DisableImplicitSystemValueTupleReference>
    <FrameworkPathOverride Condition="'$(TargetFramework)' == 'net35'">C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v3.5\Profile\Client</FrameworkPathOverride>
    
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="FSharp.Core" Version="3.0.2" Condition=" '$(TargetFramework)' == 'net35' " />
    <PackageReference Include="FSharp.Core" Version="4.2.2" Condition=" '$(TargetFramework)' != 'net35' " />
  </ItemGroup>
  
  <ItemGroup>
    <Compile Include="Library.fs" />
  </ItemGroup>

</Project>

image

And generates a (at first glance) correct nupkg:

    <dependencies>
      <group targetFramework=".NETFramework3.5">
        <dependency id="FSharp.Core" version="3.0.2" exclude="Build,Analyzers" />
      </group>
      <group targetFramework=".NETStandard1.6">
        <dependency id="NETStandard.Library" version="1.6.1" exclude="Build,Analyzers" />
        <dependency id="FSharp.Core" version="4.2.2" exclude="Build,Analyzers" />
      </group>
    </dependencies>

@matthid
Copy link
Member

matthid commented Aug 27, 2017

I think we could "fix" this is we generate a real "MSbuild" file with the actual PackageReference items (and add the conditions like we do as of today) instead of our "new" references file.

@0x53A
Copy link
Contributor Author

0x53A commented Aug 27, 2017

I think I would rather implement #2606, so that the user can condition the <PaketReference inside his main project file.

@matthid
Copy link
Member

matthid commented Aug 27, 2017

Ok go ahead :). I think it might be asking for a lot more trouble than you think (depending on how we implement it - obviously you could just use them as marker and let msbuild ignore them completely)

@0x53A
Copy link
Contributor Author

0x53A commented Aug 27, 2017

hm, my idea was to just pass them as commandline parameters to paket here: https://github.com/fsprojects/Paket/compare/master...0x53A:paket-references-xml?expand=1#diff-c17f23be41fa3f5b94dff83d2c955208R24

That means you can use them almost exactly like "PackageReference", including making them conditional, or adding them via some Directory.Build.props, etc.

@matthid
Copy link
Member

matthid commented Aug 27, 2017

I changed that process a bit :)

@enricosada enricosada changed the title [Question] NewSdk and MultiTargeting and Conditions NewSdk and MultiTargeting and Conditions Jan 23, 2018
@enricosada
Copy link
Collaborator

Closing. dupe of #2394

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

No branches or pull requests

5 participants