[Pkg-zfsonlinux-devel] Bug#686447: [RFC] First release of spl-dkms and zfs-linux packages for Debian

Fri Feb 15 23:18:04 UTC 2013

Hi!

An update here.

I was a bit busy later. Today I was talking with Aron on IRC and we
agreed that we will push your repository on Alioth in order to keep the
full history.

In fact is already there:

http://anonscm.debian.org/gitweb/?p=pkg-zfsonlinux/zfs.git
http://anonscm.debian.org/gitweb/?p=pkg-zfsonlinux/spl.git

And we will start from this codebase.

I will be rebasing some of the changes I did on a separate branch (and
splitting them in small commits) so we could discuss later each one of
this changes.

See below for the inline replies to your last mail:

On 16/12/12 09:19, Darik Horn wrote:
>> Strip from spl-dkms all files not related to kernel modules.
> 
> Why are you removing copyright attributions like the AUTHORS file?
> This could upset ZoL contributors and cause legal exposure.
> 
> 

I thought that debian/copyright file would be enough to give credit to
the authors of the software.

However you are right. A simple "apt-file search AUTHORS" give me more
than enough reasons to keep this file.

>> Rewrite postinst helper that ensures that /etc/hostid is valid and will remain constant across reboots.
> 
> The __BYTE_ORDER__ test is interesting.  I will likely add it to pkg-spl.
> 
> However, randomizing the hostid violates the principle of least
> astonishment because it causes a zpool.cache mismatch that breaks
> subsequent imports, and it can break license management for non-Debian
> software.
> 
> Stabilizing the hostid is safe, but changing the hostid is unsafe for
> the same reason that randomizing a missing hostname is wrong.
> 
> 

I'm only randomizing it when the current host's hostid is "0xffffffff",
which I understand is an invalid hostid for ZFS and would case it to
stop working properly. Isn't this the case?

> The pristine-tar branch already exists in pkg-spl and pkg-zfs.  Using
> the pristine-tar facility is certainly correct, but not currently
> practical for doing the frequent releases that ZoL users expect.

We should agree on a common way of working.

Either we use pristine-tar or not.

I'm relative new to use git for Debian packages. So I'm open to follow
yours and Aron advice.

> 
> 
>> Fix clean target and use dh_autoreconf
> 
> This breaks backports for Lucid (and its derivatives) because
> dh-autoreconf is a non-main package on those systems.  Keeping
> compatibility with all officially supported Ubuntu variants is
> worthwhile and something that I want to do.
> 
> 

Well. I love to have things as clean and small as possible.
dh_autoreconf helps with that. But I understand your point. Not big deal.

