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

[Curtis Hovey]

# juju-mongodb3.2

The Juju team wants juju-mongodb3.2 packaged in xenial.

    lp:~sinzui/ubuntu/xenial/juju-mongodb3.2/xenial-mongodb-3.2

This package is an fork and upgrade of the juju-mongodb2.6 package
that will be used in upgrades. There is a lot of change in this package
because Upstream changed a lot. the package required new rules and
deps. Many patches were needed to support wily/xenial tools chains
and arm64 and ppc64el archs.

Juju2 prefers juju-mongodb3.2 as the db for its controllers. This db
uses a different storage engine named wiredtiger. it requires JS to
operate.

There are many packages in src/third_party/. Most of them are ignored
because we choose to use the ubuntu system packages. Wiretiger appears
in third_party. 10gen/MongoDB bought the product and made it the default
storage engine. We must use this version. Mongo builds with the
embedded mozjs, there is no system option.

## juju-mongodb2.6-to-juju-mongodb3.2.diff

This is a diff of the debian dir from juju-mongodb2.6 to
juju-mongodb3.2, excluding patches. The principal changes are:

1. Rename juju-mongodb2.6 => juju-mongodb3.2.
2. Added new build deps. I tried to use system packages when possible.
   - trusty's libboost is too old so d/rules falls back to the embedded
     package for trusty only.
   - server-js is off; the server does not accept js queries, but the
     wiredtiger storage engine is compiled with JS, the server uses
     JS to talk to talk internally to the storage engine. The
     --js-engine=none option removes all js and only comppiles a server
     with the old storage engine.
3. The test infastructure also changed alot. We run only part of the
   test suite because the full suite will test JS communication from
   client to server, and we do not support that scenario.

## juju-mongodb3.2-patches.diff

There are three general kinds of change to patches.

1. The patch was upstreamed, or obsoleted by the removal of files to
   patch:
    -00git_boost_1.56.patch
    -0006-Fix-ARM-alignment-problems.patch
    -0007-Use-TIME_UTC_-macro.patch
    -0009-ignore-unused-local-typedefs.patch
    -# Added for arm64 and ppc64el enablement
    -0001-Add-option-to-disable-javascript.patch
    -0004-Support-ppc64el-builds.patch
    -0099-Pagesize-hacks.patch
    -fix-yaml-detection.patch
    -no-unused-function.patch

2. Patches found upstream in the devel release 3.3
    +scons-detect-ppc64.patch
    +support-ppc64el-mozjs.patch
    +use-4kb-sector-size-for-direct-io.patch
    +fix-debug-non-jit-builds-of-spidermonkey.patch
    +processinfo_test-fails-on-ppc64.patch
    +dont-assume-char-is-unsigned-in-logging.patch
    +use-signed-char-explicitly.patch
    +warn-user-if-locked-memory-limit-is-too-low.patch

3. Patches created by me to address wily/xenial build and test issues
    +offset-outside-of-bounds.patch
     These tests fail to build on wily and xenial. They are fine on
     trusty
    +fix-ppc64-preprocessor-blocks.patch
     A synax error only seen when attempting to build wiredtiger on
     ppc64el.
    +skip-high-mem-test.patch
     The warn-user-if-locked-memory-limit-is-too-low patch does not
     fix the test, it just suggests the user get more memory and
     fails. the test will not pass on the builders of on the ppc64el
     quests I have.
    +arm64-immediate-delete-should-wait-cursor.patch
     I disabled the test because it fails on arm64.

[Curtis Hovey (sinzui)]

A diff of the patch changes for juju-mongodb3.2. It is easier to read the actual patches then to read this diff of a patch.

[Brian Murray (brian-murray)]

*** 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.

[Robie Basak (racb)]

Hi Curtis,

Again, this looks good on the whole. As discussed on IRC, please add dep3 headers to the quilt patches to help me review and so that we can maintain this better going forward.

debian/copyright also probably needs a glance for both this and the 2.6 package, but as discussed I'll leave that between you and the archive admin.

I've not completed review yet, but the other changes look fairly straightforward to me. Hopefully this is enough to unblock you.

[Curtis Hovey (sinzui)]

Thank you for the feedback Robie. Attached is a diff of my dep-3 changes. I also updated the control arch to any per Dimitri's desire to build every where possible

[Robie Basak (racb)]

Thank you for the update. I've reviewed the patches now they have headers so I can follow what's going on. Most look OK. I'm unhappy with two:

1. arm64-immediate-delete-should-wait-cursor.patch IMHO inappropriately disables a test that correctly tests positive. This should be fixed properly, or allowed to FTBFS on arm64, or arm64 builds need to be disabled. Happy to follow Ubuntu archive admin or release team guidance.

