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

Ripgrep completions are in conflict with completions provided by ripgrep itself #5822

Closed
ronjouch opened this issue Apr 17, 2019 · 28 comments
Closed

Comments

@ronjouch
Copy link

ronjouch commented Apr 17, 2019

Since 5989a92 by @mqudsi, building the fish-git AUR package succeeds, but installation fails with:

error: failed to commit transaction (conflicting files)
fish-git: /usr/share/fish/completions/rg.fish exists in filesystem (owned by ripgrep)

I'm filing this issue but I'm uncertain who's "right" and who should be providing these completions:

  • I'm guessing it should be ripgrep...
  • ... but fish providing the completions means users with improper ripgrep installations (for example users who just dropped the binary in $PATH) will still get completions.
  • If fish keeps shipping completions, a simple "fix" for the AUR package would be to add to package() a little rm "$pkgdir/usr/share/fish/completions/rg.fish"

Anyway, you already provide tons of completions, so I guess you already know what's the right thing to do here. So, feedback welcome, I'll pass it on to AUR packagers.

As usual, thanks a million for fish!

@SanskritFritz
Copy link
Contributor

The AUR package is my concern, the fish devs will not do anything with it.
Anyway, I support the wish for dropping ripgrep completions from fish, since upstream provides it.

@faho
Copy link
Member

faho commented Apr 17, 2019

Sooo....

  1. Ripgrep should be adding the file to vendor_completions.d, not the normal "completions" directory. That's ours.

  2. @mqudsi: Why are we shipping a file already provided upstream?

@mqudsi
Copy link
Contributor

mqudsi commented Apr 17, 2019

User-provided or package-provided completions should not be installed to the fish installation directory, there's a separate directory for that.

@mqudsi
Copy link
Contributor

mqudsi commented Apr 17, 2019

@faho ripgrep installation instructions are to manually copy the completions to the shell-specific user-local completions directory. A lot of our completions are also available via other means (e.g. pip, kitty, and others) but made more accessible by fish. This isn't new, AUR is to blame here.

Rust developers (which is the community rg came out of) installation ripgrep via cargo install and not a package manager, which deploys only the binaries. AFAIK this is also the official installation method upstream.

@faho
Copy link
Member

faho commented Apr 17, 2019

AUR is to blame here.

Not really. The "AUR" package in question is the fish-git package, which just installs fish.

The ripgrep package, which installs the completions into our directory, is in the official arch repo.

Though TBH I'd just drop these completions and be done with it - it's not like they're particularly sophisticated. The autogenerated completions aren't much worse.

@SanskritFritz
Copy link
Contributor

It is true that the ripgrep Archlinux package explicitely installs the completions to
install -Dm644 "target/release/build/ripgrep-"*/out/rg.fish "$pkgdir/usr/share/fish/completions/rg.fish"
That would be worth a complaint to archlinux.

@zanchey
Copy link
Member

zanchey commented Apr 17, 2019

Probably they should go in /usr/share/fish/vendor_completions.d, though we haven't really got the story for that sorted out yet (#5029).

@mqudsi
Copy link
Contributor

mqudsi commented Apr 17, 2019

I staunchly disagree with @faho on this. /usr/local/share/fish/ belongs to fish and we do not want to set a precedent of not adding foo because package xyz already messes with the fish installation directory.

@faho
Copy link
Member

faho commented Apr 17, 2019

we do not want to set a precedent of not adding foo because package xyz already messes with the fish installation directory.

Is it worth dying on this hill?

The completion is a tad better than the autogenerated one, it's installed already in at least some distributions via upstream, so removing it just makes our lives easier.

@faho
Copy link
Member

faho commented Apr 17, 2019

Probably they should go in /usr/share/fish/vendor_completions.d, though we haven't really got the story for that sorted out yet

In this case it's easy - it's for the distribution, so it should just hardcode that path instead of /usr/share/fish/completions/.

@mqudsi
Copy link
Contributor

mqudsi commented Apr 17, 2019

Is it worth dying on this hill?

Not for the rg completions (even if they were identical to the autogenerated ones) but the principle of it. It's a package maintainer's issue, this isn't our problem.

@faho
Copy link
Member

faho commented Apr 26, 2019

cc @svenstaro.

@svenstaro
Copy link

svenstaro commented Apr 29, 2019

So hang on, all in all, what are distribution maintainers supposed to do?

I'm supposed to change the installation path in the rg package to install to the vendor path, right?

@faho
Copy link
Member

faho commented Apr 30, 2019

I'm supposed to change the installation path in the rg package to install to the vendor path, right?

@svenstaro: Exactly. Use /usr/share/fish/vendor_completions.d, not /usr/share/fish/completions.

@faho
Copy link
Member

faho commented May 28, 2019

Okay, since we're apparently not going to do anything, let's close this and see if everything works out alright.

@faho faho closed this as completed May 28, 2019
@faho faho added this to the will-not-implement milestone May 28, 2019
@zanchey
Copy link
Member

zanchey commented Sep 3, 2019

I actually think this is the package manager's job to look after. Distribution maintainers faced with conflicting files in any packages should manage this in the way they would any other conflict - patch one of the packages, or use the tools that the distribution provides (eg alternatives, diversions on Debian) to manage the problem.

