Migrate to netstandard #1248

nirinchev · 2017-02-22T13:50:11Z

Description

Notable changes:

System.Json is not part of netstandard, so I've taken dependency on Newtonsoft.Json
Schema is woven at compile time in ModuleWeaver.WeaveSchema. Had to expose public property RealmSchema.DefaultTypes and set it in the application entry point. Theoretically that would allow people to modify it before the first call to Realm.GetInstance().
Realm.BuildTasks defaults to mscorlib when resolving dependencies. Had to do that as the default assembly resolver fails to resolve dependencies such as System.Runtime, etc. that are just reference assemblies.

Fixes #1080

TODO

High-level overview of what's changed

Added Realm/Realm and Realm/Realm.Sync .NET standard 1.4 projects. Those will be packaged in the NuGet and will be the only .dll's we ship.
Renamed Tests to Manual Tests as those are run manually rather than by CI.
Moved all test projects (platform specific + shared) to Tests.
Removed platform specific Realm projects.
Got rid of the try-finally pattern for assigning handles.
Hacky: BuildTasks falls back to mscorlib if it fails to resolve an assembly (as System.Runtime, etc. are not real ones). As which types we use is very deterministic, it's not an issue, but it's something to watch out for.
Weave the default schema on compile - as we don't have access to AppDomain.Current in .NET Standard, we can't inspect the default types at runtime. The current solution will weave logic to call AddDefaultTypes either in the Assembly.EntryPoint or if it doesn't exist, in the module initializer (that's needed for unit tests on windows). It will walk the graph of all referenced assemblies and pull in their types. It's good enough for merging, but should be tested with complex graphs.
System.Json is gone, so add Newtonsoft.Json as a dependency for Realm.Sync.
Use Reflection to find Environment.SpecialFolder.Personal (default location of realm files). This will not work on UWP and will need to be extended.
Add new NuGet - Realm.DataBinding that Realm.Database depends on. It is platform specific and has PCL (bait and switch), iOS, and Android versions (a shared project) and contains all the IReflectableType-related implementations. Needed as TypeDelegator will not be available until netstandard 2.0. The weaver will weave IReflectableType implementation to user objects by calling TypeInfoHelper.GetInfo.
Updated Jenkinsfile, nuspecs, docfx.json

After merging

Test that NuGet dependencies are correct, especially on Windows (i.e. I can create a new app, add Realm and have everything working as expected).
Test default schema with non-trivial project graphs.
Verify that IReflectableType is properly woven on PCL models as well.

nirinchev · 2017-02-23T00:06:18Z

I'm happy to announce that non-sync tests pass (except IReflectableType) 🎉

nirinchev · 2017-02-28T14:05:22Z

Realm/Realm/Native/NativeCommon.cs

+                if (platformPI.GetValue(osVersion).ToString() == "Win32NT")
+                {
+                    // We know we're on Win32 so Assembly.Location is available
+                    var assemblyLocationPI = typeof(Assembly).GetProperty("Location", BindingFlags.Public | BindingFlags.Instance);


UWP might also report Win32NT, so this should be guarded

kristiandupont

A few questions and I am sure I am missing things because I can't see the entire diff in my browser, but overall looks good.

kristiandupont · 2017-03-10T10:00:42Z

DataBinding/DataBindingAssemblyInfo.cs

+using System.Reflection;
+
+[assembly: AssemblyDescription("Realm is a mobile database: a replacement for SQLite")]
+[assembly: AssemblyCopyright("Copyright © 2017 Realm")]


I read recently that the year you put in a copyright statement is supposed to represent the earliest year from which the copyright applies. Thus, it should not be updated. I am not sure if that info was valid and I don't think it really matters as at least half of the web then gets this wrong, but since we're being overly pedantic on the .net team, I thought I'd bring it up.. :-)

kristiandupont · 2017-03-10T10:04:14Z

DataBinding/Realm.DataBinding.TypeDelegator/TypeInfoHelper.cs

+namespace Realms
+{
+    [SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:ElementsMustBeDocumented")]
+    public static class TypeInfoHelper


Why does this need to be public?

It's called by the woven IReflectableType.GetType implementation.

kristiandupont · 2017-03-10T10:05:45Z

NuGet/Realm.DataBinding/Realm.DataBinding.nuspec

+    <projectUrl>http://github.com/realm/realm-dotnet</projectUrl>
+    <iconUrl>http://realm.io/img/favicon-32x32.png</iconUrl>
+    <requireLicenseAcceptance>false</requireLicenseAcceptance>
+    <description>Realm is a mobile database: a replacement for SQLite and ORMs</description>


Maybe we'd want a specific description for this package?

Yes, I didn't feel like coming up with something, so I just copy-pasted the old one.

kristiandupont · 2017-03-10T10:06:39Z

Platform.PCL/Realm.PCL/RealmObjectPCL.cs

@@ -503,11 +503,5 @@ protected void RaisePropertyChanged([CallerMemberName] string propertyName = nul
        protected virtual void OnPropertyChanged(string propertyName)
        {
        }
-
-        TypeInfo IReflectableType.GetTypeInfo()


Are automatic transactions not supported any more?

They are - the new package Realm.Databinding contains the logic for automatic transactions and the IReflectableType implementation is woven to all user types.

kristiandupont · 2017-03-10T10:12:20Z

Platform.PCL/Realm.PCL/RealmPCL.cs

@@ -293,13 +293,15 @@ public void Write(Action action)
        }

        /// <summary>
-        /// Execute an action inside a temporary <see cref="Transaction"/> on a worker thread. If no exception is thrown,
+        /// Execute an action inside a temporary <see cref="Transaction"/> on a worker thread, <b>if</b> called from UI thread. If no exception is thrown,


Maybe "Execute an action in a temporary transaction. If called from the UI thread, this will take place on a worker thread."

That's from @AndyDentFree's PR: #1253, haven't changed it.

No, but you did add to it :-)

I don't think I did - it was merged in this branch which is why you're seeing it compared to master :P

kristiandupont · 2017-03-10T10:15:24Z

Realm/Realm.Sync/Handles/SharedRealmHandleExtensions.cs

@@ -45,7 +45,7 @@ private static class NativeMethods

            public unsafe delegate void RefreshAccessTokenCallbackDelegate(IntPtr session_handle_ptr);

-            public unsafe delegate void SessionErrorCallback(IntPtr session_handle_ptr, ErrorCode error_code, sbyte* message_buf, IntPtr message_len, IntPtr user_info_pairs, int user_info_pairs_len);
+            public unsafe delegate void SessionErrorCallback(IntPtr session_handle_ptr, ErrorCode error_code, byte* message_buf, IntPtr message_len, IntPtr user_info_pairs, int user_info_pairs_len);


Was the use of sbyte just a typo?

I can't say I'm 100% sure :) The thing is, there's no longer a string ctor that accepts sbyte* in .NET standard, but converting to byte* and using Encoding.GetString seems to do the trick (passes the tests at least).

I see. Well those calls look nicer to me anyway.

AndyDentFree

Overall looks good and a huge body of work!

AndyDentFree · 2017-03-09T16:56:27Z

DataBinding/DataBindingAssemblyInfo.cs

+[assembly: AssemblyCompany("Realm Inc.")]
+[assembly: AssemblyProduct("Realm C#")]
+
+[assembly: AssemblyVersion("1.0.0.0")]


Does this correspond to the Realm release version numbers? Maybe add a comment line explaining context.

My thinking was that it should not be tied to the Realm version numbers as that is a package we'll rarely update (I hope). But it does add a bit of complexity to the release procedure if we decide to update it as that would require a bump in the dependency versions (which I'll document once this PR is finalized and merged).

AndyDentFree · 2017-03-09T17:39:42Z

