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

Adding support for multi-target builds using Mono #119

Merged

Conversation

atlemann
Copy link
Contributor

@atlemann atlemann commented Mar 4, 2018

  • Use netfx.props suggested by Don Syme.
  • Change FsUnit.MbUnit.fsproj to new SDK and fix build (not needed)
  • Figure out why I have to reference System.Runtime and System.Reflection

Fixes: #118

Removed target framework condition in .fsprojs
Update FSharp.Core to 4.2.3 for netstandard2.0
Add explicit refs to System.Runtime and System.Reflection to make build pass.
@atlemann
Copy link
Contributor Author

atlemann commented Mar 4, 2018

Not sure what to do with MbUnit. I guess I could switch to new SDK for the project file and add the fix there to build on mono and just keep it on net45.

@atlemann
Copy link
Contributor Author

atlemann commented Mar 4, 2018

Another thing, I had to add explicit references to System.Runtime and System.Reflection to get the build working, however I'm getting a lot of warnings about them not being found. Why do I have to add these?

@sergey-tihon
Copy link
Member

We could ignore MbUnit, I do not even know how to run MbUnit tests under the full framework (I do not update this package anymore)

According to FSharp.Core, we definitely should use min possible version in paket.dependencies. I think that 4.1.18 is the good target, like Expecto do FSharp.Core (>= 4.1.18)

@sergey-tihon
Copy link
Member

Dependencies are strange, I thought that xUnit and MSTest should have similar dependencies, so System.Reflection looks even more strange

@sergey-tihon
Copy link
Member

@atlemann this is beautiful! It works!
I decreased version of FSharp.Core because of Libraries should target lower versions of FSharp.Core

@atlemann
Copy link
Contributor Author

atlemann commented Mar 6, 2018

Maybe we should just go back to what it was, since the page mentions that version: Use FSharp.Core nuget 4.2.3 (assembly version 4.4.1.0) or 4.3.3 (assembly version 4.4.3.0) so we don't pull in all those deprecated PCL targets?

Does the FullFramework target have to be as late as net46? Could we also target net45 as mentioned in the link?

I have been trying to find out why it needs System.Runtime and System.Reflection, but I cannot find any consistent answers and usually the fix seems to be to add an explicit reference. We might want add some assembly redirects, since I was getting a lot of warnings about "choosing version" etc. I can try to get rid of those warnings at least, before it's merged.

@atlemann
Copy link
Contributor Author

atlemann commented Mar 6, 2018

I saw in Don Syme's comment that he also had to add references to some facade assemblies, so I guess that solution is fine. System.Reflection was required due to this error:

/home/atle/src/FsUnit/src/FsUnit.Xunit/FsUnit.fs(9,6): error FS1108: The type 'TypeInfo' is required here and is unavailable. You must add a reference to assembly 'System.Reflection, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'. [/home/atle/src/FsUnit/src/FsUnit.Xunit/FsUnit.Xunit.fsproj]

@atlemann
Copy link
Contributor Author

atlemann commented Mar 6, 2018

Looks like Travis CI lost its interwebs and Paket failed to download some deps. Other than that, I think I'm done.

@atlemann atlemann changed the title [WIP] Adding support for multi-target builds using Mono Adding support for multi-target builds using Mono Mar 6, 2018
@sergey-tihon
Copy link
Member

sergey-tihon commented Mar 7, 2018

@atlemann Thank you! Definitely make sense to return to FSharp.Core 4.2.3 (will do)

We could try net451 (because of NHamcrest) but IIRC there were compilation issues with versions lower 4.6...

Should we update paket.template files to force reference System.Reflection.dll during NuGet package install?

@atlemann
Copy link
Contributor Author

atlemann commented Mar 7, 2018

If there were issues with < net46 then we should just keep it like it is now. Do we have to force reference facade assemblies for runtime? I'm no expert when it comes to those things. I can try to google it, or we could try to ask someone.

@atlemann
Copy link
Contributor Author

atlemann commented Mar 7, 2018

Found some old threads about this. Looks like there shouldn't be an explicit reference in the NuGet package, but maybe we'll have to try to create a nuget locally and try to use it from a net46 and netcoreapp2.0 projects.

aspnet/dnx#2031
https://stackoverflow.com/questions/34136228/unable-to-resolve-assembly-reference-issue-without-frameworkassemblies

@sergey-tihon
Copy link
Member

I think that we are good for now and ready to merge.
Since, nobody report issues, 3.1.0 works for users without explicit references.

@sergey-tihon sergey-tihon merged commit bd259f8 into fsprojects:master Mar 7, 2018
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.

Support multi-target builds using Mono
2 participants