Bug 790768

Summary: bash autocompletion should use "systemctl list-unit-files" as source for units to enable/disable
Product: [Fedora] Fedora Reporter: Jóhann B. Guðmundsson <johannbg>
Component: systemdAssignee: Zbigniew Jędrzejewski-Szmek <zbyszek>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 19CC: harald, h.reindl, jeff, johannbg, leho, metherid, mkolman, mschmidt, notting, paul.klapperich, Philippe.Vouters, plautrba, psklenar, samuel-rhbugs, systemd-maint, vpavlin, zbyszek
Target Milestone: ---Keywords: Reopened
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: systemd-208-26.fc20 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2014-11-05 03:56:31 UTC Type: ---
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:    
Bug Blocks: 959971, 784611    
Attachments:
Description Flags
systemctl tab strace none

Description Jóhann B. Guðmundsson 2012-02-15 11:25:13 UTC
Description of problem:

Apparently users expect working autocompletion for units ( .targets .path .timer .sockets .service etc ) in systemd

There is a respond which in a thread that seems to add that ability for restarts in bash

http://lists.fedoraproject.org/pipermail/devel/2012-February/162741.html

Version-Release number of selected component (if applicable):

All releases on all GA releases

How reproducible:

Always

Steps to Reproduce:
1. Type systemctl start/reload/restart/stop/enable/disable first ( few ) letter of service <press tab>
2.
3.
  
Actual results:

No auto completion of an unit, list of units presented to the user

Expected results:

Lazy end users being able to press tab to auto complete the typing of unit in question 

Additional info:

The fix for this should be backported to all GA releases

Comment 1 Michal Schmidt 2012-02-15 11:49:21 UTC
It should work if they have bash-completion installed.
Some cases where it does work 100% are related to bug 748512.

> The fix for this should be backported to all GA releases

F16 is fine AFAIK. Not sure about the state of bash completion in F15. I don't use it anymore and I only intended to backport fixes for serious bugs to F15 at this point.

Comment 2 Jóhann B. Guðmundsson 2012-02-15 14:04:59 UTC
I dont think keeping the same bash auto completion script on all release is to invasive + If it silences Reindl rants which keeps people sanity in checks that should warrant it enough to be granted exeption =) 

I think users expectation with regards to auto completion is that systemctl <command> <tab> should list all units that are supported by that command line switch for example systemctl start should list all unit types that can be manually started etc.

Based on that user expected criteria let's conduct few bash auto completion test and their results on F16...

First let's establish a base to work on...

note I'm only using units in the /lib/systemd/system not the unit's that might reside in the /etc/systemd/system directory or some other related directory that units might reside in...

# for i in automount device mount path service snapshot socket swap target timer; do echo "Number of $i type units $(ls /lib/systemd/system/*.$i 2>/dev/null | wc -l)";done
Number of automount type units 1
Number of device type units 0
Number of mount type units 12
Number of path type units 5
Number of service type units 359 
Number of snapshot type units 0
Number of socket type units 15
Number of swap type units 0
Number of target type units 46
Number of timer type units 3

Which gives us total number of 431 units

Now that we have the base let's start testing...

systemctl start <tab> list only types of .service units and not even all of those .service types units 

]# systemctl start 
Display all 113 possibilities? (y or n)

Gives 113 possibilities should give atleast 359 but should give more since you can start more than type .service units.

Further example ( I will only conduct this once )

systemctl start m<tab> list only .service files and not all of them

# systemctl start m
mdmonitor.service       mimedefang.service      modclusterd.service     multipathd.service      myproxy-server.service  mysqld.service

# ls m*.service
mcelog.service              mediatomb.service   messagebus.service     miredo-server.service  mon.service     multipathd.service  mydns.service
mdmonitor.service           memcached.service   mip6d.service          mldonkey.service       motion.service  munin-node.service  mysqld.service
mdmonitor-takeover.service  memcached@.service  miredo-client.service  monotone.service       mpd.service     murmur.service

Test result = FAIL

systemctl reload <tab> lists only 1 type service and 10 .mount type units 

