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

fuse-t: init at 1.0.42 #348804

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fuse-t: init at 1.0.42 #348804

wants to merge 2 commits into from

Conversation

Enzime
Copy link
Member

@Enzime Enzime commented Oct 15, 2024

  • Added fuse-t, fuse-t.binaryDistribution and fuse-t.stubs
  • Use fuse-t instead of macFUSE for sshfs-fuse

You can test this PR with:

nix run github:numtide/nixpkgs-unfree#fuse-t --override-input nixpkgs github:Enzime/nixpkgs/fuse-t -- user@host:/path /mount/point 

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@Enzime Enzime requested review from primeos and emilazy October 15, 2024 15:01
@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 1-10 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Oct 15, 2024
@Enzime Enzime requested review from flokli and midchildan October 31, 2024 08:11
Comment on lines +171 to +173
# NFS server
# free for non-commercial use, see: https://github.com/macos-fuse-t/fuse-t/blob/main/License.txt
lib.licenses.unfreeRedistributable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit concerned about including the unfree NFS server. I believe Hydra doesn't build unfreeRedistributable packages. Wouldn't this cause more popular packages to be dropped from the Darwin binary cache as more packages switch to FUSE-T?

#83884

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AIUI packages will only link to the stubs, which are FOSS (as they have to be, because FUSE is LGPL).

(Will try to find the time to review the rest of this soon.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears that the fuse-t package isn't a stub, but rather a mixture of a source-built libfuse and the unfree NFS server. So in this case, I believe sshfs-fuse will be dropped from the binary cache.


postBuild = ''
mkdir -p lib/pkgconfig
tapi stubify --filetype=tbd-v2 lib/libfuse-t.dylib -o lib/libfuse-t.tbd
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the purpose of creating a TBD stub from a source-built package? I'm confused because I recognize TBD stubs as substitutes for actual dylib files, yet the built dylib is included in this package.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just copied it from the macFUSE derivation, I didn't actually know what they were for

Copy link
Member

@midchildan midchildan Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. So here's the background. macFUSE consists of a kernel extension, preference pane, and the userspace libfuse library. This created a few challenges.

  1. Everything except libfuse is unfree which disqualifies it from being the on binary cache
  2. Preference panes and kernel extensions can't be installed with Nix
  3. libfuse works closely with the kernel extension, so version mismatches could break things

So I took the approach of making users install macFUSE outside of Nix and have packages link to the out-of-store libfuse. To achieve this, the macfuse-stub package creates a stub of libfuse using the actual macFUSE release so that compilers can link to it without it being actually present in /usr/local/lib.

With FUSE-T, problem No. 1 remains. And if we drop the NFS server but build libfuse from source, we'll have problem No. 3 as well. So I think the FUSE-T package should take the same approach to avoid these problems.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case of FUSE-T, the location for the NFS server is compiled in a header https://github.com/NixOS/nixpkgs/pull/348804/files#diff-542ad4d614512f9a215eaba4a723df42cc4a1fdc6f1d339f73a35818a50b043e but can be overridden with the environment variable FUSE_NFSSRV_PATH

So we could change it from directly pointing to the unfree NFS server to /run/current-system/sw/bin and then nix-darwin users can install the unfree NFS server using environment.systemPackages

In addition we could create wrappers (living under different attributes, so the downstream packages still get cached) that set FUSE_NFSSRV_PATH so that users can still use the packages from Nixpkgs without needing to manually install the unfree NFS server

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t suppose there’s any chance of using the same stubs for both macFUSE and FUSE-T?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. FUSE-T changed the filename to libfuse-t.dylib for one thing.

Copy link
Member

@midchildan midchildan Nov 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we could change it from directly pointing to the unfree NFS server to /run/current-system/sw/bin and then nix-darwin users can install the unfree NFS server using environment.systemPackages

For maximum compatibility, I guess we can use the header as is and introduce a nix-darwin module that makes FUSE_NFSSRV_PATH point to the Nix store. This would allow both Nix and non-Nix applications to use FUSE-T.

It should be noted though, that some applications use libraries that bypass libfuse and talk to the NFS server directly. Those aren't guaranteed to respect FUSE_NFSSRV_PATH. This is likely not an exhaustive list, but I found one case where the path is hardcoded.

https://grep.app/search?q=FUSE_NFSSRV_PATH

There are also cases like cgofuse, which uses dlopen() to load libfuse from a hardcoded path.

https://github.com/winfsp/cgofuse/blob/f87f5db493b56c5f4ebe482a1b7d02c7e5d572fa/fuse/host_cgo.go#L176

In addition we could create wrappers (living under different attributes, so the downstream packages still get cached) that set FUSE_NFSSRV_PATH so that users can still use the packages from Nixpkgs without needing to manually install the unfree NFS server

You mean like steam-run? That sounds nice, though I think we'd need to check that two different instances of the NFS server can run at the same time. Nix being Nix, users may have multiple FUSE-T NFS servers in their Nix store.

From what I've seen in FUSE-T libfuse's code, it does appear to be okay. libfuse creates a socket pair using socketpair(), forks and launches the NFS server, and uses the created socket to communicate with the newly launched server. It's probably a good idea to check though, because we don't know exactly what happens in the NFS server.

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants