search

y-crdt / ydotnet

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

Add libyrs.dylib built for osx arm #81

Closed vdurante closed 6 months ago

vdurante commented 6 months ago

Add libyrs.dylib for OSX ARM

SebastianStehle commented 6 months ago

Btw: For testing I would just reference this project from the unit tests (temporarily) and then run the tests.

vdurante commented 6 months ago

@SebastianStehle

I used the .dylib file on my custom project and it worked, but for some reason if I add the native ARM project as a reference to the unit tests project, it still fails

This is how the project looks like:

image

Added this to YDotNet.Tests.Unit:

<ProjectReference Include="..\..\native\YDotNet.Native.MacOS.ARM\YDotNet.Native.MacOS.ARM.csproj" />

Still failing when I run. Checking the bin folder, it doesn't contain the runtimes folder.

SebastianStehle commented 6 months ago

Perhaps this is the reason why it is called "PackagePath" ;)...So I guess we have to publish it. But you have to explain me the other changes in the yml file before I can merge it.

vdurante commented 6 months ago

Regarding the yml file:

Here you can find a comment that directs to the other repository that contains a workflow file that was used as inspiration:

https://github.com/y-crdt/ydotnet/blob/070adb52dc73062295482f5fec5956a008f26b6f/.github/workflows/build-binaries.yml#L18

Going to the other repository, you can find how it builds for both Mac x64 and arm64

https://github.com/crodjer/sysit/blob/b03d0d4920ab43241d8e0976a5de3962b3bce016/.github/workflows/release.yml#L62-L71

I basically copied over the yaml config to my PR and adjusted the Content's Include and PackagePath accordingly to the new directories.

I am not entirely sure where the output\<os-arch> folder naming is configured, but from what it seems the subfolder uses the name of the build step, hence why I use macos-x86_64 and macos-aarch64 for those

https://github.com/y-crdt/ydotnet/blob/e1e2cd7ce6c9803c94d4b2f4554e2e76e9389421/.github/workflows/build-binaries.yml#L74-L84

Not easy to test this locally, since nektos/act doesn't have Docker images for MacOS

SebastianStehle commented 6 months ago

Ah, I see. aarch is just an arm variation. The point is: You cannot build for arm yet because there are not arm github runners yet. Therefore we have to build it locally and this is the reason why I cannot do it myself (because I do not have a mac).

So for now, we should:

  1. Just keep the build process as is,.
  2. In the current MacOS project file, use runtimes\osx-x64\native and target path to avoid confusion (but use the existing source file).
  3. Add your binary to the repo.

When ARM runners are available we can make another PR.

vdurante commented 6 months ago

@SebastianStehle from what I read online rust is capable to build for ARM using Intel, so it should technically be possible.

If you want, we can test it, otherwise I can follow your approach!

Also, Linux contains all architectures in the same project, why separate into Intel and ARM only for Mac? Wouldn't it make sense to follow the same pattern? The file is not that big, roughly 1.5 MB

SebastianStehle commented 6 months ago

That would be great. Can you confirm that? Just run the build in your fork and download the artifacts. If it works: Great :)

I agree with your argument about Mac and Linux project. I have forgotten that. Lets do one project again

vdurante commented 6 months ago

@SebastianStehle it works! Just tried on my project and it worked well. I built using github actions, downloaded the artifact and used it on my local yjs project.

I also reverted the other changes and kept a single Mac project