Merge lp:~brian-murray/ubuntu/oneiric/apport/conffile-handling into lp:~ubuntu-core-dev/ubuntu/oneiric/apport/ubuntu

Proposed by Brian Murray on 2011-07-19
Status: Merged
Merged at revision: 1802
Proposed branch: lp:~brian-murray/ubuntu/oneiric/apport/conffile-handling
Merge into: lp:~ubuntu-core-dev/ubuntu/oneiric/apport/ubuntu
Diff against target: 76 lines (+22/-4) (has conflicts)
3 files modified
apport/hookutils.py (+9/-2)
data/general-hooks/ubuntu.py (+2/-2)
debian/changelog (+11/-0)
Text conflict in debian/changelog
To merge this branch: bzr merge lp:~brian-murray/ubuntu/oneiric/apport/conffile-handling
Reviewer Review Type Date Requested Status
Martin Pitt 2011-07-19 Approve on 2011-07-21
Review via email: mp+68477@code.launchpad.net

Description of the change

This branch raises a yes / no dialog in the event that a conffile is modified so that the reporter can make an informed decision about including the modified conffile.

I'd really rather that ui be a required argument to attach_conffiles so that source package hooks must pass ui to it as its possible the person writing the source package hook isn't familiar with the conffiles provided by the package and their contents.

If need be I'm willing to modify the source package hooks that call attach_conffiles to pass ui and then change the argument so that it no longer defaults to None.

To post a comment you must log in.
Martin Pitt (pitti) wrote :

Thanks for this!

Some remarks:

 * Please don't change the API of attach_conffiles, i. e. add the new ui argument as the last one.
 * attach_conffiles() should not crash with a None ui, i. e. please check ui before calling yesno.
 * Needs "from apport import unicode_gettext as _" and wrapping the user visible string in _() for i18n
 * the "report[key] = '[modified]'" can just go into the else branch of the third if, and the first two ifs (which are redundant, "not False" is true) can be removed.

Thanks,

Martin

review: Needs Fixing
1802. By Brian Murray on 2011-07-20

modifications based off reviewer feedback

Brian Murray (brian-murray) wrote :

I've updated the branch but did not implement the i18n bit as I think this would be an inconsistent experience for the reporter as source package hooks calls to ui are not translated.

Martin Pitt (pitti) wrote :

Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'apport/hookutils.py'
2--- apport/hookutils.py 2011-07-14 16:13:38 +0000
3+++ apport/hookutils.py 2011-07-20 16:36:13 +0000
4@@ -80,7 +80,7 @@
5 key += '_'
6 report[key] = read_file(path)
7
8-def attach_conffiles(report, package, conffiles=None):
9+def attach_conffiles(report, package, conffiles=None, ui=None):
10 '''Attach information about any modified or deleted conffiles'''
11
12 try:
13@@ -111,7 +111,14 @@
14 calculated_md5sum = m.hexdigest()
15
16 if calculated_md5sum != default_md5sum:
17- report[key] = contents
18+ if ui:
19+ response = ui.yesno("It seems you have modified the contents of '%s'. Would you like to add the contents of it to your bug report?" % path)
20+ if response:
21+ report[key] = contents
22+ else:
23+ report[key] = '[modified]'
24+ else:
25+ report[key] = '[modified]'
26 statinfo = os.stat(path)
27 mtime = datetime.datetime.fromtimestamp(statinfo.st_mtime)
28 mtime_key = 'mtime.conffile.' + path_to_key(path)
29
30=== modified file 'data/general-hooks/ubuntu.py'
31--- data/general-hooks/ubuntu.py 2011-07-20 05:01:57 +0000
32+++ data/general-hooks/ubuntu.py 2011-07-20 16:36:13 +0000
33@@ -18,7 +18,7 @@
34 from apport.hookutils import *
35 from apport import unicode_gettext as _
36
37-def add_info(report):
38+def add_info(report, ui):
39 add_release_info(report)
40
41 add_kernel_info(report)
42@@ -61,7 +61,7 @@
43 if 'Package' in report:
44 package = report['Package'].split()[0]
45 if package and 'attach_conffiles' in dir():
46- attach_conffiles(report, package)
47+ attach_conffiles(report, package, ui)
48
49 # do not file bugs against "upgrade-system" if it is not installed (LP#404727)
50 if package == 'upgrade-system' and 'not installed' in report['Package']:
51
52=== modified file 'debian/changelog'
53--- debian/changelog 2011-07-20 05:01:57 +0000
54+++ debian/changelog 2011-07-20 16:36:13 +0000
55@@ -1,3 +1,4 @@
56+<<<<<<< TREE
57 apport (1.21.2-0ubuntu6) UNRELELASED; urgency=low
58
59 * data/general-hooks/ubuntu.py:
60@@ -7,6 +8,16 @@
61
62 -- Brian Murray <brian@ubuntu.com> Tue, 19 Jul 2011 13:01:37 -0700
63
64+=======
65+apport (1.21.2-0ubuntu6) oneiric; urgency=low
66+
67+ * apport/hookutils.py:
68+ - raise a yes no dialog in the event a conffile has been modified
69+ (LP: #811203)
70+
71+ -- Brian Murray <brian@ubuntu.com> Tue, 19 Jul 2011 15:10:21 -0700
72+
73+>>>>>>> MERGE-SOURCE
74 apport (1.21.2-0ubuntu5) oneiric; urgency=low
75
76 * data/general-hooks/ubuntu.py:

Subscribers

People subscribed via source and target branches