Restructure the C# bindings for Yoga to be based on .NET Standard 2.0 and the new .NET 6.0+ target platform work. #1207

jkoritzinsky · 2023-01-03T03:41:04Z

Updated the C# bindings to build using the new simplified .NET SDK tooling. The C# projection now builds for .NET Standard 2.0 (to support Unity, .NET Framework, UWP, and modern .NET) and .NET 6.0 for iOS (to have the native calls correctly support calling into a statically-linked library).

The native changes were needed to get the C# tests passing on Windows x64 (tested locally).

This PR does not update any build, test, or package scripts. I can either add that to this PR or follow up with another PR.

… and the new .NET 6.0+ target platform work. Tests building is currently broken due to e96a09e. Next commit will investigate how to fix that.

…the DEBUG define and fix a missing export

facebook-github-bot · 2023-01-03T03:41:12Z

Hi @jkoritzinsky!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

facebook-github-bot · 2023-01-03T05:41:14Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

NickGerleman

This is great stuff! Very excited to see the C# bindings work again.

Would be good to clean up some of the dead project configuration around here. E.g. looking at the new source tree, I can see a Xamarin sln referencing folders no longer present, along with a non-vcxproj build script that doesn't work/I'm guessing is no longer used.

I can see there is updated code for Mono binding to YogaKit on iOS. From what I can tell the previous iOS build used that build script instead of depending on the vcxproj. Will those non-Windows builds still work? If not I would reccomend removing as well (I'm okay losing parity if we get something presently working and maintainable in return).

I think we should add some Github workflows here to excersise any builds/NUnit tests which are working, for the matrix we want to support. My preference would be to do that as part of the change, so we can demonstrate everything is working. The recent "validate-js.yml" workflow should be relatively pattern matchable but I can also help with this, given the permutations we want to support.

csharp/Yoga/Yoga.vcxproj

NickGerleman · 2023-01-03T09:08:42Z

Also feel free to ignore the JavaScript linting failure. Made a change internally without realizing Prettier wasn't turned on for Yoga in internal validation. Working on fixing that up soon so it won't happen again.

…build and add some projects to build the packages

jkoritzinsky · 2023-01-05T06:40:08Z

I've added a build and test GitHub Action at the .github/workflows/validate-csharp.yml path. It will validate the build, package, and test steps for windows-x64 and linux-x64 on Ubuntu. The linux build should probably build against an older version of glibc to make it more usable across linux distributions, but at least for now my main goal was getting something working.

jkoritzinsky · 2023-01-05T06:41:00Z

I haven't hooked up testing for iOS yet as I'm not familiar enough with GitHub actions to know the best way to run tests against an iOS simulator or device.

NickGerleman · 2023-01-07T03:49:34Z

The linux build should probably build against an older version of glibc to make it more usable across linux distributions, but at least for now my main goal was getting something working.

I looked into that a bit for the CMake change I'm working on. Right now the repo is keeping things simple, with the GitHub hosted/managed runners. The oldest LTS Ubuntu supports is 18.04, with GCC 6. but GitHub only supports back to 20.04.

Ideas were:

Select different toolchain versions on top of a current install
Convert the setup-xxx.yaml files into a Dockerfile and start building our own images ala https://github.com/marketplace/actions/build-and-push-docker-images

There is lower hanging fruit though, so it's not worth thinking about too much yet.

csharp/Facebook.Yoga/Facebook.Yoga.csproj

.github/workflows/validate-csharp.yml

…nly the native project

.github/workflows/validate-csharp.yml

csharp/Facebook.Yoga/Facebook.Yoga.csproj

csharp/Yoga/CMakeLists.txt

csharp/Facebook.Yoga/Facebook.Yoga.csproj

.github/workflows/validate-csharp.yml

Co-authored-by: Nick Gerleman <nick@nickgerleman.com>

.github/actions/setup-cs/action.yml

csharp/Yoga/CMakeLists.txt

Summary: Pull Request resolved: facebook#1259 Wires C ABI to C# bindings using `System.Runtime.InteropServices`. Note that we don't have a working C# build right now, but there is [effort to address that](facebook#1207) which may get some more effort before the Yoga release, so this keeps the bindings up to date. Differential Revision: D45297676 fbshipit-source-id: 77df49f7ffe17e177db8a39e8322124058353e23

Summary: Pull Request resolved: facebook#1259 Wires C ABI to C# bindings using `System.Runtime.InteropServices`. Note that we don't have a working C# build right now, but there is [effort to address that](facebook#1207) which may get some more effort before the Yoga release, so this keeps the bindings up to date. Reviewed By: yungsters Differential Revision: D45297676 fbshipit-source-id: 9e7bd1bcce979acb6bb7abb8dc9c46416c5c3725

Summary: Pull Request resolved: facebook#1259 Wires C ABI to C# bindings using `System.Runtime.InteropServices`. Note that we don't have a working C# build right now, but there is [effort to address that](facebook#1207) which may get some more effort before the Yoga release, so this keeps the bindings up to date. Reviewed By: yungsters Differential Revision: D45297676 fbshipit-source-id: c622865232689e7cda48ec5cd566ee906962444c

Summary: Wires C ABI to C# bindings using `System.Runtime.InteropServices`. Note that we don't have a working C# build right now, but there is [effort to address that](facebook#1207) which may get some more effort before the Yoga release, so this keeps the bindings up to date. Differential Revision: https://internalfb.com/D45297676 fbshipit-source-id: 1ec7cb1f15773efdb46669db0ebf6fc5d6b759b6

Summary: Wires C ABI to C# bindings using `System.Runtime.InteropServices`. Note that we don't have a working C# build right now, but there is [effort to address that](facebook#1207) which may get some more effort before the Yoga release, so this keeps the bindings up to date. Differential Revision: https://internalfb.com/D45297676 fbshipit-source-id: aa03d1a31602051d3d2514f6fc8a7a3db5bea262

Summary: Wires C ABI to C# bindings using `System.Runtime.InteropServices`. Note that we don't have a working C# build right now, but there is [effort to address that](facebook#1207) which may get some more effort before the Yoga release, so this keeps the bindings up to date. Differential Revision: https://internalfb.com/D45297676 fbshipit-source-id: 4a7ad17192ed8d4b779b10ad834644194111b96c

Summary: Pull Request resolved: facebook#1259 Wires C ABI to C# bindings using `System.Runtime.InteropServices`. Note that we don't have a working C# build right now, but there is [effort to address that](facebook#1207) which may get some more effort before the Yoga release, so this keeps the bindings up to date. Reviewed By: yungsters Differential Revision: D45297676 fbshipit-source-id: db461b0203200b9db425f4ddebcc1798477e9cde

Summary: Wires C ABI to C# bindings using `System.Runtime.InteropServices`. Note that we don't have a working C# build right now, but there is [effort to address that](facebook#1207) which may get some more effort before the Yoga release, so this keeps the bindings up to date. Differential Revision: https://internalfb.com/D45297676 fbshipit-source-id: 5f20e0a9865d33ba4974e03d365f11fae816087c

Summary: Wires C ABI to C# bindings using `System.Runtime.InteropServices`. Note that we don't have a working C# build right now, but there is [effort to address that](facebook#1207) which may get some more effort before the Yoga release, so this keeps the bindings up to date. Differential Revision: https://internalfb.com/D45297676 fbshipit-source-id: 63f7c3b7c2e7a97d7bba12f62d9e448c8aaee538

Summary: Pull Request resolved: facebook#1259 Wires C ABI to C# bindings using `System.Runtime.InteropServices`. Note that we don't have a working C# build right now, but there is [effort to address that](facebook#1207) which may get some more effort before the Yoga release, so this keeps the bindings up to date. Reviewed By: yungsters Differential Revision: D45297676 fbshipit-source-id: 4535de6ba97f8ea87141b3a8dcd6dbb174555ec2

Summary: Pull Request resolved: facebook#1259 Wires C ABI to C# bindings using `System.Runtime.InteropServices`. Note that we don't have a working C# build right now, but there is [effort to address that](facebook#1207) which may get some more effort before the Yoga release, so this keeps the bindings up to date. Reviewed By: yungsters Differential Revision: D45297676 fbshipit-source-id: fcbbe4e3a1cd6ed02fee25c169d892b68c95e667

Summary: Wires C ABI to C# bindings using `System.Runtime.InteropServices`. Note that we don't have a working C# build right now, but there is [effort to address that](facebook#1207) which may get some more effort before the Yoga release, so this keeps the bindings up to date. Differential Revision: https://internalfb.com/D45297676 fbshipit-source-id: 171c6aa3699802e51e5e12025acce854e13c7402

Summary: Wires C ABI to C# bindings using `System.Runtime.InteropServices`. Note that we don't have a working C# build right now, but there is [effort to address that](facebook#1207) which may get some more effort before the Yoga release, so this keeps the bindings up to date. Differential Revision: https://internalfb.com/D45297676 fbshipit-source-id: d9b9949705c8654fe2705429442affd5fa4cb0c8

Summary: Pull Request resolved: facebook#1259 Wires C ABI to C# bindings using `System.Runtime.InteropServices`. Note that we don't have a working C# build right now, but there is [effort to address that](facebook#1207) which may get some more effort before the Yoga release, so this keeps the bindings up to date. Reviewed By: yungsters Differential Revision: D45297676 fbshipit-source-id: d8a455686c5a1063e7047a7a511fd70714f8b013

Summary: Wires C ABI to C# bindings using `System.Runtime.InteropServices`. Note that we don't have a working C# build right now, but there is [effort to address that](facebook#1207) which may get some more effort before the Yoga release, so this keeps the bindings up to date. Differential Revision: https://internalfb.com/D45297676 fbshipit-source-id: 324ebfdd8ba82813aab4816236b33f6bce7b3c26

Summary: Wires C ABI to C# bindings using `System.Runtime.InteropServices`. Note that we don't have a working C# build right now, but there is [effort to address that](facebook#1207) which may get some more effort before the Yoga release, so this keeps the bindings up to date. Differential Revision: https://internalfb.com/D45297676 fbshipit-source-id: fb27513efa119065a7607ae7738b3060ba098ea3

Summary: Pull Request resolved: facebook#1259 Wires C ABI to C# bindings using `System.Runtime.InteropServices`. Note that we don't have a working C# build right now, but there is [effort to address that](facebook#1207) which may get some more effort before the Yoga release, so this keeps the bindings up to date. Reviewed By: yungsters Differential Revision: D45297676 fbshipit-source-id: 3b94a7f4ec663693d686e3321ec9aa06e7cd12ca

Summary: Wires C ABI to C# bindings using `System.Runtime.InteropServices`. Note that we don't have a working C# build right now, but there is [effort to address that](facebook#1207) which may get some more effort before the Yoga release, so this keeps the bindings up to date. Differential Revision: https://internalfb.com/D45297676 fbshipit-source-id: 4a308833fc86b0fac8e2d8a04ca9fd08df4e35cd

Summary: Wires C ABI to C# bindings using `System.Runtime.InteropServices`. Note that we don't have a working C# build right now, but there is [effort to address that](facebook#1207) which may get some more effort before the Yoga release, so this keeps the bindings up to date. Differential Revision: https://internalfb.com/D45297676 fbshipit-source-id: cf9246f5c223096dcb587bd2537016ef1100b248

Summary: Pull Request resolved: facebook#1259 Wires C ABI to C# bindings using `System.Runtime.InteropServices`. Note that we don't have a working C# build right now, but there is [effort to address that](facebook#1207) which may get some more effort before the Yoga release, so this keeps the bindings up to date. Reviewed By: yungsters Differential Revision: D45297676 fbshipit-source-id: 10cfb71a6220e4c7a6fc15032cd74033d4472f4b

Summary: Wires C ABI to C# bindings using `System.Runtime.InteropServices`. Note that we don't have a working C# build right now, but there is [effort to address that](facebook#1207) which may get some more effort before the Yoga release, so this keeps the bindings up to date. Differential Revision: https://internalfb.com/D45297676 fbshipit-source-id: 07ca3b5c0509f18ad825029804b68ccb5c4e5522

Summary: Wires C ABI to C# bindings using `System.Runtime.InteropServices`. Note that we don't have a working C# build right now, but there is [effort to address that](facebook#1207) which may get some more effort before the Yoga release, so this keeps the bindings up to date. Differential Revision: https://internalfb.com/D45297676 fbshipit-source-id: 113377d2577360d53d9bc7594322fbbcf91cd55d

Summary: Pull Request resolved: #1259 Wires C ABI to C# bindings using `System.Runtime.InteropServices`. Note that we don't have a working C# build right now, but there is [effort to address that](#1207) which may get some more effort before the Yoga release, so this keeps the bindings up to date. Reviewed By: yungsters Differential Revision: D45297676 fbshipit-source-id: 408f84d74ebbc7698407e951e831627117cbc2ed

NickGerleman · 2023-05-11T20:13:43Z

@jkoritzinsky what do you think about merging this as-is? I think it is going to be a better reference than what we have right now, and we can leave tests disabled in the meantime.

jkoritzinsky · 2023-05-11T20:24:46Z

I'm fine merging this as-is. I haven't had time to investigate the test failures.

Summary: These C# bindings were contributed as part of facebook@ff8f17a. They have occasionally been refreshed, but has never really had validation it built continuously, or dedicated maintenance. There has been a surge of work with facebook#1207 to try to modernize the build for these, but checking with jkoritzinsky I'm not sure either of us have the time to commit to supporting these at the same level as other bindings. Some well-known projects like Unity had already abandoned this set of bindings for their own. Unity-Technologies/UnityCsReference@016297e#diff-c85198aaac9095a5446ed00b0fba8025072d235b2b69dea8aad85abc64a83e1e So, as part of the work for an official OSS release, and really trying to define what is deprecated, and what we will try to support, I am removing the in-tree C# bindings from Yoga. In the past, gaps in Yoga bindings we haven't supported have led to new bindings with dedicated maintainers e.g. [FlexLayout](https://github.com/layoutBox/FlexLayout), [yoga-rs](https://github.com/bschwind/yoga-rs), [yoga-wasm-web](https://github.com/shuding/yoga-wasm-web). My hope is that by removing the C# bindings that we are not supporting, we free up the opportunity for a new version to become the defacto. Differential Revision: D46425886 fbshipit-source-id: 422abc0681bf02e1ebf1ba305efa3936e063c7e1

Summary: Pull Request resolved: #1302 These C# bindings were contributed as part of ff8f17a. They have occasionally been refreshed, but has never really had validation it built continuously, or dedicated maintenance. There has been a surge of work with #1207 to try to modernize the build for these, but checking with jkoritzinsky I'm not sure either of us have the time to commit to supporting these at the same level as other bindings. Some well-known projects like Unity had already abandoned this set of bindings for their own. Unity-Technologies/UnityCsReference@016297e#diff-c85198aaac9095a5446ed00b0fba8025072d235b2b69dea8aad85abc64a83e1e So, as part of the work for an official OSS release, and really trying to define what is deprecated, and what we will try to support, I am removing the in-tree C# bindings from Yoga. In the past, gaps in Yoga bindings we haven't supported have led to new bindings with dedicated maintainers e.g. [FlexLayout](https://github.com/layoutBox/FlexLayout), [yoga-rs](https://github.com/bschwind/yoga-rs), [yoga-wasm-web](https://github.com/shuding/yoga-wasm-web). My hope is that by removing the C# bindings that we are not supporting, we free up the opportunity for a new version to become the defacto. Reviewed By: javache Differential Revision: D46425886 fbshipit-source-id: df964c4d55adf93c4d1e82c104e74ca5ad181612

NickGerleman · 2023-06-05T18:17:37Z

Talking offline, we decided to go a separate route, of removing the bindings from being in tree. This was commited with 5e74e20

jkoritzinsky added 4 commits December 26, 2022 16:16

Restructure the C# bindings for Yoga to be based on .NET Standard 2.0…

da3d65e

… and the new .NET 6.0+ target platform work. Tests building is currently broken due to e96a09e. Next commit will investigate how to fix that.

Remove usages of YogaNode.GetInstanceCount

3311ec0

Merge branch 'main' of github.com:facebook/yoga into csharp-refresh

e38554a

Update to v143 toolset and make library always compile as a DLL. Set …

4a2d8a9

…the DEBUG define and fix a missing export

facebook-github-bot added the CLA Signed label Jan 3, 2023

NickGerleman reviewed Jan 3, 2023

View reviewed changes

csharp/Yoga/Yoga.vcxproj Outdated Show resolved Hide resolved

jkoritzinsky added 4 commits January 4, 2023 20:58

Create new sln file

3d4eed2

Move the C++ build for the .NET packages over to a rudimentary CMake …

9ec47c2

…build and add some projects to build the packages

Add GitHub workflow and enable linux-x64 build

7a2f5ad

Fix indentation

763ebb2

jkoritzinsky force-pushed the csharp-refresh branch from 5d50a14 to 763ebb2 Compare January 5, 2023 06:38

NickGerleman reviewed Jan 7, 2023

View reviewed changes

csharp/Facebook.Yoga/Facebook.Yoga.csproj Show resolved Hide resolved

NickGerleman reviewed Jan 7, 2023

View reviewed changes

.github/workflows/validate-csharp.yml Outdated Show resolved Hide resolved

Install workloads and fix Build Native Components to actually build o…

26150b5

…nly the native project

NickGerleman reviewed Jan 7, 2023

View reviewed changes

.github/workflows/validate-csharp.yml Outdated Show resolved Hide resolved

NickGerleman reviewed Jan 7, 2023

View reviewed changes

.github/workflows/validate-csharp.yml Outdated Show resolved Hide resolved

jkoritzinsky and others added 4 commits January 6, 2023 21:24

Apply suggestions from code review

9d213ed

Co-authored-by: Nick Gerleman <nick@nickgerleman.com>

Refactor C# setup steps into separate action

6e57851

Apply suggestions from code review

33a7ab0

Co-authored-by: Nick Gerleman <nick@nickgerleman.com>

Specify package license expression

fb1bd56

NickGerleman reviewed Jan 7, 2023

View reviewed changes

.github/actions/setup-cs/action.yml Outdated Show resolved Hide resolved

Update .github/actions/setup-cs/action.yml

aa8be68

NickGerleman reviewed Jan 7, 2023

View reviewed changes

csharp/Yoga/CMakeLists.txt Show resolved Hide resolved

NickGerleman mentioned this pull request Jun 4, 2023

Remove In-tree C# Bindings #1302

Closed

NickGerleman closed this Jun 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Restructure the C# bindings for Yoga to be based on .NET Standard 2.0 and the new .NET 6.0+ target platform work. #1207

Restructure the C# bindings for Yoga to be based on .NET Standard 2.0 and the new .NET 6.0+ target platform work. #1207

jkoritzinsky commented Jan 3, 2023

facebook-github-bot commented Jan 3, 2023

facebook-github-bot commented Jan 3, 2023

NickGerleman left a comment •

edited

NickGerleman commented Jan 3, 2023

jkoritzinsky commented Jan 5, 2023

jkoritzinsky commented Jan 5, 2023

NickGerleman commented Jan 7, 2023

NickGerleman commented May 11, 2023

jkoritzinsky commented May 11, 2023

NickGerleman commented Jun 5, 2023

Restructure the C# bindings for Yoga to be based on .NET Standard 2.0 and the new .NET 6.0+ target platform work. #1207

Restructure the C# bindings for Yoga to be based on .NET Standard 2.0 and the new .NET 6.0+ target platform work. #1207

Conversation

jkoritzinsky commented Jan 3, 2023

facebook-github-bot commented Jan 3, 2023

Action Required

Process

facebook-github-bot commented Jan 3, 2023

NickGerleman left a comment • edited

Choose a reason for hiding this comment

NickGerleman commented Jan 3, 2023

jkoritzinsky commented Jan 5, 2023

jkoritzinsky commented Jan 5, 2023

NickGerleman commented Jan 7, 2023

NickGerleman commented May 11, 2023

jkoritzinsky commented May 11, 2023

NickGerleman commented Jun 5, 2023

NickGerleman left a comment •

edited