rust-lv2 icon indicating copy to clipboard operation
rust-lv2 copied to clipboard

Add macos support to lv2-sys?

Open JustAnotherCodemonkey opened this issue 2 years ago • 13 comments

As it stands, macos is unsupported, as stated by an error message that greets anyone who tries to compile on mac. It's kind of odd to me that windows is supported but not macos. I'd like to use the crate but I can't on my macbook as it stands.

JustAnotherCodemonkey avatar May 06 '23 18:05 JustAnotherCodemonkey

If i remember correctly, we don't have macos support because this require having a macos to generate the lv2 binding and no maintainer have one. However, We have a tool You may use to enable macos support. Step to follow

  • cloning this repo
  • follow instruction of the systool to generate binding. The lv2 C API is included in sys/lv2
  • edit sys/src/lib.rs. You just need to add a line like this #[cfg_attr(target_os = "macos", path = <your_binding.rs>)]

YruamaLairba avatar May 22 '23 11:05 YruamaLairba

Ok! I'd be happy to help out where I can with the maintaining. I don't have a huge amount of experience with c or llvm but I'd love to help and I can do macos testing.

I'm trying to run the tool with the following command: "cargo r -p systool -- --lv2 sys/lv2 --out x86_64-apple-darwin" but get the following error: "thread 'main' panicked at 'called Option::unwrap() on a None value', sys/tool/src/main.rs:96:54". The clang args should not be required so why is it unwraping on this?

JustAnotherCodemonkey avatar May 22 '23 18:05 JustAnotherCodemonkey

thread 'main' panicked at 'called Option::unwrap() on a None value', sys/tool/src/main.rs:96:54

Ooops, this is a bug from systool. try to workaround by passing a clang args. For example, use "-v", for verbose, this shouldn't affect the generated file

YruamaLairba avatar May 22 '23 22:05 YruamaLairba

It generated a ton of warnings but it does work. Still wondering why you needed an actual mac to generate the bindings as the tool docs literally have a section on how to cross-compile.

JustAnotherCodemonkey avatar May 22 '23 22:05 JustAnotherCodemonkey

honestly, i don't remember why we didn't include mac suppport. I was convinced this was because it require some header that can only be found on mac, but i just tried to generate from my linux machine and it worked.

YruamaLairba avatar May 23 '23 23:05 YruamaLairba

Ok. What's the process to fix this for the repo?

JustAnotherCodemonkey avatar May 26 '23 20:05 JustAnotherCodemonkey

You just need to do a pull request that pass all the checks and wait somebody validate the pull request. This may take a long...

YruamaLairba avatar May 27 '23 21:05 YruamaLairba

Well what do I change for the PR? I include the generated file, right? Just want to clarify

JustAnotherCodemonkey avatar May 27 '23 21:05 JustAnotherCodemonkey

Yes, name your file "macos.rs", include it in sys/src, and add #[cfg_attr(target_os = "macos", path = "macos.rs")] on top of mod unsupported;

YruamaLairba avatar May 28 '23 09:05 YruamaLairba

Ok new to github. I made my fork and am trying to merge onto master in my fork. It says that it requires maintainer permission and is waiting on workflow checks. What do I do?

JustAnotherCodemonkey avatar May 28 '23 22:05 JustAnotherCodemonkey

I don't know why workflow checks stuck. I just realized we need to add workflow checks for macos, but this is beyond the scope of my knowledge for now. Except that, what you did is correct, you just need to wait for a maintainer to validate the pull request. Maybe we can call one :)

@prokopyl are you around to check and merge #113 ?

YruamaLairba avatar May 29 '23 10:05 YruamaLairba

I also would like to see this getting merged please!

herrernst avatar Dec 28 '23 20:12 herrernst