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

changed ix-async dependencies to System.Interactive.Async version 3.0.0 #7519

Closed

Conversation

apolcyn
Copy link
Contributor

@apolcyn apolcyn commented Jul 25, 2016

This is a first attempt to fix #7103.

I think that it would be best to make that change now rather than later.

Ran into Nuget issues locally when running tests and building on OSX, but saw this build and restore packages on Windows and MonoDevelop on linux.

One issue seems to be that System.Interactive.Async relies on Nuget client being greater than 2.12. When the available version is less than 2.12, then the package seems to fail to restore during the run_tests.py script, or just when building with Xamarin Studio's default Nuget client on mac. When Nuget client is updated to the latest version, then it fails wby trying to look for a seemingly updated MSBuild.exe

@apolcyn
Copy link
Contributor Author

apolcyn commented Jul 25, 2016

builds seem to be failing only because the Nuget client used on docker needs to get updated to 2.12 or greater to restore the latest System.Interactive.Async. Unfortunately apt-get update && apt-get install seems to point at 2.8.7 as the latest

@apolcyn
Copy link
Contributor Author

apolcyn commented Jul 26, 2016

After updating the Grpc.Core project.json to depend on System.Interactive.Async instead of Ix-Async (forgot to update this earlier and only updated its packages.config), a nuget restore of the Grpc.Examples.MathClient package (depends on the Grpc.Core project), results in an error message along the lines of "package x provides compile time reference assembly for package x on netstandard1.5 but no runtime assemblies were found", for System.AppContext and System.Runtime.InteropServices.

Using nuget 3.4 here.
Saw other people having a similar issues with dotnet cli, but stuck on this at the moment.

cc @jskeet, if you're familiar with this.

@jskeet
Copy link
Contributor

jskeet commented Jul 26, 2016

Hmm. The mixture of dotnet cli and "old school" projects makes this harder to work with. Do you have a built version of Grpc.Core that you could put somewhere (e.g. on Drive or an experimental myget.org feed) and I can try it in a few different scenarios? I don't have very much time before I go away for a couple of weeks, but I'll see what I can do.

@jskeet
Copy link
Contributor

jskeet commented Jul 26, 2016

Also note that it may partly be problems with the .NET Core dependencies not being to the RTM versions, as per #7334.

@apolcyn
Copy link
Contributor Author

apolcyn commented Jul 26, 2016

I actually had to increase the netstandad1.5 NETStandardLibraries dependency from a 1.5.0-rc2 to 1.6.0 in order to compile using System.Interactive.Async 3.0.0.

I pushed a Grpc.Core package with System.Interactive.Async, created with "dotnet pack", to https://www.myget.org/feed/Packages/alex-polcyn

@apolcyn
Copy link
Contributor Author

apolcyn commented Jul 26, 2016

That Grpc.Core pacakge was made with the latest commit by the way.

What I'm seeing is basically:

For netstandard1.5 framework, currently depend on libraries pulled in from NETStandard.Library 1.5.0-rc2. The System.Interactive.Async 3.0.0 depends on libraries versions contained in 1.6.0, and the project doesn't compile with the 1.5.0-rc2 libraries and System.Interactive.Async. With the libraries at 1.6.0 though, Grpc.Core builds, but I get errors with "compile time references but no runtime assemblies for platform"

@apolcyn
Copy link
Contributor Author

apolcyn commented Jul 27, 2016

NETStandard libraries like System.AppContext reference netstandard1.6 rather than netstandard1.5. And System.Interactive.Async specifies netstandard1.3 and netstandard1.0 in its frameworks. Not sure if this would this be preventing them from building with the netstandard1.5 target of GRPC?

@jskeet
Copy link
Contributor

jskeet commented Jul 27, 2016

I believe it should be fine with netstandard1.5. I'll try to reproduce and then investigate today.

@apolcyn
Copy link
Contributor Author

apolcyn commented Jul 27, 2016

Made some progress in getting everything to build. It seems to be able to build when targeting net46 and netstandard1.6 rather than net45 and netstandard1.5.

Read over https://docs.microsoft.com/en-us/dotnet/articles/core/tutorials/libraries, it seems to me like I can't pull in a library that targets netstandard1.6, if I'm using netstandard1.5.

Also I think from the tables in those articles, net45 platform doesn't implement the netstandard1.6 APIs that System.Interactive.Async needs, and it was then failing to find the assemblies it needed because net45 just doesn't have them.

@jskeet
Copy link
Contributor

jskeet commented Jul 27, 2016

That would be very weird - I wouldn't expect System.Interactive.Async to require netstandard 1.6. I'm looking now.

@jskeet
Copy link
Contributor

jskeet commented Jul 27, 2016

Looking at https://www.nuget.org/packages/System.Interactive.Async, it should only need netstandard 1.0. About to download your Grpc.Core to try it in my own project.

@jskeet
Copy link
Contributor

