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

Include packages for x86/x64 onigwrap.dll, and dynamic binding between them #51

Closed
dmitry-medvedev opened this issue May 4, 2023 · 20 comments · Fixed by #52
Closed

Comments

@dmitry-medvedev
Copy link

We use TextMateSharp in our projects; on our tests, projects compiled with AnyCPU platform either do not copy onigwrap.dll to the bin folder (in case of packages.config), or copy x86 version of onigrwrap.dll (in case of sdk-style projects/package references). On the x64 platform, the project cannot load this dll and fails to run.

Please consider providing additional packages like TextMateShar.onigwrap.dll.Native.win-x64/TextMateShar.onigwrap.dll.Native.win-x86,

and implementing run-time binding to the correct version of onigwrap.dll depending on the platform.

Please take a look at ClearScript project which does it this way:
https://www.nuget.org/packages/Microsoft.ClearScript

(different platforms are handled inside V8SplitProxyNative.Generated.cs)

@danipen
Copy link
Owner

danipen commented May 4, 2023

@dmitry-medvedev are you planning a PR for this or do you want me to take a look (not sure if I will able to take a look soon since I am quite busy now)?

@dmitry-medvedev
Copy link
Author

We will need this to work quite soon, as our release depends on it. We will do local changes first ourselves, and then submit a pull request.

@danipen
Copy link
Owner

danipen commented May 4, 2023

Ok then. Thanks.

@danipen
Copy link
Owner

danipen commented May 4, 2023

Not sure I understand the proposed solution. Are you proposing separating different nuget packages for each platform? In that case, how would you need to reference each nuget reference in the .csproj based on the current platform?

Note that, right now, the nuget package packs every version of the native libraries, and it places it on the right path, so the runtime is able to load them based on the platform:

  • runtimes/win-x86/native/onigwrap.dll for x86
  • runtimes/win-x64/native/onigwrap.dll for x64
  • runtimes/linux/native/libonigwrap.so for linux
  • runtimes/osx/native/libonigwrap.dylib for macOS

An existing issue is that, in debug mode, using Visual Studio, only the x86 version is copied to the output dir, and probably, using x64 won't work (in my case it works fine for the Demo project but it fails for the unit tests).

But I never saw it failing when using nuget packages.

@danipen
Copy link
Owner

danipen commented May 4, 2023

So probably, finding a better way of copying the native deps into the bin folder (bin\Debug\netstandard2.0) may fix the issue.

@dmitry-medvedev
Copy link
Author

Yes, separate NuGet packages for every platform looks like an overkill. We will try the solution you proposed.

@danipen
Copy link
Owner

danipen commented May 6, 2023

@dmitry-medvedev I have some time to have a look. Could you please confirm if #52 resolves the problem you were facing?

@dmitry-medvedev
Copy link
Author

Many thanks for that!

We will test changes in #52 and will let you know ASAP.
Changes to the theme names work great for us.

Dmitry

@danipen
Copy link
Owner

danipen commented May 6, 2023

Awesome. Thanks for the contribution BTW! 😉

@dmitry-medvedev
Copy link
Author

We've tested the new interop code with different platforms, and it works perfectly. Thank you!

@danipen
Copy link
Owner

danipen commented May 8, 2023

A new version of TextMateSharp is available in NuGet including the PR that fixes this issue: https://www.nuget.org/packages/TextMateSharp/1.0.55

@dmitry-medvedev
Copy link
Author

Thanks, we're now using it, and it works great. I will let you know once we release the software which uses TextMate package.

@danipen
Copy link
Owner

danipen commented May 11, 2023

BTW and out of curiosity ... are you using AvaloniaEdit for the editor or are you using a different implementation?

@dmitry-medvedev
Copy link
Author

No, we're not using Avalonia, it's our own implementation.

@dmitry-medvedev
Copy link
Author

dmitry-medvedev commented Jun 3, 2023

Hi Daniel,

I thought you might be interested. We've released a new version of AlterNET Studio, with a new TextMate-based parser as one of the major features:
https://www.alternetsoft.com/news/alternet-studio-9-0-released

There are a couple of things that we had to work around when using the TextMateSharp package. Maybe that's something you may address in future releases:

  1. TextMateSharp depends on System.Text.Json of version 7.0.0.2, while other libraries that we use, such as Microsoft Code Analysis, use version 7.0.0.0. I wonder if there is a specific reason why you're using that specific version or if it can be downgraded to 7.0.0.0.

  2. We found that for .net applications using .Net Framework 4.6.2, x86 and x64 onigwrap libraries are not copied in the bin folder. On our tests, either the x64 or x86 bit version is copied. We solved this issue by adding onigwrap libraries to our own package that uses TextMateSharp with the following build target:

<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <ItemGroup Condition="'$(TargetFramework)' == '' Or '$(TargetFramework.TrimEnd(`0123456789`))' == 'net'">
    <None Include="$(MSBuildThisFileDirectory)..\runtimes\win-x64\native\onigwrap-x64.dll">
      <Link>onigwrap-x64.dll</Link>
      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
      <Visible>false</Visible>
    </None>
    <None Include="$(MSBuildThisFileDirectory)..\runtimes\win-x86\native\onigwrap-x86.dll">
      <Link>onigwrap-x86.dll</Link>
      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
      <Visible>false</Visible>
    </None>
  </ItemGroup>
</Project>

Kind regards,
Dmitry

@danipen
Copy link
Owner

danipen commented Jun 3, 2023

I wonder if there is a specific reason why you're using that specific version or if it can be downgraded to 7.0.0.0.

No prob about downgrading. I was just using latest.

@danipen
Copy link
Owner

danipen commented Jun 3, 2023

We found that for .net applications using .Net Framework 4.6.2, x86 and x64 onigwrap libraries are not copied in the bin folder. On our tests, either the x64 or x86 bit version is copied.

Not sure about this one. The nugget package should deploy the native deps automatically...

Are you consuming TexmateSharp from the nuget package?

@dmitry-medvedev
Copy link
Author

WindowsFormsApp7.zip

I'm attaching a sample project created with .NET Framework referencing TextMateSharp and TextMateSharp.Grammars packages. On our tests it does not copy onigwrap DLLs to the bin folder.

@danipen
Copy link
Owner

danipen commented Jun 5, 2023

In this case you're not consuming TexmateSharp as a nuget package so probably the deps are not copied due to this reason.

If you want to add a PR with the needed changes to copy deps when referencing the project directly for .net framework it's ok for me.

@danipen
Copy link
Owner

danipen commented Jun 5, 2023

No prob about downgrading. I was just using latest.

Same for this one. Add a PR if you want and I'll be happy to merge it 😊. Thanks!

Andrew1Medvedev added a commit to Andrew1Medvedev/TextMateSharp that referenced this issue Jun 9, 2023

Verified

This commit was signed with the committer’s verified signature.
weswigham Wesley Wigham
…n them danipen#51
Andrew1Medvedev added a commit to Andrew1Medvedev/TextMateSharp that referenced this issue Jul 3, 2023

Verified

This commit was signed with the committer’s verified signature.
weswigham Wesley Wigham
…n them danipen#51
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 a pull request may close this issue.

2 participants