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

Support netstandard along side dotnet. #525

Merged
merged 43 commits into from
Nov 4, 2017
Merged

Conversation

aignas
Copy link
Contributor

@aignas aignas commented Oct 16, 2017

This is a PR with changes that I made whilst experimenting with bringing full support of netstandard to mathnet numerics. As it seems that the original author of #499 is not working anymore on it, I propose to finish the support of netstandard here.

Could anyone review this?

using NUnitLite;
using System.Reflection;

namespace NodaTime.Test
Copy link
Member

Choose a reason for hiding this comment

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

The namespace and copyright header would have to be changed. But the change towards an executable makes sense, I wanted to go in this direction as well (for other reasons, unrelated to .Net Core). Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed it. Let me know if it is still unsuitable. :)

@dsyme
Copy link

dsyme commented Oct 16, 2017

Why not target netstandard 2.0?

I presume many of the contracts your are eliding are in netstandard 2.0.

@aignas
Copy link
Contributor Author

aignas commented Oct 16, 2017

@dsyme, this is because it is best to target as low as possible and since there is already code for targetting portable, I thought that it would be easiest to do that first. There is some stuff (Complex number serialization support) that is comming out only in v2.1. Hence, it would be nice to do that at the same time.

Unless there is a lot of gain when going with 2.0, I would say that for initial netstandard support let's try the proposed versions as they are as low as possible in the 1.x range. I suppose there will be also work to be done in order to get all of the packaging and other stuff going.

@cdrnet, I am not really familliar with the packaging of this project. Would you be willing to give any pointers on what are the requirements? I assume that this is multitargeting various .NET versions. Should we just pack using the 2017 csproj files, because they make the multitargeting really easy and whatever magic is involved elsewhere most likely can be done cleaner with the new format. :)

@cdrnet
Copy link
Member

cdrnet commented Oct 16, 2017

Isn't .Net Standard 1.x basically deprecated?

@aignas great that you bring up packaging. We currently automate the whole process, which to my understanding is more than the dotnet tools provide out of the box:

For a specific bundle (i.e. one of: core, coreSigned (strong name), mklWin, mklLinux, cuda, openBlas):

  • Parse the release notes file to get both the version number to use and the release notes text
  • Clean (conditionally)
  • Enforce the version from the release notes as assembly and file version
  • Build (net40, net35, pcl7, pcl47, pcl78, pcl259, pcl328; with or without a strong name depending on bundle)
  • Run Tests
  • Generate zip files (including a generated readme based on the release notes) for the release archive.
  • Generate NuGet packages (often multiple; including generated release notes in the meta data); originally also a source + symbol package but we stopped doing that.
  • Generate the website (including generated release notes)
  • Generate the API docs website
  • Generate and publish a git release tag, also containing the release notes
  • Publish project website and API docs website
  • Publish the zip file to the public release archive

I'm quite happy with this process, as it turns the whole thing in a single command, and up to the tests it works both in Windows and on Linux. I understand there may be a need to change some aspects of this to move forward, but it is important to me that the release process does not get significantly more complicated in practice.

I'm ok with dropping the PCLs and net35 for the next major version, but we need to continue including full .Net Framework builds besides .Net Standard.

@cdrnet
Copy link
Member

cdrnet commented Oct 16, 2017

Btw, thanks a lot @aignas for moving forward here!

@cdrnet
Copy link
Member

cdrnet commented Oct 16, 2017

I'd hope we can "simply" modify the some of the build steps to call out to the dotnet SDK, but otherwise keep it as is. But I cannot say yet whether this is feasible.

@aignas
Copy link
Contributor Author

aignas commented Oct 16, 2017

OK, I'll try to read up on it more, so that the CI starts working again.

As for the deprecation of the netstandard1.x, I have not heard about it. In the announcement of netstandard 2.0 they officially deprecated the PCLs, so maybe removing the PORTABLE ifdefs would be something to get done after or before this gets merged.

The only benefit of the new tooling, which comes with the VS 2017 is the fact that one can generate a single NuGet package, which can be targetting multiple .NET frameworks (i.e. multiple dlls in one NuGet package). This is also nice, and the things that one can do with the conditional referencing of the assemblies and other things is also nice. However, I'll try to go the least resistance path here and will see how it goes.

@cdrnet
Copy link
Member

cdrnet commented Oct 16, 2017

VS 2017 is the fact that one can generate a single NuGet package, which can be targetting multiple .NET frameworks.

We also do such multi-targeting, but currently we need multiple project files (one per target) and we generate the nuspec to pack them explicitly into a single package. Having to maintain these multiple project files is somewhat painful; I'd be happy if we could replace them with a single multi-targeting project file each, as supported by the new tooling. Maybe we can also let the new tooling then generate the NuGet package, and then patch that later to our needs (if we cannot fine-control the process enough, e.g. to include the release notes).

NB: I think we can drop the readme.txt in the root of the package; this dates back to the very early days of NuGet and seems to be deprecated.

