Install bashcompletion file in appropriate location #15304
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 3 commits
September 21, 2023 16:06
Certain Linux distributions (Debian/Ubuntu at least) expect bash-completion snippets to be installed in /usr/share/bash-completion/completions instead of /etc/bash_completion.d. This patch sets the bashcompletiondir variable based on the vendor, inspired by similar settings for initdir and initconfdir. It seems that commit 612b8df caused the file to be installed in the first-place (thus the error when building debian packages only became apparent when testing a 2.2.0-rc4 build) The change only sets the variable in Makefile context - the rpm/zfs.spec.in file has the path hardcoded as %{_sysconfdir}/bash_completion.d/zfs, but since running ``` ./configure --sysconfdir=/myetc ; make rpm ``` also results in all relevant files to be installed in /etc instead of /myetc I assume this can remain as is. Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
Follows b191f9a13d3005621ead9a727b811892264505ef from Debian's packaging team at: https://salsa.debian.org/zfsonlinux-team/zfs/ The previous build-dependency is kept as option, to still be able to build on older Debian based distros (e.g. Ubuntu 20.04). Without this building on Debian 12/bookworm does not work, as `dkms` is a virtual package. Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
tested by running: ``` ./configure --with-config=user; cp -a contrib/debian . dpkg-buildpackage -b -uc -us ``` on a Debian 12 based system. and checking where the completion file got installed. Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
siv0
force-pushed
the
fix_bashcompletion_dir_defaults
branch
from
9b51ef1 to
cc132e3
Compare
|
@usaleem-ix would you mind reviewing this. |
usaleem-ix
approved these changes
Oct 2, 2023
behlendorf
added
Status: Accepted
behlendorf
pushed a commit
that referenced
this pull request
Oct 3, 2023
Follows b191f9a13d3005621ead9a727b811892264505ef from Debian's packaging team at: https://salsa.debian.org/zfsonlinux-team/zfs/ The previous build-dependency is kept as option, to still be able to build on older Debian based distros (e.g. Ubuntu 20.04). Without this building on Debian 12/bookworm does not work, as `dkms` is a virtual package. Reviewed-by: Umer Saleem <usaleem@ixsystems.com> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com> Closes #15304
behlendorf
pushed a commit
that referenced
this pull request
Oct 3, 2023
tested by running: ``` ./configure --with-config=user; cp -a contrib/debian . dpkg-buildpackage -b -uc -us ``` on a Debian 12 based system. and checking where the completion file got installed. Reviewed-by: Umer Saleem <usaleem@ixsystems.com> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com> Closes #15304
behlendorf
pushed a commit
to behlendorf/zfs
that referenced
this pull request
Oct 3, 2023
Certain Linux distributions (Debian/Ubuntu at least) expect bash-completion snippets to be installed in /usr/share/bash-completion/completions instead of /etc/bash_completion.d. This patch sets the bashcompletiondir variable based on the vendor, inspired by similar settings for initdir and initconfdir. It seems that commit 612b8df caused the file to be installed in the first-place (thus the error when building debian packages only became apparent when testing a 2.2.0-rc4 build) The change only sets the variable in Makefile context - the rpm/zfs.spec.in file has the path hardcoded as %{_sysconfdir}/bash_completion.d/zfs, but since running ``` ./configure --sysconfdir=/myetc ; make rpm ``` also results in all relevant files to be installed in /etc instead of /myetc I assume this can remain as is. Reviewed-by: Umer Saleem <usaleem@ixsystems.com> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com> Closes openzfs#15304
behlendorf
pushed a commit
to behlendorf/zfs
that referenced
this pull request
Oct 3, 2023
Follows b191f9a13d3005621ead9a727b811892264505ef from Debian's packaging team at: https://salsa.debian.org/zfsonlinux-team/zfs/ The previous build-dependency is kept as option, to still be able to build on older Debian based distros (e.g. Ubuntu 20.04). Without this building on Debian 12/bookworm does not work, as `dkms` is a virtual package. Reviewed-by: Umer Saleem <usaleem@ixsystems.com> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com> Closes openzfs#15304
behlendorf
pushed a commit
to behlendorf/zfs
that referenced
this pull request
Oct 3, 2023
tested by running: ``` ./configure --with-config=user; cp -a contrib/debian . dpkg-buildpackage -b -uc -us ``` on a Debian 12 based system. and checking where the completion file got installed. Reviewed-by: Umer Saleem <usaleem@ixsystems.com> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com> Closes openzfs#15304
behlendorf
pushed a commit
that referenced
this pull request
Oct 3, 2023
Certain Linux distributions (Debian/Ubuntu at least) expect bash-completion snippets to be installed in /usr/share/bash-completion/completions instead of /etc/bash_completion.d. This patch sets the bashcompletiondir variable based on the vendor, inspired by similar settings for initdir and initconfdir. It seems that commit 612b8df caused the file to be installed in the first-place (thus the error when building debian packages only became apparent when testing a 2.2.0-rc4 build) The change only sets the variable in Makefile context - the rpm/zfs.spec.in file has the path hardcoded as %{_sysconfdir}/bash_completion.d/zfs, but since running ``` ./configure --sysconfdir=/myetc ; make rpm ``` also results in all relevant files to be installed in /etc instead of /myetc I assume this can remain as is. Reviewed-by: Umer Saleem <usaleem@ixsystems.com> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com> Closes #15304
behlendorf
pushed a commit
that referenced
this pull request
Oct 3, 2023
Follows b191f9a13d3005621ead9a727b811892264505ef from Debian's packaging team at: https://salsa.debian.org/zfsonlinux-team/zfs/ The previous build-dependency is kept as option, to still be able to build on older Debian based distros (e.g. Ubuntu 20.04). Without this building on Debian 12/bookworm does not work, as `dkms` is a virtual package. Reviewed-by: Umer Saleem <usaleem@ixsystems.com> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com> Closes #15304
behlendorf
pushed a commit
that referenced
this pull request
Oct 3, 2023
tested by running: ``` ./configure --with-config=user; cp -a contrib/debian . dpkg-buildpackage -b -uc -us ``` on a Debian 12 based system. and checking where the completion file got installed. Reviewed-by: Umer Saleem <usaleem@ixsystems.com> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com> Closes #15304
lundman
pushed a commit
to openzfsonwindows/openzfs
that referenced
this pull request
Dec 12, 2023
Certain Linux distributions (Debian/Ubuntu at least) expect bash-completion snippets to be installed in /usr/share/bash-completion/completions instead of /etc/bash_completion.d. This patch sets the bashcompletiondir variable based on the vendor, inspired by similar settings for initdir and initconfdir. It seems that commit 612b8df caused the file to be installed in the first-place (thus the error when building debian packages only became apparent when testing a 2.2.0-rc4 build) The change only sets the variable in Makefile context - the rpm/zfs.spec.in file has the path hardcoded as %{_sysconfdir}/bash_completion.d/zfs, but since running ``` ./configure --sysconfdir=/myetc ; make rpm ``` also results in all relevant files to be installed in /etc instead of /myetc I assume this can remain as is. Reviewed-by: Umer Saleem <usaleem@ixsystems.com> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com> Closes openzfs#15304
lundman
pushed a commit
to openzfsonwindows/openzfs
that referenced
this pull request
Dec 12, 2023
Follows b191f9a13d3005621ead9a727b811892264505ef from Debian's packaging team at: https://salsa.debian.org/zfsonlinux-team/zfs/ The previous build-dependency is kept as option, to still be able to build on older Debian based distros (e.g. Ubuntu 20.04). Without this building on Debian 12/bookworm does not work, as `dkms` is a virtual package. Reviewed-by: Umer Saleem <usaleem@ixsystems.com> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com> Closes openzfs#15304
lundman
pushed a commit
to openzfsonwindows/openzfs
that referenced
this pull request
Dec 12, 2023
tested by running: ``` ./configure --with-config=user; cp -a contrib/debian . dpkg-buildpackage -b -uc -us ``` on a Debian 12 based system. and checking where the completion file got installed. Reviewed-by: Umer Saleem <usaleem@ixsystems.com> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com> Closes openzfs#15304
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patchset is the result of trying to build Debian Packages for 2.2.0-rc4 for Proxmox[0]
The changes in debian/contrib were only targeted at the fix (I did not try to install the resulting packages)
[0] https://git.proxmox.com/?p=zfsonlinux.git;a=tree
Motivation and Context
Somewhere between zfs-2.1.12 and zfs-2.2.0-rc4 a change caused the bash completion to
actually get installed to DESTDIR - I assume 612b8df
introduced it.
Instead of working around the different location used in Debian based distributions (/usr/share/bash-completion/completions/, instead of /etc/bash_completion.d) in debian/rules [1] , setting the path during ./configure seems preferable)
[1]
zfs/contrib/debian/rules.in
Line 75 in 4647353
(similar workaround can be found in Debian upstream and Proxmox rules files)
Description
commit 1/3 sets the bashcompletiondir variable based on the vendor, inspired by similar settings for initdir and initconfdir
commit 2/3 is a small update to contrib/debian/control, in order to make building packages work on Debian Bookworm (the dkms package is virtual and should be replaced by dh-sequence-dkms)
commit 3/3 adapts contrib/debian/rules to not move the bashcompletion file around anymore
How Has This Been Tested?
on Debian 12 I ran:
git clean -fdx; ./autogen.sh; ./configure --with-config=user; cp -a contrib/debian .; dpkg-buildpackage -b -uc -us
(the --with-config=user was a shorter way instead of providing the custom kernel-header location in Proxmox VE)
on Fedora 38:
git clean -fdx; ./autogen.sh ; ./configure ; make rpm
and verifying that the bash completion file was installed in the correct location
Types of changes
Checklist:
Signed-off-by.