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.

Sign up for GitHub

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

mount /sdcard/ #1801

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

mount /sdcard/ #1801

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
64f72c6
mount /sdcard/
lollilol May 6, 2024
ff1930e
Merge branch 'main' into patch-1
ferdnyc May 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/service/plugins/sftp.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ const SFTPPlugin = GObject.registerClass({

// This is the actual call to mount the device
const host = this.device.channel.host;
const uri = `sftp://${host}:${packet.body.port}/`;
const uri = `sftp://${host}:${packet.body.port}/sdcard/`;
Copy link
Member

Choose a reason for hiding this comment

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

If I'm right, then this should be...

Suggested change
const uri = `sftp://${host}:${packet.body.port}/sdcard/`;
const uri = `sftp://${host}:${packet.body.port}/${packet.body.path}/`;

Copy link
Contributor

Choose a reason for hiding this comment

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

This looks correct if we're happy to access only the Android device's internal storage.

However, if I plug a USB thumb-drive into my phone, I get:

"path": "/storage/emulated/0",
"multiPaths": [
  "/storage/emulated/0",
  "/mnt/media_rw/301B-13FB"
],
"pathNames": [
  "Internal storage",
  "Lexar USB drive"
]

We should probably mount each of these.

Copy link
Member

Choose a reason for hiding this comment

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

@mavit

Ugggh, that's really ugly. The worst part of it is, the packet.body.path is no longer even an ancestor of both (all) of the multiPaths listed. I was hoping at least browsing packet.body.path and seeing both of the mounts under it would be possible.

Given the data you've provided, I agree — we should stop trying to browse a single "device filesystem root" at all, and just separately mount each of the multiPaths entries, however meany there are.

In theory we could have UI to select which one to mount, but that already feels more complicated than necessary. If the user has multiple filesystems exported on the device, and chooses to mount it, they should just get all of them.

But it's even more clear now that we should never try to browse either sftp://${host}:${packet.body.port}/ or sftp://${host}:${packet.body.port}/${packet.body.path}/, since neither one represents the root of the remote filesystem. (There is, in fact, no path that does.)

Copy link
Member

Choose a reason for hiding this comment

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

The really sucky thing is that, if I'm interpreting this right, there isn't going to be anything to indicate which mount point is which. I don't think we can set volume labels on the remote mount points, and I'm not sure GVFS will pick them up from the KDEConnect side either.

I guess we can modify _addSymlink to create ${by_name_dir.get_path()}/${safe_device_name} as a directory, instead of as the symbolic link itself, and then create one symlink for each mount point inside that directory. Then at least we'll have a mapping of the remote mount points to their names.

const file = Gio.File.new_for_uri(uri);

await file.mount_enclosing_volume(GLib.PRIORITY_DEFAULT, op,
Expand Down