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.

[thernstig]

@jorgebucaran Did you get the stamina to continue on this?

[jorgebucaran]

Hey! Good question! :)

I'm just a bit worried about adding more debt, but I'm not against adding this feature either.

How many people (do you think) want or need this divided by other Fisher users?

[thernstig]

@jorgebucaran that is a very good question. I cannot say. Anything would be anecdotal. I do think using dotfiles is becoming more common, so in that sense the number of users that would want this will probably increase as time goes. But that is still a guess.

[talal]

@jorgebucaran I have a minor request.

I manage my fish config across different machines by modifying the universal variables. I do not sync the fish_variables instead I use the following script:

#!/usr/bin/env fish

if test -e "$HOME/.config/fish/fish_variables"
    rm "$HOME/.config/fish/fish_variables"
    fish -c echo "" # This will autogenerate the default fish_variables file.
end

set_universal_settings # this is a function that holds my universal settings

This works quite nicely, since I don't change things often. The problem is that when I run this, it deletes and recreates the fish_variables file and consequently all the _fisher_ entries get deleted.
If I run fisher update, it complains that there are conflicting files and I have to go and remove them first. Another small inconvenience is that fisher update ends up removing the plugins with orphaned files from fish_plugins file, which I then have to update.

This could be resolved by having a force/overwrite flag (-f) so that when we use fisher update -f, it always downloads and updates the plugin files without complaining about conflicts.

I know that you don't want to add more debt but does this sound like a reasonable feature to you?

[jorgebucaran]

I made this comment on a PR somewhere else, but can't find it now. 😅

Basically, I'd prefer to prevent you from getting into this situation instead. If that's really not possible, then we could look into adding a --force flag.

[talal]

@jorgebucaran do you have something in mind that would help me to prevent this situation?

[jorgebucaran]

Actually, implementing this issue (#611) would prevent you from getting into the wanting a way to force install situation, no?

[talal]

Yeah 🙂

[adamcstephens]

I've been committing my fish_variables into my dotfiles but am trying to move away from that. Paths are different across my machines and it causes too much churn in my dotfiles.

That brought me here because I'm experiencing the same issue as the others. Either a force flag, or better logic that will re-bootstrap plugins would be helpful for me.

I'd be happy to help test any changes if needed.

[andreiborisov]

fish documentation encourages users to use universal variables for the configuration and the easiest way to transfer those across machines is to commit fish_variables file.

Now, all this should be addressed on the fish shell level (they're planning to remove universal variables altogether fish-shell/fish-shell#7317 (comment) actually), but keeping in mind that more and more work is being done in virtual environments like Codespaces recently, I think this issues definitely has its merits, especially after VS Code added dotfiles sync support right into the editor.

[jorgebucaran]

4.4.0 is out and this is in. Now it should be easier to sync dotfiles across machines that have different $HOME locations.

A script runs once after updating Fisher that migrates your existing variables to the new architecture.

fisher/functions/fisher.fish

Lines 234 to 237 in 357d682

	for var in (set --names | string match --entire --regex '^_fisher_.+_files$')
	set $var (string replace -- ~ \~ $$var)
	end
	functions --erase _fisher_fish_postexec

Please do let me know if you find any bugs or have suggestions. 🙏

[thernstig]

Great news @jorgebucaran! Now if only fish nvm and the universal variable nvm_data could do the same 😄

[jorgebucaran]

That's a discussion for nvm.fish. 😁