python utilities script suffix (.py) should be removed as per Policy 10.4

[Eric Desrochers]

[Impact]

The zfs utilities scripts shipped with debian and ubuntu such as :

    * arc_summary.py : Provides a summary of the statistics
    * arcstat.py : Print out ZFS ARC Statistics exported via kstat(1)
    * dbufstat.py : Print out statistics for all cached dmu buffers

should be renamed according to Policy 10.4 :
https://www.debian.org/doc/debian-policy/ch-files.html#s-scripts

"When scripts are installed into a directory in the system PATH, the script name should not include an extension such as .sh or .pl that denotes the scripting language currently used to implement it."

Ideally, this should be fixed and upstreamed to Debian (or to zfslinux upstream) as well.

[Test Case]

 * Install zfsutils-linux
 * List files

 $ dpkg -L zfsutils-linux | egrep "arc|dbufstat"
   /usr/sbin/arc_summary.py
   /usr/sbin/arcstat.py
   /usr/sbin/dbufstat.py

[Regression Potential]

 * none expected, this is a trivial change that simply rename the scripts to remove the extension (.py) with a simple 'mv' before the dh_install invocation in order to get this expected result :

$ dpkg -L zfsutils-linux | egrep -i "arc|dbuf"
  /usr/sbin/arc_summary
  /usr/sbin/arcstat
  /usr/sbin/dbufstat

Yakkety include the renaming as shown above.

Xenial will have a symlink to not break user experience until next release upgrade:
https://bugs.launchpad.net/ubuntu/xenial/+source/zfs-linux/+bug/1628279/comments/19

So user will be able to use both for instance : arcstat.py or arcstat

[Other Info]

 * Justification/argument for Xenial
https://bugs.launchpad.net/ubuntu/+source/zfs-linux/+bug/1628279/comments/13
https://bugs.launchpad.net/ubuntu/+source/zfs-linux/+bug/1628279/comments/14

* Debian bug
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=839645

 * Upstream (ZoL) is reluctant to change it at upstream code level.
https://bugs.launchpad.net/ubuntu/+source/zfs-linux/+bug/1628279/comments/2

 * This has been flag by Adam Conrad
https://bugs.launchpad.net/ubuntu/+source/zfs-linux/+bug/1574342/comments/19

 * Debian policy
https://www.debian.org/doc/debian-policy/ch-files.html#s-scripts

[Eric Desrochers (slashd)]

It seems like "dh-exec" might do the trick.

http://manpages.ubuntu.com/manpages/xenial/man1/dh-exec-install.1.html

Example :
#! /usr/bin/dh-exec
debian/default.conf => /etc/my-package/start.conf

[Richard Laager (rlaager)]

