Next Session: TBD |
1 [13:00] <dholbach> welcome everybody!
2 [13:00] <dholbach> who do we have here of the training session?
3 [13:01] <dholbach> come on, don't be shy :)
4 [13:01] <dholbach> or is the channel +m'ed again?
5 [13:01] <effie_jayx> me
6 [13:01] <dholbach> ahhh, here we go :)
7 [13:01] <SevenMachines_> i was on -chat
8 [13:01] <effie_jayx> me
9 [13:01] <dholbach> let's all chat in here - that's totally fine
10 [13:01] <bobbo> o/
11 [13:01] <dholbach> yeeehaw!
12 [13:01] * dholbach is EXCITED
13 [13:02] <dholbach> alright… I guess we'll have a few stragglers - we'll just ask them to check out the logs later on :)
14 [13:02] <dholbach> bobbo and I are going to talk about Operation Cleansweep and the Patch Reviewers Team today
15 [13:03] <dholbach> as you can see from https://wiki.ubuntu.com/OperationCleansweep the goal of this initiative is to clear up the quite big backlog of bugs with patches
16 [13:03] <dholbach> bobbo: at the moment, we have how many total?
17 [13:03] <bobbo> We started with 2100
18 [13:03] <bobbo> but we've hit about 1650 now :D
19 [13:04] <dholbach> NICE
20 [13:04] <dholbach> alright, the problem with a lot of these patches is that people didn't know about the "proper process to submit patches" or sometimes added patches we couldn't easily make a decision about
21 [13:05] <dholbach> so the goal now is to triage those patches, see if they still work, then get them to the authors of the software and debian and get them included in Ubuntu that way
22 [13:05] <dholbach> so everybody benefits
23 [13:05] <dholbach> another problem we had was that Launchpad didn't offer something like "patch status"
24 [13:06] <dholbach> so we always knew there's a huge number of patches, but we didn't know anything about them, because we couldn't query launchpad easily
25 [13:06] <dholbach> our goal for this cycle is, to check out ALL of them and make sure they get attention
26 [13:07] <dholbach> we put quite a lot of effort into writing good documentation that explains how to do it and how to make all the decisions regarding patches and where they should go
27 [13:07] <dholbach> https://wiki.ubuntu.com/ReviewersTeam/ReviewGuide is the page you should probably bookmark :-D
28 [13:07] <dholbach> bobbo: care to take us through https://wiki.ubuntu.com/ReviewersTeam/ReviewGuide#Workflow ?
29 [13:07] <bobbo> sure :D
30 [13:07] <dholbach> awesome
31 [13:08] <bobbo> Basically, going through that process you first need to find an appropriate bug to look at
32 [13:08] <bobbo> There's a few links in the wiki, but http://tinyurl.com/2u7kf3b is the bug list we've been working from mostly
33 [13:08] <bobbo> all of those bugs have patches that need a bit of love
34 [13:09] <bobbo> The first step in working with a patch is to make sure the bug is still valid
35 [13:09] <bobbo> there's no point trying to patch a bug that doesn't exist anymore
36 [13:10] <bobbo> if you can reproduce the bug properly, then you should head on to the next step, if not you can generally set the bug to Incomplete and ask other people to check if it's still an issue for them
37 [13:11] <txwikinger> o/
38 [13:11] <bobbo> txwikinger, question?
39 [13:11] * txwikinger jusst said hi
40 [13:11] <bobbo> txwikinger, haha :D
41 [13:12] <bobbo> once you have a bug you can reproduce, you now need to test the patch, by downloading the packages source code (apt-get source packagename), downloading the patch attached to the bug and attempting to apply it
42 [13:13] <bobbo> if it fails to apply, you should add the 'patch-needswork' tag to the bug, and add a comment saying it failed to apply (adding some output from patch showing which patch hunk(s) failed is also a great idea)
43 [13:14] <dholbach> we have some documentation on how to do that at https://wiki.ubuntu.com/Bugs/HowToFix and I'll integrate that into the docs in a bit
44 [13:14] <bobbo> if the patch applies fully, you'll next want to test that it actually fixes the bug (no point applying a patch if it doesn't actually fix anything)
45 [13:15] <bobbo> you'll probably need to build the package (I'm sure there're docs on the Wiki somewhere to guide you with that, dholbach?)
46 [13:15] <nigelbabu> we'll get docs on that stat, 24 hours perhaps. there are docs, but not geared to reviewers
47 [13:15] <dholbach> bobbo: yes, that's on HowToFix too
48 [13:16] <dholbach> I'll make sure to update it after the session
49 [13:16] <bobbo> once we write some docs and you know how to build packages, build it and go through the steps you went through initially to try to reproduce the bug
50 [13:17] <dholbach> as bobbo said: if it doesn't work tag it as patch-needswork and it's off the list for now
51 [13:17] <SevenMachines_> is it best/needed to be a member of the bug control team for this?
52 [13:17] <dholbach> SevenMachines_: no, not at all
53 [13:17] <dholbach> SevenMachines_: ubuntu-bugcontrol is only useful if you want to set the bug importance or mark it as won't fix
54 [13:17] <dholbach> to tag the patch you can do it just like that
55 [13:18] <bobbo> if the bug is still reproducible, hit up the bug with 'patch-needswork'
56 [13:18] <bobbo> otherwise, you should send the bug upstream or to Debian, dholbach could you take us through sending a patch upstream?
57 [13:19] <dholbach> sure
58 [13:19] <dholbach> if you scroll down on the reviewers guide a bit, down to "Getting patches accepted", you'll find information on how to make a decision where to send the patch
59 [13:19] <dholbach> in an ideal world, it'd work like this:
60 [13:19] <dholbach> - we get a patch in ubuntu
61 [13:20] <dholbach> - we send it to the software authors of that piece of software (upstream)
62 [13:20] <dholbach> - they like it
63 [13:20] <dholbach> - it gets integrated
64 [13:20] <dholbach> - the new release makes it into debian
65 [13:20] <dholbach> - then we get it
66 [13:20] <dholbach> sometimes you'll find that it's a patch that has only to do with packaging
67 [13:20] <dholbach> in that case and if the package is in debian too, you'd just send it to debian
68 [13:21] <dholbach> if the package is just in ubuntu, we need to integrate it ourselves
69 [13:21] <dholbach> the good thing about getting in touch with upstream and debian is that we get a few more sets of additional eyes to have a look at the patch
70 [13:21] <dholbach> and make an informed decision
71 [13:21] <dholbach> however
72 [13:22] <dholbach> if it's really an important bug fix, like a crash or when data gets deleted
73 [13:22] <dholbach> etc
74 [13:22] <karyo> i'm sorry to interrupt but how do I "apply" a patch? does it need to be dealt with in a case by case basis?
75 [13:22] <dholbach> we integrate it ourselves first
76 [13:22] <dholbach> karyo: up until a few days ago it was a "case by case basis"
77 [13:22] <dholbach> but luckily, we have the ultimate solution to things, which is
78 [13:22] <dholbach> BOBBO
79 [13:22] <bobbo> edit-patch?
80 [13:22] <bobbo> amirite?
81 [13:22] <dholbach> he extended "edit-patch" which is in ubuntu-dev-tools
82 [13:23] <dholbach> so you'd just download the patch and the source package
83 [13:23] <dholbach> then call edit-patch <patch file>
84 [13:23] <dholbach> and it'd get applied for you
85 [13:23] <dholbach> then you can test-build the package
86 [13:23] <dholbach> as bobbo, nigelbabu and I said: we'll work on getting the docs for that right ASAP
87 [13:23] <karyo> in general, would you say applying patches would require packaging skills?
88 [13:23] <karyo> or "just run edit-patch and your done"
89 [13:23] <bobbo> (the only caveat there is that if the package doesn't have a patchsystem (dpatch/quilt/cdbs), edit-patch wont work)
90 [13:24] <dholbach> bobbo: we can fix that too
91 [13:24] <dholbach> bobbo: I'll add it to the TODO list
92 [13:24] <karyo> do most packages have a patch system?
93 [13:24] <dholbach> karyo: I'd say so
94 [13:24] <dholbach> but we'll fix that
95 [13:25] <bobbo> karyo, a lot do, yeah, but I'll patch edit-patch as soon as we're done here so it works for all packages
96 [13:25] <dholbach> thanks a bunch already for all your questions and feedback - as you can see you sparked a lot of ideas :-D
97 [13:25] <dholbach> ok, so let's say you forwarded the patches
98 [13:25] <dholbach> you'd use patch-forwarded-upstream
99 [13:26] <dholbach> and/or patch-forwarded-debian
100 [13:26] <karyo> as tags?
101 [13:26] <dholbach> If upstream requests patch rework, you'd add the patch-needswork tag
102 [13:26] <dholbach> karyo: yes
103 [13:26] <dholbach> if upstream rejects the patch, remove patch-forwarded-upstream tag, and add patch-rejected-upstream (or patch-rejected-debian) tag
104 [13:27] <karyo> what if I cannot get an answer from upstream?
105 [13:27] <dholbach> and if the patch is unnecessary or addresses something that does not need to be fixed, add tag patch-rejected, give reason in the comments, and if required close the bug to Won't Fix
106 [13:27] <dholbach> karyo: that's indeed a problem - in that case we'd need to assess the patch and the bug and decide if we want to carry the difference ourselves
107 [13:27] <dholbach> it always comes at a maintenance cost
108 [13:28] <dholbach> (but maybe worthwhile)
109 [13:28] <karyo> i'll ask you about a specific case about this later. please carry on
110 [13:28] <dholbach> ok cool
111 [13:28] <dholbach> as you can see… the tags we use are really self-explaining
112 [13:29] <dholbach> and apart from the technical details which might require some knowledge (which you can ask for in #ubuntu-reviews), the process itself is pretty straight-forward
113 [13:30] <dholbach> if upstream accepts the patch, remove patch-forwarded-upstream tag, and add patch-accepted-upstream (or patch-accepted-debian) tag. Indicate the VCS revision and/or expected upstream/debian version/revision that will include the bugfix
114 [13:30] <dholbach> the only thing left (apart from a juicy example or two) is: how do I get it into Ubuntu if it's urgent enough?
115 [13:31] <dholbach> there's the sponsors team - they will take patches from people who can't upload themselves yet, where all the patch assessment has happened already
116 [13:31] <dholbach> you'll just attach an updated patch (if necessary), add a changelog entry (if you can) and subscribe ubuntu-sponsors
117 [13:31] <dholbach> do you have any more questions up until now?
118 [13:32] <dholbach> bobbo has picked a few nice and juicy examples
119 [13:33] <dholbach> let's first go through one that was already done
120 [13:33] <dholbach> and then do a live example
121 [13:33] <bundo> hayanbom, Hi !
122 [13:33] <hayanbom> bundo, hi
123 [13:33] <effie_jayx> @dholbach there are a few bugs with patches with very specific hardware. what can one do to help get those sorted if one does not have the needed hardware
124 [13:34] <bobbo> https://bugs.edge.launchpad.net/ubuntu/+source/libpcap/+bug/523349
125 [13:34] <ubot2> Launchpad bug 523349 in libpcap (Debian) (and 1 other project) "Bad /sys path to text-based usbmon (affects: 2) (dups: 1) (heat: 42)" [Unknown,Fix released]
126 [13:34] <dholbach> effie_jayx: good question - I'd probably ask in #ubuntu-devel for help with that
127 [13:34] <karyo> reproducing the patch would be very difficult too
128 [13:34] <bobbo> This is a bug which had a patch which has gone through the patch-review process
129 [13:35] <bobbo> Brian Murray confirmed the bug and it was then sent up to Debian (unfortunately whoever did that forgot to add patch-forwarded-debian tag)
130 [13:36] <bobbo> it was taken and applied in the Debian package, and was tagged 'patch-accepted-debian'
131 [13:36] <bobbo> and now we are waiting for the patch to trickle down and hit the Ubuntu archives
132 [13:36] <bundo> oh karyo good English conversation ^^..
133 [13:36] <dholbach> also
134 [13:37] <dholbach> we have it tagged now, so we practically have another "todo list" now
135 [13:37] <dholbach> we can regularly have a look at the patch-accepted-* bugs and make sure we integrate them one way or another
136 [13:37] <dholbach> (there's instructions on the review guide for that as well)
137 [13:37] <karyo> isn't it automatic?
138 [13:37] <dholbach> karyo: it depends
139 [13:38] <dholbach> karyo: only if we're in the beginning in the release cycle and the package in ubuntu was not modified, we'll get it automatically
140 [13:38] <dholbach> karyo: in the other cases we'll have to do this semi-manually
141 [13:38] <dholbach> like: if we're very close to release and it's a HUGE CHANGE, we probably choose to fix it in the next release instead
142 [13:38] <dholbach> even if it fixes a bug
143 [13:39] <dholbach> so it can get more testing, etc.
144 [13:39] <dholbach> bobbo: please go on - just wanted to quickly interrupt :-)
145 [13:39] <bobbo> dholbach, awesome :D
146 [13:39] <karyo> Oh, I thought it only involved work in our part. Not decisions. bobbo go on please
147 [13:39] <bobbo> okay https://bugs.edge.launchpad.net/ubuntu/+source/empathy/+bug/544242 is another quick example of a patch going through the process
148 [13:39] <ubot2> Launchpad bug 544242 in empathy (Ubuntu) (and 1 other project) "[PATCH] Empathy should allow users to toggle auto-away mode on/off (affects: 1) (heat: 45)" [Wishlist,Fix committed]
149 [13:39] <bobbo> this time a bug was created with a patch attached
150 [13:40] <bobbo> it was reviewed by Omer Akram who sent it upstream
151 [13:40] <bobbo> nigelbabu, saw this and marked it -frowarded-upstream
152 [13:41] <karyo> isn't this a feature rather than a but fix?
153 [13:41] <bobbo> There was some discussion about the patch upstream with people debating the correct way to implement it before the patch was accepted and tagged patch-accepted-upstream
154 [13:41] <bobbo> karyo, the process can work for both features and fixeds
155 [13:42] <bobbo> karyo, in the case of feature requests it's especially important that we talk to the upstream about it and forward patches up to the original authors
156 [13:42] <dholbach> karyo: it's probably one of those examples for "late in the release, maybe we shouldn't include it right away, even if it has upstream approval"
157 [13:42] <dholbach> yep
158 [13:42] <bobbo> dholbach, do you want to take us through a "live" untouched example from the queue?
159 [13:43] <dholbach> let me check the list
160 [13:43] <karyo> I understand. btw am I interrupting too much?
161 [13:43] <dholbach> let me check the list
162 [13:43] <dholbach> karyo: not at all
163 [13:43] <dholbach> karyo: it's way more fun if this is a conversation
164 [13:43] <bobbo> karyo, the more questions you ask, the more we understand where we need to improve our documentation and processes
165 [13:44] <karyo> that's nice 'cause questions I've plenty
166 [13:45] <dholbach> karyo: do you have any more questions right now?
167 [13:46] <dholbach> ok let's see if https://bugs.edge.launchpad.net/ubuntu/+source/onboard/+bug/526791 makes any sense for us to work on
168 [13:46] <ubot2> Launchpad bug 526791 in onboard (Ubuntu) (and 1 other project) "onboard crashed with SIGSEGV in XkbGetNames() (affects: 19) (dups: 4) (heat: 123)" [Medium,Fix committed]
169 [13:46] <dholbach> if you have a look at the last comment
170 [13:47] <dholbach> you see that marmuta said "I've pushed a workaround to onboard and virtkey trunks"
171 [13:47] <dholbach> that means that the patches were already pushed upstream
172 [13:47] <dholbach> so it's easy, I'd just add patch-accepted-upstream and it'd be off our list
173 [13:48] <dholbach> nigelbabu, bobbo: do I remove the 'patch' tag? O:-)
174 [13:48] <bobbo> dholbach, yes :)
175 [13:48] <nigelbabu> dholbach: yes
176 [13:48] <dholbach> oh, it doesn't even have it
177 [13:48] <karyo> I don't have to reproduce and confirm that the patch is working?
178 [13:48] <dholbach> alright, another one off the list
179 [13:48] <nigelbabu> it doesn't have it, because its an older patch that's part of clean sweep
180 [13:48] <dholbach> karyo: if upstream is totally happy with it, that's good enough for us, of course, if you want you can still test it
181 [13:48] <dholbach> nigelbabu: gotcha
182 [13:49] <bobbo> once you start going through the list, you'll find a lot of the bugs can be easily taken off our list as they've already been forwarded or accepted in Upstreams or Debian
183 [13:49] <bobbo> the majority of bugs I've touched, I haven't had to apply the patch and test it as someone has done that already and sent it to Debian, the bug just hasn't been tagged yet
184 [13:49] <karyo> so all I have to do is find out if upstream/debian is happy with the patch?
185 [13:50] <dholbach> karyo: if it wasn't tested first, you'd need to test it though
186 [13:50] <SevenMachines> is there a cutoff for these patches, ie, do we still try and fix the warty release :)
187 [13:50] <karyo> how do I know if it was tested?
188 [13:51] <dholbach> karyo: if it already was forwarded or even accepted, that's easy
189 [13:51] <dholbach> karyo: in the other cases you'd want to test
190 [13:51] <karyo> @sevenmachines I believe warthy is no longer supported
191 [13:51] <dholbach> https://bugs.edge.launchpad.net/ubuntu/+source/openttd/+bug/503725 is another one of those
192 [13:51] <ubot2> Launchpad bug 503725 in openttd (Ubuntu) "CVE-2009-4007 (DoS of OpenTTD < 0.7.5) (affects: 2) (heat: 252)" [Low,Confirmed]
193 [13:51] <bobbo> SevenMachines, often if there's a bug from a long time ago, the patch will have already been applied somewhere, you just have to use your google-fu to check
194 [13:51] <dholbach> it's about a security issue, directly in the initial comment it says
195 [13:51] <dholbach> There are two ways to fix this:
196 [13:51] <dholbach> - update to 0.7.5 (or higher)
197 [13:51] <dholbach> - apply the patch of 0.6.2-1+lenny1 (from Debian)
198 [13:52] <SevenMachines> scroogle-fu!
199 [13:52] <dholbach> so it's been accepted both in upstream and in debian
200 [13:52] <dholbach> I'll add patch-accepted-debian and patch-accepted-upstream
201 [13:52] <dholbach> as you can see: a lot of our work is to put those patches into buckets
202 [13:52] <dholbach> and make it clear what happened to the patches up until now
203 [13:52] <karyo> is update to 0.7.5 an option?
204 [13:52] <dholbach> so that further action can be taken easily
205 [13:53] <dholbach> karyo: sure
206 [13:53] <karyo> isn't that changing packages?
207 [13:53] <dholbach> if you have a look at http://packages.debian.org/search?searchon=sourcenames&keywords=openttd
208 [13:53] <bobbo> SevenMachines, normally if it's been languishing in LP for a long time, it's either not suitable to be applied or there's some other reason. In most cases of really old patches, I've looked around a bit, found the source code has evolved so much that the patch wont even apply any more and just slapped with with a 'patch-rejected' and explained why
209 [13:53] <dholbach> you can see that it's already in Debian
210 [13:54] <bundo> hayanbom, Your happy?
211 [13:54] <dholbach> another interesting example is https://bugs.edge.launchpad.net/ubuntu/+source/file-roller/+bug/354136
212 [13:54] <ubot2> Launchpad bug 354136 in file-roller (Ubuntu) (and 1 other project) "unintuitive back and forward (heat: 11)" [Low,Triaged]
213 [13:54] <dholbach> as you can see the bug was forward to upstream already
214 [13:54] <dholbach> click on the "gnome-bugs #578837" link
215 [13:54] <ubot2> Launchpad bug 578837 in blueprint "Proposing a blueprint for a sprint cannot be undone (dup-of: 198982)" [Undecided,New] https://launchpad.net/bugs/578837
216 [13:55] <ubot2> Launchpad bug 198982 in blueprint "Can't un-propose a blueprint for a meeting (affects: 1) (dups: 1) (heat: 12)" [Low,Triaged] https://launchpad.net/bugs/198982
217 [13:55] <dholbach> if you compare the two suggested patches, you'll see they're identical
218 [13:55] <dholbach> http://bugzilla-attachments.gnome.org/attachment.cgi?id=132594
219 [13:55] <dholbach> http://launchpadlibrarian.net/24728371/file-roller-2.24.1_history-fix2.diff
220 [13:55] <dholbach> the patch didn't get a review upstream though yet
221 [13:55] <dholbach> so we'd just add a patch-forwarded-upstream
222 [13:58] <dholbach> alright… let's close the session at this point and move over to #ubuntu-reviews where we can further discuss patches, bugs and the process involved
223 [13:58] <dholbach> thanks a lot everybody
224 [13:58] <bobbo> thanks guys :D
225 [13:58] <dholbach> thanks bobbo for being a great host
226 [13:58] <dholbach> I'll make sure we get the logs up later on for others who are interested
227 [13:58] <karyo> thx
228 [13:58] <SevenMachines> thanks everyone
229 [13:58] <dholbach> I'll update statistics here: http://daniel.holba.ch/review/report
230 [13:58] <dholbach> (every week, so we know how we're doing compared to last week)
231 [13:59] <SevenMachines> sort of related, is there a developer week soon? or did i imagine it
232 [13:59] <dholbach> SevenMachines: a couple of weeks left still :)
233 [13:59] <karyo> what do I do when I cannot get an answer from upstream?
234 [14:01] <dholbach> karyo: I'd try to ask again, on the bug, maybe on their mailing list too
235 [14:01] <dholbach> karyo: and if it's not possible at all to get a reply, decide if we're willing to carry the load of taking the patch ourselves