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

Fish doesn't have /usr/local/share/fish/completions in $fish_complete_path #5029

Closed
io12 opened this issue Jun 6, 2018 · 18 comments · Fixed by #6428 or #6508
Closed

Fish doesn't have /usr/local/share/fish/completions in $fish_complete_path #5029

io12 opened this issue Jun 6, 2018 · 18 comments · Fixed by #6428 or #6508
Assignees
Milestone

Comments

@io12
Copy link

io12 commented Jun 6, 2018

$ fish --version
fish, version 2.7.1
$ uname -a
Linux thinkpad 4.14.47-1-lts #1 SMP Wed May 30 21:09:23 UTC 2018 x86_64 GNU/Linux
$ echo $fish_complete_path
/home/blevy/.config/fish/completions /etc/fish/completions /usr/share/fish/vendor_completions.d /usr/share/fish/completions /home/blevy/.local/share/fish/generated_completions

I downloaded fish through the default arch repos.

@faho faho added the question label Jun 6, 2018
@faho
Copy link
Member

faho commented Jun 6, 2018

The /usr/local/ bit is the "PREFIX" that is chosen when compiling.

In your case, it is just "/usr". There is no need for the "local/" subdirectory because it's tracked by the package manager anyway.

@io12
Copy link
Author

io12 commented Jun 6, 2018

But let's say I, without using the system package manager, install a program that includes a fish completion script. It would need to install to /usr/local to avoid conflicts with the package manager. Even though fish was installed to /usr, completion scripts might not be able to be installed there safely. Bash and zsh both include this behavior.

@faho
Copy link
Member

faho commented Jun 7, 2018

It would need to install to /usr/local to avoid conflicts with the package manager.

It would not. This is where that "vendor" stuff comes in.

If the package wants to install completion scripts, it should use /usr/share/fish/vendor_completions.d. This can either be hard-coded (again, by the packager for the distribution) or gotten via pkg-config fish --variable completionsdir). Functions and configuration snippets work similarly.

@io12
Copy link
Author

io12 commented Jun 7, 2018

The problem with that approach is while /usr is the PREFIX of fish, /usr/local is the PREFIX of the local package. pkg-config only works when fish is installed, which isn't necessarily true, and hardcoding /usr might not work (because fish wasn't necessarily compiled with that PREFIX). Packages really shouldn't need to install files outside their PREFIX. I think the best solution to something like this would be for $fish_complete_path to be:

~/.config/fish/completions

/etc/fish/completions

$PREFIX/share/fish/completions (if $PREFIX is not /usr or /usr/local)

/usr/share/fish/vendor_completions.d

/usr/share/fish/completions

/usr/local/share/fish/vendor_completions.d

/usr/local/share/fish/completions

~/.local/share/fish/generated_completions

@faho
Copy link
Member

faho commented Jun 7, 2018

pkg-config only works when fish is installed

Sure. And if it's not, you won't want to install completions either. So then you ask the user to put it somewhere in $fish_complete_path.

and hardcoding /usr might not work

When I say "hardcoding" here, I was thinking of "hard-coded in the package build script" - by setting it as a compile-time variable or cping files there.

I'm sorry, but unless someone can come up with a compelling usecase (and installing half your system and not installing the other isn't one), I'm inclined to reject this as unnecessary complication.

@io12
Copy link
Author

io12 commented Jun 7, 2018

Here's my usecase:

I have an AUR package run-git that installs https://gitlab.com/blevy/run. The Makefile has a make install that uses DESTDIR and PREFIX, but the default for PREFIX is /usr/local. The reason I can't check if fish is installed, then use pkg-config in the Makefile is because the user should be able to install the AUR package, then install fish, and automatically get completions without having to reinstall the AUR package. The best solution in my case would be to hardcode /usr/share/fish/vendor_completions.d in the Makefile, but fish wasn't necessarily installed with /usr as its PATH.

@zanchey
Copy link
Member

zanchey commented Jul 2, 2018

Bash and zsh both include this behavior.

I don't think this is the case. bash-completion only loads scripts from its own directory, plus from ~/.bash_completion. zsh is the same, by default, but true to its versatile nature this can be controlled by the build process. Debian's builds add multiple additional fpaths, including /usr/local/share/zsh/site-functions - interestingly, Arch's builds don't use the same flags and I'd be interested to see what the output of echo $fpath under zsh on an Arch system is.

The whole idea of the vendor directories is that they can (should?) be set outside of the prefix - Homebrew does this, for example. If fish is installed into a prefix by a package manager, then the package manager assumes it has complete knowledge of all files in the prefix, and can install extra completions into fish's directories - so distribution-compiled packages can use $PREFIX/share/fish/completions with relative impunity. However, non-distribution files should use the vendor completions directory, which should be set appropriately by the distribution build system.

I've only really thought closely about this recently, and I'm tempted to change the default vendor functions/completions/snippets path to a /usr/local directory (which would fix your issue), but other comments are welcome.

@faho
Copy link
Member

faho commented Sep 30, 2018

I'd be interested to see what the output of echo $fpath under zsh on an Arch system is.

@zanchey:

$ printf '%s\n' $fpath
/usr/local/share/zsh/site-functions
/usr/share/zsh/site-functions
/usr/share/zsh/functions/Calendar
/usr/share/zsh/functions/Chpwd
/usr/share/zsh/functions/Completion
/usr/share/zsh/functions/Completion/Base
/usr/share/zsh/functions/Completion/Linux
/usr/share/zsh/functions/Completion/Unix
/usr/share/zsh/functions/Completion/X
/usr/share/zsh/functions/Completion/Zsh
/usr/share/zsh/functions/Exceptions
/usr/share/zsh/functions/Math
/usr/share/zsh/functions/MIME
/usr/share/zsh/functions/Misc
/usr/share/zsh/functions/Newuser
/usr/share/zsh/functions/Prompts
/usr/share/zsh/functions/TCP
/usr/share/zsh/functions/VCS_Info
/usr/share/zsh/functions/VCS_Info/Backends
/usr/share/zsh/functions/Zftp
/usr/share/zsh/functions/Zle

@faho
Copy link
Member

faho commented Apr 26, 2019

If fish is installed into a prefix by a package manager, then the package manager assumes it has complete knowledge of all files in the prefix, and can install extra completions into fish's directories - so distribution-compiled packages can use $PREFIX/share/fish/completions with relative impunity.

#5822 shows that this isn't quite true - it installs ripgrep's completions there, but then we added them as well, so now the package needs to be adjusted.

Though in that case they could have been using vendor_completions.d.

So... how about just adding /usr/local/share/fish/vendor* always as well as the prefixed version?

@io12
Copy link
Author

io12 commented Apr 26, 2019

Sounds like a good solution

@zanchey
Copy link
Member

zanchey commented Sep 3, 2019

OK, having thought about this today, there needs to be directories for the following things:

  • Completions installed by the package manager (currently $PREFIX/share/fish/completions or otherwise controlled by DATADIR)
  • Completions installed by other software ($__extra_completionsdir, which will be $PREFIX/share/fish/vendor_completions.d by default)
  • Custom system-wide completions ($PREFIX/etc/fish/completions or otherwise controlled by SYSCONFDIR)
  • User completions (~/.config/fish/completions/)

My feeling is that $__extra_completionsdir should default to /usr/local rather than $PREFIX. This would allow software which is installed locally to drop files into that directory. fish installed from source into /usr/local would still work exactly the same as it does today.

If fish is installed into another prefix by a package manager (say /usr), then managing conflicting files is the job of the package manager. It already has the most information about what packages install what files, and should have methods of managing these conflicts - either by patching the package to not install conflicting files, or something like dpkg's diversions (as usual Debian has had this solved since 1995).

Some alternatives include:

  • Adding /usr/local/share/fish/completions to the complete path unconditionally - this would mean a path that cannot be controlled by configuration, either build-time or load-time (at least initially)
  • Encouraging locally-installed software (eg make install) to install completions in /etc/fish/completions - this a reasonable thing to do but will make many systems administrators Extremely Nervous
  • Adding a third level of directories - something like $PREFIX/share/fish/completions.d as well as __extra_completionsdir, to allow for a split between fish-supplied/package manager-shipped completions and externally-supplied/package manager-shipped completions - presumably the pkg-config tools would still point to __extra_completionsdir? I feel like this complicates things too much

(All the above applies to functions and configuration snippets as well, of course.)

@Darkshadow2

This comment has been minimized.

@zanchey

This comment has been minimized.

@sniperrifle2004
Copy link

sniperrifle2004 commented Nov 26, 2019

Maybe I'm missing something, but couldn't $__extra_*dirs be determined based on XDG_DATA_DIRS? ie. simply tell vendors that if they drop their completions into $XDG_DATA_DIRS/fish/vendor_completions fish will pick them up.

@krobelus
Copy link
Member

Yeah I think the vendor dirs should be independent of $PREFIX.
And it should be possible to override the defaults of /usr/local/fish/vendor_{completions,functions,conf}.d.
Then we have two directories, one managed by the package manager in $PREFIX, and the other one for additional local installations.

Sanity check: does fish actually need to create those (empty) vendor directories?
I guess yes because an installation script might drop a file in /usr/local/share/fish/vendor_completions.d without creating the directory.
OTOH, zsh on Arch has an $fpath that starts with

/usr/local/share/zsh/site-functions # Not installed by the zsh package
/usr/share/zsh/site-functions       # Owned by the zsh package

It might makes sense to do the same - only create the directories below $PREFIX, and include both in the $fish_{complete,function}_path by default, as suggested in #5029 (comment). That should resolve #5719.

@krobelus
Copy link
Member

krobelus commented Dec 12, 2019

Maybe I'm missing something, but couldn't $__extra_*dirs be determined based on XDG_DATA_DIRS? ie. simply tell vendors that if they drop their completions into $XDG_DATA_DIRS/fish/vendor_completions fish will pick them up.

Yeah that could work - reading $XDG_DATA_DIRS at runtime.
For systems that don't define it we probably want to fall back to the canonical /usr/local/share:/usr/share default, should this be configurable as well?

$XDG_DATA_DIRS includes directories installed by flatpak for example.

krobelus added a commit to krobelus/fish-shell that referenced this issue Dec 13, 2019
Vendor configuration provided by third party packages is sourced first
from $EXTRA_VENDOR_PATH/vendor_* and $PREFIX/share/fish/vendor_*
instead of only the latter.  The default for $EXTRA_VENDOR_PATH is
/usr/local/share/fish:/usr/share/fish, which enables fish to source
third party configuration from packages installed there, even if fish is
installed with a different prefix. The directories in $EXTRA_VENDOR_PATH
will not be created by (installing) fish.

In particular, with this change
/usr/bin/fish also uses /usr/local/share/fish/vendor_completions.d, and
/usr/local/bin/fish also uses /usr/share/fish/vendor_completions.d

Fixes fish-shell#5029
krobelus added a commit to krobelus/fish-shell that referenced this issue Dec 17, 2019
Vendor configuration provided by third party packages is sourced first
from $EXTRA_VENDOR_PATH/vendor_* and $PREFIX/share/fish/vendor_*
instead of only the latter.  The default for $EXTRA_VENDOR_PATH is
/usr/local/share/fish:/usr/share/fish, which enables fish to source
third party configuration from packages installed there, even if fish is
installed with a different prefix. The directories in $EXTRA_VENDOR_PATH
will not be created by (installing) fish.

In particular, with this change
/usr/bin/fish also uses /usr/local/share/fish/vendor_completions.d, and
/usr/local/bin/fish also uses /usr/share/fish/vendor_completions.d

Fixes fish-shell#5029
@zanchey zanchey removed the RFC label Jan 16, 2020
@zanchey
Copy link
Member

zanchey commented Jan 16, 2020

This isn't actually fixed by what got merged in #6428.

@zanchey zanchey reopened this Jan 16, 2020
@zanchey
Copy link
Member

zanchey commented Jan 16, 2020

What we've ended up with is the machinery to support "adding a third level" as referenced in #5029 (comment). Looking at what zsh has in Arch and Debian I actually think this is reasonable, but it is proving very difficult for me to explain in the documentation! In particular the use of pkg-config is now totally situational (is this software being built for package manager installation or not), and it seems difficult to give sensible guidance.

Although the machinery is there, by default /usr/local is not currently added to the search path.

I'll work on a PR over the next 24 hours.

@zanchey zanchey added this to the fish 3.1.0 milestone Jan 18, 2020
zanchey added a commit to zanchey/fish-shell that referenced this issue Jan 18, 2020
zanchey added a commit to zanchey/fish-shell that referenced this issue Jan 20, 2020
zanchey added a commit to zanchey/fish-shell that referenced this issue Jan 25, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.