# systemctl reload 
<list truncated 11 units offered there>

systemctl reload m<tab> offers only media.mount ( granted it's the only one starting with m that in the tab list offered in the first place to user to reload )

Test result = FAIL

systemctl restart <tab> lists various types of units but not all of them which can be restarted

# systemctl restart 
Display all 164 possibilities? (y or n)

systemctl restart m<tab> list not all available types of units that can be restarted

Test result = FAIL

systemctl stop <tab> lists various types of units but not all the units that can be stopped 

# systemctl stop
<list truncated 57 units listed there )

systemctl stop m<tab> list not all available types of units that can be stopped

systemctl enable <tab> triggers an bug

# systemctl enable Failed to issue method call: Bad message

systemctl disable <tab> triggers a bug 

# systemctl disable Failed to issue method call: Bad message

I also did test creating a new unit in the /lib/systemd/system directory and redo the previous mentioned tests to see if it got picked up by bash auto completion which it did not I even rebooted in between and re did all the tests with the same result. ( not sure if it should have matter but I tested it anyway ).

Conclusion....

All the conducted test that used the most commonly used commands in systemd with bash auto completion utterly and miserably failed thus are far from being "fine" in F16...

Comment 3 Michal Schmidt 2012-02-15 15:08:20 UTC
(In reply to comment #2)
> If it silences Reindl rants which keeps people sanity in checks
> that should warrant it enough to be granted exeption =)

Heh, Mr. Reindl's rude demands and insults have exactly the opposite influence on my enthusiasm to fix anything.

But for the benefit of other users, I can take a look at what commits are needed to backport.

> I think users expectation with regards to auto completion is that systemctl
> <command> <tab> should list all units that are supported by that command line
> switch for example systemctl start should list all unit types that can be
> manually started etc.

Yes, that would be ideal.

> Gives 113 possibilities should give atleast 359 but should give more since you
> can start more than type .service units.

That's what I meant in bug 748512.

> systemctl enable <tab> triggers an bug
> 
> # systemctl enable Failed to issue method call: Bad message
> 
> systemctl disable <tab> triggers a bug 
> 
> # systemctl disable Failed to issue method call: Bad message

I can't reproduce these errors.

> Conclusion....
> 
> All the conducted test that used the most commonly used commands in systemd
> with bash auto completion utterly and miserably failed thus are far from being
> "fine" in F16...

And I assume it is the same in the latest upstream.
Anyway, I don't understand the bash completion script, so I'll let someone else improve it.

Comment 4 Jóhann B. Guðmundsson 2012-02-15 15:53:28 UTC
Created attachment 562253 [details]
systemctl tab strace

I can reproduce it in systemd-37-13.fc16.x86_64 everytime.

Attaching the strace from that if it helps.

Comment 5 Michal Schmidt 2012-02-15 16:26:15 UTC
The strace shows a result of calling "systemctl enable", but that's not what happens when you try to autocomplete the command by pressing Tab. Or at least it should not be.

