cockpit-project / cockpit-files

A Featureful File Browser for Cockpit (Modernized and tested version of https://github.com/45Drives/cockpit-navigator)
GNU Lesser General Public License v2.1
40 stars 25 forks source link

Support symlink creation #391

Open jelly opened 4 months ago

jelly commented 4 months ago

Our current implementation is an arbitrary symlink creation dialog

image

The target path can be edited to something else and is shown as an absolute path while the link name can't be an absolute path by only relative. (As the dialog prefills /home/admin).

This is confusing, because it made me think I should provide an absolute path in both inputs.

Symlink creation arbitrary creates a symlink under directory if directory is passed

image

Moving symlinks

Moving relative existing symlinks can break, do we show a warning?

jelly commented 4 months ago

Some more questions:

garrett commented 4 months ago

Here's a comparison of all the sorts of links:

The important part is intent. Absolute is for referring to files outside of a project that will not move, whereas relative is for within self-contained directory trees (such as projects, git checkouts, etc.), which could possibly be reparented (renaming or moving the toplevel container directory). They're both valid uses of symlinks, and depending on what an administrator is using, they may want one or the other.

I guess we should probably show them untruncated and relative (so someone could see the actual files and path fragments that matter) and perhaps have an option to make it an absolute link in the background.

I'm not sure if hardlinks really make sense. I think they probably do not.

Reflinks, we get for free with a standard copy when on supported filesystems.

garrett commented 4 months ago

Taken all of the above into consideration, it looks like I need to design a new link modal.

We could put that on pause on the dialog for now and make linking make a "Link to" prefixed link like in GNOME.

However, I guess that would imply that it's an absolute link or some relative one that "smartly" updates when moved (which means we'd need to look at what the link links to and update it in the new location). The absolute link would be much easier to implement, even if it's less useful in most cases.

From https://stackoverflow.com/questions/8523159/how-do-i-move-a-relative-symbolic-link#8523293:

You can turn relative paths into full paths using readlink -f foo. So you would do something like:

ln -s $(readlink -f $origlink) $newlink
rm $origlink

and:

I noticed that you wish to keep the paths relative. In this case, after you move the link, you can use symlinks -c to convert the absolute paths back into relative paths.

...But this requires a command from the symlinks package, at least on Fedora, so it's out.

Someone in the thread mentioned this ln command as an alternative, which converts an absolute symlink to a relative one (by overwriting the absolute link with a relative one):

ln -srf $newlink

I don't see a way to do the opposite (relative to absolute), however, aside from the code block above.

So, moving/renaming a relative link could be:

  1. convert to absolute link
  2. do the move/rename
  3. convert back to relative link

However, it's more complex than just moving an absolute link.

(I wonder if Nautilus handles this? Edit: Nope, it does not properly handle moving symlinks separate from moving origin files, apparently.)

jelly commented 4 months ago

In my opinion we should make a "create link" context menu item which makes a link for a directory or file "Link to X" which would be an absolute symlink.

If the link already exists, either we don't show the context menu or overwrite or append (n).

allisonkarlitskaya commented 4 months ago

I don't think we should have a UI option to create symlinks.

jelly commented 4 months ago

We concluded to drop symlink creation for now and we will re-visit this functionality later https://github.com/cockpit-project/cockpit-files/pull/419

allisonkarlitskaya commented 3 months ago

I don't think we should have a UI option to create symlinks.

I think I retract this opinion. Some of our users expect this feature.

garrett commented 3 months ago

I suppose we have three options:

  1. Create placeholder symlink with an autogenerated name
  2. Create placeholder symlink and instantly trigger renaming (either inline or in a dialog)
  3. Create interactively using a modal (which is similar to the create and rename, but wouldn't create until done and would allow for relative vs. absolute)

If we're doing # 1 (placeholder only), then someone will always have to manually rename and possibly even move the symlink, so we might as well do # 2 (placeholder and rename). But if we're doing # 2 with a rename in a dialog, then it might as well be # 3 instead.

If we can do renaming in-place with inline renaming (not a dialog), then # 2 is more of a viable option. (But we could still do # 3).

We had # 3 implemented in a way, but it could be done more simply:

Like the left one of these two (or the right if a toggle isn't clear enough). Both would default to absolute and would let you pick relative.

image

We might want to explain some non-obvious things with some helper text and/or (?) icon with a bubble.

jelly commented 3 months ago

@allisonkarlitskaya and I did some thinking about how we would make a symlink and usually you would think about it in the other direction. So you have a target and want to make a link to it in the current directory. For example symlink /etc/os-release to my $CWD ln -s ../../etc/os-release os-release or ln -s /etc/os-release os-release. ln makes this a relative or absolute symlink depending on the given path .. => relative / absolute.

So in a UI we could make this as following, the target field is our component which should only accept absolute paths (due to a bug it does also accept ../etc for example). The destination is filename and we should not allow ../ or /foo. the downside is that the dialog is more free form and not a right click action. The upside is that we can easily support a relative orabsolutetype radio button as this just passes a--relativetoln` depending on the selected radio button

image

garrett commented 3 months ago

The problem is that how you specify the paths doesn't necessarily matter for symlink creation. They're different. You can create a relative symlink by specifying absolute paths, for example.

ln -sr /etc etc

It's also weird to have to use a fiddly fileselector complete dropdown instead of the file manager we're building.

garrett commented 3 months ago

FWIW; here's a document on symlinks, which handles moving too: https://www.baeldung.com/linux/absolute-and-relative-symlinks#3-directly-create-symlink-at-destination

garrett commented 3 months ago

As per the call: the original one was bad, but because of the labeling and the symlink vs. hard link inclusion, not the autocomplete part.

We can have a similar dialog, but it would have a mandatory target with an autocomplete and optional destination. (Optional as it would reuse the same name as the target in the current directory if not specified.)

jelly commented 1 month ago

I would like to get to work on this again, and I am not sure if agree with what was last stated. What we want to support is:

Also lets only talk about target or original and link name, all other terms become very confusing fast.

There are multiple options on how we can implement this:

One thing we probably should never do is allow relative paths in any input (ie ../bots), we should defer to passing --relative with a full path:

ln --symbolic --relative /etc/os-release os-release
os-release -> ../usr/lib/os-release

Follow up

garrett commented 1 month ago

Creating a relative symlink (if anyone still uses those)

People use it; even the customer mentioned on the call.

The typical way of using it is symlinking something in the current directory or for when someone is working on a project. In most cases, they make much more sense, as they survive copying to other places (including backups), whereas absolute ones do not.

Relative symlinks can be converted to absolute and vice versa.

One thing we probably should never do is allow relative paths in any input (ie ../bots), we should defer to passing --relative with a full path:

If someone types in a relative path, then we could resolve it in the dialog. If ../bots exists, we could check that it exists while typing and convert it to the full path equivalent when focus changes.

Relative/Absolute would be an option below that.


I'd love to discuss this more at some point in time, but: We all have other things to work on (including landing open PRs and fixing follow up issues), and I need to focus on finishing up designs for both Cockpit Files and Anaconda, so can we please put this one on hold for a short while?