Bug 1030440

Summary: [RFE] bash completion support
Product: [Fedora] Fedora Reporter: Alberto Ruiz <aruizrui>
Component: dnfAssignee: Elad Alfassa <elad>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: low    
Version: 20CC: akozumpl, danielkza2, elad, mhroncok, packaging-team-maint, pnemade, rholy, Simon.Gerhards
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: dnf-0.4.13-2.fc20 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2014-02-09 03:54:22 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 1048468    
Bug Blocks:    

Description Alberto Ruiz 2013-11-14 13:02:59 UTC
Description of problem:
While using dnf from the command line bash does not complete commands or package names using bash completion. This is a regression from the current yum experience.

Version-Release number of selected component (if applicable):
0.4.6
  Installed: dnf-0:0.4.6-1.fc20.noarch at 2013-11-08 14:24
  Built    : Fedora Project at 2013-10-30 10:03

  Installed: rpm-0:4.11.1-7.fc20.x86_64 at 2013-09-26 11:16
  Built    : Fedora Project at 2013-09-09 12:13


How reproducible:
Always

Steps to Reproduce:
1. type "dnf inst" on the command line
2. press tab
3. wait for ever

Actual results:
no completion is shown

Expected results:
complete!

Additional info:

Comment 1 Ales Kozumplik 2013-11-15 08:43:51 UTC
Hello Alberto, we already had this reported in bug 886912. There is currently no way to implement the package completion so it is at the same time quick, doesn't *constantly* (=each time a shell is running) occupy MBs of memory and doesn't require an extensive effort on implementation.

We'll keep this on low prio for now, some kind of completion will probably be provided later, it just not might be package completion. (people prefer shell that is responsive to a shell that is completing everything but halts for 2 seconds on pressing a <tab>).

Comment 2 Elad Alfassa 2013-12-05 20:54:35 UTC
I assume package completion support for removal/update is easier, because all the bash-completion script will have to do is process the output of rpm -ql.

As for installing, if we do it from cache only, it should be at least as fast as the yum equivalent. Plus, if we can somehow display a message in the console when looking for packages to tab-complete, it would already be better UX than the yum equivalent which will case the console to be stuck until completion is completed.

Comment 3 Ales Kozumplik 2013-12-05 21:01:57 UTC
(In reply to Elad Alfassa from comment #2)
> I assume package completion support for removal/update is easier, because
> all the bash-completion script will have to do is process the output of rpm
> -ql.

That's still pretty slow (over 1s on my box with SSD) .

> As for installing, if we do it from cache only, it should be at least as
> fast as the yum equivalent. 

And that's just slow.

Comment 4 Elad Alfassa 2013-12-05 21:20:21 UTC
right.

the list of installed packages is few hundreds of kilobytes, we can hold that in memory as long as the shell is running. the list of available packages is a bit more meomory-expensive (3.2MB on my test machine), but even that is not really a lot for modern day computers (especially if we unload it when/if memory usage spikes).

We can also keep those two lists in plain-text files on disk, because all we need is package names and nothing more than that. grep'ing a 3.2MB file takes 0.020s on my test machine (and should be faster with SSD). As long as we keep those lists up-to-date (ie. regenerate them in the background on metadata update and/or transaction complete), I see no problem with this implementation.

Comment 5 Elad Alfassa 2013-12-05 21:24:20 UTC
(I can write this as a 3rd party utility if you don't want it to be shipped with dnf, which is understandable because my approach is quite hackish... but I can't seem to find documentation on how to bind plugin functions to metadata download complete and transaction complete events)

Comment 6 Ales Kozumplik 2013-12-06 07:47:11 UTC
(In reply to Elad Alfassa from comment #4)
> right.
> 
> the list of installed packages is few hundreds of kilobytes, we can hold
> that in memory as long as the shell is running.

Does that imply that every login shell startup would be slowed down by approximately 1 second? That is absolutely not acceptable.

> We can also keep those two lists in plain-text files on disk, because all we
> need is package names and nothing more than that. grep'ing a 3.2MB file
> takes 0.020s on my test machine (and should be faster with SSD). As long as
> we keep those lists up-to-date (ie. regenerate them in the background on
> metadata update and/or transaction complete), I see no problem with this
> implementation.

I don't like it but it's along the lines of what bug 886912 suggested.

Comment 7 Ales Kozumplik 2013-12-06 07:48:59 UTC
(In reply to Elad Alfassa from comment #5)
> (I can write this as a 3rd party utility if you don't want it to be shipped
> with dnf, which is understandable because my approach is quite hackish...
> but I can't seem to find documentation on how to bind plugin functions to
> metadata download complete and transaction complete events)

If we do this then we'll have to ship it with dnf package---I don't think that FPG would let us have a special package for that. So you are welcome to even send patches directly for dnf-plugins[1]. For the API bindings and hooks you are missing please open bugzillas tagged [api].

[1] https://github.com/akozumpl/dnf-plugins

Comment 8 Elad Alfassa 2013-12-06 08:22:05 UTC
> Does that imply that every login shell startup would be slowed down by approximately 1 second? 

Not if we do it in the background.

> I don't think that FPG would let us have a special package for that
I don't see why it can't be a subpackage. 

As for the bugs, done:
bug #1038936
bug #1038937

Comment 9 Ales Kozumplik 2013-12-17 08:33:29 UTC
(In reply to Elad Alfassa from comment #8)
> As for the bugs, done:
> bug #1038936
> bug #1038937

Both these are fixed in the master now.

Comment 10 Ales Kozumplik 2014-01-02 10:57:04 UTC
(discussed on fedora user's list: https://lists.fedoraproject.org/pipermail/users/2014-January/444547.html)

Comment 11 Elad Alfassa 2014-01-02 19:23:45 UTC
I'm probably going to work on this during the weekend. Do you prefer it as a patch to dnf-plugins or as a completely separated project due to the "hackish" implementation? I'm fine with either.

One more clarification I'm going to need is the license, can a plugin be licensed say with MIT or GPLv3+, or is there a requirement to have plugins as GPLv2+?

Comment 12 Ales Kozumplik 2014-01-02 20:52:17 UTC
You can start with dnf-core-plugins. but mind you: the solution of storing a couple of MBs permanently in memory (what if 20 users are logged in at the same time?), just in case somebody might need to do dnf completion is not acceptable. So if you want to be sure that this goes in Fedora and does not get rejected on the pull request review, please start a separate project.

Also---will you take this bug then?

Comment 13 Elad Alfassa 2014-01-02 21:39:26 UTC
My intended solution is to keep one flat file in /var/cache/dnf, one with a list of available packages.

A dnf plugin will update that file on every metadata refresh. Completion search will be done by a simple grep.

This will add approximately 4 to 8 seconds to the metadata caching process, but I'll make sure it won't affect performance of dnf itself by either threading it or spawning a separate process.

For completing installed files, "dnf list installed" piped to grep y (for example) takes less than a second to complete on my non-ssd test machine, so I won't need a separate file for that.


Is this acceptable for dnf-plugins-core?

Comment 14 Ales Kozumplik 2014-01-03 09:00:26 UTC
(In reply to Elad Alfassa from comment #13)
> My intended solution is to keep one flat file in /var/cache/dnf, one with a
> list of available packages.
> 
> A dnf plugin will update that file on every metadata refresh. Completion
> search will be done by a simple grep.
> 
> This will add approximately 4 to 8 seconds to the metadata caching process,
> but I'll make sure it won't affect performance of dnf itself by either
> threading it or spawning a separate process.
> 
> For completing installed files, "dnf list installed" piped to grep y (for
> example) takes less than a second to complete on my non-ssd test machine, so
> I won't need a separate file for that.
> 
> 
> Is this acceptable for dnf-plugins-core?

Yes if it the performance hit is negligible. Else we will have to disable it by default/make you package it separately.

Comment 15 Elad Alfassa 2014-01-04 12:41:51 UTC
Submitted a pull request. https://github.com/akozumpl/dnf-plugins-core/pull/3

It has some flaws:
1 . completing the package name for installed packages for removal is a bit slow (guess I'll have to use a flat file for that as well)

2. Right now I'm using rpm -qav piped to egrep to complete installed packages, I wanted to use dnf but bug #1048468 prevented me from doing that

3. Due to sack() being ran when a sack is activated (instead of refreshed), the plugin adds few ms of delay to most dnf operations, even when cache refresh is not needed. The delay was extremely small tho, so I did not bother to make the plugin use a separate process or thread

4. Due to issue #3, the completion cache will not be generated unless the user did a dnf operation that caused a sack to be activated. The completion cache will not be refreshed by dnf makecache or the dnf makecache timer.

Comment 16 Ales Kozumplik 2014-01-06 14:49:17 UTC
Thanks for putting the first version together, let's look at the details:

(In reply to Elad Alfassa from comment #15)
> Submitted a pull request. https://github.com/akozumpl/dnf-plugins-core/pull/3
> 
> It has some flaws:
> 1 . completing the package name for installed packages for removal is a bit
> slow (guess I'll have to use a flat file for that as well)

Yes, flat file sounds good.

> 2. Right now I'm using rpm -qav piped to egrep to complete installed
> packages, I wanted to use dnf but bug #1048468 prevented me from doing that

Yeah, once that's fixed we get there.

> 3. Due to sack() being ran when a sack is activated (instead of refreshed),
> the plugin adds few ms of delay to most dnf operations, even when cache
> refresh is not needed. The delay was extremely small tho, so I did not
> bother to make the plugin use a separate process or thread

This is a bit unfortunate. Until we get this settled I suggest we disable the plugin by default, the completion script should work still (perhaps slow). Is that OK?

> 4. Due to issue #3, the completion cache will not be generated unless the
> user did a dnf operation that caused a sack to be activated. The completion
> cache will not be refreshed by dnf makecache or the dnf makecache timer.

Same as 3).

Comment 17 Elad Alfassa 2014-01-06 15:10:16 UTC
As for 2, I'm starting to doubt the benefit of using dnf for this if not doing a flat file. If I do switch that to a flat file as well, there'd be no need of invoking dnf or rpm there

For 3, the completion script won't work without a flat cache file as it is now. If I add the relevant lines to make it work, it'd be extremely slow without that file present. Adding threading/multiproccessing to the plugin will of course improve performance, but having the completion only work after you actually do an action is a bit ridiculous.

If you still think sack() should be called only when the sack is activated, I have two solutions: 
1) add a hook that fires after a "makecache" completes. This will mean that the completion cache file will be updated by the timer, but won't be updated if the user does (for example) "dnf update" and as a side effect downloads the cache.
2) I could write a script of my own (instead of the plugin) to run with a timer and generate the cache file. I do not like this approach very much tho.

Comment 18 Ales Kozumplik 2014-01-06 16:03:48 UTC
I think the best course of action is:

1) provide a bash completion script that can work without the plugin, even if it's slow. you're going to have to have it pulled into dnf core and not the plugins, sorry about that.

2) open a bug for the proper plugin hook

3) finish the plugin using the new hook

4) update the completion script to work faster if the plugin is in place (but still do something if dnf wasn't run a single time).

completions are tricky and that's why we avoided them until now.

Comment 19 Elad Alfassa 2014-01-10 15:07:03 UTC
Done. updated pull requests as well.

Note that until bug #1048468 is fixed, the completion script won't be able to complete names of available packages (unless the plugin generated the cache)

Comment 20 Fedora Update System 2014-02-07 06:43:51 UTC
dnf-0.4.13-2.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/dnf-0.4.13-2.fc20

Comment 21 Fedora Update System 2014-02-08 05:07:38 UTC
Package dnf-0.4.13-2.fc20:
* should fix your issue,
* was pushed to the Fedora 20 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing dnf-0.4.13-2.fc20'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2014-2172/dnf-0.4.13-2.fc20
then log in and leave karma (feedback).

Comment 22 Elad Alfassa 2014-02-08 11:49:23 UTC
May I ask for an update of dnf-plugins-core as well, so people can use the completion caching plugin if they wish to?

Comment 23 Daniel Miranda 2014-02-09 00:41:30 UTC
Trying to autocomplete package names for 'dnf install' prompts me for my sudo password then breaks my terminal prompt.

Comment 24 Fedora Update System 2014-02-09 03:54:22 UTC
dnf-0.4.13-2.fc20 has been pushed to the Fedora 20 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 25 Elad Alfassa 2014-02-09 07:42:52 UTC
(In reply to Daniel Miranda from comment #23)
> Trying to autocomplete package names for 'dnf install' prompts me for my
> sudo password then breaks my terminal prompt.

The only way this could possibly happen is if you'd alias dnf to sudo dnf. Did you?

Comment 26 Daniel Miranda 2014-02-09 14:51:32 UTC
Not at all. Just to be sure I did a clean reinstall of dnf, and looking at /etc/bash_completion.d/dnf_completion.bash, line 83:

$ grep sudo /etc/bash_completion.d/dnf-completion.bash -C 5 -n
78-    if [[ "$command" == "install" || "$command" == "update" || "$command" == "info" ]]; then
79-        if [ -r '/var/cache/dnf/available.cache' ]; then
80-            COMPREPLY=($(compgen -W "`egrep ^$cur /var/cache/dnf/available.cache`" -- "$cur"))
81-            return
82-        else
83:            COMPREPLY=($(compgen -W "`sudo dnf list --cacheonly 2>/dev/null | cut -d' ' -f1 | egrep ^$cur`" -- "$cur"))
84-            return
85-        fi
86-    fi
87-    if [[ "$command" == "remove" || "$command" == "erase" ]]; then
88-        if [ -r '/var/cache/dnf/installed.cache' ]; then

Comment 27 Elad Alfassa 2014-02-09 18:58:53 UTC
Right, my fault. The problem is it won't work *at all* without sudo until bug #1058224 is fixed.

Workaround for now: Install dnf-plugins-core from git, then run dnf clean all; dnf makecache, this will make sure you have the completion cache plugin and that the cache is built, so it won't hit the code-path that breaks your shell.

I only tested it on passwordless-sudo, so that's why I didn't notice breakage while writing this. I agree sudo shouldn't be there, but we need bug #1058224 fixed for that to be possible.

Sorry.

Comment 28 Ales Kozumplik 2014-02-10 07:54:28 UTC
(In reply to Elad Alfassa from comment #22)
> May I ask for an update of dnf-plugins-core as well, so people can use the
> completion caching plugin if they wish to?

right: https://admin.fedoraproject.org/updates/dnf-plugins-core-0.0.3-1.fc20

Comment 29 Miro Hrončok 2015-04-30 14:38:08 UTC
What is the current status of dnf bash completion?

I know that completing the names of packages is problematic, and I don't need it. But yum was able to construct the following command:

    yum downgrade openscad\* --disablerepo=\* --enablerepo fedora\*

very easily (option names --disablerepo and --enablerepo as well as names of the repos were autocompleted). However with dnf I can only autocomplete 'dnf' itself and 'downgrade' and I'm done.

Comment 30 Elad Alfassa 2015-04-30 14:44:18 UTC
dnf on Fedora 22 completes the option names as well, however it does not complete names of repos.

Comment 31 Miro Hrončok 2015-05-02 19:42:37 UTC
dnf-0.6.5-1.fc22 does not.