Comment 6 Jóhann B. Guðmundsson 2012-02-15 18:21:07 UTC
(In reply to comment #5)
> The strace shows a result of calling "systemctl enable", but that's not what
> happens when you try to autocomplete the command by pressing Tab. Or at least
> it should not be.

You are correct that strace was useless and we found that cause on irc ( and bug )

Anyway I have figured out was wrong with the bash completion script. 

We are overcomplicating what is expected we are filtering out units based on their property type as in we only list units that are "inactive" etc which is plane stupid for everything execpt units that dont have Install secton ( enable/disable ) plus I think that __filter_units_by_property () is broken. 

To do a simple test fix for systemctl start <tab>

First backup /etc/bash_completion.d/systemd-bash-completion.sh

Then open that file in your favorit editor and add to line 32

__test               () { __systemctl list-unit-files  | awk '                 {print $1}' ; }  

has out lines 136 and 137

#                comps=$( __filter_units_by_property CanStart yes \
#                      $( __get_inactive_units | grep -Ev '\.(device|snapshot)$' ))

Add this line to 138 

                comps=$( __test | grep -Ev '\@.service' | grep -Ev '\.(device|snapshot)$' 

( as you can see I added additional filter for @.service units which cant be started )

Save the file and logout ( exit ) then log in or open a new terminal and voila 

All units are now listed when systemctl start <tab> is pressed with the exception of @,service ( notice that bla is still listed there )

Now I do believe it's only a matter on how we want to proceed with fixing this. 

Unless we add something like "systemctl show-unit-files", a proper property filtering cant be performed.

Comment 7 Philippe Vouters 2012-05-26 18:48:32 UTC
[philippe@victor ~]$ rpm -qa | grep bash
bash-4.2.24-1.fc16.i686
bash-completion-1.3-6.fc16.noarch

bash does not auto-complete environment variables. Instead it adds a backslah in front of $

Reproducer:
[philippe@victor ~]$ ls $HOME/<tab>

Result:
[philippe@victor ~]$ ls \$HOME/

For an example of environment variables auto-copmpletion, read the following GNU readline example at:
http://vouters.dyndns.org/tima/Unix-Linux-OpenVMS-GNU_readline-readline_with_filename_completion_example.html

Yours truly,
Philippe Vouters (Fontainebleau/France)

Comment 8 Michal Schmidt 2012-05-28 10:09:40 UTC
Philippe,
the problem you're describing does not seem to be related to this bug, which is about auto-completion of systemctl commands. It would be better filed as a separate bug.

Comment 9 Jóhann B. Guðmundsson 2012-08-07 11:49:08 UTC
Did not Harald fix this already or was that never committed?

Comment 11 Jóhann B. Guðmundsson 2012-08-07 15:20:44 UTC
Here is the v2 version
http://lists.freedesktop.org/archives/systemd-devel/2012-February/004573.html

Comment 12 Václav Pavlín 2012-08-08 08:10:43 UTC
(In reply to comment #11)
> Here is the v2 version
> http://lists.freedesktop.org/archives/systemd-devel/2012-February/004573.html

I went through the patch and I don't think it solves this issue. It still uses systemctl list-units --all as a source for __get_all_units function, which gives you only loaded units. Patch only removes dependency on grep and awk and uses clean bash only.

To solve this, we need to merge outputs of list-units --all and list-unit-files (it can be done in systemctl or in autocomlete script).

Comment 13 Lennart Poettering 2012-10-30 21:07:55 UTC
*** Bug 865467 has been marked as a duplicate of this bug. ***

Comment 14 Fedora End Of Life 2013-04-03 19:47:14 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 19 development cycle.
Changing version to '19'.

(As we did not run this process for some time, it could affect also pre-Fedora 19 development
cycle bugs. We are very sorry. It will help us with cleanup during Fedora 19 End Of Life. Thank you.)

More information and reason for this action is here:
https://fedoraproject.org/wiki/BugZappers/HouseKeeping/Fedora19

Comment 15 Harald Hoyer 2013-04-10 10:44:48 UTC
(In reply to comment #12)
> (In reply to comment #11)
> > Here is the v2 version
> > http://lists.freedesktop.org/archives/systemd-devel/2012-February/004573.html
> 
> I went through the patch and I don't think it solves this issue. It still
> uses systemctl list-units --all as a source for __get_all_units function,
> which gives you only loaded units. Patch only removes dependency on grep and
> awk and uses clean bash only.
> 
> To solve this, we need to merge outputs of list-units --all and
> list-unit-files (it can be done in systemctl or in autocomlete script).

Is this still an issue with systemd-201?

Comment 16 Zbigniew Jędrzejewski-Szmek 2013-12-09 01:07:33 UTC
After an archeology expedition, I learned that the fix was committed in v37-19-g8aea83c :)

Comment 17 Zbigniew Jędrzejewski-Szmek 2013-12-09 01:07:49 UTC
After an archeology expedition, I learned that the fix was committed in v37-19-g8aea83c :)

Comment 18 Paul Klapperich 2014-07-26 04:38:58 UTC
(In reply to Harald Hoyer from comment #15)
> (In reply to comment #12)
> > (In reply to comment #11)
> > > Here is the v2 version
> > > http://lists.freedesktop.org/archives/systemd-devel/2012-February/004573.html
> > 
> > I went through the patch and I don't think it solves this issue. It still
> > uses systemctl list-units --all as a source for __get_all_units function,
> > which gives you only loaded units. Patch only removes dependency on grep and
> > awk and uses clean bash only.
> > 
> > To solve this, we need to merge outputs of list-units --all and
> > list-unit-files (it can be done in systemctl or in autocomlete script).
> 
> Is this still an issue with systemd-201?

Based on the description from comment #2, this is still an issue in systemd-215 as __get_startable_units () and __get_all_units () both use 'list-units --all' and not 'systemctl list-unit-files'. Comment 2 explains that tab completion should list units that are startable, not just units that are enabled and not running.

Steps to reproduce:
1. systemctl start [tab] 
2. systemctl enable [tab]
3. Notice that none of the units in step 2 are listed in step 1, even though it's possible to start disabled units.

Comment 19 Zbigniew Jędrzejewski-Szmek 2014-10-15 00:18:37 UTC
*** Bug 1024379 has been marked as a duplicate of this bug. ***

Comment 20 Fedora Update System 2014-10-26 23:10:22 UTC
systemd-208-25.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/systemd-208-25.fc20

Comment 21 Harald Reindl 2014-10-27 01:29:19 UTC
thanks but

[root@rh:~]$ systemctl stop dovecot.service; sleep; systemctl start dovToo many arguments.
should complete to "dovecot" instead "Too many arguments"

[root@rh:~]$ systemctl start mpToo many arguments.
should complete to "mpd" instead "Too many argumenst"

[root@rh:~]$ rpm -q systemd
systemd-208-25.fc20.x86_64

Comment 22 Zbigniew Jędrzejewski-Szmek 2014-10-27 02:08:14 UTC
That's ... discouraging. Thanks for the quick answer.

Comment 23 Fedora Update System 2014-10-30 11:59:58 UTC
systemd-208-26.fc20,kmod-15-2.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/systemd-208-26.fc20,kmod-15-2.fc20

Comment 24 Harald Reindl 2014-10-31 11:57:11 UTC
looks fine now 

bash autocompletion should use "systemctl list-unit-files" as source for units to enable/disable - well, "systemctl start" should do the same - sequences as below are not that uncommon (stop, do something, do something, start) and the completion for the start command don't work just because it is running at the moment


systemctl stop postfix.service; /bin/true; systemctl start post

Comment 25 Zbigniew Jędrzejewski-Szmek 2014-10-31 12:38:47 UTC
You have a point there. This would simplify things a lot.

Comment 26 Fedora Update System 2014-11-01 01:40:44 UTC
Package systemd-208-26.fc20, kmod-15-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 systemd-208-26.fc20 kmod-15-2.fc20'
as soon as you are able to, then reboot.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2014-14032/systemd-208-26.fc20,kmod-15-2.fc20
then log in and leave karma (feedback).

Comment 27 Fedora Update System 2014-11-02 07:26:45 UTC
systemd-208-25.fc20 has been pushed to the Fedora 20 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 28 Harald Reindl 2014-11-02 17:22:03 UTC
uhm 208-25.fc20 should not have made it to stable instead 208-26.fc20 which fixes the breakage of autocompletion (see comment 21)

Comment 29 Zbigniew Jędrzejewski-Szmek 2014-11-02 17:50:20 UTC
Yeah, it shouldn't have. Normally I would pull that update when -26 was made, but I didn't make the -26 update, so I didn't think about it. Sorry. Please add karma to -26, I'll look into the KVM problem, and hopefully it can be quickly pushed out.

Comment 30 Fedora Update System 2014-11-05 03:56:31 UTC
systemd-208-26.fc20, kmod-15-2.fc20 has been pushed to the Fedora 20 stable repository.  If problems still persist, please make note of it in this bug report.