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.

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

contrib: link zpool to zfs in bash-completion #16376

Merged
merged 1 commit into from
Aug 5, 2024

Conversation

Harry-Chen
Copy link
Contributor

Motivation and Context

Currently user won't have completion of zpool command until they trigger completion of zfs first. This patch adds a link to zfs, thus user can use both to initialize the completion.

Fixes: #16320

Description

Create a symlink to zfs as zpool under contrib/bash_completion.d.

How Has This Been Tested?

Tested the whole configuration, compilation and installation process. It wors as expected. No other functionality is affected.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

@tonyhutter tonyhutter added the Status: Code Review Needed Ready for review and testing label Jul 25, 2024
@tonyhutter
Copy link
Contributor

Can we get more more reviewer on this PR?

Copy link
Member

@robn robn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It hardly matters, and I have no real opinion nor any idea what good practice is for completion scripts, but:

Would it be better to install this as a hard link instead of a symlink, or even a whole separate copy of the file?

@Harry-Chen
Copy link
Contributor Author

It hardly matters, and I have no real opinion nor any idea what good practice is for completion scripts, but:
Would it be better to install this as a hard link instead of a symlink, or even a whole separate copy of the file?

I look at /usr/share/bash-completion/completions on my debian system, and many packages choose to use a symlink, e.g. vg* all go to lvm, and lxc_* go to _lxc. I think making hard links or copying might be confusing to users, and a symlink clearly shows the relationship between files.

@robn
Copy link
Member

robn commented Aug 1, 2024

Works for me, thanks for checking 👍

@Harry-Chen Harry-Chen mentioned this pull request Aug 1, 2024
13 tasks
@tonyhutter
Copy link
Contributor

For some reason the Centos and Fedora builders were failing. I just started a re-run of those tests. If all goes well I will merge the PR.

@Harry-Chen
Copy link
Contributor Author

For some reason the Centos and Fedora builders were failing. I just started a re-run of those tests. If all goes well I will merge the PR.

There is an error

  CC       contrib/pam_zfs_key/pam_zfs_key_la-pam_zfs_key.lo
ln -s zfs contrib/bash_completion.d/zpool
ln: failed to create symbolic link 'contrib/bash_completion.d/zpool': File exists
make[4]: *** [Makefile:14165: contrib/bash_completion.d/zpool] Error 1

zpool is created and preserved in the release tarball, thus causing the File exists error when being created again.

Sorry for not finding such situation, I will fix it soon.

Currently user won't have completion of `zpool` command until they
trigger completion of `zfs` first. This patch adds a link to `zfs`,
thus user can use both to initialize the completion.

Fixes: openzfs#16320

Signed-off-by: Shengqi Chen <harry-chen@outlook.com>
@Harry-Chen
Copy link
Contributor Author

The building issue is now fixed. @tonyhutter

@tonyhutter tonyhutter merged commit 46ebd0a into openzfs:master Aug 5, 2024
20 of 24 checks passed
@Harry-Chen Harry-Chen deleted the zpool-bash-completion branch August 5, 2024 16:50
@Harry-Chen Harry-Chen mentioned this pull request Aug 23, 2024
13 tasks
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Sep 4, 2024
Currently user won't have completion of `zpool` command until they
trigger completion of `zfs` first. This patch adds a link to `zfs`,
thus user can use both to initialize the completion.

Fixes: openzfs#16320

Signed-off-by: Shengqi Chen <harry-chen@outlook.com>
Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Rob Norris <rob.norris@klarasystems.com>
Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Code Review Needed Ready for review and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

zpool <TAB> completion only works after running zfs <TAB>
4 participants