2. offset-outside-of-bounds.patch: tests disabled due to GCC-5. As noted this should be fixed properly and upstreamed. Perhaps this should be combined into disable-warnings.patch? Not sure if this is OK to upload or not. Happy to follow Ubuntu archive admin or release team guidance.

The changelog needs updating to match changes made based on 2.6 - I don’t think it’s accurate any more. Ideally it would include changes not uploaded to the archive, so perhaps rebase on 2.6 and squash into previous PPA entry? rbasak is happy to do this before upload when the other items are resolved (so he doesn’t have to keep changing the description of what changed as we change things).

I've not tested build since resolving above items may influence outcome.

My tree (with some additional changes) is at https://git.launchpad.net/~racb/+git/juju-mongodb/log/?h=sinzui-3.2-fixes

[Robie Basak (racb)]

Minor omission: changelog entry should close this bug. I can fix when uploading, just leaving a note for myself here.

[Robie Basak (racb)]

To clarify, points 1 and 2 in my comment 6 above need fixing or otherwise to be addressed somehow. Once done, I'm happy to upload.

[Curtis Hovey (sinzui)]

Hi Robie

Attached are my changes. that I pushed to

    lp:~sinzui/ubuntu/xenial/juju-mongodb3.2/xenial-mongodb-3.2

> Thank you for the update. I've reviewed the patches now they have
> headers so I can follow what's going on. Most look OK. I'm unhappy
> with two: 1. arm64-immediate-delete-should-wait-cursor.patch IMHO
> inappropriately disables a test that correctly tests positive. This
> should be fixed properly, or allowed to FTBFS on arm64, or arm64
> builds need to be disabled. Happy to follow Ubuntu archive admin or
> release team guidance.

I revisited upstream bugs, commits and commits from other repos. I
found a patch that affected the failing test. In my experience, only
arm64 was failing, but the upstream report implies the tests can
fail on many types of hardware.

https://github.com/mongodb/mongo/commit/b6d309a15215a9243f21eb33cc6bcb6fbca58e56.patch

I successfully built and tested xenial arm64, amd64, and ppc64el.

> 2. offset-outside-of-bounds.patch: tests disabled due to GCC-5. As
> noted this should be fixed properly and upstreamed. Perhaps this
> should be combined into disable-warnings.patch? Not sure if this is OK
> to upload or not. Happy to follow Ubuntu archive admin or release team
> guidance.

I can file an bug upstream. I am not inclined attempt this fix myself
as I have other tasks that I am told are higher priority.

> The changelog needs updating to match changes made based on 2.6 - I

I updated the changelog to match what I propose for juju-mongodb2.6

[Michael Hudson-Doyle (mwhudson)]

Something very odd is going on with the problem offset-outside-of-bounds.patch is addressing. I hacked the offending source file down to http://paste.ubuntu.com/15573220/.

This fails to compile with -O2:

ubuntu@promonarchy-remi:~/xenial-mongodb-3.2$ g++ -o foo.o -c -O2 -Werror -std=c++11 -Ibuild/opt -Isrc src/mongo/bson/bson_obj_test.cpp
In file included from src/mongo/db/jsobj.h:49:0,
                 from src/mongo/bson/bson_obj_test.cpp:28:
src/mongo/bson/bsonelement.h: In member function ‘virtual void {anonymous}::UnitTest__BSONObj__getFields::_doTest()’:
src/mongo/bson/bsonelement.h:231:23: error: offset outside bounds of constant string [-Werror]
         return data + 1;
                       ^
cc1plus: all warnings being treated as errors

It succeeds with -O1 though. It _also_ succeeds with -O1 and all the -fxxx flags that -O2 implies on top of -O1. It _also_ succeeds if almost any line is removed from the file. But if you remove a few lines and change it to -O3, it fails again. So I guess some threshold-based optimization eventually transforms the code in the file into something that triggers the warning. I'm not quite confident enough to call this a compiler bug, but it's certainly user hostile.

[Michael Hudson-Doyle (mwhudson)]

I made a smaller testcase: http://paste.ubuntu.com/15573440/. I still have no idea what's going on, but I think the patch in Curtis' branch is a reasonable response to the issue.

[Michael Hudson-Doyle (mwhudson)]

I filed https://jira.mongodb.org/browse/SERVER-23460 fwiw

[Michael Hudson-Doyle (mwhudson)]

And they came back with a patch: http://paste.ubuntu.com/15585695/. I'll try that in a PPA on Monday if noone beats me to it.

[Curtis Hovey (sinzui)]

I am testing the patch now.

[Curtis Hovey (sinzui)]

The patch is good. arm64 passes just like amd64 and ppc64el
    https://launchpadlibrarian.net/251720877/buildlog_ubuntu-xenial-arm64.juju-mongodb3.2_3.2.0-0ubuntu1~juju9_BUILDING.txt.gz

I have pushed the changes to
     lp:~sinzui/ubuntu/xenial/juju-mongodb3.2/xenial-mongodb-3.2

[Michael Hudson-Doyle (mwhudson)]

Following the instructions on https://github.com/linux-on-ibm-z/docs/wiki/Building-MongoDB-3.2-on-RHEL-7-and-SLES-12 I've added a (large) patch to support s390x and uploaded it to my ppa: https://launchpad.net/~mwhudson/+archive/ubuntu/devirt/+sourcepub/6282808/+listing-archive-extra.

[Michael Hudson-Doyle (mwhudson)]

Having read through the diff with a bit more attention, I'm a little concerned that it seems to have a lot more endian-related changes than appear in mongo tip. I think it would be good to ask someone at IBM if the v3.2-s390 branch is still considered the right thing to use.

[Michael Hudson-Doyle (mwhudson)]

I've come up with a better s390x-support.patch by backporting fixes from mongo master. It's building in the juju/experimental ppa now: https://launchpad.net/~juju/+archive/ubuntu/experimental/+packages but only the changelog differs from the version in my PPA: https://launchpad.net/~mwhudson/+archive/ubuntu/devirt/+packages

I've been working in git, you can see my branch here: https://code.launchpad.net/~mwhudson/ubuntu/+source/juju-mongodb/+git/juju-mongodb3.2.

I'm not sure which branch Robie last reviewed so I'm not sure I can provide a sensible debdiff (the diff will be huge regardless because I also updated to the upstream 3.2.4 release).

[Michael Hudson-Doyle (mwhudson)]

Oh and to be clear: I think that the 3.2.4-0ubuntu1~juju1 is ready for review and (hopefully) upload.

[Michael Hudson-Doyle (mwhudson)]

I've uploaded the package and it's in the NEW queue: https://launchpad.net/ubuntu/xenial/+queue

[Steve Langasek (vorlon)]

Reviewing the diff vs the current juju-mongodb package in xenial:

- libgoogle-perftools-dev [linux-amd64 linux-i386 armhf],
+ libgoogle-perftools-dev [linux-amd64],

What's the reason for this delta? This is a delta from both juju-mongodb and mongodb 2.6.10-0ubuntu1. (Ditto changes in debian/rules)

--- juju-mongodb-2.4.10/debian/tests/control 2014-04-29 15:20:27.000000000 -0700
+++ juju-mongodb2.6-2.6.10/debian/tests/control 2016-04-04 09:18:30.000000000 -0700
@@ -1,2 +1,2 @@
 Tests: binaries
-Depends: juju-mongodb
+Depends: juju-mongodb2.6

Should really just be 'Depends: @'.

[Michael Hudson-Doyle (mwhudson)]

I don't know (Curtis?). But note that these packages do not build on 32 bit anyway, so this doesn't make much difference (I don't know why Dimitri asked Curtis to leave Architecture: as any, see comment #5).

[Steve Langasek (vorlon)]

The package has built on armhf and i386 in xenial...

[Michael Hudson-Doyle (mwhudson)]

No it didn't? https://launchpad.net/ubuntu/+source/juju-mongodb3.2/3.2.4-0ubuntu1

[Steve Langasek (vorlon)]

@Michael sorry, the problem here is that I was commenting on the wrong bug (because I assumed there was one bug for both 3.2 and 2.6). The above build-dep diff, and comment, referrs to juju-mongodb2.6.

[Curtis Hovey (sinzui)]

Hi Steve. Continuing with the conversation on the wrong bug...I prepared the Juju2 specific packages to be 64 bit only. Neither Juju or upstream support 32-bit servers.

But there was a change in plan that I was not privy to. Since 1.25 does want juju-mongodb2.6, I think we could make the i386 and armhf libgoogle-perftools-dev. Juju doesn't use it. It was provided as a curtesy for those that might want more diagnostics.

On the other hand, the Juju team is moving closer with abandoning 32 bit servers. 1.25.5 will probably be the last juju to support 32 bit archs.

[Michael Hudson-Doyle (mwhudson)]

I guess we should add a Trusty subordinate(?) task for tracking the backporting?

[Michael Hudson-Doyle (mwhudson)]

I'm closing this bug as the package is in xenial-releases now. I was advised that there should be a separate bug for tracking the SRU to trusty.