I just talked to Brian Behlendorf (since I'm sitting next to him at the OpenZFS Developer Summit). He doesn't have strong feelings either way, but is concerned about breaking people who are already relying on those names.

My thought is we should just fix it in Debian/Ubuntu regardless of whether it is fixed upstream.

[Eric Desrochers (slashd)]

rlaager, yes, this is reasonable.

[Eric Desrochers (slashd)]

I look at the dh_exec approach and that requires the following :

· The package must be using compatibility level 9 or later (see debhelper(7))
· The package will need a build-dependency on dh-exec.
· The install file must be marked as executable.

Debian unstable is already at compat "9"

v9 also seems to be the recommended mode of operation.

reference:
https://www.mankier.com/1/dh_install
https://www.mankier.com/7/debhelper

The other method, would be to rename the utilities during the debian/rules "build" target before you get to the dh_install invocation.

I'll have a look at both approaches.

Eric

[Eric Desrochers (slashd)]

Note that this is based on a comment previously made by Adam that can be found here :

https://bugs.launchpad.net/ubuntu/+source/zfs-linux/+bug/1574342/comments/19

Eric

[Eric Desrochers (slashd)]

Following a discussion with Aron Xu, and he feels more comfortable with the approach to rename the utilities during the debian/rules "build" target before you get to the dh_install invocation.

Richard Laager, feel free to resubmit the patch that you did, otherwise I'll do it on your behalf.

Eric

[Richard Laager (rlaager)]

The previous patch was here:
https://launchpadlibrarian.net/275709492/zfs-fix-filenames.debdiff

It probably still applies cleanly (aside for the changelog).

[Eric Desrochers (slashd)]

Thanks Richard,

I'll do the SRU on your behalf.

Eric

[Eric Desrochers (slashd)]

Richard the patch is working as expected

Confirmation :

$ dpkg -L zfsutils-linux | egrep -i "arc|dbuf"
/usr/sbin/arc_summary
/usr/sbin/arcstat
/usr/sbin/dbufstat

I'll go ahead and start the SRU for Yakkety (16.10).
Since this is not a "bug", I will only submit it for the devel release.

Aron Xu is supposed to take care to update debian pkg.

Eric

[Eric Desrochers (slashd)]

debdiff for Yakkety (16.10)

[Aron Xu (happyaron)]

Uploaded, needs approval from queue because of freeze.

[Eric Desrochers (slashd)]

Thanks Aron.

[Richard Laager (rlaager)]

Once this is in Yakkety, I think we should SRU it to Xenial. The sooner we change this, the less likelihood we have people relying on the old names (with .py) of these utilities.

@kirkland, it'd be really nice to address this before ZFS lands in the default Server Seed for 16.04.

[Eric Desrochers (slashd)]

@rlaager
@kirkland
@happyaron

The SRU are usally for "High-impact bugs"... but the current SRU for Xenial might possibly fit in section :

2. When
2.2. Other safe cases

For Long Term Support releases we sometimes want to introduce new features. They must not change the behaviour on existing installations (e. g. entirely new packages are usually fine). If existing software needs to be modified to make use of the new feature, it must be demonstrated that these changes are unintrusive, have a minimal regression potential, and have been tested properly. To avoid regressions on upgrade, any such feature must then also be added to any newer supported Ubuntu release. Once a new feature/package has been introduced, subsequent changes to it are subject to the usual requirements of SRUs to avoid regressions.

[1] - https://wiki.ubuntu.com/StableReleaseUpdates

Eric

[Launchpad Janitor (janitor)]

This bug was fixed in the package zfs-linux - 0.6.5.8-0ubuntu4

---------------
zfs-linux (0.6.5.8-0ubuntu4) yakkety; urgency=low

  [Richard Laager]
  * Remove .py extension from utilities in /usr/sbin as per policy 10.4 Scripts (LP: #1628279)

 -- Eric Desrochers <email address hidden> Fri, 30 Sep 2016 07:47:54 -0400

[Eric Desrochers (slashd)]

@rlaager,

I will submit a debdiff for Xenial, as soon as my other SRU[1] for zfs-linux will be completed.

[1] - https://bugs.launchpad.net/ubuntu/+source/zfs-linux/+bug/1607920

Eric

[Robie Basak (racb)]

13:48 <slashd> Hi SRU vanguard, could you please nominate LP: #1628279 affecting Xenial ?

13:48 <ubot5> Launchpad bug 1628279 in zfs-linux (Ubuntu) "python utilities script suffix (.py) should be removed as per Policy 10.4" [Low,Fix released] https://launchpad.net/bugs/1628279

13:48 <apw> slashd, done

13:49 <slashd> apw, tks

14:00 <rbasak> slashd: won't that regress Xenial users by definition?

14:01 <apw> yep, looks like it will ... perhaps we should be offering them under both names in this

14:02 <rbasak> Yeah, adding symlinks makes sense. Then users can rely on the new names, but users relying on the old names won't break before a release upgrade.

14:02 <apw> rbasak, perhaps copy that into the bug, i do not believe they have done the work on teh sru yet

14:02 <rbasak> Will do

[Eric Desrochers (slashd)]

Debian bug :
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=839645

[Eric Desrochers (slashd)]

@rbasak, yes symlink is the best way to not break user experience until release upgrade.

dpkg -L zfsutils-linux | egrep "arc|dbuf" | sort
/usr/sbin/arcstat
/usr/sbin/arcstat.py
/usr/sbin/arc_summary
/usr/sbin/arc_summary.py
/usr/sbin/dbufstat
/usr/sbin/dbufstat.py

# ls -altr /usr/sbin/arcstat
lrwxrwxrwx 1 root root 10 Oct 3 11:10 /usr/sbin/arcstat -> arcstat.py

# ls -altr /usr/sbin/arc_summary
lrwxrwxrwx 1 root root 14 Oct 3 11:10 /usr/sbin/arc_summary -> arc_summary.py

# ls -altr /usr/sbin/dbufstat
lrwxrwxrwx 1 root root 11 Oct 3 11:10 /usr/sbin/dbufstat -> dbufstat.py

Eric

[Eric Desrochers (slashd)]

As per irc conversation with apw, I'm merging both LPs #1607920 & #1628279 into one debdiff.

Reference:
https://bugs.launchpad.net/ubuntu/xenial/+source/zfs-linux/+bug/1607920/comments/17

Eric

[Andy Whitcroft (apw)]

Hello Eric, or anyone else affected,

Accepted zfs-linux into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/zfs-linux/0.6.5.6-0ubuntu14 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Eric Desrochers (slashd)]

I confirmed it works as expected :

root@test01:~# dpkg -l | grep -i zfsutils-linux
ii zfsutils-linux 0.6.5.6-0ubuntu14 amd64 Native OpenZFS management utilities for Linux

root@test01:~# cat /etc/lsb-release
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=16.04
DISTRIB_CODENAME=xenial
DISTRIB_DESCRIPTION="Ubuntu 16.04 LTS"

root@test01:~# ls -altr /usr/sbin/arc*
-rwxr-xr-x 1 root root 40752 Oct 5 09:05 /usr/sbin/arc_summary.py
-rwxr-xr-x 1 root root 13058 Oct 5 09:05 /usr/sbin/arcstat.py
lrwxrwxrwx 1 root root 14 Oct 5 09:05 /usr/sbin/arc_summary -> arc_summary.py
lrwxrwxrwx 1 root root 10 Oct 5 09:05 /usr/sbin/arcstat -> arcstat.py
root@test01:~# ls -altr /usr/sbin/dbufstat
lrwxrwxrwx 1 root root 11 Oct 5 09:05 /usr/sbin/dbufstat -> dbufstat.py

Eric

[Richard Laager (rlaager)]

This is installing the files using the old names, and symlinking using the new ones. Is that intentional? I expected it would install using the new names and symlinks for the old ones.

[Eric Desrochers (slashd)]

@rlaager, as you know, the upstream source code has the suffix (.py)

In order to do less manipulation since this is not fixing anything broken, I symlinked that way using dh_links.

IMHO, the symlink order doesn't really matter since the result will be the same either way, to have both old name and new name working until release upgrade.

(Note that starting with Yakkety ONLY the new name will be available.)

If users uses [Tab] key to auto-complete the command sequence, the script without .py will come up first.

I don't see how changing the symlink could give special importance or prominence to the new name.

Eric

[Launchpad Janitor (janitor)]

This bug was fixed in the package zfs-linux - 0.6.5.6-0ubuntu14

---------------
zfs-linux (0.6.5.6-0ubuntu14) xenial; urgency=medium

  * Fix zfs services to fail on first boot due to zfs services starting up
    before /etc/mtab has a chance to be symlinked to /proc/mounts. (LP: #1607920)
    (upstream commit 792517389fad5c495a2738b61c2e9c65dedaaa9a)

  * Symlink utilities in /usr/sbin to remove suffix as per policy 10.4 Scripts (LP: #1628279)
    Users can rely on the new names, but relying on the old names won't break before
    release upgrade.

 -- Eric Desrochers <email address hidden> Tue, 04 Oct 2016 15:07:52 +0200

[Brian Murray (brian-murray)]

The verification of the Stable Release Update for zfs-linux has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.