Jenkinsfile


 xbuildCmd = '/usr/local/bin/xbuild'
-def nuget = '/usr/local/bin/nuget'
+nugetCmd = '/usr/local/bin/nuget'
+def windowsNugetCmd = 'C:\\ProgramData\\chocolatey\\bin\\NuGet.exe'


Does this require that nuget was installed on Windows using the Chocolatey package manager? If so, is that documented anywhere? Does it matter to anyone building outside of Realm?

Yes - it requires a chocolatey install of nuget on our CI slaves. I'm not sure if we have documentation about CI setup anywhere. Ping @fealebenpae.

Nope, we don't. And this change only matters to Jenkins. People building manually (us, included) will just restore packages like they always would - through Visual Studio.

AndyDentFree · 2017-03-09T18:26:00Z

NuGet/Realm.DataBinding/Realm.DataBinding.nuspec

+    <projectUrl>http://github.com/realm/realm-dotnet</projectUrl>
+    <iconUrl>http://realm.io/img/favicon-32x32.png</iconUrl>
+    <requireLicenseAcceptance>false</requireLicenseAcceptance>
+    <description>Realm is a mobile database: a replacement for SQLite and ORMs</description>


Maybe the description could indicate this is useless by itself and is a supporting nuget for Realm.Database?

AndyDentFree · 2017-03-10T10:48:36Z

NuGet/Realm.Database/Realm.Database.nuspec

@@ -17,8 +17,18 @@
    <tags>Realm database db persistence sqlite</tags>
    <dependencies>
      <dependency id="Fody" version="1.29.4"/>
+      <dependency id="Realm.DataBinding" version="1.0.0"/>
      <dependency id="DotNetCross.Memory.Unsafe" version="0.2.2"/>


Are we removing DotNetCross.Memory.Unsafe or leaving that change for a separate PR later?

Let's leave it for later as this one's already huge and would prefer not to add more points where things might break :)

AndyDentFree · 2017-03-10T11:03:26Z

DataBinding/Realm.DataBinding.TypeDelegator/TypeInfoHelper.cs

