Merge lp://qastaging/~vorlon/lxc-android-config/apport-job-cleanup into lp://qastaging/lxc-android-config

Proposed by Steve Langasek
Status: Needs review
Proposed branch: lp://qastaging/~vorlon/lxc-android-config/apport-job-cleanup
Merge into: lp://qastaging/lxc-android-config
Diff against target: 161 lines (+23/-101)
5 files modified
debian/changelog (+8/-0)
debian/lxc-android-config.maintscript (+2/-0)
etc/init/apport-config.conf (+13/-10)
etc/init/apport-reconfig.conf (+0/-33)
etc/init/apport.override (+0/-58)
To merge this branch: bzr merge lp://qastaging/~vorlon/lxc-android-config/apport-job-cleanup
Reviewer Review Type Date Requested Status
Ken VanDine Needs Fixing
Review via email: mp+285016@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2015-10-15.

Commit message

Rework and simplify the apport overrides. Instead of adding two new upstart
jobs and an override for apport.conf, we can achieve everything we need to
with a single new apport job.

This leaves outstanding the issue that switching a device between channels
from the commandline will not cause the config to be correctly switched.

Description of the change

Rework and simplify the apport overrides. Instead of adding two new upstart
jobs and an override for apport.conf, we can achieve everything we need to
with a single new apport job.

This leaves outstanding the issue that switching a device between channels
from the commandline will not cause the config to be correctly switched.

Verified to work correctly on a stock Ubuntu 14.04 desktop system; will need
validating against the phone images also.

To post a comment you must log in.
Revision history for this message
Ken VanDine (ken-vandine) wrote : Posted in a previous version of this proposal

This no longer merges cleanly, please update the branch

review: Needs Fixing
Revision history for this message
Ken VanDine (ken-vandine) wrote :

Looks great and works well

review: Approve
Revision history for this message
Ken VanDine (ken-vandine) wrote :

QA failed this during landing. It seems to do the right thing at boot, but when toggling it via system-settings it no longer sets the core_pattern. It must not be starting and stopping the apport job on FILE=/var/lib/apport/autoreport

Not sure why though... but it does regress

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

It looks like there is an EVENT missing for the file watch, could that be the cause of the problem?

Revision history for this message
Ken VanDine (ken-vandine) wrote :

> It looks like there is an EVENT missing for the file watch, could that be the
> cause of the problem?

Oh, yeah I bet that's it, good catch. It needs the start on EVENT=create and stop on EVENT=delete.

Revision history for this message
Steve Langasek (vorlon) wrote :

On Fri, Feb 26, 2016 at 01:56:32PM -0000, Ken VanDine wrote:
> > It looks like there is an EVENT missing for the file watch, could that be the
> > cause of the problem?

> Oh, yeah I bet that's it, good catch. It needs the start on EVENT=create
> and stop on EVENT=delete.

FWIW I hadn't replied yet because I haven't had time to investigate for
sure, but I don't think this is the case. The EVENT= env var declaration
makes the match more specific; a match which does not include the EVENT=
should match a 'file' event with /any/ value of EVENT (or none at all).

Unmerged revisions

14. By Steve Langasek

Merge from trunk

13. By Steve Langasek

Further refinement: no apport override needed at all, do everything in the apport-config helper job

12. By Steve Langasek

rm -f is always polite

11. By Steve Langasek

Further reduction of the apport override

10. By Steve Langasek

Simplify apport handling, to avoid duplication of the main apport.conf
where avoidable and to simplify the event start model (avoiding possible
deadlocks).

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches