Revision 1 as of 2010-06-10 13:07:24

Clear message

   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