Giter Site home page Giter Site logo

Comments (12)

scop avatar scop commented on June 25, 2024

The suggested approach is apparently Linux specific, so it cannot be just slapped in (the place to do this is the _pnames function in bash_completion BTW). It also has some generic portability issues: the -z argument to cut is not portable (doesn't even exist in some recentish Linux distros such as my Fedora 23), neither is -0 to xargs. And we don't want to use external utilities like basename when internal suitable mechanisms exist, such as bash's parameter expansion in this case. Search for POSIX in CONTRIBUTING.md for more info on portability.

from bash-completion.

ThomasFaivre avatar ThomasFaivre commented on June 25, 2024

Hello,

sorry for digging this up, but I fixed this in my bashrc by redefining _killall and calling '_pnames -s' instead of just '_pnames'.

I am not familiar with Solaris & others but it seems _pnames is defined differently on those OS's so it should not do anything.

I can make the patch if you agree with that statement.

from bash-completion.

scop avatar scop commented on June 25, 2024

@ThomasFaivre it would be good to first hear what exactly doesn't work for you, and what is your OS and version.

from bash-completion.

ThomasFaivre avatar ThomasFaivre commented on June 25, 2024

Fair enough.

Debian 10

bash --version
GNU bash, version 5.0.3(1)-release (x86_64-pc-linux-gnu)
Copyright (C) 2019 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html

This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

bash-completion/testing,now 1:2.11-2 all [installed]

psmisc/stable,now 23.2-1 amd64 [installed,automatic]

How to reproduce:

$ <Start some process with a process name longer than 16 bytes. I'm using qemu>
$ ps -eo cmd | grep qemu
/usr/bin/qemu-system-x86_64 -k fr -daemonize --enable-kvm [...]
$ killall qemu< TAB >
$ killall -v qemu-system-x86_64
qemu-system-x86_64: no process found
$ killall -v qemu-system-x86 # manually written
Killed qemu-system-x86(31622) with signal 15

Read too fast the original post, I don't know about the space issue but I saw that pnames was reworked so that might have already been fixed. I don't know.

Using '_pnames -s' here, fixes this issue for me:

$ killall qemu< TAB >
$ killall -v qemu-system-x86

from bash-completion.

scop avatar scop commented on June 25, 2024

I think _pnames -s "fixes" this particular case just because qemu-system-x86 happens to be short enough, and that it wouldn't work for longer (> 15 char) command basenames. Please follow the Troubleshooting section in README.md to get debug output, review it for possible information you may not want to share here, redact as appropriate, and attach the output here as an attachment.

from bash-completion.

ThomasFaivre avatar ThomasFaivre commented on June 25, 2024

I can do that on monday, but I don't really understand. Qemu basename is qemu-system-x86_64 which is longer than 15, that is why I need to strip '_64' to make killall command happy

In [19]: len('qemu-system-x86_64')                                              
Out[19]: 18

In [20]: len('qemu-system-x86')                                                 
Out[20]: 15

I used the ps ax -o comm (== _pnames -s) to list processes and looked for truncated basenames and tried to killall one:

$ killall -vi at-spi2-registryd 
at-spi2-registryd: no process found
$ killall -vi at-spi2-registr
Kill at-spi2-registr(28590) ? (y/N) n
at-spi2-registr: no process found <<< This is due to the 'n' answer to the interactive option

Am I missing something here?

from bash-completion.

scop avatar scop commented on June 25, 2024

Oh, I missed the x86 vs x86_64 difference. But for me on Ubuntu 18.04, killall seems to work with the complete process names, not abbreviations, so I believe switching to _pnames -s would break the completion which seems to work fine as it is. psmisc is 23.1-1ubuntu0.1.

$ ps wax | grep [g]sd-print-noti
 2203 tty1     Sl+    0:00 /usr/lib/gnome-settings-daemon/gsd-print-notifications
 3973 tty2     Sl+    0:00 /usr/lib/gnome-settings-daemon/gsd-print-notifications
$ ps axo comm= | grep [g]sd-print-noti
gsd-print-notif
gsd-print-notif
$ killall -s 0 gsd-print-notif
gsd-print-notif: no process found
$ killall -s 0 gsd-print-notifications
$ echo $?
0

from bash-completion.

scop avatar scop commented on June 25, 2024

Also:

$ killall -vi at-spi2-registryd 
Kill at-spi2-registr(3595) ? (y/N) n
at-spi2-registryd: no process found  <<< also due to the 'n' choice
$ killall -vi at-spi2-registr
at-spi2-registr: no process found

from bash-completion.

ThomasFaivre avatar ThomasFaivre commented on June 25, 2024

It seems it was fixed in killall:

https://gitlab.com/psmisc/psmisc/-/blob/master/ChangeLog#L19

It was backported to ubuntu 18.04, and is present in Ubuntu 20.04 and Debian testing (future 11)

I can live with the workaround in my bashrc (or I could upgrade to testing version). Do we still want to do something here? Probably not useful since Debian 10 wil probably not ship the fix we make...

from bash-completion.

scop avatar scop commented on June 25, 2024

Ok, confused here. psmisc 23.2 according to the referred changelog entry is supposed to work with the longer process names ("Command names increased from 16 to 64 characters" I guess), right? Your report says you have 23.2 but it actually doesn't work with the long ones, but only the truncated ones?

from bash-completion.

ThomasFaivre avatar ThomasFaivre commented on June 25, 2024

Hmmm that's very true.

Let's try that again:

I believe the issue was actually introduced in 23.2 with this commit (and a fix):

https://gitlab.com/psmisc/psmisc/-/commit/1e2f38a202798a78554ae5f5d12f697f3607f89f
https://gitlab.com/psmisc/psmisc/-/commit/acdcbf30c68053a25a1a5b5a6604a8382912be71

This was done to handle the new comm length for "new kernel". Modification I still can't understand since kernel 5.12-rc1 still uses 16 as TASK_COMM_LEN:

https://github.com/torvalds/linux/blob/master/include/linux/sched.h#L215
https://gitlab.com/psmisc/psmisc/-/blob/master/src/comm.h#L30

And someone got as confused as me:

https://gitlab.com/psmisc/psmisc/-/issues/19

But anyway, it was fixed in 23.3:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=912748 <<< this is actually the issue I'm seeing. Found in changelog for 23.3
https://gitlab.com/psmisc/psmisc/commit/1188315cd037d73bf946a0003b70c6423cc330d2 <<< note that there is a typo in changelog
https://gitlab.com/psmisc/psmisc/-/commit/2209bc12fedd75aca8b79801a842b35f87f9b727 <<< fixes the typo

To summarize:

Ubuntu-18.04: Version is 23.1-1ubuntu0.1 - does not handle the new comm length of 64 => No issue
Ubuntu-20.04: Version is 23.3-1 - handle new (64) and old (16) comm length => No issue
Debian-10: Version is 23.2-1 - handle new (64) comm length but not the old one => Issue
Debian-11: Version will be at least 23.3-1 - handle new (64) and old (16) comm length => No issue

So yeah, I don't think anything should change here. I have contacted the maintainer for the psmisc package about releasing the fix in Debian 10, but maybe a debian patch should be added to the bash-completion package in (and only in) Debian 10 to handle that specific issue. It will depend on the answer from the maintainer.

Sorry for the noise!

from bash-completion.

scop avatar scop commented on June 25, 2024

No problem, thanks for the thorough detective work. Guess we should keep this open anyway as I gather at least some of the originally reported issues still do persist.

from bash-completion.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.