/etc/init/apport-noui.conf is non-functional on the phone

[Steve Langasek]

I've uploaded a fix to upstart for bug #1235387, but /etc/init/apport-noui.conf in the apport-noui package still doesn't work here. There's no /etc/apport/autoreport in the image, so it fails on the first line of the script:

    [ -e /etc/apport/autoreport ] || exit 0

Apparently further integration work is needed.

The job is also structured quite confusingly, with an 'instance' that means the job will be launched once for each file found in /var/crash - even though the command it runs is called /usr/share/apport/whoopsie-upload-*all*. So we'll get multiple copies of this running in parallel the first time it successfully triggers. I suspect this may result in multiple submissions, as whoopsie sees the .upload stamp updated?

In any case, I don't see the point of having this as a separate upstart job from whoopsie at all. whoopsie is already monitoring the /var/crash directory with inotify; it should just check /etc/apport/autoreport itself (or maybe this should be /etc/whoopsie/autoreport?), and if present, monitor the .crash files directly.

[Steve Langasek (vorlon)]

So just to confirm, after manually touching /etc/apport/autoreport, my phone is now being crushed by several instances of the apport-noui job processing *the same crash file*, in parallel - apparently, for some reason whoopsie-upload-all finds it necessary to run gdb over the crash files, which is not pretty.

This reinforces my belief that we should ditch /etc/init/apport-noui.conf completely, and let whoopsie handle those crash files directly.

But it also looks to me like /usr/share/apport/apport does not write its crash files atomically... it opens the .crash file, writes to it, and closes it. Which means that regardless of who's watching /var/crash with inotify, there's a race condition here that means some things may try to process the crash files before they're fully written. And on armhf, writing these crash files out is sometimes VERY slow. I think we need apport fixed to write to a temp file and atomically rename into place as .crash, to ensure this all works reliably.

[Martin Pitt (pitti)]

> But it also looks to me like /usr/share/apport/apport does not write its crash files atomically... it opens the .crash file, writes to it, and closes it.

FTR, It opens the file with mode 0, and only chmods it to 600 once it's done writing (after closing). By any other method, inotify watches would also be triggered, so it seems best to only consider crash files which you can actually access.

[Steve Langasek (vorlon)]

missed the boat on this for 13.10; most likely doesn't warrant an SRU since we're aiming to focus all our phone development efforts on trusty.

[Brian Murray (brian-murray)]

If the location of /etc/apport/autoreport changes, which it should since /etc/ is read-only on the phone images, then whoopsie-preferences needs to have the location updated too. This should end up fixing the following bug report:

https://bugs.launchpad.net/ubuntu/+source/whoopsie-preferences/+bug/1239811

[Launchpad Janitor (janitor)]

This bug was fixed in the package apport - 2.12.7-0ubuntu6

---------------
apport (2.12.7-0ubuntu6) trusty; urgency=medium

  * Merge from trunk:
    - setup.py: Make updating of hashbangs work when building without Java,
      and also apply it on bin/.
  * Bump Standards-Version to 3.9.5, no changes necessary.
 -- Martin Pitt <email address hidden> Tue, 07 Jan 2014 18:41:12 +0100

[Martin Pitt (pitti)]

So Brian changed /etc/apport/autoreport to /var/lib/apport/autoreport, and added that path in

lxc-android-config (0.126) trusty; urgency=low

  * Adding /var/lib/apport to writable persistent paths. (LP: #1239811)

 -- Brian Murray <email address hidden> Mon, 06 Jan 2014 12:03:20 -0800

Why can't we use /etc/ for this instead? This *is* a configuration file after all, and moving it to /var/lib/ is just a bad hack to work aorund our bad hack of non-writable /etc/. Can't we keep that in /etc/apport/ and make /etc/apport/autoreport writable instead?

[Steve Langasek (vorlon)]

On Thu, Jan 09, 2014 at 11:50:14AM -0000, Martin Pitt wrote:
> So Brian changed /etc/apport/autoreport to /var/lib/apport/autoreport,
> and added that path in

> lxc-android-config (0.126) trusty; urgency=low

> * Adding /var/lib/apport to writable persistent paths. (LP: #1239811)

> -- Brian Murray <email address hidden> Mon, 06 Jan 2014 12:03:20 -0800

> Why can't we use /etc/ for this instead? This *is* a configuration file
> after all, and moving it to /var/lib/ is just a bad hack to work aorund
> our bad hack of non-writable /etc/. Can't we keep that in /etc/apport/
> and make /etc/apport/autoreport writable instead?

I asked him to do this because I consider bind-mounting of individual files
a horrible hack. Since this is a configuration setting that's going to be
changed almost exclusively through the phone UI anyway, there's no logical
advantage of considering this a config file rather than "state", as it
should not be a conffile.

[Martin Pitt (pitti)]

It has pretty much the same purpose and semantics like an /etc/default/ file. On a desktop it should very much be a config file (not a conffile). It seems to me it only is not any more because our phones have a broken /etc/, not because we actually don't want it to be configuration?

[Steve Langasek (vorlon)]

On Thu, Jan 09, 2014 at 04:56:00PM -0000, Martin Pitt wrote:
> It has pretty much the same purpose and semantics like an /etc/default/
> file. On a desktop it should very much be a config file (not a
> conffile). It seems to me it only is not any more because our phones
> have a broken /etc/, not because we actually don't want it to be
> configuration?

In general, if the primary way the file is edited is from the software and
not via using an editor on the filesystem, I don't think there's a strong
argument that the file belongs in /etc.

[Oliver Grawert (ogra)]

this was never fixed, apport needs to create the dir from postinst or ship it in debian/apport.dirs, writable-paths only works with existing directories by design, so the change to lxc-android-config was a no-op.

additionally there are also the lines:

env MATCH=NULL
 [ "$MATCH" = NULL ] && exit 0

in the apport-noui.conf job, so it would always exit anyway.

[Brian Murray (brian-murray)]

While I've addressed oliver's comments from comment #10 there is still no user interface for the setting of automatic crash reporting and I've reported this as bug 1319516. However, if you now manually create /var/lib/apport/autoreport most crash reports will be automatically sent.

[Brian Murray (brian-murray)]

I say most because of bug 1319099 which I'll be working on soon.

[Launchpad Janitor (janitor)]

This bug was fixed in the package apport - 2.14.2-0ubuntu3

---------------
apport (2.14.2-0ubuntu3) utopic; urgency=medium

  * debian/apport-noui.upstart: remove early exit (LP: #1235436)
  * debian/apport-noui.dirs: create /var/lib/apport (LP: #1235436)
 -- Brian Murray <email address hidden> Wed, 14 May 2014 12:26:39 -0700