@cdrnet
Copy link
Member

cdrnet commented Oct 16, 2017

If it helps we can also completely separate the bundles (so there is no touching point of this work with the native provider build process, which builds the native library NuGet packages).

@aignas
Copy link
Contributor Author

aignas commented Oct 16, 2017

Sounds good. So as I understand the number of the solution files would also decrease? Or is there any other purpose to them?
Now we have:

  • Math.Numerics.2017.sln - The new solution file, temporary.
  • MathNet.Numerics.All.sln
  • MathNet.Numerics.Data.sln
  • MathNet.Numerics.NativeProviders.sln
  • MathNet.Numerics.Net35Only.sln
  • MathNet.Numerics.sln
  • MathNet.Numerics.XamariniOSOnly.sln

As I understand, we could safely remove *.Net35Only, *.XamariniOSOnly, *.All and replace MathNet.Numerics.sln with the new format? From your comment I understand that the *.Data and *.NativeProviders could be left untouched for the time being.

@cdrnet
Copy link
Member

cdrnet commented Oct 16, 2017

Yes. I propose in the end to call the new solution simply MathNet.Numerics.sln.

The Net35Only solution was originally created to support very old Visual Studio versions, but this is no longer required. We can drop it.

The Xamarin one was created for similar reasons, to exclude projects not relevant nor supported in the old Xamarin Studio. The new Visual Studio for Mac seems to be focused on .Net Core entirely, so I expect this can be dropped as well (I cannot verify).

@petriashev
Copy link

Any progress?
Now i have to use unofficial package from @iamcarbon
https://www.nuget.org/packages/MathNet.Numerics.Core/

@aignas
Copy link
Contributor Author

aignas commented Oct 31, 2017 via email

@cdrnet
Copy link
Member

cdrnet commented Nov 1, 2017

If the F# part is blocking, we can migrate it in a second step (we'll have to reduce the platform support for the F# part to net45+/netstandard1.6+ anyway, since FSharp.Core no longer supports older targets).

@aignas
Copy link
Contributor Author

aignas commented Nov 1, 2017 via email

@aignas
Copy link
Contributor Author

aignas commented Nov 1, 2017

@cdrnet, I managed to get the F# projects building with the new multitargeting feature of the new project format. It seems that the Visual Studio tooling support may be limited until v15.5 is released, but it should be OK to build it with the dotnet CLI.

Now the FSharp project supports netstandard1.6 and I needed to drop support for net35 for the TestData.

So my plan now is to remove the old portable projects and do the solution file consolidation and drop support for everything below NET4.5. This should be more straightforward since I think I managed to get the F# going.

Let me know if you would like to merge this as is and then tackle the consolidation and packaging as a separate PR. I am also fine if we decide to do everything in one go.

ryanblackwell and others added 14 commits November 2, 2017 19:53
Use a different way to initialize the PRNG creation for the .NET
framework as described on this SO answer:
https://stackoverflow.com/a/38644970

Add System.Security.Cryptography.Algorithms NuGet on netstandard1.6.
Use NUnitLite to run tests
This is for easier debugging in the future of the Complex64
MatrixStorage serialization tests.
It seems that serialization support for `Complex` in
`System.Runtime.Numerics` got cleaned up a while ago.  It is
scheduled for reintroduction in `netstandard2.1`.

See the following PR for more info:
dotnet/corefx#20222
Apparently, the serialization support for `System.Random` is not present
in `netstandard1.3` and even though it was added at some point, it is
again removed from the future `netstandard` versions.

See the following PR for more info:
dotnet/coreclr#11765
@petriashev
Copy link

Error on build server: error : Assets file 'C:\projects\mathnet-numerics\src\Numerics\obj\project.assets.json' not found. Run a NuGet package restore to generate this file. [C:\projects\mathnet-numerics\src\Numerics\Numerics.csproj]

build.fsx Outdated
@@ -46,8 +46,8 @@ traceHeader releases

let summary = "Math.NET Numerics, providing methods and algorithms for numerical computations in science, engineering and every day use."
let description = "Math.NET Numerics is the numerical foundation of the Math.NET project, aiming to provide methods and algorithms for numerical computations in science, engineering and every day use. "
let support = "Supports .Net 4.0, .Net 3.5 and Mono on Windows, Linux and Mac; Silverlight 5, WindowsPhone/SL 8, WindowsPhone 8.1 and Windows 8 with PCL portable profiles 7, 47, 78, 259 and 328; Android/iOS with Xamarin."
let supportFsharp = "Supports F# 3.0 on .Net 4.0, .Net 3.5 and Mono on Windows, Linux and Mac; Silverlight 5 and Windows 8 with PCL portable profile 47; Android/iOS with Xamarin."
let support = "Supports .Net 4.0, netstandard1.6 and Mono on Windows, Linux and Mac, Android/iOS."

Choose a reason for hiding this comment

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