jskeet commented Jul 27, 2016

Looking at the project.json file, I believe part of the problem is that the Grpc.Examples.MathClient project is built as a library (targeting netstandard1.5) but specifies a runtimes section. My understanding is that you'd normally do that for an application, and only a self-contained application at that.

Just removing the runtimes part of the project.json file removes the error for me.

@jskeet
Copy link
Contributor

jskeet commented Jul 27, 2016

Having said that, given that it's an application rather than a library, I believe it should target dotnetcoreapp1.0 rather than netstandard1.5 anyway.

@apolcyn
Copy link
Contributor Author

apolcyn commented Jul 27, 2016

Removing the the runtimes section from the projects used as applications rather than libraries seems to work for me too.

It makes sense that applications should be targeting an actual framework runtime rather than the netstandard libraries, not quite sure how it's working when targeting netstandard (maybe it just picks the framework from the environment?).

What made me think of targeting netstandard1.6 was that the core libraries like System.AppContext, whose assemblies weren't being found, seem to be targeting it in their project.json's, at least on their master branches (https://github.com/dotnet/corefx/blob/master/src/System.AppContext/src/project.json). But I'd rather not update to 1.6 if we didn't have to.

Thanks for looking into this! Just figuring this out...

@jskeet
Copy link
Contributor

jskeet commented Jul 27, 2016

I'm not sure why System.AppContext was being checked at all, but basically we should be fine. I think the project.json files need a bit of revising in general, when Jan is back.

@apolcyn apolcyn force-pushed the update_ix_async_to_system_interactive branch from aed058f to ad37133 Compare July 27, 2016 17:56
@apolcyn
Copy link
Contributor Author

apolcyn commented Jul 27, 2016

test this please

@@ -42,11 +42,6 @@
}
}
},
"runtimes": {
Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect removing these will break running tests using run_tests.py -l csharp --compiler coreclr

@jtattermusch
Copy link
Contributor

I think #7334 should be addressed before this one (the System.AppContext issues might be related to using netstandard1.5 for projects that are probably supposed to be netcoreapp1.0 instead as Jon is suggesting). I have some work in progress in that direction.

@jtattermusch
Copy link
Contributor

jtattermusch commented Jul 29, 2016

Also, as mentioned earlier in #7103, the problem with this change is that once we publish Grpc.Core that depends on System.Interactive.Async, all the existing users (they are on .NET45 currently) will need to update to a newer nuget version to prevent being broken, while at the same time this change won't bring them any value. Can we instead stick with Ix-Async for .NET45 targets to not force users to take any unnecessary actions while switching to System.Interactive.Async for .NET core netstandard targets? (the API in both nugets is the same, right?).

@apolcyn
Copy link
Contributor Author

apolcyn commented Jul 29, 2016

So possibly target System.Interactive.Async for netstandard, and Ix-Async for .NET45? For the APIs between the two, I understand that they are the same, so shouldn't have to make code changes to do this.

But for the breaking changes, I see that this breaks users of the old nuget client (anyone using Xamarin Studio or Monodevelop, although they can update nuget and restore through the cli still), and also users depending on Ix-Async. But I take it that the IDEs are expected to update soon.

I wasn't certain how to weigh this, but we thought that it would be better to make the breaking change for the Ix-Async vs System.Interactive.Async sooner rather than later.

cc @jskeet for context

@jskeet
Copy link
Contributor

jskeet commented Jul 30, 2016

I would really rather not have different dependencies for the different
targets here. That will force the Google Cloud Client Libraries to do the
same thing... I far prefer a short time of pain now while we have few users
than a long term pain as we try to handle users who want
System.Interactive.Async, which now feels far more "official" than Ix-Async
ever did.

(The fact that ASP.NET Core uses System.Interactive.Async is also compelling, I think... and note that it's going to need that even when running under net45.)

@apolcyn
Copy link
Contributor Author

apolcyn commented Aug 2, 2016

To get ready to potentially use the fix to the project.json files in #7580, I "pre-merged" those changes into this one.

Unfortunately, targetting dotnetcoreapp in the project.jsons still lead me to the same issue in restoring nugets: 'x provides a compile-time assembly compatible with but not run-time assembly could be found for <runtime, e.g. osx>'. But from NuGet/Home#2915, I believe it is a bug in the new nuget client.

Restoring packages with dotnet cli seems to work though, so the most recent commit switches the pre_build scripts to use dotnet cli instead of nuget commands.

My plan is now to just install dotnet cli on test workers.

cc @jtattermusch

@apolcyn
Copy link
Contributor Author

apolcyn commented Aug 3, 2016

Switching the tests and build setup to use dotnet cli seems like too much for this PR, switching to use an updated nuget client, and unfortunately, likely having to unnecessarily delete the "runtime" node section of some project.jsons.

@apolcyn apolcyn closed this Aug 3, 2016
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants