[needs packaging] xe-guest-utils

Bug #1459455 reported by Ben Howard
14
This bug affects 1 person
Affects Status Importance Assigned to Milestone
xe-guest-utilities (Ubuntu)
Fix Released
High
Unassigned
Wily
Fix Released
High
Unassigned

Bug Description

xe-guest-utilities is a set of user-land scripts for running Linux on Citrix Xen Server. This package is needed for Hardware/Cloudware enablement.

Scripts for monitoring Virtual Machines on a Xen Hypervisor. It Writes distribution version information and IP address to XenStore.

Tags: bot-comment
Revision history for this message
Ben Howard (darkmuggle-deactivatedaccount) wrote :
Changed in ubuntu:
assignee: nobody → Ben Howard (utlemming)
description: updated
Revision history for this message
Ubuntu Foundations Team Bug Bot (crichton) wrote :

Thank you for taking the time to report this bug and helping to make Ubuntu better. It seems that your bug report is not filed about a specific source package though, rather it is just filed against Ubuntu in general. It is important that bug reports be filed about source packages so that people interested in the package can find the bugs about it. You can find some hints about determining what package your bug might be about at https://wiki.ubuntu.com/Bugs/FindRightPackage. You might also ask for help in the #ubuntu-bugs irc channel on Freenode.

To change the source package that this bug is filed about visit https://bugs.launchpad.net/ubuntu/+bug/1459455/+editstatus and add the package name in the text box next to the word Package.

[This is an automated message. I apologize if it reached you inappropriately; please just reply to this message indicating so.]

tags: added: bot-comment
Revision history for this message
Micah Gersten (micahg) wrote :
Download full text (8.5 KiB)

Here's my initial review:
debian/rules should have get-orig-source target that handles the upstream ISO/tarball processing (Policy 4.9, see https://www.debian.org/doc/debian-policy/ch-source.html#s-debianrules)
This should not be a native package (as listed in debian/source/format) as it comes from an upstream source and has patches on top of the upstream code
debian/copyright isn't in DEP-5 format (http://dep.debian.net/deps/dep5/)
I don't believe you need the empty postinst and prerm scripts
Should be arch:all since there isn't anything that's architecture dependent in this package.

Source lintian output:
N: Processing source package xe-guest-utilities (version 6.2.0-1120-0ubuntu3, arch source) ...
W: xe-guest-utilities source: native-package-with-dash-version
N:
N: Native packaging should only be used if a piece of software was written
N: specifically to be turned into a Debian package. In this case, the
N: version number should not contain a Debian revision part.
N:
N: Native source packages are sometimes created by accident. In most cases
N: the reason is the location of the original source tarball. For version
N: 1.0 source packages, dpkg-source determines whether they're non-native
N: by looking for a file named <package>_<upversion>.orig.tar.gz in the
N: parent directory, where <upversion> is the upstream version from the
N: most recent debian/changelog entry. For version 3.0 packages, check
N: debian/source/format for an erroneous "(native)" package format.
N:
N: Severity: normal, Certainty: certain
N:
N: Check: cruft, Type: source
N:
W: xe-guest-utilities source: quilt-series-but-no-build-dep
N:
N: The package contains a debian/patches/series file usually used by quilt
N: to apply patches at build time, but quilt is not listed in the build
N: dependencies.
N:
N: You should either remove the series file if it's effectively not useful
N: or add quilt to the build-dependencies if quilt is used during the build
N: process.
N:
N: If you don't need quilt during build but only during maintenance work,
N: then you can override this warning.
N:
N: Severity: normal, Certainty: possible
N:
N: Check: patch-systems, Type: source
N:

Binary lintian output:
N: Processing binary package xe-guest-utilities (version 6.2.0-1120-0ubuntu3, arch amd64) ...
W: xe-guest-utilities: copyright-without-copyright-notice
N:
N: The copyright file for this package does not appear to contain a
N: copyright notice. You should copy the copyright notice from the upstream
N: source (or add one of your own for a native package). A copyright notice
N: must consist of Copyright, Copr., or the Unicode symbol of C in a circle
N: followed by the years and the copyright holder. A copyright notice is
N: not required for a work to be copyrighted, but Debian requires the
N: copyright file include the authors and years of copyright, and including
N: a valid copyright notice is the best way to do that. Examples:
N:
N: Copyright YYYY Firstname Lastname <email address hidden>
N: Copr. YYYY-YYYY Firstname Lastname <email address hidden>
N: © YYY...

Read more...

Revision history for this message
Ben Howard (darkmuggle-deactivatedaccount) wrote :

Micah, thanks for taking a look. I've uploaded a new package for consideration.

Please read debian/README.source for how the source is generated.

Revision history for this message
Ben Howard (darkmuggle-deactivatedaccount) wrote :
Revision history for this message
Steve Langasek (vorlon) wrote :

Hi Ben,

I've had a look at this package and identified a few changes that I think are required before it can be uploaded to wily. Please find attached a debdiff of the changes I've made so far.

There is one outstanding issue; namely, that debian/rules attempts to key on the release version to decide whether or not to build --with systemd. This is incorrect for two reasons. First, packages should support all init systems, not just one, so that they don't have to be upgraded in lockstep with the init system (and I've partially addressed this in my patch, with changes to install both the upstart job and systemd unit in the dh_installinit target). Second, dh_systemd doesn't exist at all in precise, so this package will fail to build on precise no matter what due to an unsatisfied build dependency.

I'd like to know your thoughts on the best way to resolve this.

Revision history for this message
Ben Howard (darkmuggle-deactivatedaccount) wrote :

Ingestion of feedback. Package builds and installs for 12.04 and later, installs init/systemd jobs properly

Revision history for this message
Ben Howard (darkmuggle-deactivatedaccount) wrote :
Revision history for this message
Steve Langasek (vorlon) wrote :

The package looks good to me now, except for one last thing:

override_dh_installinit:
        dh_installinit --name xe-daemon
        dh_installinit --name xe-linux-distribution
        dh_installinit --no-start --no-restart-on-upgrade

The last line of this is definitely wrong; it's going to look for files named debian/{xe-guest-utilities.,}{service,upstart,init} in the source directory, install them to the package, and configure the maintainer scripts to not start this service on package install/upgrade. This is a no-op, since none of the named files are present in the source.

What it *doesn't* do is prevent the xe-daemon and xe-linux-distribution jobs/services from being started on install/upgrade. If that's the goal, you need the --no-start option passed to the other two dh_installinit calls, for their respective scripts. (Note that --no-restart-on-upgrade is redundant with --no-start.)

Revision history for this message
Ben Howard (darkmuggle-deactivatedaccount) wrote :

Thanks Steve for the feedback, I've fixed, tested and confirmed that using "dh_inistallinit --no-restart-on-upgrade" gives me what was needed.

Revision history for this message
Ben Howard (darkmuggle-deactivatedaccount) wrote :

Here is another cut after feedback from infinity.

Revision history for this message
Ben Howard (darkmuggle-deactivatedaccount) wrote :

In further review, the udev rule for the CPU hot add looked suspect. Its been updated to match the hyperV rule. In talking this over with an archive admin, there is the question whether or not the hyper-V rule should be generalized to include both Xen and HyperV rather than carrying the rule in some obscure universe package.

Revision history for this message
Ben Howard (darkmuggle-deactivatedaccount) wrote :
Adam Conrad (adconrad)
affects: Ubuntu Wily → xe-guest-utilities (Ubuntu Wily)
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package xe-guest-utilities - 6.2.0-1120+dsf1-0ubuntu1

---------------
xe-guest-utilities (6.2.0-1120+dsf1-0ubuntu1) wily; urgency=medium

  [ Ben Howard ]
  * Initial packaging/forking for usability in Ubuntu.
    - depend on xenstore-utils rather than pre-packaged binary blobs.
    - created systemd files
    - created upstart files
  * Source changes:
    - deleted xenstore-sources.tar.bz2
    - deleted xenstore.tar.bz2
    - deleted apt source 'citrix.list' as both broken and unneeded
    - deleted debian/*.{postinst,prerm} as unneeded
    - modified debian/rules
  * Generalized upstart/systemd descriptions.
  * Changed systemd target for proc-xen.mounts.
  * Fix systemd job for xe-daemon.service to use ConditionPathExists.
  * Modernize the CPU hotplug udev rule.
  * Closes (LP: #1459455).

  [ Steve Langasek ]
  * Adjust debian/copyright to comply with copyright-format 1.0; and fix
    inclusion of personal copyright statement on Canonical code.
  * Remove debian/postinst and debian/postrm that are empty except for the
    debhelper tokens; fixes lintian warning due to ignored errors. Let
    debhelper create the scripts itself so it gets them right.
  * Drop a no-op debian/docs.
  * Rename debian/nova-agent.upstart to
    debian/xe-guest-utilities.xe-daemon.upstart and let it be picked up
    automatically in debian/rules.
  * Don't make systemd vs. upstart support conditional on the target release.

 -- Ben Howard <email address hidden> Thu, 13 Aug 2015 16:28:48 -0600

Changed in xe-guest-utilities (Ubuntu Wily):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.