Fisher: Fisher should stop using universal variables for plugin state

[lilyball]

Fisher 4.1.0 uses universal variables to maintain both the list of what plugins are installed, and what paths are installed for each plugin. This is potentially problematic in the scenario where I'm syncing my fish config between two machines, as having the universal variables get out of date may cause fisher to orphan installed files. There are two main scenarios here:

1. If I sync my fish config including fish_variables file. The fish documentation says not to modify the fish_variables file externally but to always use fish commands to do so. Experimentally, if I modify my fish_variables and then query variables from another pre-existing shell, the shell won't pick up on the variables until I've shown a new prompt first. This means if I sync my config and then immediately run fisher update, it will be using the old values of the universal variables, meaning if the list of files installed by plugins changed as part of the sync, fisher will then potentially orphan files.

2. If I sync my fish config but omit the fish_variables file (which represents what I actually do on my own machines), syncing changes to the list of installed plugin files will allow fisher to orphan files. Basically the same scenario as the first one except I don't have to run fisher update as the very first action after the sync, I can run it arbitrarily later instead.

Instead of using universal variables, fisher should put a file on disk in $fisher_path that describes what's currently installed. It's still possible for me to sync ignoring that file, but that's really my fault. Or it could even be clever and make the file something like conf.d/_fisher_state.fish which would set global variables, and fisher could re-source that file upon invocation (to ensure it picks up on any synced changes). Since any syncing I'm doing is presumably syncing that dir, this would ensure I don't skip the file by virtue of not allowlisting its path (which is to say, my original sync setup actually explicitly listed which paths inside ~/.config/fish to sync instead of listing which files to ignore).

[jorgebucaran]

@lilyball 2. [...] syncing changes to the list of installed plugin files will allow fisher to orphan files.

No, it will not, unless you modify Fisher's state in $_fisher_plugins _or_ $_fisher_PLUGIN_NAME_files.

@lilyball This is potentially problematic in the scenario where I'm syncing my fish config

As long as you don't modify Fisher's state as described above, Fisher won't orphan files. The only caveat I can think of is if you were syncing UVARS. But if you omit _fisher_* entries when modifying fish_variables, then it should be fine:

string match --entire --all --invert --regex _fisher_ 

An improvement I can think of would be using $fisher_data/plugins, $fisher_data/PLUGIN_NAME_files, instead of UVARS.

[jorgebucaran]

Should one even attempt to sync universal variables? 🤔

From the Fish docs:

Universal variables are stored in the file .config/fish/fish_variables. Do not edit this file directly, as your edits may be overwritten. Edit the variables through fish scripts or by using fish interactively instead.

We could interpret that as "don't sync uvars", as it would require replacing fish_variables with your own.

[thernstig]

I also now noticed that fisher 4.1 sets universal variables such as SETUVAR _fisher_jorgebucaran_2F_fisher_files:/home/myuser/\x2econfig/fish/functions/fisher\x2efish\x1e/home/myuser/\x2econfig/fish/completions/fisher\x2efish

This suddenly broke my machine syncs, and most likely many others. Notice how it contains the full path to my home directory which varies through machines. Anyone using dotfiles and syncs the fish config ought to have this problem now.

Syncing universial variables is not uncommon since a lot of config in there is for various, installed fisher plugins. Examples:

SETUVAR FZF_CD_COMMAND:fd\x20\x2d\x2dtype\x20d\x20\x2d\x2dno\x2dignore\x2dvcs\x20\x2d\x2dexclude\x20\x2egit
SETUVAR FZF_CD_WITH_HIDDEN_COMMAND:fd\x20\x2d\x2dtype\x20d\x20\x2d\x2dno\x2dignore\x2dvcs\x20\x2d\x2dexclude\x20\x2egit\x20\x2d\x2dhidden
SETUVAR --export FZF_DEFAULT_COMMAND:fd\x20\x2d\x2dtype\x20f\x20\x2d\x2dhidden\x20\x2d\x2dno\x2dignore\x2dvcs\x20\x2d\x2dexclude\x20\x2egit
SETUVAR --export FZF_DEFAULT_OPTS:\x2d\x2dreverse\x20\x2d\x2dheight\x2020\x25\x20\x2d\x2dinline\x2dinfo
...
SETUVAR fish_color_status:red
SETUVAR fish_color_user:brgreen

