-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
zfs: divide under subpackages #42484
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the point of this? We aren't Debian and don't split packages into minuscule sub-packages. Most of these packages function as a unit anyway. What good is the DKMS package without the userspace utilities, and vice versa?
You could make a case that the tests might be split (or at least toggled with a build option), but the rest seems superfluous and undesirable.
Also, especially when completely refactoring a package, please tag its maintainer.
cc: @Vaelatern
conf_files="/etc/zfs/zed.d/zed.rc" | ||
short_desc="Z File System -- userland, pyzfs, and kernel modules (using DKMS)" | ||
makedepends="pam-devel zlib-devel libuuid-devel libblkid-devel libtirpc-devel attr-devel eudev-libudev-devel" | ||
short_desc="Z File System --" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This never gets overwritten in the main package and leaves an incomplete package description.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahh, this is valid. I will fix it.
|
||
replaces="spl<=0.7.13_1" # Because SPL was merged into zfs in 0.8.0 | ||
# the zfs-tools package needs to be done always the last! | ||
subpackages="libzfs zfs-devel zfs-dkms zfs-pam zfs-test zfs-zed zfs-tools" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Relying on specific ordering for subpackages is incredibly fragile and should not be required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no other way to force the zfs-tools be the last. Otherwise all the specific files would need to be defined, which is then much more fragile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not more fragile, just more explicit and tedious. In any case, the "catchall" package should always be the primary package, which is what you have left after stripping out any specific subpackages.
@@ -61,16 +57,23 @@ post_install() { | |||
rm -rf ${DESTDIR}/usr/share/zfs/zfs-tests | |||
} | |||
|
|||
zfs_package() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function will never run, see
https://github.com/void-linux/void-packages/actions/runs/4283529441/jobs/7459365515#step:7:7990
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. On local xbps-src it is built. I have tested this completely and it is working as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It didn't build locally, it is just shadowed by the main package that shares its name.
The main point is to have a support for custom kernel with zfs built in. I came from Gentoo and have a long history of making my own kernel which then boots not only the Gentoo, but even others. It is easy to mask the linux package, but I'm not able to install zfs userland. Actually there are two things, kernel module and userland tools packed to one tar. And the userland tools does not need to be strictly 1:1 to the kernel ZFS version (to some point). I was thinking about opting that out, but since all of this is can be packed to different packages, this seems to be much better to do it that way. I'm not insisting on this change and I'm free to keep it myself for myself. I tried to do that the way it can be shared with others. |
I am strictly opposed to a package graph that facilitates these kinds of mismatches. Ubuntu has been bitten HARD by shipping different versions of the ZFS kmod and userspace utilities. Because we strive for a first-class ZFS experience in Void, the userspace tools and DKMS package that we ship should definitely be part of a matched pair. Just because our package installs the DKMS sources doesn't mean that you have to build them. Void kernel packages will build the kmod automatically, but your kernel workflow can skip the XBPS hooks. You can also |
Thank you for your contribution! Unfortunately, there are some red flags here that can not be cleaned up. ZFS lives in that special place in our systems where, if it dies, we end up with massive cleanup efforts (and probably restoration from backups). As such, anything that threatens stability, even so innocuous as separating userspace commands and kernel modules installed (which is arguably tested... in one direction), risks data. I'm afraid we won't be able to merge this into Void. |
I understand the intention to keep this as simple as possible from the maintenance point of view. I have also refactored it the way it would not interfere the stock users and allow customization if wanted. I definitely do not want to brake something in spite of some custom users. I understand the noextract option, but wouldn't be viable just create subpackages for stuff that is able to be "noextracted" with nearly no cost? Or if it is some non-written convention in Void not to create subpackages much (as for example Alpine do) I would understand that. Wouldn't then be an option to keep the zfs-userland in zfs package and only create subpackages with dkms and zed? The zfs-test is something that should not be present on any semi-production/user system anyway (but this is true also for the zinject). With the version incompatibility breakage I remember that form the time of 0.6 versions myself, but since 2.0 or 2.1 I have never see any commit in 2.x branch to break the interoperability between kernel-useralnd on the same branch. And it is not an intention of this refactor. DKMS users will still have the same version on both and the ones that build the zfs inside kernel just need to care themselves. |
Ok I undestand that. |
The main intention of this PR is to separate the zfs tools from the kernel module. This change introduce several new subpackages for the
zfs
package.Namely:
The original zfs package remain as a metapackage for the needed subpackages. This helps the transition (eg: for those having zfs installed, the system update will only introduce several new packages and will work as before). It is now possible to install purely the zfs-tools package if a non-distributed kernel is built with zfs.
I have outplaced the ztest to a new subpackage which is not as part of the zfs metapackage any more. Ztest is intended to be run only on testing systems.
I may provide additional information if needed. For example the contents of the packages, etc.
Testing the changes
Local build testing