@ronjouch
Copy link
Author

Same thing today with the new rustup completions shipped in 85f93ff .

error: failed to commit transaction (conflicting files)
fish-git: /usr/share/fish/completions/rustup.fish exists in filesystem (owned by rustup)

@JeremyKennedy
Copy link

I had this same issue, found it mentioned in the Gitter chat. This is what fixed it for me: apt -o Dpkg::Options::="--force-overwrite" -f upgrade

@ronjouch
Copy link
Author

Yup. On the Ubuntu side, as of today 13 February 2020, the fish PPA package conflicts (at least) with the completions provided by:

Should we file issues for each?

@krobelus
Copy link
Member

Distributions should prefer the completion script provided by fish.

@ronjouch
Copy link
Author

ronjouch commented Feb 13, 2020

Distributions should prefer the completion script provided by fish.

@krobelus distributions will do their job and do the right thing in their ecosystem, ensuring no conflict happens in the virtual tree of the union of all their packages.

But here I'm asking how to solve the conflicts between PPA packages & "Early, directly from authors" deb packages like the {bat, ripgrep} examples I shared above. I'll be happy to file bat & ripgrep issues and offer a suggestion, but I'd like a confirmation and I'm not even sure what to suggest:

@krobelus
Copy link
Member

Yes, /usr/share/fish/vendor_completions.d (assuming prefix is /usr) is a good place for rg and others to ship fish completions. It will take precedence over /usr/share/fish/completions, so distributions could delete the vendor-provided completion in order to get the fish-provided one.

Or should we ask them to stop distributing completions altogether in their deb/rpm packages?

Not sure about this, probably not anytime soon (since old fish versions will be around for some time).
For softwares that autogenerate their fish completions, the ones provided by fish could be better (but can also be outdated if the software changes their CLI).

@adiabatic
Copy link
Contributor

  • Or should we ask them to stop distributing completions altogether in their deb/rpm packages?

This is probably a bad idea, at least in the general case.

It's easier to keep, say, bat.fish updated by the bat people in the bat repository and have distributions like Homebrew take that bat.fish and put it in the highest-priority system-wide spot.

On the other hand, FreeBSD's pkg suite doesn't do this. If I use bat on FreeBSD (assume it's available as a package), nothing's going to put an updated bat.fish in $SOMETHING/share/fish/vendor_completions.d.

Here's another way to think of the problem. Imagine you're a person who likes using a program called foo with fish. Whom do you send foo.fish files to in order to get the most-updated completions files to as many people as possible? A lot of programs have their command-line flags changed more frequently than fish ships updates.

@zanchey
Copy link
Member

zanchey commented Feb 19, 2020

Distributions should prefer the completion script provided by fish.

This is not the case; I would like to encourage more developers to write and ship fish completions! The problem is that completions shipped with (say) rg have to be compatible with a wide range of fish versions, while our completions have to be compatible with a wide range of rg versions.

I'm tempted to drop our bat and rg completions for 3.1.1.

@faho
Copy link
Member

faho commented Feb 19, 2020

I'm tempted to drop our bat and rg completions for 3.1.1.

I agree. We don't have a good workaround to offer those who meet conflicted packages (forcing overwrites isn't one), so we should drop them at least until they've been moved to the proper location by distribution packagers.

There are four groups of people affected by this.

  • People who don't use rg/bat, who don't care either way
  • People who use rg/bat from distro packages that install the file to the right location, who have completions either way
  • People who use rg/bat and don't have the completions installed, who would have no completions if we drop them
  • People who use rg/bat from distro packages that conflict, who would have completions either way but now have to force an overwrite

Frankly the fourth group is affected much worse than the third, so we should drop the completions.

(personally I've also said that I wouldn't ship upstream completions like the rg ones to begin with, but that's a different discussion)

@krobelus
Copy link
Member

Distributions should prefer the completion script provided by fish.

This is not the case; I would like to encourage more developers to write and ship fish completions!

Whoops, my bad; my train of thought was that sometimes a package simply includes auto-generated fish completions (for example from rustup completions fish), in that case our manually written ones may be better.
Which doesn't apply to rg and bat, our and upstream completions seem similar.

Anyway, dropping them from 3.1.1 is a good idea imo; eventually packages should be using vendor_completions.d

@ronjouch
Copy link
Author

eventually packages should be using vendor_completions.d

Note that rg just did that ( BurntSushi/ripgrep#1485 + BurntSushi/ripgrep@01eeec5 ), and bat is about to do it too; sharkdp/bat#651 (comment)

@zanchey
Copy link
Member

zanchey commented Feb 22, 2020

In the interim, the following workaround commands are useful:

dpkg-divert --add --divert  /usr/share/fish/completions/rg.fish.0 --rename --package ripgrep /usr/share/fish/completions/rg.fish
dpkg-divert --add --divert  /usr/share/fish/completions/bat.fish.0 --rename --package bat /usr/share/fish/completions/bat.fish

zanchey added a commit that referenced this issue Feb 22, 2020
These are shipped upstream.

Closes #5822.
zanchey added a commit that referenced this issue Feb 22, 2020
These are shipped upstream.

Closes #5822.

(cherry picked from commit f036d01)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants