si icon indicating copy to clipboard operation
si copied to clipboard

Fix si-fs project on macos

Open JakeGinnivan opened this issue 6 months ago • 5 comments

Added libfuse to fuser features Switch EBADFD to EBADF for cross platform bad file descriptor Pass additional link args on mac so the rust binary can link the the macfuse symbols

I've tried to not make changes which would affect linux, but don't have a linux machine at the moment so I've not tested.

nix:nix-shell-env) MacBookPro:si jake$ buck2 build //bin/si-fs
Starting new buck2 daemon...
Connected to new buck2 daemon.
[lots of warnings]
Build ID: 0655c259-4946-42e3-b5fb-e813fe62a0f3
Network: Up: 0B  Down: 26MiB
Loading targets.   Remaining     0/25     
Analyzing targets. Remaining     0/740    
Executing actions. Remaining     0/3041   
Command: build.    Finished 905 local                                                                   
Time elapsed: 2:17.2s
BUILD SUCCEEDED
(nix:nix-shell-env) MacBookPro:si jake$ 
image image image

JakeGinnivan avatar May 27 '25 12:05 JakeGinnivan

Will continue trying to figure out how to make these changes only apply on macos

JakeGinnivan avatar May 28 '25 00:05 JakeGinnivan

/try

zacharyhamm avatar May 28 '25 17:05 zacharyhamm

Okay, starting a try! I'll update this comment once it's running...\n 🚀 Try running here! 🚀

github-actions[bot] avatar May 28 '25 17:05 github-actions[bot]

@JakeGinnivan we just need to test this out and make sure the changes still work on linux. But looks good to me so far!

zacharyhamm avatar May 28 '25 17:05 zacharyhamm

@zacharyhamm feedback on the approaches welcome, not used fuse, nix or buck2 before so probably better ways, but looked for existing patterns to solve OS specific ways to solve in buck and seemed it was mostly done through fixups.

JakeGinnivan avatar May 29 '25 00:05 JakeGinnivan

@JakeGinnivan sorry for losing track of this one. I have tested it on linux and it builds, so we should be good to go. Can you rebase this with the latest main? Let me know and I'll test it out again on macOS and Linux.

zacharyhamm avatar Jul 30 '25 19:07 zacharyhamm

@zacharyhamm I've rebased.

I've gone through it all again, I think the fixup was the wrong approach, I've removed. Here is some things i've noticed which having someone who is more familiar with the build system setup to validate.

Cargo.lock

The conditional feature in third-party/rust/Cargo.toml is causing pkg-config to be added to the Cargo.lock file, not sure if this is going to cause an issue.

Also, I've had to duplicate the fuser version number, which will break if the conditional dep and the normal dep don't both get upgraded.

Reindeer adding to buildscript_run

The above causes reindeer to add the additional platform targets to buildscript_run. But this isn't supported by _cargo_buildscript_rule which is called in prelude/rust/cargo_buildscript.bzl. I just remove it from the map, not sure if there is a better way.

platform_fixup.

I removed this in some windows fixup files as that syntax has been removed in reindeer, I was using it previously, but I don't think that's the right approach because it doesn't automatically add the dependencies in, which causes issues downstream.

JakeGinnivan avatar Sep 29 '25 06:09 JakeGinnivan

@zacharyhamm I've rebased.

I've gone through it all again, I think the fixup was the wrong approach, I've removed. Here is some things i've noticed which having someone who is more familiar with the build system setup to validate.

Cargo.lock

The conditional feature in third-party/rust/Cargo.toml is causing pkg-config to be added to the Cargo.lock file, not sure if this is going to cause an issue.

Also, I've had to duplicate the fuser version number, which will break if the conditional dep and the normal dep don't both get upgraded.

Reindeer adding to buildscript_run

The above causes reindeer to add the additional platform targets to buildscript_run. But this isn't supported by _cargo_buildscript_rule which is called in prelude/rust/cargo_buildscript.bzl. I just remove it from the map, not sure if there is a better way.

platform_fixup.

I removed this in some windows fixup files as that syntax has been removed in reindeer, I was using it previously, but I don't think that's the right approach because it doesn't automatically add the dependencies in, which causes issues downstream.

Great! taking a look

zacharyhamm avatar Sep 29 '25 17:09 zacharyhamm

hey @JakeGinnivan - so we started work on a new tool - called si-conduit that will replace the abilities for si-fs - it is going to be more portable as it's written in deno and can be changed / extended easily

I am going to close this out - sorry we were so bad about responding to this :/ I will make sure this doesn't happen again in the future

stack72 avatar Oct 22 '25 18:10 stack72

@stack72 all good, was interesting learning the toolchain enough to try fix this. New tool sounds like a nice approach

JakeGinnivan avatar Oct 22 '25 23:10 JakeGinnivan