[needs-packaging] juju-mongo-tools3.2 in xenial, wily, and trusty

Bug #1558336 reported by Curtis Hovey
16
This bug affects 1 person
Affects Status Importance Assigned to Milestone
juju-mongo-tools3.2 (Ubuntu)
Fix Released
Wishlist
Unassigned

Bug Description

# juju-mongo-tools3.2

The Juju team wants juju-mongo-tools3.2 packaged in xenial.

    lp:~sinzui/ubuntu/xenial/juju-mongo-tools3.2/xenial-mongo-tools3.2

This package is new. Upstream replaced the mongo tools included in
MongoDB 2.x with new tools from the mongo-tools project. This project
is written in Go. This package required a patch to support arm64.

## juju-mongo-tools3.2-debian.diff

This diff shows my debianisation of juju-mongo-tools.
1. The package uses five golang-dev packages from xenial
   * There ware four matching golang-dev packages in wily.
2. This package uses the system ssl (instead of go's x-crypto)
3. This is a dep-5 package.
   * upstream has utility to manage golang package deps and save them
     to th vendor dir.
   * This implementation is similar Juju's godeps tool so i record
     the git commit for the package in the license comment.
4. The embedded and minimise JS is a nightmare. I collected the
   missing sources and include them in the copyright file and in
   the missing-sources dir. see juju-mongo-tools3.2-missing-sources.diff.
5. I patched one file to fix an arm64 build error.
   syscall.Dup2 is not supported by arm64, use syscall.Dup3.
6. The rules to build the many mongo tools are simple, though there
   is a few steps to setup the G0PATH.
   * Using the same technique as the juju-core package in xenial,
     I create a SYSTEM_GOCODE path and link all the installed golang dev
     packages to it. SYSTEM_GOCODE is the first item in the GOPATH.
   * The mongo-tools package is not a valid Go package path :/
     I follow the packages own example of creating a sensible
     GO package path that I call LOCAL_GOCODE. I symlink the content
     of this package into the LOCAL_GOCODE path. This is the second item
     in the GOPATH.
    * I also add the vendor path as the last item in GOPATH.

## juju-mongo-tools3.2-missing-sources.diff

The missing sources for the emedded JS and CSS>
Minimised JS is evil. Sane servers compress the text, which is
superior to removing whitespace.

Revision history for this message
Curtis Hovey (sinzui) wrote :
Revision history for this message
Curtis Hovey (sinzui) wrote :

A diff of the missing sources added to debian/.

Revision history for this message
Brian Murray (brian-murray) wrote :

*** This is an automated message ***

This bug is tagged needs-packaging which identifies it as a request for a new package in Ubuntu. As a part of the managing needs-packaging bug reports specification, https://wiki.ubuntu.com/QATeam/Specs/NeedsPackagingBugs, all needs-packaging bug reports have Wishlist importance. Subsequently, I'm setting this bug's status to Wishlist.

Changed in juju-mongodb (Ubuntu):
importance: Undecided → Wishlist
Revision history for this message
Robie Basak (racb) wrote :

I presume (with mgz) that the bzr branch is what I'm reviewing/uploading, as opposed to those diffs applied to the upstream source?

Where can I find the orig tarball for this, please?

Revision history for this message
Robie Basak (racb) wrote :

(some way to verify the orig tarball against upstream would be good, too - I don't see a debian/watch file)

Revision history for this message
Robie Basak (racb) wrote :

I reviewed
~sinzui/ubuntu/xenial/juju-mongo-tools3.2/xenial-mongo-tools3.2 revno 19
for upload.

Summary: looks good, one thing to fix in debian/rules, then I can make the
other changes and am happy to upload.

Good job sending the syscall patch upstream. I appreciate the dep3
header telling me the status.

In override_dh_clean, rm -rf /tmp/go-build* will interfere with other
concurrent builds. We shouldn't stomp on the system like this. I'm not
sure the build should leave anything in /tmp at all. Maybe use TMPDIR to
make it use somewhere else, then clean that? Is there a standard pattern
for this that we can use?

Other notes (I can fix up):

No need for override_dh_strip any more (golang-go produced binaries can
now be stripped according to bug 1318027). When fixed, can remove
lintian overrides. Maybe best to leave for now as we're short on time,
but can leave a bug open to do later.

No need for override_dh_builddeb since xz is default on Xenial, or are
you leaving this for backporting ease? If so, then do we know xz will
work as far back as you want to backport?

debian/copyright looks superficially OK to me, and I'm happy that Curtis
can maintain these accurately. Leaving it for the AA to review once.
Same with debian/missing-sources. Good job on identifying that this was
required and taking care of it.

Embedded dependencies: I see 10 bundled dependencies, and four -dev
packages which presumably override them (cannot check due to lack of
build, see below). Are the others acceptable to AAs to bundle?

Changelog should close LP bug. Changelog arch reference is presumably
superseded now as we're bulding Arch: any?

I'm not sure where to get the orig tarball and there is no debian/watch
file.

Cannot (easily) check lintian without orig tarball.

Cannot (easily) build test without orig tarball. So I basically just
reviewed the debian/ directory by looking at it only. It is nice and
small and easy to review - thanks.

Revision history for this message
Robie Basak (racb) wrote :

> In override_dh_clean, rm -rf /tmp/go-build* will interfere with other
concurrent builds. We shouldn't stomp on the system like this. I'm not
sure the build should leave anything in /tmp at all. Maybe use TMPDIR to
make it use somewhere else, then clean that? Is there a standard pattern
for this that we can use?

This needs addressing please.

Changed in juju-mongodb (Ubuntu):
status: New → Incomplete
Revision history for this message
Curtis Hovey (sinzui) wrote :

Hi Robie.

> In override_dh_clean, rm -rf /tmp/go-build* will interfere with other

Thank you for calling this out. I have removed it. It is not needed.

> Other notes (I can fix up):

The rules were written to work for wily and trusty. I look forward
to removing the work arounds or using a better technique. My hope
is that when officially backporting packages to wily and trusty, only
the control file will change.

> No need for override_dh_strip any more (golang-go produced binaries can
> now be stripped according to bug 1318027). When fixed, can remove
> lintian overrides. Maybe best to leave for now as we're short on time,
> but can leave a bug open to do later.
>
> No need for override_dh_builddeb since xz is default on Xenial, or are
> you leaving this for backporting ease? If so, then do we know xz will
> work as far back as you want to backport?

I think this is needed to trusty.

> debian/copyright looks superficially OK to me, and I'm happy that Curtis
> can maintain these accurately. Leaving it for the AA to review once.
> Same with debian/missing-sources. Good job on identifying that this was
> required and taking care of it.
>
> Embedded dependencies: I see 10 bundled dependencies, and four -dev
> packages which presumably override them (cannot check due to lack of
> build, see below). Are the others acceptable to AAs to bundle?

I have reviewed the build logs for my trials on xenial, wily, and trusty.
Only the trusty built with all the embedded go packages.

> Changelog should close LP bug. Changelog arch reference is presumably
> superseded now as we're bulding Arch: any?

Oops, yes a bullet point needs to be removed, Done.

> I'm not sure where to get the orig tarball and there is no debian/watch
> file.

/me hangs head in shame. I didn't commit the watch file. It is not in
the tree.

> Cannot (easily) check lintian without orig tarball.

> Cannot (easily) build test without orig tarball. So I basically just
> reviewed the debian/ directory by looking at it only. It is nice and
> small and easy to review - thanks.

I recreate the tarfile when creating the source package. The quickest
command is
    bzr bd S -- -uc -us

Attached is a diff of the changes made per the review

Jon Grimm (jgrimm)
Changed in juju-mongodb (Ubuntu):
status: Incomplete → New
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

I've had a whack at the packaging. I started by switching over to use dh-golang more canonically, which gets some nice things like running the test suite as part of the build and help making a Built-Using header. But then I found that the upstream tests fail (https://github.com/mongodb/mongo-tools/issues/67) and odd things about the package mean that dh-golang's behaviour is not so useful. But I've whacked my way to something that builds, you can see it at https://code.launchpad.net/~mwhudson/+git/juju-mongo-tools3.2. You can get the orig from my branch with pristine-tar, but I just got r3.2.4.tgz from https://github.com/mongodb/mongo-tools/releases.

Are we going to be applying for an MIR for this package? If I was on the security team I'd have some .... questions, I think. (IMO, as mongo-tools at least claims to support multiple versions of mongo, I think aggressively tracking upstream is probably the best policy).

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

PS: I'd like either Curtis or Robie to have a look at my package for basic sanity before I do anything rash like uploading it.

Revision history for this message
Curtis Hovey (sinzui) wrote :

Thank you Michael for improving the rules for this package. The Juju team agreed that a universal mongo-tools is ideal, but too risky to assume is true. That is why this package has the 3.2 in its name.

I think this package needs to be in main because "main" means supported. Juju is supported and Canonical support recommends uses backup the juju db. Users cannot backup without this package.

I am saddened to see juju-monogdb3.2 arrive before juju-mongo-tools3.2 in xenial-release. Juju can find the new mongodb, but it cannot do backups.

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

I've uploaded the package to the queue now. Juju needs some more fixes to work with it but wallyworld has those in hand I believe.

Revision history for this message
Curtis Hovey (sinzui) wrote :

Hi Doko and Michael.

<doko>
mwhudson, sinzui: juju-mongodb-tools: and vendor/src/golang.org/x/ reference a LICENSE file which doesn't exist in the sources. Please add it
<doko>
mwhudson, sinzui: rejected for now. afaics the other license information, and the packaging seem to be okish

I apologise. I was very proud of my copyright documentation, but I did leave vendor/src/golang.org/x/ undocumented. I have a branch based on michaels' master (https://code.launchpad.net/~mwhudson/+git/juju-mongo-tools3.2) that adds the missing missing. The license was in place, but the directory didn't have its own stanza.
    https://code.launchpad.net/~sinzui/+git/juju-mongo-tools3.2/+ref/licence-fix

affects: juju-mongodb (Ubuntu) → juju-mongo-tools3.2 (Ubuntu)
Changed in juju-mongo-tools3.2 (Ubuntu):
status: New → 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.