[LU-16392] Bash completion regressions, lctl and lfs Created: 13/Dec/22  Updated: 07/Dec/23  Resolved: 03/Feb/23

Status: Resolved
Project: Lustre
Component/s: None
Affects Version/s: Lustre 2.16.0
Fix Version/s: Lustre 2.16.0

Type: Bug Priority: Minor
Reporter: Thomas Bertschinger Assignee: Thomas Bertschinger
Resolution: Fixed Votes: 0
Labels: None

Issue Links:
Related
is related to LU-5170 lfs usability Open
is related to LU-8621 Parser_execarg() prints help to stder... Resolved
is related to LU-13225 add bash completion for lfs Resolved
is related to LU-12734 bash completion (for now: lctl set/ge... Resolved
is related to LU-930 Update or improvement to a Lustre man... Open
Severity: 3
Rank (Obsolete): 9223372036854775807

 Description   

There are a few regressions introduced in the bash completions for lctl and lfs.

1. Attempting completion for lctl prints a message to stderr and does not generate any completions. To test this behavior interactively, type lctl <tab>. To test this behavior programmatically, run

source lustre/scripts/bash-completion/lustre; COMPLETIONS=$( _lustre_cmds lctl) && [ ! -z "$COMPLETIONS" ]

An exit status of 1 indicates the bug is present.

 

2. Completion for lfs check gives incorrect results.

 

3. Attempting completion for lfs find -component-flags erroneously prints a message to stderr.



 Comments   
Comment by Andreas Dilger [ 14/Dec/22 ]

It looks like this is a regression from patch https://review.whamcloud.com/47162 "LU-8621 utils: cmd help to stdout or short cmd error"

Comment by Andreas Dilger [ 14/Dec/22 ]

Thomas, thanks for your bug report. Any chance you could investigate how to change bash completion to call "lctl --list-commands" to generate to command list instead of just a null input?

Otherwise, it should be possible to revert the above patch to restore the basic functionality, with the accompanying spew of lines for any error (which is why it was changed originally).

Comment by Andreas Dilger [ 14/Dec/22 ]

To test this behavior programmatically, run:

source lustre/scripts/bash-completion/lustre; COMPLETIONS=$( _lustre_cmds lctl) && [ ! -z "$COMPLETIONS" ]

Thanks for this. I was wondering how we might be able to write a regression test for this without having interactive usage.

Comment by Thomas Bertschinger [ 14/Dec/22 ]

I considered resolving this by using lctl --list-commands, but there are two problems:

  1. --list-commands truncates long options, thus generating incorrect completions (example: changelog_deregister)
  2. lctl --list-commands lists options for sub-commands which do not apply to the top-level command, and I don't know how to work around this within the bash completion interface (example: lctl pcc add: bash would suggest both pcc and add as completions)

I have a patch in progress that adds an option --list-commands-brief which prints out subcommands without the above two problems. This has the advantage of not changing existing behavior of the CLI tools, but the disadvantage is adding a new, perhaps superfluous option. What do you think of this idea? An alternative would be modifying the format of lctl --list-commands.

Note, in this bug report I only mentioned lctl but this problem applies to lfs as well.

Comment by Andreas Dilger [ 14/Dec/22 ]

I was looking at the command truncation issue as well. It looks like we are out by only a character or two in a few rare cases. It would be better to misalign the output by a few characters to fit in the longest options I think, rather than creating a new option.

Comment by Andreas Dilger [ 14/Dec/22 ]

For the sub-commands, is it possible to list only the top-level command, and then add eg. "lctl pcc --list-commands" to generate the list of sub commands for the completion (ie. it passes the sub-command as well, not just "lctl --list-commands").?

Also, in a semi-related note, I'm not sure why "--list-commands" was used here instead of using "--help" to get this list, but it would be good to allow both (though "--list-commands" should be used for the completions since it has existed much longer).

Comment by Gerrit Updater [ 22/Dec/22 ]

"Thomas Bertschinger <bertschinger@lanl.gov>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/49484
Subject: LU-16392 utils: use --list-commands for bash completion
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 7c02a52fd57e2a0fa7e35c6cf3dad16ba12cb8d1

Comment by Peter Jones [ 03/Jan/23 ]

Does not affect b2_15- just master 

Comment by Gerrit Updater [ 03/Feb/23 ]

"Oleg Drokin <green@whamcloud.com>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/49484/
Subject: LU-16392 utils: use --list-commands for bash completion
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: b4cc570ad11c1c07a6e1d825787ccc62c1245ca1

Comment by Peter Jones [ 03/Feb/23 ]

Landed for 2.16

Generated at Sat Feb 10 03:26:37 UTC 2024 using Jira 9.4.14#940014-sha1:734e6822bbf0d45eff9af51f82432957f73aa32c.