| === ziroday` is now known as ziroday | ||
| dholbach | Packaging Training Session in 15m: Operation Cleansweep and the Patch Reviewers Team! | 12:45 |
|---|---|---|
| dholbach | welcome everybody! | 13:00 |
| dholbach | who do we have here of the training session? | 13:00 |
| dholbach | come on, don't be shy :) | 13:01 |
| dholbach | or is the channel +m'ed again? | 13:01 |
| effie_jayx | me | 13:01 |
| dholbach | ahhh, here we go :) | 13:01 |
| SevenMachines_ | i was on -chat | 13:01 |
| effie_jayx | me | 13:01 |
| dholbach | let's all chat in here - that's totally fine | 13:01 |
| bobbo | o/ | 13:01 |
| dholbach | yeeehaw! | 13:01 |
| * dholbach is EXCITED | 13:01 | |
| dholbach | alright… I guess we'll have a few stragglers - we'll just ask them to check out the logs later on :) | 13:02 |
| dholbach | bobbo and I are going to talk about Operation Cleansweep and the Patch Reviewers Team today | 13:02 |
| 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 | 13:03 |
| dholbach | bobbo: at the moment, we have how many total? | 13:03 |
| bobbo | We started with 2100 | 13:03 |
| bobbo | but we've hit about 1650 now :D | 13:03 |
| dholbach | NICE | 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 | 13:04 |
| 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 | 13:05 |
| dholbach | so everybody benefits | 13:05 |
| dholbach | another problem we had was that Launchpad didn't offer something like "patch status" | 13:05 |
| 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 | 13:06 |
| dholbach | our goal for this cycle is, to check out ALL of them and make sure they get attention | 13:06 |
| 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 | 13:07 |
| dholbach | https://wiki.ubuntu.com/ReviewersTeam/ReviewGuide is the page you should probably bookmark :-D | 13:07 |
| dholbach | bobbo: care to take us through https://wiki.ubuntu.com/ReviewersTeam/ReviewGuide#Workflow ? | 13:07 |
| bobbo | sure :D | 13:07 |
| dholbach | awesome | 13:07 |
| bobbo | Basically, going through that process you first need to find an appropriate bug to look at | 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 | 13:08 |
| bobbo | all of those bugs have patches that need a bit of love | 13:08 |
| bobbo | The first step in working with a patch is to make sure the bug is still valid | 13:09 |
| bobbo | there's no point trying to patch a bug that doesn't exist anymore | 13:09 |
| 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 | 13:10 |
| txwikinger | o/ | 13:11 |
| bobbo | txwikinger, question? | 13:11 |
| * txwikinger jusst said hi | 13:11 | |
| bobbo | txwikinger, haha :D | 13:11 |
| 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 | 13:12 |
| 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) | 13:13 |
| 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 | 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) | 13:14 |
| 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?) | 13:15 |
| nigelbabu | we'll get docs on that stat, 24 hours perhaps. there are docs, but not geared to reviewers | 13:15 |
| dholbach | bobbo: yes, that's on HowToFix too | 13:15 |
| dholbach | I'll make sure to update it after the session | 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 | 13:16 |
| dholbach | as bobbo said: if it doesn't work tag it as patch-needswork and it's off the list for now | 13:17 |
| SevenMachines_ | is it best/needed to be a member of the bug control team for this? | 13:17 |
| dholbach | SevenMachines_: no, not at all | 13:17 |
| dholbach | SevenMachines_: ubuntu-bugcontrol is only useful if you want to set the bug importance or mark it as won't fix | 13:17 |
| dholbach | to tag the patch you can do it just like that | 13:17 |
| bobbo | if the bug is still reproducible, hit up the bug with 'patch-needswork' | 13:18 |
| bobbo | otherwise, you should send the bug upstream or to Debian, dholbach could you take us through sending a patch upstream? | 13:18 |
| dholbach | sure | 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 | 13:19 |
| dholbach | in an ideal world, it'd work like this: | 13:19 |
| dholbach | - we get a patch in ubuntu | 13:19 |
| dholbach | - we send it to the software authors of that piece of software (upstream) | 13:20 |
| dholbach | - they like it | 13:20 |
| dholbach | - it gets integrated | 13:20 |
| dholbach | - the new release makes it into debian | 13:20 |
| dholbach | - then we get it | 13:20 |
| dholbach | sometimes you'll find that it's a patch that has only to do with packaging | 13:20 |
| dholbach | in that case and if the package is in debian too, you'd just send it to debian | 13:20 |
| dholbach | if the package is just in ubuntu, we need to integrate it ourselves | 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 | 13:21 |
| dholbach | and make an informed decision | 13:21 |
| dholbach | however | 13:21 |
| dholbach | if it's really an important bug fix, like a crash or when data gets deleted | 13:22 |
| dholbach | etc | 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? | 13:22 |
| dholbach | we integrate it ourselves first | 13:22 |
| dholbach | karyo: up until a few days ago it was a "case by case basis" | 13:22 |
| dholbach | but luckily, we have the ultimate solution to things, which is | 13:22 |
| dholbach | BOBBO | 13:22 |
| bobbo | edit-patch? | 13:22 |
| bobbo | amirite? | 13:22 |
| dholbach | he extended "edit-patch" which is in ubuntu-dev-tools | 13:22 |
| dholbach | so you'd just download the patch and the source package | 13:23 |
| dholbach | then call edit-patch <patch file> | 13:23 |
| dholbach | and it'd get applied for you | 13:23 |
| dholbach | then you can test-build the package | 13:23 |
| dholbach | as bobbo, nigelbabu and I said: we'll work on getting the docs for that right ASAP | 13:23 |
| karyo | in general, would you say applying patches would require packaging skills? | 13:23 |
| karyo | or "just run edit-patch and your done" | 13:23 |
| bobbo | (the only caveat there is that if the package doesn't have a patchsystem (dpatch/quilt/cdbs), edit-patch wont work) | 13:23 |
| dholbach | bobbo: we can fix that too | 13:24 |
| dholbach | bobbo: I'll add it to the TODO list | 13:24 |
| karyo | do most packages have a patch system? | 13:24 |
| dholbach | karyo: I'd say so | 13:24 |
| dholbach | but we'll fix that | 13:24 |
| 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 | 13:25 |
| dholbach | thanks a bunch already for all your questions and feedback - as you can see you sparked a lot of ideas :-D | 13:25 |
| dholbach | ok, so let's say you forwarded the patches | 13:25 |
| dholbach | you'd use patch-forwarded-upstream | 13:25 |
| dholbach | and/or patch-forwarded-debian | 13:26 |
| karyo | as tags? | 13:26 |
| dholbach | If upstream requests patch rework, you'd add the patch-needswork tag | 13:26 |
| dholbach | karyo: yes | 13:26 |
| dholbach | if upstream rejects the patch, remove patch-forwarded-upstream tag, and add patch-rejected-upstream (or patch-rejected-debian) tag | 13:26 |
| karyo | what if I cannot get an answer from upstream? | 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 | 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 | 13:27 |
| dholbach | it always comes at a maintenance cost | 13:27 |
| dholbach | (but maybe worthwhile) | 13:28 |
| karyo | i'll ask you about a specific case about this later. please carry on | 13:28 |
| dholbach | ok cool | 13:28 |
| dholbach | as you can see… the tags we use are really self-explaining | 13:28 |
| 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 | 13:29 |
| 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 | 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? | 13:30 |
| 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 | 13:31 |
| dholbach | you'll just attach an updated patch (if necessary), add a changelog entry (if you can) and subscribe ubuntu-sponsors | 13:31 |
| dholbach | do you have any more questions up until now? | 13:31 |
| dholbach | bobbo has picked a few nice and juicy examples | 13:32 |
| dholbach | let's first go through one that was already done | 13:33 |
| dholbach | and then do a live example | 13:33 |
| bundo | hayanbom, Hi ! | 13:33 |
| hayanbom | bundo, hi | 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 | 13:33 |
| bobbo | https://bugs.edge.launchpad.net/ubuntu/+source/libpcap/+bug/523349 | 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] | 13:34 |
| dholbach | effie_jayx: good question - I'd probably ask in #ubuntu-devel for help with that | 13:34 |
| karyo | reproducing the patch would be very difficult too | 13:34 |
| bobbo | This is a bug which had a patch which has gone through the patch-review process | 13:34 |
| 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) | 13:35 |
| bobbo | it was taken and applied in the Debian package, and was tagged 'patch-accepted-debian' | 13:36 |
| bobbo | and now we are waiting for the patch to trickle down and hit the Ubuntu archives | 13:36 |
| bundo | oh karyo good English conversation ^^.. | 13:36 |
| dholbach | also | 13:36 |
| dholbach | we have it tagged now, so we practically have another "todo list" now | 13:37 |
| dholbach | we can regularly have a look at the patch-accepted-* bugs and make sure we integrate them one way or another | 13:37 |
| dholbach | (there's instructions on the review guide for that as well) | 13:37 |
| karyo | isn't it automatic? | 13:37 |
| dholbach | karyo: it depends | 13:37 |
| 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 | 13:38 |
| dholbach | karyo: in the other cases we'll have to do this semi-manually | 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 | 13:38 |
| dholbach | even if it fixes a bug | 13:38 |
| dholbach | so it can get more testing, etc. | 13:39 |
| dholbach | bobbo: please go on - just wanted to quickly interrupt :-) | 13:39 |
| bobbo | dholbach, awesome :D | 13:39 |
| karyo | Oh, I thought it only involved work in our part. Not decisions. bobbo go on please | 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 | 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] | 13:39 |
| bobbo | this time a bug was created with a patch attached | 13:39 |
| bobbo | it was reviewed by Omer Akram who sent it upstream | 13:40 |
| bobbo | nigelbabu, saw this and marked it -frowarded-upstream | 13:40 |
| karyo | isn't this a feature rather than a but fix? | 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 | 13:41 |
| bobbo | karyo, the process can work for both features and fixeds | 13:41 |
| 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 | 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" | 13:42 |
| dholbach | yep | 13:42 |
| bobbo | dholbach, do you want to take us through a "live" untouched example from the queue? | 13:42 |
| dholbach | let me check the list | 13:43 |
| karyo | I understand. btw am I interrupting too much? | 13:43 |
| dholbach | let me check the list | 13:43 |
| dholbach | karyo: not at all | 13:43 |
| dholbach | karyo: it's way more fun if this is a conversation | 13:43 |
| bobbo | karyo, the more questions you ask, the more we understand where we need to improve our documentation and processes | 13:43 |
| karyo | that's nice 'cause questions I've plenty | 13:44 |
| dholbach | karyo: do you have any more questions right now? | 13:45 |
| dholbach | ok let's see if https://bugs.edge.launchpad.net/ubuntu/+source/onboard/+bug/526791 makes any sense for us to work on | 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] | 13:46 |
| dholbach | if you have a look at the last comment | 13:46 |
| dholbach | you see that marmuta said "I've pushed a workaround to onboard and virtkey trunks" | 13:47 |
| dholbach | that means that the patches were already pushed upstream | 13:47 |
| dholbach | so it's easy, I'd just add patch-accepted-upstream and it'd be off our list | 13:47 |
| dholbach | nigelbabu, bobbo: do I remove the 'patch' tag? O:-) | 13:48 |
| bobbo | dholbach, yes :) | 13:48 |
| nigelbabu | dholbach: yes | 13:48 |
| dholbach | oh, it doesn't even have it | 13:48 |
| karyo | I don't have to reproduce and confirm that the patch is working? | 13:48 |
| dholbach | alright, another one off the list | 13:48 |
| nigelbabu | it doesn't have it, because its an older patch that's part of clean sweep | 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 | 13:48 |
| dholbach | nigelbabu: gotcha | 13:48 |
| 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 | 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 | 13:49 |
| karyo | so all I have to do is find out if upstream/debian is happy with the patch? | 13:49 |
| dholbach | karyo: if it wasn't tested first, you'd need to test it though | 13:50 |
| SevenMachines | is there a cutoff for these patches, ie, do we still try and fix the warty release :) | 13:50 |
| karyo | how do I know if it was tested? | 13:50 |
| dholbach | karyo: if it already was forwarded or even accepted, that's easy | 13:51 |
| dholbach | karyo: in the other cases you'd want to test | 13:51 |
| karyo | @sevenmachines I believe warthy is no longer supported | 13:51 |
| dholbach | https://bugs.edge.launchpad.net/ubuntu/+source/openttd/+bug/503725 is another one of those | 13:51 |
| ubot2 | Launchpad bug 503725 in openttd (Ubuntu) "CVE-2009-4007 (DoS of OpenTTD < 0.7.5) (affects: 2) (heat: 252)" [Low,Confirmed] | 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 | 13:51 |
| dholbach | it's about a security issue, directly in the initial comment it says | 13:51 |
| dholbach | There are two ways to fix this: | 13:51 |
| dholbach | - update to 0.7.5 (or higher) | 13:51 |
| dholbach | - apply the patch of 0.6.2-1+lenny1 (from Debian) | 13:51 |
| SevenMachines | scroogle-fu! | 13:52 |
| dholbach | so it's been accepted both in upstream and in debian | 13:52 |
| dholbach | I'll add patch-accepted-debian and patch-accepted-upstream | 13:52 |
| dholbach | as you can see: a lot of our work is to put those patches into buckets | 13:52 |
| dholbach | and make it clear what happened to the patches up until now | 13:52 |
| karyo | is update to 0.7.5 an option? | 13:52 |
| dholbach | so that further action can be taken easily | 13:52 |
| dholbach | karyo: sure | 13:53 |
| karyo | isn't that changing packages? | 13:53 |
| dholbach | if you have a look at http://packages.debian.org/search?searchon=sourcenames&keywords=openttd | 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 | 13:53 |
| dholbach | you can see that it's already in Debian | 13:53 |
| bundo | hayanbom, Your happy? | 13:54 |
| dholbach | another interesting example is https://bugs.edge.launchpad.net/ubuntu/+source/file-roller/+bug/354136 | 13:54 |
| ubot2 | Launchpad bug 354136 in file-roller (Ubuntu) (and 1 other project) "unintuitive back and forward (heat: 11)" [Low,Triaged] | 13:54 |
| dholbach | as you can see the bug was forward to upstream already | 13:54 |
| dholbach | click on the "gnome-bugs #578837" link | 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 | 13:54 |
| 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 | 13:55 |
| dholbach | if you compare the two suggested patches, you'll see they're identical | 13:55 |
| dholbach | http://bugzilla-attachments.gnome.org/attachment.cgi?id=132594 | 13:55 |
| dholbach | http://launchpadlibrarian.net/24728371/file-roller-2.24.1_history-fix2.diff | 13:55 |
| dholbach | the patch didn't get a review upstream though yet | 13:55 |
| dholbach | so we'd just add a patch-forwarded-upstream | 13:55 |
| 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 | 13:58 |
| dholbach | thanks a lot everybody | 13:58 |
| bobbo | thanks guys :D | 13:58 |
| dholbach | thanks bobbo for being a great host | 13:58 |
| dholbach | I'll make sure we get the logs up later on for others who are interested | 13:58 |
| karyo | thx | 13:58 |
| SevenMachines | thanks everyone | 13:58 |
| dholbach | I'll update statistics here: http://daniel.holba.ch/review/report | 13:58 |
| dholbach | (every week, so we know how we're doing compared to last week) | 13:58 |
| SevenMachines | sort of related, is there a developer week soon? or did i imagine it | 13:59 |
| dholbach | SevenMachines: a couple of weeks left still :) | 13:59 |
| karyo | what do I do when I cannot get an answer from upstream? | 13:59 |
| dholbach | karyo: I'd try to ask again, on the bug, maybe on their mailing list too | 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 | 14:01 |
| SevenMachines | joining ubuntu-reviews crashes my xchat :) | 14:02 |
| karyo | so then it's : try hard to talk to upstream, and then try talking to the relevant ubuntu team. right? | 14:02 |
| dholbach | karyo: ubuntu-sponsors | 14:02 |
| dholbach | and the relevant team | 14:02 |
| dholbach | be it desktop, server or kubuntu or wherever else :) | 14:02 |
| * dholbach heads out of here - see you in #ubuntu-reviews | 14:03 | |
| karyo | okay. I think I had enough for the day. It's bedtime here. good bye | 14:03 |
| dholbach | karyo: sleep tight! :) | 14:03 |
| * karyo heads out ZZZ | 14:03 | |
| dholbach | it was great to have you around | 14:03 |
| SevenMachines | bye all | 14:03 |
| MTecknology | dholbach: I missed it, didn't i? | 14:04 |
| nigelbabu | MTecknology: yes you did | 14:04 |
| dholbach | MTecknology: yep, I'll put up logs in a bit | 14:04 |
| MTecknology | dholbach: gosh darned you people that don't all use my timezone :P - I'm just waking up | 14:04 |
| === daker_ is now known as daker | ||
| === Esquire_ is now known as Esquire | ||
| === yofel_ is now known as yofel | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!