Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

{ls,ins,rm}mod: update help, man and {bash,fish,zsh} shell completion #138

Closed
wants to merge 3 commits into from
Closed

{ls,ins,rm}mod: update help, man and {bash,fish,zsh} shell completion #138

wants to merge 3 commits into from

Conversation

evelikov
Copy link
Collaborator

@evelikov evelikov commented Sep 17, 2024 •
edited
Loading

As the title says, this series focuses on lsmod/rmmod/insmod.

In general, we've ensured they're consistent among other another - syslog/verbose/version/help for all - aligning the help and manual pages.

Some potentially picky bits:

  • removed unused and undocumented since day 1 - rmmod -p and -s
  • added longopt and documentation for insmod -f

Input welcome:

  • the rmmod/insmod force messages (in help and shell completion) should be trimmed and made scarier IMHO

@evelikov evelikov changed the title {ls,ins,rm}: update help, man and {bash,fish,zsj shell {ls,ins,rm}: update help, man and {bash,fish,zsh} shell completion Sep 17, 2024
@evelikov evelikov force-pushed the foomod-help-man-shell branch from a27f71b to dba5502 Compare September 17, 2024 21:58
@evelikov
Copy link
Collaborator Author

This is the first batch for:

@evelikov evelikov changed the title {ls,ins,rm}: update help, man and {bash,fish,zsh} shell completion {ls,ins,rm}mod: update help, man and {bash,fish,zsh} shell completion Sep 18, 2024
@evelikov evelikov marked this pull request as draft September 18, 2024 10:06
@evelikov
Copy link
Collaborator Author

Switching to draft since there are some small bugs and cleanups needed.

Would love to hear on the fiddly bits mentioned above and generally if this is something we'd want before polishing it.

@lucasdemarchi
Copy link
Contributor

Some potentially picky bits:

  • removed unused and undocumented since day 1 - rmmod -p and -s
  • added longopt and documentation for insmod -f

agree

Input welcome:

  • the rmmod/insmod force messages (in help and shell completion) should be trimmed and made scarier IMHO

agree

@lucasdemarchi
Copy link
Contributor

Would love to hear on the fiddly bits mentioned above and generally if this is something we'd want before polishing it.

yes... this all looks good to me. Thanks

@evelikov evelikov force-pushed the foomod-help-man-shell branch from dba5502 to d98f3a9 Compare September 18, 2024 16:23
@evelikov evelikov marked this pull request as ready for review September 18, 2024 16:28
@evelikov
Copy link
Collaborator Author

Glad to hear - series re-spinned.

Will look at the rest (modinfo, modprobe, depmod) at a later date.

lucasdemarchi pushed a commit that referenced this pull request Sep 21, 2024
@evelikov @lucasdemarchi
tools: add kmod_version() helper
b644b8d 
Add a trivial helper that prints the version + features combo. I will be
adding another instance of those, so I'm aiming to keep the boilerplate
code to a minimum.

Signed-off-by: Emil Velikov <[email protected]>
Link: #138
Signed-off-by: Lucas De Marchi <[email protected]>
lucasdemarchi pushed a commit that referenced this pull request Sep 21, 2024
@evelikov @lucasdemarchi
tools/lsmod: add basic opts like rmmod
6313d40 
This allows for more consistent experience across tools, allows to see
the version, observe/debug if "Used by" reports -1, etc.

Also don't forget to update the man page :-)

Signed-off-by: Emil Velikov <[email protected]>
Link: #138
Signed-off-by: Lucas De Marchi <[email protected]>
lucasdemarchi pushed a commit that referenced this pull request Sep 21, 2024
@evelikov @lucasdemarchi
tools/rmmod: remove unused -w shortopt
519621b 
The (long)option and handling was removed some 10 years ago, yet the
shortopt remained... oops

Fixes: a4bd144 ("Remove "rmmod -w" documentation and getopt entry")
Signed-off-by: Emil Velikov <[email protected]>
Link: #138
Signed-off-by: Lucas De Marchi <[email protected]>
lucasdemarchi pushed a commit that referenced this pull request Sep 21, 2024
@evelikov @lucasdemarchi
tools/rmmod: tweak --force help message
9d2cb67 
Drop the kernel configuration details - the CONFIG toggle name is more
useful (since you can see it in /proc/config.*) and generally the part
is better suited for the manual page.

Move DANGEROUS at the front, so it's obvious from the start

Signed-off-by: Emil Velikov <[email protected]>
Link: #138
Signed-off-by: Lucas De Marchi <[email protected]>
lucasdemarchi pushed a commit that referenced this pull request Sep 21, 2024
@evelikov @lucasdemarchi
man/rmmod: reorder, add --help
4cc728f 
Stay consistent with the output of rmmod --help and update the
respective options.

Signed-off-by: Emil Velikov <[email protected]>
Link: #138
Signed-off-by: Lucas De Marchi <[email protected]>
lucasdemarchi pushed a commit that referenced this pull request Sep 21, 2024
@evelikov @lucasdemarchi
tools/rmmod: make opt variables non-global
24fe68d 
They're used locally within one function so declare them there.

Signed-off-by: Emil Velikov <[email protected]>
Link: #138
Signed-off-by: Lucas De Marchi <[email protected]>
lucasdemarchi pushed a commit that referenced this pull request Sep 21, 2024
@evelikov @lucasdemarchi
tools/rmmod: consistently use ERR logging facility
a6f9cd0 
Currently we'll print "Module foo is in use by:" to syslog, while the
modules themselves will end up in strerr.

Signed-off-by: Emil Velikov <[email protected]>
Link: #138
Signed-off-by: Lucas De Marchi <[email protected]>
lucasdemarchi pushed a commit that referenced this pull request Sep 21, 2024
@evelikov @lucasdemarchi
tools/rmmod: return EXIT_FAILURE if kmod_module_new_* fails
6317b4a 
In the unlikely case where kmod_module_new_from_{path,name} fails,
rmmod will return success. Change that to EXIT_FAILURE.

Signed-off-by: Emil Velikov <[email protected]>
Link: #138
Signed-off-by: Lucas De Marchi <[email protected]>
lucasdemarchi pushed a commit that referenced this pull request Sep 21, 2024
@evelikov @lucasdemarchi
tools/insmod: remove unused -p -s shortopts
b6b04af 
These have been around since the import to git, doing nothing. There are
no users that I can see, the manual page does not list them and the
shell completions (fish and bash lack any for insmod, zsh has one) don't
suggest it either.

Remove them.

Signed-off-by: Emil Velikov <[email protected]>
Link: #138
Signed-off-by: Lucas De Marchi <[email protected]>
lucasdemarchi pushed a commit that referenced this pull request Sep 21, 2024
@evelikov @lucasdemarchi
tools/insmod: add syslog and verbose options
ca8f04e 
Add the extra options for consistency with the rest of kmod.

Document all options in the man page.

Signed-off-by: Emil Velikov <[email protected]>
Link: #138
Signed-off-by: Lucas De Marchi <[email protected]>
lucasdemarchi pushed a commit that referenced this pull request Sep 21, 2024
@evelikov @lucasdemarchi
tools/insmod: add --force longopt and document it
e102d2c 
As of the initial import to git, the option was silently ignored.
Shortly afterwards, functionality was reinstated lacking a longopt and
any documentation.

As per insmod(8) for most use-cases you'd want to use modprobe(8).
Although since the option is available and we have an equivalent in
rmmod(8) having it documented and consistent triumphs.

Signed-off-by: Emil Velikov <[email protected]>
Link: #138
Signed-off-by: Lucas De Marchi <[email protected]>
@lucasdemarchi
Copy link
Contributor

I pushed up the patches that add shell completion. I think we also need to integrate with autotools before merging the rest.

@evelikov evelikov force-pushed the foomod-help-man-shell branch from d98f3a9 to f232ec5 Compare September 22, 2024 10:23
@evelikov
shell-completion/*/lsmod: add bash/fish/zsh completion
6e34d3e 
Note that completions are explicitly aimed to be simple, depending on as
little as possible shell specific helpers.

The goal is that people unfamiliar with these can extend them with zero
ramp-up. Doing things efficiently or "properly" is somewhat secondary.

v2:
 - wire the completions to the autotools build

v3:
 - use SPDX style copyright statements

Signed-off-by: Emil Velikov <[email protected]>
@evelikov
shell-completion/*/rmmod: add bash/fish/zsh completion
f4d6a54 
v2:
 - use e(x)clusive answers for fish, tweak force string

v3:
 - wire the completions to the autotools build

v4:
 - use SPDX style copyright statements

Signed-off-by: Emil Velikov <[email protected]>
@evelikov
shell-completion/*/insmod: add bash/fish/zsh completion
e2d1c0e 
A few inline todos and some odd fish behaviour as mentioned inline.
Otherwise things just work :-)

v2:
 - use e(x)clusive answers for fish, tweak force string

v3:
 - wire the completions to the autotools build

v4:
 - use SPDX style copyright statements

Signed-off-by: Emil Velikov <[email protected]>
@evelikov evelikov force-pushed the foomod-help-man-shell branch from f232ec5 to e2d1c0e Compare September 22, 2024 12:10
lucasdemarchi pushed a commit that referenced this pull request Sep 22, 2024
@evelikov @lucasdemarchi
shell-completion/*/lsmod: add bash/fish/zsh completion
257034a 
Note that completions are explicitly aimed to be simple, depending on as
little as possible shell specific helpers.

The goal is that people unfamiliar with these can extend them with zero
ramp-up. Doing things efficiently or "properly" is somewhat secondary.

v2:
 - wire the completions to the autotools build

v3:
 - use SPDX style copyright statements

Signed-off-by: Emil Velikov <[email protected]>
Link: #138
Signed-off-by: Lucas De Marchi <[email protected]>
lucasdemarchi pushed a commit that referenced this pull request Sep 22, 2024
@evelikov @lucasdemarchi
shell-completion/*/rmmod: add bash/fish/zsh completion
76fbadb 
v2:
 - use e(x)clusive answers for fish, tweak force string

v3:
 - wire the completions to the autotools build

v4:
 - use SPDX style copyright statements

Signed-off-by: Emil Velikov <[email protected]>
Link: #138
Signed-off-by: Lucas De Marchi <[email protected]>
lucasdemarchi pushed a commit that referenced this pull request Sep 22, 2024
@evelikov @lucasdemarchi
shell-completion/*/insmod: add bash/fish/zsh completion
069869f 
A few inline todos and some odd fish behaviour as mentioned inline.
Otherwise things just work :-)

v2:
 - use e(x)clusive answers for fish, tweak force string

v3:
 - wire the completions to the autotools build

v4:
 - use SPDX style copyright statements

Signed-off-by: Emil Velikov <[email protected]>
Link: #138
Signed-off-by: Lucas De Marchi <[email protected]>
@lucasdemarchi
Copy link
Contributor

Applied, thanks.

@evelikov evelikov deleted the foomod-help-man-shell branch September 23, 2024 00:06
@zanchey zanchey mentioned this pull request Nov 7, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lucasdemarchi lucasdemarchi lucasdemarchi left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@evelikov @lucasdemarchi