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

[WIP] Ported core projects to project.json #151

Closed
wants to merge 6 commits into from
Closed

[WIP] Ported core projects to project.json #151

wants to merge 6 commits into from

Conversation

khellang
Copy link
Member

I'm opening this up early to see if anyone has some feedback... πŸ‘€

Currently I have the 4 main projects building for net40, net45 and dotnet5.4 (netstandard1.3), without any code changes πŸ˜„
This should cover a minimum of net40, net45, net46, uap10, netcore50 and dnxcore50. I think this includes the Xamarin products as well ✨

I tried to aim for dotnet5.2 (netstandard1.1), which would include minimum win8 and wpa8 as well, but could only get Interfaces to compile. Will give it another go soon. ⌚
I currently don't have the tooling for sl5, wp7 and wp8, so I punted on those for now πŸ‘―

All the preprocessor directives are directly from Common.targets, so hopefully they should be good. Anyway, they're a horrible mess and should πŸ”₯ ASAP, once the older targets are deprecated.

There are still a few features that are not supported for project.json yet, like .tt-files etc., so this'll probably have to stay side-by-side for a while 😞

For reference, see the standard platform document.

Should close #148

"licenseUrl": "http://go.microsoft.com/fwlink/?LinkID=261272",
"requireLicenseAcceptance": true,

"exclude": [ "*/**/ImmutableList.cs" ],
Copy link
Member Author

Choose a reason for hiding this comment

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

For some reason this file caused trouble because of a duplicate declaration in the Core project. This one was picked over the other, but it lacked the ImmutableList<T>.Empty property, so it wouldn't compile. The implementations looks pretty similar - is there a reason why this exists?

@shiftkey
Copy link
Contributor

shiftkey commented Feb 8, 2016

@khellang is there anything in particular I can help with here? (feedback, testing etc)

@khellang
Copy link
Member Author

khellang commented Feb 9, 2016

I don't know. There are still quite a few projects that needs porting, but it would be nice with some feedback on the target platforms. What do we want to support going forward?

@shiftkey
Copy link
Contributor

shiftkey commented Feb 9, 2016

What do we want to support going forward?

For simplicity's sake, I'd target whatever can be built OOTB in VS2015. But I'm feeling a bit radical this evening and really want to simplify contributing to the codebase.

@clairernovotny
Copy link
Member

Is there any reason to support Net4? Microsoft has killed it support-wise too. Only .NET 4.5.2 and higher will be getting security updates.

I don't think there's any point in supporting WP7 or SL5. WP8 is still a stretch, but maybe leave that one for another version/year and kill it then.

@CADbloke
Copy link

Is there any reason to support Net4?

I know of weird edge-cases like older versions of AutoCAD that fall over when they see modern things. Most of this was fixed in their service packs but some things break obscurely. Honestly, if a client wanted me to support an older version I'd either drag them kicking and screaming into the present day or make them pay for the custom tweakage. Or walk away.

tldr; Don't listen to the laggards. Modern is good.

@shiftkey
Copy link
Contributor

@onovotny @CADbloke excellent points

@onovotny

WP8 is still a stretch, but maybe leave that one for another version/year and kill it then.

We're talking about the wp Target Platform Name here and not wpa, right?

@clairernovotny
Copy link
Member

Yes

@clairernovotny
Copy link
Member

Otoh, rxui 6.5 still works for wp8. Maybe housecleaning is in order. πŸ”₯ Wp8 now?

@shiftkey
Copy link
Contributor

@khellang I sat down and tried to get the test project Tests.System.Reactive building - unfortunately that's all using MSTest so it's not really ideal. But here's some more hacking I got to today:

https://github.com/khellang/Rx.NET/compare/core-port...shiftkey:some-build-script-improvements?expand=1

There's a few other projects that are needed by the tests:

  • System.Reactive.Providers
  • System.Reactive.Observable.Aliases

So I'm just wrapping my head around those and any portability issues right now...

@M-Zuber
Copy link
Contributor

M-Zuber commented Feb 19, 2016

I'd like to have a shot at converting the tests to xunit. I hope to start over the weekend and will update after mondayish if it looks like something I can pull through to the finish. Sounds okay?

@shiftkey
Copy link
Contributor

@M-Zuber πŸ‘ I'm curious what you find out about the migration!

@khellang
Copy link
Member Author

@M-Zuber Before you start doing anything manually, you should know that the Roslyn team wrote a tool that can translate MSTest to xUnit automatically πŸ˜„ See https://github.com/dotnet/codeformatter/tree/master/src/XUnitConverter

@M-Zuber
Copy link
Contributor

M-Zuber commented Feb 19, 2016

πŸ‘ that's what I was planning on using
πŸ˜€

Let's hope that all we need to do is run that

@shiftkey
Copy link
Contributor

Just a heads up - if we're going to opt-in for the new way of creating packages, the parent folders must be renamed to match the existing package, e.g System.Reactive.Interfaces becomes Rx-Interfaces

@clairernovotny
Copy link
Member

Aren't Xamarin specific impls still needed for some of the light-up? If so, then those can't yet be built with xproj, afaik.

@shiftkey
Copy link
Contributor

@onovotny I have no idea what the support looks like over on that side of the fence...

@shiftkey
Copy link
Contributor

@onovotny it's been a while since I've been looked at WinRT (is it now what we call UWP?), and I'm kinda stumped as to how to resolve types under the Windows.* namespace now that all the dependencies are distributed via NuGet, e.g TypedEventHandler<TSender, TEventArgs>

