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.

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

Fisher initial install command overwrites fish_plugins #705

Closed
winghouchan opened this issue Feb 9, 2022 · 13 comments
Closed

Fisher initial install command overwrites fish_plugins #705

winghouchan opened this issue Feb 9, 2022 · 13 comments

Comments

@winghouchan
Copy link

I have a fish_plugins file committed to my dotfiles so that I can quickly install Fish plugins on new machines. My config.fish also detects the presence of the fisher command and installs Fisher if it's not present. The current installation instructions specifies the following command to run:

curl -sL https://git.io/fisher | source && fisher install jorgebucaran/fisher

The issue is the above command seems to overwrite whatever is in the fish_plugins file which means any other plugins in fish_plugins get removed. I'm working round this by doing the following instead:

curl -sL https://git.io/fisher | source && fisher update

Could the README be updated with the above recipe for installing Fisher if fish_plugins exists. Or could the fisher function be updated to not have this behaviour?

@winghouchan
Copy link
Author

I took a look at fisher.fish and I think this is happening because:

  1. $_fisher_plugins is initially empty while the fish_plugins file is not.
  2. The installed plugin gets appended to $_fisher_plugins:
    contains -- $plugin $_fisher_plugins || set --universal --append _fisher_plugins $plugin
  3. $_fisher_plugins then gets written to fish_plugins file:
    printf "%s\n" $_fisher_plugins >$fish_plugins ||

winghouchan added a commit to winghouchan/dotfiles that referenced this issue Feb 9, 2022
Running `fisher install jorgebucaran/fisher` as per the instructions (https://github.com/jorgebucaran/fisher#installation) overwrites the plugin file, see jorgebucaran/fisher/issues/705.
@winghouchan
Copy link
Author

Hmm, #611 is kinda related to this. Syncing my fish_variables will ensure $_fisher_plugins is populated on initial install however syncing fish_variables isn't possible because it contains absolute paths which may break on different machines. If #611 is resolved in a way that allows fish_variables to be synced then this issue will be resolved too.

@jorgebucaran
Copy link
Owner

fisher install foo/bar

will (attempt to) install foo/bar and remove any plugins in your fish_plugins that are not already installed. This happens whether you are bootstrapping Fisher or not. I made it that way so that fish_plugins would always reflect the current state.

Try adding a bunch of words to fish_plugins, then fisher install jorgebucaran/foobar. Open your fish_plugins.

Anyhow, it seems that by using fisher update you were able to work around the issue.

Could the README be updated with the above recipe for installing Fisher if fish_plugins exists. Or could the fisher function be updated to not have this behaviour?

You are welcome to send me a PR adding the text you think is missing from the README, but I'm not too keen on changing how Fisher works here at this time.

@winghouchan
Copy link
Author

I made it that way so that fish_plugins would always reflect the current state.

I see. I was making the assumption that fish_plugins would act like a plugin manifest and could be used to install plugins that weren't already installed given the following is in the README:

fisher/README.md

Lines 89 to 91 in 0a0c489

## Using your `fish_plugins` file
Whenever you install or remove a plugin from the command line, Fisher will write down all the installed plugins plugins to `$__fish_config_dir/fish_plugins`. Adding this file to your dotfiles or version control is the easiest way to share your configuration across different systems.

And also making the assumption that fisher install would behave like the install commands of other package managers which don't modify the manifest at all.

But thanks for clarifying expectations here.

The README does mention the following:

fisher/README.md

Lines 99 to 111 in 0a0c489

```diff
jorgebucaran/fisher
ilancosman/tide
jorgebucaran/[email protected]
+ PatrickF1/fzf.fish
- /home/jb/path/to/plugin
```
```console
fisher update
```
That will install **PatrickF1**/**fzf.fish**, remove /**home**/**jb**/**path**/**to**/**plugin**, and update everything else.

This is what prompted me to try fisher update instead of the command specified in the installation instructions.

If the README was to be updated, I'd do something like this:

## Installation

`` `console
curl -sL https://git.io/fisher | source && fisher install jorgebucaran/fisher
`` `

+ Installing a previous configuration with plugins? Use `curl -sL https://git.io/fisher | source && fisher update`.

What do you think? I'll open a PR if you think it's good.

@jorgebucaran
Copy link
Owner

...would act like a plugin manifest and could be used to install plugins that weren't already installed

Without arguments fisher update installs plugins not already installed, removes (already installed) plugins that are not present in fish_plugins, and updates everything else. Does this not act as a plugin manifest?

What do you think?

I'll definitely think about it. 😉

@winghouchan
Copy link
Author

Well, not entirely since the typical expectation is a package manager's install command installs things on the manifest that aren't installed instead of treating the contents as a representation of the state of the system – which is more like a lockfile.

Ultimately, I've found a workaround, but it would be great for others to be able to find the workaround more easily hence the suggested addition in the README. Or for the behaviour of the install command to behave more intuitively. I'll leave it with you 🙂

@jorgebucaran
Copy link
Owner

I see! Perhaps we should discuss changing fisher install's default behavior without arguments. Right now it doesn't do anything. Instead, we could go ahead and install new plugins in fish_plugins and perhaps re-add removed entries of already installed plugins. I'm not sure about that last one.

@winghouchan
Copy link
Author

Perhaps we should discuss changing fisher install's default behavior without arguments. Right now it doesn't do anything. Instead, we could go ahead and install new plugins in fish_plugins [...].

I think this is a sensible middle ground.

[...] and perhaps re-add removed entries of already installed plugins. I'm not sure about that last one.

I'm not sure either. Ultimately, I think it's a question of what is the source of truth for what packages should be installed. My view is, if fisher_plugins is supposed to be a package manifest, then it should be the source of truth for what packages are installed. This means:

  • Items on the manifest are not removed by fisher install.
  • Not installed packages that appear on the manifest are installed when running fisher install.
  • Installed packages that don't appear on the manifest are uninstalled when running fisher install.

@LinuxIsCool
Copy link

Whether I try fisher update or fisher install both commands are removing lines from fish_plugins in very unexpected ways. This makes it impossible for me to user fisher as a version controlled fish plugin manager and I will be seeking alternative options.

@LinuxIsCool
Copy link

This kind of works:
curl -sL https://git.io/fisher | source && fisher install (cat ~/.config/fish/fish_plugins)

But if any of the plugins fail to install then fisher removes them from the fish_plugins file which is very annoying.

@jorgebucaran
Copy link
Owner

Fisher's install command could add or remove lines from fish_plugins if you manually added or removed entries and then run fisher install. You probably meant to use fisher update instead, which works as I described here.

Perhaps we shouldn't let you run fisher install after you've modified the file, as that is probably a mistake anyway.

If the documentation is not clear about this, I'd love to fix it.

@LinuxIsCool Can you show one or more examples of either command removing lines from fish_plugins in unexpected ways?

@LinuxIsCool
Copy link

Thank you for the reply. fisher update was not working for me as you described here.

I got it working with the line of code I posted above. I don't have time right now, but if I get a chance, I'll see if I can re-create the situation I was in and post examples of what was happening when I tried fisher update.

@jorgebucaran
Copy link
Owner

Btw, you could fisher install < ~/.config/fish/fish_plugins as well. Sorry, I just have low tolerance for useless use of cat. 😂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants