Bug 1030440
Summary: | [RFE] bash completion support | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Alberto Ruiz <aruizrui> |
Component: | dnf | Assignee: | Elad Alfassa <elad> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | unspecified | Docs Contact: | |
Priority: | low | ||
Version: | 20 | CC: | 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
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>). 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. (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. 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. (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) (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. (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 > 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 (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. (discussed on fedora user's list: https://lists.fedoraproject.org/pipermail/users/2014-January/444547.html) 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+? 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? 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? (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. 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. 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). 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. 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. 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) 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 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). May I ask for an update of dnf-plugins-core as well, so people can use the completion caching plugin if they wish to? Trying to autocomplete package names for 'dnf install' prompts me for my sudo password then breaks my terminal prompt. 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. (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? 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 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. (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 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. dnf on Fedora 22 completes the option names as well, however it does not complete names of repos. dnf-0.6.5-1.fc22 does not. |