-
Notifications
You must be signed in to change notification settings - Fork 416
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
[spec] Add Recommends %{_bindir}/sqlite3 for bash-completion for Fedora #1985
[spec] Add Recommends %{_bindir}/sqlite3 for bash-completion for Fedora #1985
Conversation
I checked the internal implementation and I think we don't need the recommend: See:
If I am not mistaken if sqlite3 is not present it uses an alternative approach. May I ask you for additional reasons why we should add the recommend? |
The complaint here centers around the speed of the completion. I suspect some of this has to do with the cache being expired, but this could be mitigated somewhat by having a better internet connection. I do think this was worse back when I was utilizing cellular for internet. When the cache is up to date, completion still appears to be a slower (.5 - 1 second) without sqlite on my machine. (AMD Ryzen 5 5600G ) Thanks |
Ok, good point. |
dnf.spec
Outdated
@@ -85,6 +85,8 @@ Requires: python3-%{name} = %{version}-%{release} | |||
%if 0%{?rhel} && 0%{?rhel} <= 7 | |||
Requires: python-dbus | |||
Requires: %{_bindir}/sqlite3 | |||
%elif 0%{?fedora} | |||
Recommends: (%{_bindir}/sqlite3 if bash-completion) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am proposing to add additional condition (%{_bindir}/sqlite3 if (bash-completion and python3-dnf-plugins-core))
.
If python3-dnf-plugins-core
is not installed then the database cannot be updated and bash completion provides outdated results.
Be honest this approach is not perfect, because it uses resources during RPM transaction. DNF5 uses an alternative approach without external database.
8fff7e4
to
4aaa0ff
Compare
dnf.spec
Outdated
@@ -85,6 +85,8 @@ Requires: python3-%{name} = %{version}-%{release} | |||
%if 0%{?rhel} && 0%{?rhel} <= 7 | |||
Requires: python-dbus | |||
Requires: %{_bindir}/sqlite3 | |||
%elif 0%{?fedora} | |||
Recommends: (%{_bindir}/sqlite3 if (bash-completion && python3-dnf-plugins-core)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am really sorry but operator &&
is not supported. See https://rpm-software-management.github.io/rpm/manual/boolean_dependencies.html. May I ask you to replace &&
by and
?
Sorry about that, will fix later today.
|
05fbdf7
to
ab3ba85
Compare
rpm-software-management#1817 added the ability to utilize sqlite cache when available. https://discussion.fedoraproject.org/t/why-bash-auto-completion-is-so-slow-with-dnf/88944 and https://discussion.fedoraproject.org/t/dnf-autocompletion-is-too-slow/64038 Are users noting that bash completion is slow without sqlite installed. = changelog = msg: [spec] Add Recommends %{_bindir}/sqlite3 for bash-completion for Fedora type: enhancement related: rpm-software-management#1817 related: https://discussion.fedoraproject.org/t/why-bash-auto-completion-is-so-slow-with-dnf/88944 related: https://discussion.fedoraproject.org/t/dnf-autocompletion-is-too-slow/64038
ab3ba85
to
0761e81
Compare
#1817 added the ability to utilize sqlite cache when available.
https://discussion.fedoraproject.org/t/why-bash-auto-completion-is-so-slow-with-dnf/88944 and
https://discussion.fedoraproject.org/t/dnf-autocompletion-is-too-slow/64038 Are users noting that bash completion is slow without sqlite installed.
= changelog =
msg: [spec] Add Recommends %{_bindir}/sqlite3 for bash-completion for Fedora
type: enhancement
related: #1817
related: https://discussion.fedoraproject.org/t/why-bash-auto-completion-is-so-slow-with-dnf/88944
related: https://discussion.fedoraproject.org/t/dnf-autocompletion-is-too-slow/64038