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

xdg-document-portal: implement flock #1353

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

lufia
Copy link

@lufia lufia commented Apr 29, 2024

I implemented flock operation of FUSE.

This is needed for Steam client.

Tested

I tested whether the new xdg-document-portal accepts flock(2) syscall with flock(1).

$ umount /run/user/60331/doc
$ meson compile -C _build
$ ./_build/document-portal/xdg-document-portal &

$ cd /run/user/60331/doc
$ touch lock.L
$ flock -n lock.L sleep 100

Then, within 100 seconds, I ran flock(1) in other terminal session.

$ flock -n lock.L ls || echo BAD
BAD

I checked blocking operations are rejected.

$ flock lock.L ls
flock: lock.L: Function not implemented

$ flock -s lock.L ls
flock: lock.L: Function not implemented

@lufia lufia marked this pull request as ready for review April 29, 2024 08:43
@swick
Copy link
Contributor

swick commented Apr 29, 2024

Looks like this will block the entire document portal when any file is blocked due to flock.

@lufia
Copy link
Author

lufia commented Apr 29, 2024

Hi, @swick
I mitigated not to block entire document-portal by rejecting blocking operations of flock.
Steam client works well in my Arch Linux box.

@hfiguiere Thanks for your suggestion!

@GeorgesStavracas
Copy link
Member

Thanks for the patch. Please squash the suggestion commit into a single commit.

@swick
Copy link
Contributor

swick commented Apr 29, 2024

Supporting flock only for certain option is very suspicious to me. This needs review from someone who knows about fuse and filesystems.

This is needed for Steam client

Signed-off-by: Kyohei KADOTA <[email protected]>
Co-authored-by: Hubert Figuière <[email protected]>
@Dutt-A
Copy link

Dutt-A commented Jul 23, 2024

Hello,
I'm wondering if someone can again take a look at this branch and merge it in. Without this addition, the Steam flatpak is unable to handle adding another filesystem.

@swick
Copy link
Contributor

swick commented Jul 23, 2024

@alexlarsson ^

@smcv
Copy link
Collaborator

smcv commented Jul 29, 2024

Without this addition, the Steam flatpak is unable to handle adding another filesystem

You can give the unofficial Steam Flatpak app direct access to locations where you want to add an extra Steam library by completely exiting from Steam, and then using something like:

$ mkdir /media/other-disk/steam-library
$ flatpak override --user --filesystem=/media/other-disk/steam-library com.valvesoftware.Steam

before starting Steam again and selecting /media/other-disk/steam-library as your new Steam library. This avoids the performance overhead of going through a FUSE filesystem, as well as avoiding any limitations like the one fixed in this branch.

(documentation)

@Dutt-A
Copy link

Dutt-A commented Jul 29, 2024

Without this addition, the Steam flatpak is unable to handle adding another filesystem

You can give the unofficial Steam Flatpak app direct access to locations where you want to add an extra Steam library by completely exiting from Steam, and then using something like:

$ mkdir /media/other-disk/steam-library
$ flatpak override --user --filesystem=/media/other-disk/steam-library com.valvesoftware.Steam

before starting Steam again and selecting /media/other-disk/steam-library as your new Steam library. This avoids the performance overhead of going through a FUSE filesystem, as well as avoiding any limitations like the one fixed in this branch.

(documentation)

Upon using the override I precisely get the original errorno 38 (function not implemented) that lufia mentions here, and I believe that the additions are need to allow the line to work, unless I am misunderstanding something.

@chrisawi
Copy link

Upon using the override I precisely get the original errorno 38 (function not implemented) that lufia mentions here, and I believe that the additions are need to allow the line to work, unless I am misunderstanding something.

No, the override is sufficient by itself. You do need to restart Steam, and you may need to remove and re-add the library in Steam so it doesn't continue to use the portal path. That error means it's probably still going through the document portal.

Also, just to be clear, you need to modify the path to match your actual Steam library location; don't use literally /media/other-disk/steam-library.

@Dutt-A
Copy link

Dutt-A commented Jul 29, 2024

Upon using the override I precisely get the original errorno 38 (function not implemented) that lufia mentions here, and I believe that the additions are need to allow the line to work, unless I am misunderstanding something.

No, the override is sufficient by itself. You do need to restart Steam, and you may need to remove and re-add the library in Steam so it doesn't continue to use the portal path. That error means it's probably still going through the document portal.

Also, just to be clear, you need to modify the path to match your actual Steam library location; don't use literally /media/other-disk/steam-library.

Really? I'll take a look and try again in a few days (occupied with moving right now).

I actually recall that the override was working for me perfectly fine on the Fedora KDE spin, but somehow it has not been working for me on the Fedora Sway spin. Both installations should have me running on Wayland.

I appreciate the input.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Needs Triage
Development

Successfully merging this pull request may close these issues.

7 participants