All these are things I do not wish to setup on every machine, that is the reason why many use dotfiles to sync.

[jorgebucaran]

@thernstig Well spotted! We need to store each file's relative path without the prefix and append $fisher_path whenever we use $_fisher_PLUGIN_NAME_files to access the files. Do you want to give this a shot and send me a PR?

[thernstig]

@jorgebucaran I should be the good citizen and do this, but I have no time currently, as I have too many other obligations. I apologize for this beforehand as I feel I should help out since I am using your great tools.

[jorgebucaran]

I'm finally free to work on this. ✊

The plan is to:

• Store only relative paths without $fisher_path
• Use $fisher_path/$_fisher_FOOBAR_files whenever we access plugin files

Looks easy to do, but the implementation needs to take care of the migration smoothly for users, which could be tricky.

Fisher state example:

_fisher__2F_Users_2F_jb_2F_fisher_files:

• /Users/jb/.config/fish/functions/fisher.fish
• /Users/jb/.config/fish/completions/fisher.fish

_fisher__2F_Users_2F_jb_2F_nvm_2E_fish_files:

• /Users/jb/.config/fish/functions/_nvm_index_update.fish
• /Users/jb/.config/fish/functions/_nvm_list.fish
• /Users/jb/.config/fish/functions/_nvm_version_activate.fish
• /Users/jb/.config/fish/functions/_nvm_version_deactivate.fish
• /Users/jb/.config/fish/functions/nvm.fish
• /Users/jb/.config/fish/conf.d/nvm.fish
• /Users/jb/.config/fish/completions/nvm.fish

_fisher_jorgebucaran_2F_autopair_2E_fish_files:

• /Users/jb/.config/fish/functions/_autopair_backspace.fish
• /Users/jb/.config/fish/functions/_autopair_insert_left.fish
• /Users/jb/.config/fish/functions/_autopair_insert_right.fish
• /Users/jb/.config/fish/functions/_autopair_insert_same.fish
• /Users/jb/.config/fish/functions/_autopair_tab.fish
• /Users/jb/.config/fish/conf.d/autopair.fish

_fisher_plugins:

• /Users/jb/fisher
• jorgebucaran/autopair.fish
• /Users/jb/nvm.fish

File variables are the easiest to patch (and the bulk of this issue), but there's not much we can do about local plugins, e.g., even if we store the relative path, variables tracking local plugins like:

• _fisher__2F_Users_2F_jb_2F_nvm_2E_fish_files

cannot be changed. I thought we could remove the expanded ~, but that won't cut it if Machine A has, say, a local plugin X in ~/foo/x and Machine B in ~/bar/x, thus I'm leaving local plugins out of the scope of this patch at this time.

[jorgebucaran]

@thernstig Do you happen to be using a custom $fisher_path? Just curious.

[thernstig]

No, no custom fisher_path path.

[jorgebucaran]

Store relative path

There's an annoying issue where changing $fisher_path can orphan files / corrupt Fisher's state. I can't find a clean way to address this yet.

If you decide to change your $fisher_path, you'll need to uninstall everything first, set the var, and install everything again.

$ fisher list >plugins
$ fisher remove <plugins
$ set --universal fisher_path /path/to/location
$ curl -L git.io/fisher | source && fisher install<plugins && rm plugins

Store full path, don't expand ~

Maybe we could still hardcode the path, but not expand ~ instead.

[kidonng]

I would suggest the former option, as syncing dotfiles through machines which may have different home location is more common than changing fisher_path.

P. S. Not related to this issue though, I think storing the full path of local plugins also causes inconvenience for syncing dotfiles. I prefer previous Fisher versions where ~ doesn't get expanded.

[kidonng]

@jorgebucaran Sorry I didn't make myself clear, I mean fish_plugins:

I better open a new issue about this...

[kidonng]

@kidonng Do you mean the latter, though, right?

Just realized I haven't answered this. Both options cover my concern (use relative path for home), so I don't have strong opinions about either.

[thernstig]

@jorgebucaran If "store the entire path, don't expand ~" solves the case where users changes fisher_path, I do believe that is the least bad one.

[jorgebucaran]

As you all know, I have a working solution in a branch, but I'm still not 100% sure if I want to add this. I'm definitely not closing yet. I feel that there's something amiss about storing ~ unexpanded. I need more time to think about it and feedback from everyone. 🙏