Plugin installation can clobber existing files

[mattgreen]

Hi,
I had a user file an issue where they installed a package I wrote and then didn't realize it'd clobber their existing fish_prompt.fish. It'd be nice if Fisher warned users that it was about to overwrite an existing file that was outside the current package. (Though I realize the last part of that is non-trivial.)

[jorgebucaran]

We could copy any existing prompt files somewhere, and restore them after you uninstall the prompt. We already attempt to restore fish_prompt with the default Fish prompt here:

fisher/fisher.fish

Line 165 in 4ff6128

functions -q fish_prompt || source $__fish_data_dir/functions/fish_prompt.fish

[jorgebucaran]

This is a non-trivial issue. Asking the user Overwrite "$file"? isn't great, as it would effectively only half-install the plugin if the user doesn't want to overwrite. We may also need to introduce a --yes or --overwrite flag, since people may be using Fisher in automated scripts.

Here are some of the options we have, all with their own cons and pros:

1. Do nothing. Users should be careful not to install plugins that could potentially overwrite other plugins. The most typical example of this is installing a prompt. What should we do about: fisher install a/foobar b/foobar. Now, prompt authors could provide a custom *_install event and deal with this themselves, but that's not ideal either.

2. Abort installing plugins that would overwrite files (that Fisher isn't already tracking) in $fisher_path. That means a/foobar gets installed above, but b/foobar isn't. It also means, neither gets installed if the user already had a foobar.fish in their $fisher_path. Print a good error message.

3. Same as (2), but instead of aborting, handle it somehow. For example, copy all would-be-overwritten files somewhere so we can restore them if the user fisher remove PLUGIN.

4. Any ideas?

[kopischke]

Having run into that exact issue recently (I forgot fish prompts are just a function and overwrote my old prompt via fisher; I’m happy with the new one, so no harm done, but that is not necessarily the case for everybody in my situation), I’d be in favour of solution two.

Solution one means mistakes may delete user data that might or might not be easily restored (if it is another plugin, easy enough; if you had customisations, hrrrm … how’s your backup strategy?). Data loss on a lapse of awareness is what I tend to consider the worst case scenario for any application.

Solution three means tracking a lot of state that can change outside fisher installs. That would be, I think, what you call a “can of worms” and rather contrary to fisher’s stated aim of being lightweight.

A good warning message à la “fisher cannot install repo/plugin because this would overwrite your path/to/file. Check if you still need that file and retry with --force if you don’t.” would avoid accidental data loss while leaving enough room for user intervention – including the preemptive use of --force (or whatever the flag would be called).

[IlanCosman]

Scuba adopted a similar approach. Scuba used solution number three, and it's the one I would advocate for. In Scuba, the installation would warn you if you were about to clobber something, list the conflicting plugins (though that really should have been files), and then ask if you want to continue. So essentially the same thing, except with a [y/N] instead of having the user run the command again with --force.

[jorgebucaran]

Vast majority of people running into this got their fish_prompt.fish overwritten. So, while the issue isn't exclusive to prompts, it's by far the most common scenario. Could we get away with (3) but only for prompts? Otherwise, I tend to agree with @kopischke.

I'd be curious to hear your opinion as well: @PatrickF1 @franciscolourenco and @neersighted. Maybe there's another way we're not even considering.

[PatrickF1]

I'm of the opinion that Fisher just alerts the user with a message during installation that files are being clobbered and get on with it. Too much code to add/maintain/debug to do anything more but this gives the user at least a chance at debugging it themselves.

[joallard]

As a first step, I would be okay with aborting when a file that Fisher doesn't own would be overwritten. That sounds "good enough" to me.

Like with a message like this:

'.config/fish/functions/fish_prompt.fish' already exists and would be overwritten by this plugin. Aborting.

You may either remove the file, or move it to somewhere else safe. You may then re-run this command.

If I look at some prior art, I can think of brew link:

> brew link node
Warning: Already linked: /usr/local/Cellar/node/15.1.0
To relink:
  brew unlink node && brew link node

(brew link --force also exists, iirc)

I am also reminded of various generators or installer that prompt something like this:

File X already exists. What would you like to do? 

o: overwrite
k: keep
d: diff
s: shell
b: backup
q: quit

It's a pretty common operation so I'm not sure whether a library exists for this, but it's quite reasonable. If we want to simplify it, it could be only between Overwrite and Keep and abort.

[jorgebucaran]

@joallard I have two questions for you.

1. Why would you install Lucid if you already had a custom prompt?
2. What did you expect would happen?

I'm just looking to gain more insight into the situation and possibly come up with a fix quickly. Thanks!

[kopischke]

@joallard I have two questions for you.

1. Why would you install Lucid if you already had a custom prompt?
2. What did you expect would happen?

I'm just looking to gain more insight into the situation and possibly come up with a fix quickly. Thanks!

You weren’t addressing me, but seeing that was exactly what I did, I’ll jump in with an answer: I forgot for a moment how fish prompts work and thought I’d have two options between which I could switch. This kind of “ah, drat, how silly” situations is what I meant with “lapses in awareness”.

If users would be perfectly aware of what they did all the time, could execute their intent flawlessly all the time and had perfect oversight over all consequences at all times, they’d never be a need for any program safety measures. In reality, people do lose awareness of what they do at times, or cannot think all consequences through, or mistype, or, or or … and they should not lose data because of trivial errors. That’s why today’s *nixes won’t let you simply rm -Rf / – too easy to fat finger when you intended to do rm -Rf /#notused* and slipped a key too far to the right.**

* German keyboard layout user here. No need to discuss key proximity, t’was but an example.

** Yes, you’d need to be root. Just assume you did sudo su first.

[mattgreen]

Answering only for myself here.

1. Why would you install Lucid if you already had a custom prompt?

This presumes I remember that I have a custom prompt. Given that the current state of the world, I wouldn't expect users to remember all of the custom functions they have. I use fish because I can mostly forget about it. Generally, tools should put up guard rails before they behave destructively, and perform their function quietly otherwise.

2. What did you expect would happen?

I'd expect fisher to say, "installing lucid would overwrite functions/fish_prompt.fish; aborting. To overwrite existing files with those from lucid, please re-run fisher: fisher install --force url/to/lucid".

[faho]

(@jorgebucaran asked me to weigh in)

There are numerous approaches here:

• Just clobber it (the status quo), with a message
• Backup the old file
• Ask to backup/clobber?
• Separate the user's function directory from the plugin one via $fish_function_path
• Stop and tell the user to deal with it

Most of these are acceptable (if done well - you want to check before installing if files exist, so you don't get into a half-installed state).

I think just clobbering is probably the worst because, well, you're overwriting their work.

Using $fish_function_path is probably the most complicated but also has the most payoff - you basically get an "overlay" like effect, where people can remove a plugin and their old prompt automatically comes back.

Backing up sounds alright, the problem is how many backup files you keep - do you leave fish_prompt.bak15? Or does the user lose their prompt after the second prompt switch?

So I think just checking and asking or refusing to overwrite should be simple to implement yet effective and understandable. I wouldn't add diffing etc because... really, you're installing a different prompt, the diff is going to be pretty meaningless. If you want to give the user tools to deal with it, use a temp directory and tell them where to find it.

[jorgebucaran]

Backing up sounds alright, the problem is how many backup files you keep - do you leave fish_prompt.bak15? Or does the user lose their prompt after the second prompt switch?

We only need to save/back up the user's prompt once. Fisher-managed plugins/prompts are managed by... Fisher.

Of course we should restore the user's prompt after they uninstall a prompt they just installed with Fisher.

When creating the backup, we print a message to tell the user what we just did and that to restore their old prompt, they just need to uninstall the new prompt.

If we choose to abort instead, the user is responsible for backing up their old prompt, and attempting to install any new prompt again.

Which one is better?

[kopischke]

We only need to save/back up the user's prompt once. Fisher-managed plugins/prompts are managed by... Fisher.

That might be a bit too narrowly scoped a scenario. For instance, what happens when the user installs a second prompt via Fisher, losing the previously Fisher installed one … and that had been customised (don’t ask me what made me imagine that case, *cough)? How would restoring the fish default before the first Fisher installed prompt help? I think we need a to focus less on the prompt scenario and more on generic prevention of loss of configuration data.

[jorgebucaran]

Fisher tracks plugins/files, so the same scenario described above applies for any plugin that would clobber files other than fish_prompt.

For instance, what happens when the user installs a second prompt via Fisher, losing the previously Fisher installed one … and that had been customised (don’t ask me what made me imagine that case, *cough)?

Nice horror story. Trying to deal with users tampering with installed plugins is non-trivial and out of the scope of this feature. Don't do that.

I think we're down to abort and backup. I'll go ahead and start working on both features to see which one feels more fishy and snappy.

[kopischke]

Fisher tracks plugins/files, so the same scenario described above applies for any plugin that would clobber files other than fish_prompt.

My point exactly. Glad to see we agree on this.

Nice horror story. Trying to deal with users tampering with installed plugins is non-trivial and out of the scope of this feature. Don't do that.

Ibid.

I think we're down to abort and backup.

I think we should be down to the nicest way to abort, because the point of my horror story was that there is no way for Fisher to do backup reliably. “We saved some state that might or might not be relevant to you, so we went ahead and overwrote your files” feels … wrong.

EDIT: unless by “backup” you mean “backup the state before every install“, thus allowing immediate rollback. In that case, I retract my objection.

[jorgebucaran]

Remember that Fisher already keeps track of all your plugin files (#603). So, it's not hard to back up or prevent clobbering of user files as we already know which files are ours. Whether we decide to abort or back up, the implementation and implications are about the same, because we already decided to draw the line at "user files", i.e., not our files.

for plugin in $plugins_to_install
  set -l new_files $plugin/{functions,completions,conf.d}/*
  set -l our_files # ... all the files tracked by Fisher

  for file in $new_files
    if test -e $fisher_path/$file && not contains -- $file $our_files
      # To abort, or to back up, that is the question!
    end
  end
end

We can already reliably create backups, as well as prevent clobbering of user files. What we can't easily do (and probably won't do for now) is cope with users tampering with Fisher-tracked files.

And even if we decide to work on this later (Fisher 2077 Codename: Cyberpunk), we should start with user files, since we can detect those right now. My point is, either aborting or creating backups is not a challenge, the question is which approach is more convenient for everyone?

[kopischke]

I guess this is a question of workflow: stop the user before they clobber their files, or offer them an option to restore them when it happens (we are talking about backing up the actual file state, yes? Not just saying “you can reinstall from X”? Because that would not work out well in all cases…). I find the former the more pleasant UX, but the latter seems far more forgiving of user error.

Maybe a stupid suggestion, but: why not do both? I.e. ask if we may clobber stuff during install, and if the user confirms, squirrel the files away and notify the user they still can have second thoughts … As you said, Fisher knows about which files are concerned already, so there is little extra effort involved doing belt and braces, AFAICS.

[jorgebucaran]

As long as we can avoid losing your data, everyone wins. I'll write the code and see how it goes. 🙌

[joallard]

Some great points have been made, so I'll pitch in my own answer for myself, actual case: 😄

Why would you install Lucid if you already had a custom prompt?

I saw it on some page and thought it'd be cool to try out. I thought "Hey, maybe this works even better for me and needs less maintenance than my own improvised code, let's try it, what's the worse that could happen, it's just installing a plugin"

What did you expect would happen?

Install plugin, it hooks somewhere and replaces my prompt (the invocation, not the stored file).

My mental model was roughly:

set fish_prompt_file ~/.config/fish/functions/fish_prompt.fish

$ fisher install lucid

set fish_prompt_file ~/.../package/lucid/prompt.fish

$ fisher uninstall lucid

set fish_prompt_file (random file)  # as long as it didn't delete my file, I didn't care

That's not how it works, but that's how I expected it to. Again, if I was told simply "hey, move your file out of the way because of practical reasons", I would've found that totally reasonable.

[jorgebucaran]

Thanks for sharing your story. This is very much in line with the fix I'm working on for this issue. Installing Lucid should save your prompt somewhere. Uninstalling Lucid should restore it.

[kopischke]

Nice one; glad to see this resolved!

[jorgebucaran]

I've pushed the first implementation of this feature. I decided to go with what I felt most people here leaned towards or were the least against, i.e., aborting and telling the user to deal with conflicting files. I feel this concurs with Fisher's minimalism ethos: what's the most straightforward thing we can do here that isn't over-architected or tries to be too smart?

Here's what happens when you try to install a plugin with conflicting user files:

$ fisher install mattgreen/lucid.fish
fisher install version 4.2.0-rc-1
Fetching https://codeload.github.com/mattgreen/lucid.fish/tar.gz/HEAD
fisher: Cannot install "mattgreen/lucid.fish": please remove or move conflicting files first:
        /Users/jb/.config/fish/functions/fish_prompt.fish

You simply cannot install the plugin. You have to either move fish_prompt elsewhere, remove it or use fisher remove a/b if you believe the conflicting files belong to a plugin (which is often the case when you try to install a prompt and then another).

Notice that this should work for all files, not just for fish_prompt.fish.

[torhovland]

Great! It would be nice if the next step was implementing the --force flag :-)

[jorgebucaran]

How would that flag work?

[torhovland]

By just overwriting all the files. If that's what I want, it's a pain to have to delete them all manually first.

[jorgebucaran]

I'd prefer to look into why you are running into that situation first and try to prevent it from happening.

[torhovland]

Sure! What happened is that I keep my dotfiles in Git, and brought them over to a new machine. However, I didn't have .config/fish/fish_variables. So while I got all my fish plugins, I didn't get the info that they were installed, hence Fisher needed to install them again, on top of the existing files.

I understand this is an edge case, and my own fault, etc. But I do think there are other reasons why someone might run into a similar situation, and would like to force-overwrite the files.

[torhovland]

By the way, here's somebody else with the same experience:
https://www.reddit.com/r/fishshell/comments/msqw7y/comment/gvrthvd

[jorgebucaran]

@torhovland We may have an alternative solution to this problem, #611. Could you have a look too?

[torhovland]

I agree, we really need #611. Although, I don't see any harm in adding a --force flag as well, for those occasions when somebody runs into a similar situation, for whatever reason (e.g. having too few or too many files synced by Git).