C:\Users\shiftkey\Documents\GitHub\Rx.NET\Rx.NET\SOurce\System.Reactive.WindowsRuntime\EventPatternSource.cs(15,15): .NETCore,Version=v5.0 error CS0246: The type or namespace name 'TypedEventHandler<TSender, TEventArgs>' could not be found (are you missing a using directive or an assembly reference?)

Some other errors talk about Windows, Version=255.255.255.255 but that's clearly not right:

The type 'IAsyncActionWithProgress<>' is defined in an assembly that is not referenced. You must add a reference to assembly 'Windows, Version=255.255.255.255, Culture=neutral, PublicKeyToken=null, ContentType=WindowsRuntime'.

Even referencing Microsoft.NETCore.UniversalWindowsPlatform didn't seem to resolve this, so I'm kinda puzzled what I'm missing here. Thoughts?

EDIT: maybe this just isn't achievable from a xproj due to the custom import for $(MSBuildExtensionsPath)\Microsoft\WindowsXaml\v$(VisualStudioVersion)\Microsoft.Windows.UI.Xaml.CSharp.targets?

@clairernovotny
Copy link
Member

I believe those types come from Windows.winmd? That's not currently on NuGet; that's part of the SDK. It also may not be possible to build with xproj.

@shiftkey
Copy link
Contributor

@onovotny basically

C:\Program Files (x86)\Windows Kits\10\References\Windows.Foundation.FoundationContract\1.0.0.0\Windows.Foundation.FoundationContract.winmd

Oh well, back to the drawing board on that project...

some more projects to build using project.json
@clairernovotny
Copy link
Member

That's not quite the reference you'd want, it'd be

C:\Program Files (x86)\Windows Kits\10\UnionMetadata\Windows.winmd

However, all is not lost, there might still be a way, but it might require "borrowing" some of the private targeting packages that the corefx team is using

https://github.com/dotnet/corefx/tree/8737f673b55ccd3eee7fbb8f914b5bcbb7c44d7b/src/System.Runtime.WindowsRuntime/src

Needs some spelunking, but I see Microsoft.TargetingPack.Private.WinRT": "1.0.1"

@clairernovotny
Copy link
Member

And here it is:

https://dotnet.myget.org/F/dotnet-core/ or
https://dotnet.myget.org/F/dotnet-core/api/v3/index.json

Then look for Microsoft.TargetingPack.Private.WinRT 1.0.1

It has a lib\dotnet5.1\windows.winmd in it.

I know it'd be better to use the main nuget feeds, but perhaps later once stuff is baked?

@M-Zuber
Copy link
Contributor

M-Zuber commented Feb 22, 2016

@khellang the MSFT too didn't seem to work for me.
In addition we have to use xUnit 1.9.2 until 4.0 is dropped.
Now converting by hand πŸ’ͺ and will make a PR onto this when I am done

@clairernovotny
Copy link
Member

Don't use xUnit 1.9.2. It's really old and won't work on devices. Just make sure your unit test project is .NET 4.5.

@clairernovotny
Copy link
Member

Or better yet, put the unit tests into a shared code project so they can be compiled on different platforms for per-platform testing.

@M-Zuber
Copy link
Contributor

M-Zuber commented Feb 22, 2016

Don't use xUnit 1.9.2. It's really old and won't work on devices. Just make sure your unit test project is .NET 4.5.

I get this, which I do not know where to find
image

Or better yet, put the unit tests into a shared code project so they can be compiled on different platforms for per-platform testing.

Currently I am working off master. Do I need to be switched to be based off this PR in order to use a shared code project? (either way a pointer or two in how to do would be appreciated as I am not that sure exactly what you mean)

@LeeCampbell
Copy link
Contributor

+1 for dropping .NET 4.0 support. It seems at odds with itself to expect users who don't upgrade (or are on WinXP) to jump to the latest build of Rx :-)

@khellang
Copy link
Member Author

I'll just close this. There's work, based on this, that will bring Rx to the new world of .NET core in the pipeline. Watch #148

@khellang khellang closed this May 24, 2016
@khellang khellang deleted the core-port branch May 24, 2016 08:21
@xperiandri
Copy link

How to target UWP APIs in .NET Core library project with uap10.0 profile?
@onovotny solution for RT and dotnet5.1 works, but what about uap10.0?

@clairernovotny
Copy link
Member

you can use an "imports": ["dotnet5.4"] in the uap10.0 framework section.

@xperiandri
Copy link

And just use the Microsoft.TargetingPack.Private.WinRT?

@xperiandri
Copy link

Like this?

{
  "version": "1.0.0-*",

  "frameworks": {
    ".NETPortable,Version=v4.6,Profile=Profile32": {
      "imports": "dotnet5.1",
      "frameworkAssemblies": {
        "mscorlib": "",
        "System": "",
        "System.Core": "",
        "System.Collections": "",
        "System.Globalization": "",
        "System.Linq": "",
        "System.Linq.Expressions": "",
        "System.Reflection": "",
        "System.Reflection.Extensions": "",
        "System.Runtime": "",
        "System.Runtime.Extensions": "",
        "System.Runtime.InteropServices": "",
        "System.Windows": ""
      },
      "dependencies": {
        "Microsoft.TargetingPack.Private.WinRT": "1.0.1"
      }
    },
    "uap10.0": {
      "buildOptions": {
        "define": [ "WINDOWS_UWP" ]
      },
      "imports": "dotnet5.4",
      "dependencies": {
        "Microsoft.NETCore.UniversalWindowsPlatform": "5.2.0",
        "Microsoft.TargetingPack.Private.WinRT": "1.0.1"
      }
    }
  },
  "buildOptions": {
    "xmlDoc": true
  }
}

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.

CoreCLR
7 participants