+    [SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:ElementsMustBeDocumented")]
+    public static class TypeInfoHelper
+    {
+        // Holds Type -> RealmObjectTypeInfo map to avoid creating a new TypeDelegator for each IReflectableType.GetTypeInfo invocation.


Is this comment still valid with dropping using IReflectableType directly for RealmObject?

Yes - it's still valid, as we're weaving call to TypeInfoHelper.GetInfo in the user's assembly, so caching types is useful.

…emblies

* Updated XML comment to clarify behaviour of WriteAsync on threads. * Updated XML comment to clarify behaviour of WriteAsync on threads (in PCL version)

Update jenkinsfile to use Debug

nirinchev added S:In Progress T-Feature labels Feb 22, 2017

nirinchev added this to the uwp milestone Feb 22, 2017

realm-ci assigned nirinchev Feb 22, 2017

nirinchev commented Feb 28, 2017

View reviewed changes

nirinchev force-pushed the ni/netstandard2 branch 4 times, most recently from 6a91d2b to 52c032b Compare March 9, 2017 01:42

nirinchev added S:Review and removed S:In Progress labels Mar 9, 2017

kristiandupont approved these changes Mar 10, 2017

View reviewed changes

AndyDentFree approved these changes Mar 10, 2017

View reviewed changes

This was referenced Mar 10, 2017

NetStandard Support sushihangover/Realm.Json.Extensions#45

Closed

NetStandard Support sushihangover/RealmThread#20

Open

nirinchev added 14 commits March 15, 2017 15:20

Add netstandard packages

07cb25f

Rename Tests -> Manual Tests

cfe76e9

Remove old projects and reorganize folders a bit

9c6ebcb

Build Realm with a bunch of TODO-s

16db36a

Update tests to semi-buildable state

d8df645

Add a dummy tester for BuildTasks

8b0ace7

Hack BuildTasks to resolve mscorlib as a fallback for netstandard ass…

40fffb5

…emblies

Fix PCL build

15720a8

Weave the default schema

3a374a4

Add files to Realm.Sync

daf571f

Add reference to Tests.Sync.Shared

a395cc7

Restore native location for Win32

9ef4d84

Fix Sync InternalsVisibleTo attribute

1612bc8

Exclude [Explicit] classes from default schema

eb143ed

nirinchev and others added 23 commits March 15, 2017 15:21

Fix configurations for new netstandard projects

5156997

Update tests to latest nugets

0eb95d0

Ad/doc/write async comments (#1253)

56c38e6

* Updated XML comment to clarify behaviour of WriteAsync on threads. * Updated XML comment to clarify behaviour of WriteAsync on threads (in PCL version)

Fix rebase issues

67a3f35

Remove Platform/Win32

f820dc6

Minor jenkinsfile update

b5040ed

Update docfx.json

74f506e

Weave IReflectableType

4b23ce3

Fix marshalling of exception strings

d47ae3f

Add netstandard to DataBinding.Android

12d62bb

Update jenkinsfile to use Debug

Update Jenkinsfile, add DataBinding.nuspec

b84b5f1

jenkins fixes

6022ced

Restore nuget packages on windows

6f33c41

Fix schema weaving

33a7592

Clean up Realm dependencies

11f0954

Remove some package references from tests

c7a2bb2

Add RealmSchema to fake realm

9748b96

fix stylecop violation

d44aac0

add missing package for Win32 tests

b3148ab

Fix nuspecs

dc58bfc

Ignore RuntimeInformation fail on Android

b6f6dae

Generate docs for .NET Standard assemblies

69abd0d

Fix schema weaving for projects that don't use realm

a9f6533

nirinchev force-pushed the ni/netstandard2 branch from 845d1f5 to a9f6533 Compare March 15, 2017 14:21

nirinchev added 2 commits March 15, 2017 16:38

Fix rebase issue

3043fd7

Add changelog entry

40fabc7

nirinchev merged commit 9e0e15a into master Mar 21, 2017

nirinchev removed the S:Review label Mar 21, 2017

nirinchev deleted the ni/netstandard2 branch March 21, 2017 14:10

github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate to netstandard #1248

Migrate to netstandard #1248

nirinchev commented Feb 22, 2017 •

edited

Loading

nirinchev commented Feb 23, 2017

nirinchev Feb 28, 2017

kristiandupont left a comment

kristiandupont Mar 10, 2017

kristiandupont Mar 10, 2017

nirinchev Mar 10, 2017

kristiandupont Mar 10, 2017

nirinchev Mar 10, 2017

kristiandupont Mar 10, 2017

nirinchev Mar 10, 2017

kristiandupont Mar 10, 2017

nirinchev Mar 10, 2017

kristiandupont Mar 10, 2017

nirinchev Mar 10, 2017

kristiandupont Mar 10, 2017

nirinchev Mar 10, 2017

kristiandupont Mar 10, 2017

AndyDentFree left a comment

AndyDentFree Mar 9, 2017

nirinchev Mar 10, 2017

AndyDentFree Mar 9, 2017

nirinchev Mar 10, 2017

fealebenpae Mar 10, 2017 •

edited

Loading

AndyDentFree Mar 9, 2017

AndyDentFree Mar 10, 2017

nirinchev Mar 10, 2017

AndyDentFree Mar 10, 2017

nirinchev Mar 10, 2017

Migrate to netstandard #1248

Migrate to netstandard #1248

Conversation

nirinchev commented Feb 22, 2017 • edited Loading

Description

TODO

High-level overview of what's changed

After merging

nirinchev commented Feb 23, 2017

Choose a reason for hiding this comment

kristiandupont left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndyDentFree left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fealebenpae Mar 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nirinchev commented Feb 22, 2017 •

edited

Loading

fealebenpae Mar 10, 2017 •

edited

Loading