netstandard2.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It compiles on netstandard2.0, but the tests may fail. :) Now it is still pretty much WIP and exact descriptions will need to be reviewed. :)

@aignas
Copy link
Contributor Author

aignas commented Nov 2, 2017

A tentative todo list:

Target "RestoreMain" (fun _ -> DotNetCli.Restore (fun p ->
{ p with
Project = "MathNet.Numerics.sln"
NoCache = true }))
Copy link
Member

Choose a reason for hiding this comment

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

What's the rationale behind NoCache=true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was just to be on the safe side for starters. IMHO, once the project structure is stabilised the cache should be enabled.

@cdrnet cdrnet merged commit 2a594ce into mathnet:master Nov 4, 2017
@petriashev
Copy link

Can not build on travis:

Error:
/usr/share/dotnet/sdk/2.0.0/Microsoft.Common.CurrentVersion.targets(1122,5): error MSB3644: The reference assemblies for framework ".NETFramework,Version=v4.0" were not found. To resolve this, install the SDK or Targeting Pack for this framework version or retarget your application to a version of the framework for which you have the SDK or Targeting Pack installed. Note that assemblies will be resolved from the Global Assembly Cache (GAC) and will be used in place of reference assemblies. Therefore your assembly may not be correctly targeted for the framework you intend. [/home/travis/build/mathnet/mathnet-numerics/src/Numerics/Numerics.csproj]

Reason:
you can build only netstandard with new .net core sdk on non windows machine.
see: dotnet/sdk#826
you need to specify netstandard only build on linux or build 4.0 by mono runtime

@cdrnet
Copy link
Member

cdrnet commented Nov 4, 2017

Ah yes, we used to build with mono on travis up to know (indeed, that was the point of the travis setup). Thanks!

@aignas
Copy link
Contributor Author

aignas commented Nov 5, 2017

There was a link in the to-do list on how to get it compile against mono. So it may be not that difficult to do.

@cdrnet
Copy link
Member

cdrnet commented Nov 6, 2017

Once more, thanks a lot for all the work so far. I do like the direction this is going. I've already merged to master, but I think we can continue using this PR (maybe rebase or back-merge from master from time to time).

  • This will become v4, among others due to dropping some target platforms.
  • I'm fine with moving towards using the dotnet SDK more (FAKE where needed, e.g. for public releases, docs and website management).
  • There will be a need for some steps between building and packing when creating actual releases (e.g. I intend to sign (code signing cert, not strong name) the assemblies; only maintainers will be able to do this, but everyone should be able to build, test, etc). Hence build should not pack implicitly, and vice versa.
  • It seems to me using the automatic nuspec and AssemblyInfo generation can work for what we need (with properties in the csproj, see current Numerics.csproj). This means we can drop a major part of the fake scripts.
  • If possible I'd like to continue using Paket to lock dependency versions globally. But this can be addressed later.
  • We can skip the examples for now.
  • We should explicitly target .Net Standard 2.0 as well (even if e.g. 1.3 or 1.6 is already supported).
  • I intend to add an explicit Net46 target as well, where we can finally leverage SIMD (also in netstandard2.0 afaik). But this is out of scope of this PR.

@aignas
Copy link
Contributor Author

aignas commented Nov 6, 2017

Thanks for the comment. I was wondering how we should do this since you merged it into master, but I agree, that we can continue to use this PR.

@cdrnet
Copy link
Member

cdrnet commented Nov 6, 2017

Great!

Unfortunately it seems the AssemblyInfo generation like in Numerics.csproj does not work for F# projects yet: dotnet/fsharp#3113.

@cdrnet
Copy link
Member

cdrnet commented Nov 26, 2017

Update: I've just pushed a first alpha release to NuGet (v4.0.0-alpha01). The Windows build is now passing again on AppVeyor.

Known Issues:

  • F# package currently references the newest FSharp.Core - we should explicitly reference a (likely older) version instead.
  • Description is badly formatted.
  • Readme missing.

@cdrnet
Copy link
Member

cdrnet commented Nov 26, 2017

Do not recompile when testing

When using the build scripts, recompilation can be disabled with "incremental", e.g. ./build.sh test incremental.

@aignas
Copy link
Contributor Author

aignas commented Nov 26, 2017

I think that supporting older F# could be as simple as:

aignas@46ba6ba

I tried building on Linux locally and it didn't work, but that is probably a known issue, or maybe my setup is wrong... :)

@cdrnet
Copy link
Member

cdrnet commented Nov 26, 2017

Thanks, yes, that's what I had in mind (but non-floating).

@cdrnet
Copy link
Member

cdrnet commented Nov 26, 2017

I tried building on Linux locally and it didn't work, but that is probably a known issue, or maybe my setup is wrong...

Yes, it fails on travis as well build 649 when I try to run dotnet clean on the solution.

@cdrnet cdrnet added this to the Numerics v4.0 milestone Feb 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants