Linux List GVFS shares in system folders
AbandonedPublic

Authored by Roel Koster (kostex) on Sun, Oct 28, 9:16 PM.

Details

Summary

Hi Brecht,

Since my previous patch submission was about Snaps in particular, I thought I'd create a new revision instead of updating the old one. Hope I did the right thing.
Anyway, you talked about finding a freedesktop standard for accessing gnome or other desktop created shares.
I've looked into it but it seems there is only an environment variable pointing to the location of the gvfs shares.
But then it occurred to me, in the same setmntent list used currently, the gvfs share is in there already!
So no need to do anything extra but excluding that share from the filter so it shows up too.

Looking at the complete unfiltered list I did not see any other useful shares to list ;-) so hopefully this patch is the last regarding this issue!

BTW. I noticed 2 typo's as well (filesystemts instead of filesystems)

Diff Detail

Repository
rB Blender
Brecht Van Lommel (brecht) requested changes to this revision.Mon, Oct 29, 2:52 PM

With this patch I always see "gvfs" in the list. I think it should show the gvfs subfolders directly in the list, like other file browsers. The "gvfs" name should not appear to users I think, it's an implementation detail.

This revision now requires changes to proceed.Mon, Oct 29, 2:52 PM
  • Get environment variable pointing to the XDG Runtime folder
  • Loop through the gvfs folder to find/add shares

I´m not perfectly sure if all folders have the same name convention, but unaltered they don't make good names to add to the list as is.
I've made the assumption that every entry has a "share=xxx" at the end for now.

NB.
Put the variable declarations near the loop.. just for quick reference
My additions to the code are mostly "kitbashing"
I have no real insight in the blender code and available tools (yet)
Not a C++ coder :-(
(en de leeftijd speelt ook mee)

Hmm,

it seems that when disconnecting the network, the share entries won't go away and the os will keep trying to look for them when asking for the directory list until timeout.
This slows down blender startup while waiting for the timeout..

Removed one check for existance of /run/user/xxx/gvfs folder
If it doesn't exist no real problem arises.
Benefit however is that when the network is down, it won't have to wait twice for a dir-exists and dir-contents

All in all I find it very difficult to get a good solution.. I'm starting to think that just adding the "gvfs" folder with another label name like "Gnome Shares" or something like that solves a lot of problems.
but then again, inside it shows those stupid folder names like in my case:
'smb-share:server=imac2009-capitan.local,share=data_1/'
'smb-share:server=imac2009-capitan.local,share=data_2/'
'smb-share:server=imac2009-capitan.local,share=datapartition/'

On the other hand, now it looks nice and just lists:
data_1
data_2
datapartition
but with the risk that starting blender and opening the file browser will take a lot of time IF the user decides to disconnect the network while the shares are still mounted.

NB. I did find another mountpoint that should not be in the system folder list: /boot/efi

forgot to include the #include "BLI_fileops_types.h" this last diff.

And while looking over the code for the nth time.. I know the addition of 6 to the label pointer is dangerous and at least foolish.. but like it said in the code.. I'm still assuming on every platform the directory entries look the same.. if not.. than my filtering out the share name is obsolete anyhow ; -)

General note, It'd be good if there were comments explaining why each of these checks are needed.

  • The check for if (STREQLEN(mnt->mnt_fsname, "/dev/loop", 9)) has no reference to snap packages for eg (even though this might be a more general convention it was added because of snap packages).
  • The STRPREFIX macro is more readable, would prefer this be used where possible (don't need to count characters to read the code).

As noted by Campbell, using STRPREFIX makes more sense.
Made the filtering a bit smarter/error proof.

Do you think the timeout will be a dealbreaker for implementing when the user has active gvfs shares AND a disconnected network?
Or does this fall under "expected and nothing we can do about"

This revision was not accepted when it landed; it landed in state Needs Review.Wed, Nov 7, 1:57 AM
This revision was automatically updated to reflect the committed changes.

Rebase on 2.8, applied minor cleanup to master/2.8

Campbell Barton (campbellbarton) requested changes to this revision.Wed, Nov 7, 4:16 AM

testing this patch am getting leaks. (just opening and closing blender)

This revision now requires changes to proceed.Wed, Nov 7, 4:16 AM
Roel Koster (kostex) updated this revision to Diff 12435.EditedWed, Nov 7, 11:49 AM

Added BLI_filelist_free. (should have been in there from the start.. seems I forgot to copy/paste that line before, and never noticed it)
It solves the leak.

To be sure I've built a debug version with ASAN.. and I'm getting no leaks anymore.
Also confirmed by Dalai (IRC)

Got to say.. for a newb it was not easy to get ASAN up and running. In the end it's simple.. like everything. learned a lot

source/blender/editors/space_file/fsmenu.c
633

Shouldn't this be checked for NULL?

Checking for NULL on XDG_RUNTIME_DIR
As if there are people not using systemd ;-)

I took the liberty not to change (foolproof) more of the code (check existance of the gvfs folder inside the runtime_dir..
But if that would be required, I'll surely hear from you again ;-)
Not really sure how deep/foolproof the checking should be.

Thanks,
Roel