Page MenuHomePhabricator

efl-mono: Add support for dotnet core
ClosedPublic

Authored by lauromoura on Feb 28 2019, 3:54 PM.

Details

Summary

This commits adds dotnet as a supported C# platform for EFL# bindings.

Due to differences between Mono and Dotnet regarding DllImport, the
bindings now are using an imperative approach to load the function
pointers through the NativeModule and FunctionWrapper classes. These
classes handle the dlopen/LoadLibrary and dlsym/GetProcAddress calls.

Also, the previous caching of non-owned strings returned to native code
was removed until further memory checks.

We also had to create workaround for bool and chars in Structs for C#
marshaling. Going through System.Byte instead and Marshaling manually
to their respective types.

In order to actually build efl_mono.dll with dotnet right now,
issue #4782 from Meson should be fixed to make it properly detect and
used the Dotnet compiler. Also use "-Ddotnet=true" when running meson.

Fixes T7394

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
lauromoura created this revision.Feb 28 2019, 3:54 PM
lauromoura requested review of this revision.Feb 28 2019, 3:54 PM

Most of the patch is final. Marked with DO NOT MERGE just until a few build issues are fixed tomorrow (Friday).

lauromoura updated this revision to Diff 19881.Mar 1 2019, 4:37 PM

Updating with dotnet build fixes.

Allowing to build with dotnet and a patched meson (see commit msg).

vitor.sousa requested changes to this revision.Mar 1 2019, 5:49 PM

Very good. I could find only one small problem:

src/bindings/mono/eo_mono/iwrapper.cs
52

A trailling NULL and a parent pointer are mandatory. So, I think this overload may cause a crash.

This revision now requires changes to proceed.Mar 1 2019, 5:49 PM
lauromoura updated this revision to Diff 19889.Mar 1 2019, 6:48 PM

Update with vitor comments

vitor.sousa accepted this revision.Mar 1 2019, 6:49 PM
This revision is now accepted and ready to land.Mar 1 2019, 6:49 PM
This revision was automatically updated to reflect the committed changes.
bu5hm4n added a comment.EditedMar 2 2019, 12:13 AM

Hey, this adds stuff to our meson build that is not even supported by current upstream master, and there is exactly 0 warning about it. Additionally, this adds a giant amount of:

if asdf 
  sample-one
else
  sample-one just with one more argument
endif

In other words, this adds a giant amount of code duplication in the build tool.
Additionally, why the heck do you copy libraries arround? This must be handled by meson and not the build script here. Meson must take care of dependencies etc. and they are right now broken here completly, ninja will not trigger build updates at all (Or build it all the time).
Additionally number 2: The intend of the new meson stuff is wrong in every single case (except 1)
Additionally number 3: Whenever someone used LD_LIBRARY_PATH, you broke this for him here.

I cannot comment on the .cs stuff, but could we probebly wait with meson patches until THE MESON SUPPORT for it is done. And NOT treat efl master like its our playground ?

Hey, this adds stuff to our meson build that is not even supported by current upstream master, and there is exactly 0 warning about it.

What would be the appropriate warning here? Comments in the meson.build where this is used and/or a warning message when this support is actually used (i.e. cs_is_dotnet is true).

Using an upstream meson with dotnet as CSC will cause meson to fail with dotnet as an unrecognized compiler. Using it with mono ignores the calls depending on the patched meson.

Additionally, this adds a giant amount of:

if asdf 
  sample-one
else
  sample-one just with one more argument
endif

In other words, this adds a giant amount of code duplication in the build tool.

This duplication is actually the calls to the 3 main build targets for the C# bindings: efl_mono.dll and the two test executables. It's not that this duplication is giant (but indeed it is better to be without it and cut while it is small). In any case, I've refactored the meson patch and I'm passing the dlls through cs_args (like -r:System.Runtime.dll) and will remove the runtime_assemblies parameter in the next diff. This should allow using executable and library calls compatible with the current meson version, dropping the duplication.

Patch in D8092

Additionally, why the heck do you copy libraries arround? This must be handled by meson and not the build script here. Meson must take care of dependencies etc. and they are right now broken here completly, ninja will not trigger build updates at all (Or build it all the time).

I'm tried changing the patch to avoid copying stuff around but run into some issues about dotnet not being able to load assemblies directly from other folders, even with LD_LIBRARY_PATH handling. Maybe will need to create a project file or something like that.

Additionally number 2: The intend of the new meson stuff is wrong in every single case (except 1)

Will fix.

Additionally number 3: Whenever someone used LD_LIBRARY_PATH, you broke this for him here.

LD_LIBRARY_PATH is also set in the load_lib block right after the copying block. Wouldn't this block also overwrite external LD_LIBRARY_PATH and should be changed to env.prepend? Btw, I tried using env.append and moving the dotnet block after load_lib but it still did overwritten instead of appending.

I cannot comment on the .cs stuff, but could we probebly wait with meson patches until THE MESON SUPPORT for it is done. And NOT treat efl master like its our playground ?

Sorry for the trouble. Working to fix this.

Thank you for your quick respons, lets continue the discussion in the other revision :)