y-crdt / ydotnet

.NET bindings for yrs.
MIT License
29 stars 9 forks source link

Binary Strategy #56

Closed SebastianStehle closed 8 months ago

SebastianStehle commented 9 months ago

Hi,

I have seen that you changed the strategy how you distribute binaries in my PR. I understand the intention that you want to have a single package that just works. But I would prefer not to skip the binaries for all platforms, especially for client applications.

So I would do that:


I also noticed that we should build with an older linux version (ubuntu-20.04). The reason is that y-crdt needs libc and the version is dependent on the build platform.

ubuntu-20.04 uses gclib 2.31 which is the same version as the default .NET docker images for 7.0. So if you we build with ubuntu latest, we would link again gclib 2.33, which would then not run with the official images. libc is backwards compatible, so a newer version should not be a problem.

LSViana commented 9 months ago

Hi, @SebastianStehle!

I'll roll back my changes to include the binaries via the main project .csproj file. I am sure your approach is the best way forward and I just pushed those changes back into your branch so I could run tests easily, however, that approach isn't gonna last.

I'll leave this issue open until #55 is merged with your approach only (since I'll remove mine).

LSViana commented 8 months ago

Hi, @SebastianStehle!

I just noticed rolling back my changes to include the .dll, .so, and .dylib files from this location.

But, please, can you just confirm to me how urgent that issue is? I mean, the binaries themselves are not added to the repository and if the project is built without those files inside /YDotNet/ nothing will happen.

This means you won't get the binaries for all platforms/architectures unless you put them there before building the project.

I'm still going to remove that but I'm still curious about how problematic that was because, from my point of view, it shouldn't break your flows, right?

SebastianStehle commented 8 months ago

Should be fine.