>> Update debian/watch to track upstream official release tarballs
> 
> Is the Github redirector fully obsolete?  (nb:
> http://wiki.debian.org/debian/watch/)
> 
> The pkg-spl and pkg-zfs watch files were added after an earlier
> private ITP review.
> 
> 

github redirector is not longer needed, so why use it?

http://wiki.debian.org/debian/watch?action=diff&rev2=10&rev1=9

Also the url on the debian/watch on your packages is not working.

This is what the current master on Alioth (your package) reports:

$ uscan --report-status
Processing watchfile line for package zfs-linux...
Newest version on remote site is 0~master, local version is 0.6.0.97
zfs-linux: remote site does not even have current version

This is what the package that I did previously reports:

$ uscan --report-status
Processing watchfile line for package zfs-linux...
Newest version on remote site is 0.5.2, local version is 0.6.0~rc12
zfs-linux: remote site does not even have current version

>> Strip from zfs-dkms all files not related to kernel modules.
>> Clean debian directory for unneeded *.docs
>> (copyright notices should be added to debian/copyright properly)
> 
> The OPENSOLARIS.LICENSE file should be unmodified and bundled in every
> ZFS package, even if the CDDL is duplicated in the debian/copyright
> file.
> 
> Modifying or omitting Oracle legal notices will attract Oracle
> lawyers.  Saving less than 64 kilobytes of boilerplate per
> installation is just not worth the risk.
> 
> 
Ok.

>> Add zfs-linux metapackage for convenience to install all ZFS
> 
> Consider naming this debian-zfs to fit the naming convention of other
> meta packages already in distribution, and to better accommodate the
> kFreeBSD platform in case the meta package can be shared.
> 
> Big or important source packages do not typically provide their own
> meta.  Doing this makes it more difficult for large sites to do local
> overrides and customization.  (And it follows that I should rename the
> ubuntu-zfs source package to something like meta-ubuntu-zfs for better
> conformance.)
> 
> 

I don't see the point of sharing such metapackage with kFreeBSD. The
whole point of the metapackage is to pull the right versions of the spl
and zfs dkms modules (which are linux specific) plus the right versions
of the user space tools that are also linux specific.

>> ensure dependencies are also always updated to the right version.
> 
> This reintroduces a dkms ordering bug where the zfs build races the
> spl build.  Notice how the BUILD_DEPENDS directive is handled by dkms.
> 
> 

Is that a bug on dkms? was reported?

>> General cleaning of files not needed (dracut/sudoers.d/...)
> 
> These things were submitted by new ZoL contributors.  Stripping them
> discourages further contribution from these people.
> 

I don't agree in this.

Shipping a commented file in /etc/sudoers.d will only cause trouble when
the package is upgraded and tries to overwrite your local changes.

The right place for such file would be  /usr/share/doc/$package/examples/

About dracut helpers, that should be moved to another package
(zfs-dracut) as there is already one zfs-initramfs.

But, honestly, given the popularity of dracut inside Debian/Ubuntu, I
won't spend time on this. However I don't have problems if you want to
work on it.

http://qa.debian.org/popcon.php?package=dracut

>> Add a debconf helper that checks if the running kernel is a 64-bit one.
>> If it detects that the kernel is 32-bit or it couldn't detect the kind of kernel
>> shows a warning to the user asking for confirmation before continuing.
> 
> I added this kind of nagging to some private builds and got negative
> feedback. YMMV. Consider disabling second-class architectures entirely
> because Debian publishes updates very slowly between major releases.
> 

IMHO enabling second-class architectures (non-x86) is a goal to achieve.
It would help to find bugs on the codebase.

Debian publishes updates very slowly between major releases? I don't
understand what you mean with this.

> Double-check that the debconf can handle a non-default
> /etc/dkms/framework.conf file.  The "/boot/config-$(uname -r)" test
> could be problematic.
> 

That check don't requires dkms for nothing. It basically checks in your
kernel config if you are running a 32 bit kernel.

If it detects your kernel is a 32 bit one, it requires you to explicit
accept the warning message.

If it is unable to find your kernel config then it prints another
warning saying that it couldn't detect if your kernel is 32 or 64 bit,
and that you should only install this on a 64-bit kernel.

The warning is only show once. Once you have accepted it, it won't show
anymore whenever you upgrade or reinstall.

I understand that this could be annoying, but this is exactly for what's
intended. Better annoy people when they install the package for the firs
time, that let them run this without knowing that it could cause data
corruption or instability on their systems on the long term.

> 
>> Add /etc/init.d/zfs and remove /etc/init.d/{zfs-mount,zfs-share}.
>> There is not need at all for two different initscripts.
> 
> This races on systems that have event driven init stacks like upstart
> or systemd, and it can break in a regular sysv init stack because
> networking can come online a long time after local storage is ready.
> 
> What happens if /etc/fstab has a legacy line that causes an automatic
> import before /etc/init.d/zfs is called?
> 
> What happens if zfs invokes /usr/bin/net or /usr/sbin/exportfs before
> the network comes up?
> 
> What happens if /tmp, /usr, or /var is on a zfs mount point?
> 
> 

Ok. Makes sense.

>> Integrate all lib* packages into libzfs1. This keep the package cleaner.
>> To me seems overkill have one package for each .so file
> 
> The libnvpair and libzfs packages are separate in all other ZFS
> implementations, and I don't see the benefit in doing something
> unusual for Debian.  Note that the current library breakout was
> approved by upstream.
> 
> 
>> when there is no real benefit (I don't expect any other package other than
>> zfsutils to link against this libraries)
> 
> Why do you expect that ZFS libraries will not be linked by other
> packages?  At least one person has mentioned on the discussion list
> that they are working on a web interface, somebody else is working on
> gparted and nagios integration, and there are several commercial
> efforts doing things on top of ZoL.
> 
> 

Ok. Makes sense.

>> Many other minor cleans/fixes
> 
> The total diff is 6,515 lines.  Splitting functional changes into
> separate commits would be easier to review.  Right now:
> 
> * General compatibility with Ubuntu and Linux Mint is broken.
> * Upgrades to existing systems are broken.
> * Third party consumers are broken.
> 

Yes.

I will be rebasing some of this work on top of the current master that
is on Alioth on a new branch.

And will be posting a mail with a summary of the changes on the mailing
list pkg-zfsonlinux-devel at lists.alioth.debian.org to discuss them prior
merging it.

I will do it on small iterations to avoid this kind of big mails that
are hard to follow.

PS: Darik, subscribe yourself to
pkg-zfsonlinux-devel at lists.alioth.debian.org if you are not already.

Regards!
--------

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 900 bytes
Desc: OpenPGP digital signature
URL: <http://lists.alioth.debian.org/pipermail/pkg-zfsonlinux-devel/attachments/20130216/00ca5f2b/attachment.pgp>