Dev Week -- Learning from mistakes - REVU reviewing best practices -- mok0 -- Thu Sep 3rd, 2009


(02:04:53 PM) mok0: OK, so this class is "Learning from mistakes - REVU reviewing best practices"
(02:05:03 PM) mok0: Can we have a count of hands, please?
[14:06:50] <mok0> Can I chat here too?
[14:07:08] <dlightle> sure :)
[14:07:19] <frandieguez> sure
[14:07:27] <mok0> This class is going to be about packaging and reviewing packages on the REVU site.
[14:07:40] <^arky^> o/
[14:07:44] <mok0> We will look at some uploads at the REVU site, developed and maintained by MOTUs... you are probably all familiar with it...
[14:07:53] <AntoineLeclair> now you're on the wrong channel :P
[14:08:01] <EagleScreen_> hi
[14:08:12] <mok0> AntoineLeclair: Uhm ... no
[14:08:32] <mok0> I refuse to switch windows back and forth all the time
[14:08:38] <HobbleAlong> hi
[14:08:38] <RainCT_> haha
[14:08:53] <mok0> Reviewing other peoples packages is a great way to learn. First of all, you can learn new tricks and see how others solve problems. But you can also learn from mistakes, and via looking at many packages will familiarize you with the Ubuntu Policy Manual which of course is the packager's bible :-)
[14:08:53] <AntoineLeclair> ha, ok then :P as you wish
[14:08:56] <RainCT_> mok0: best to ask for an unmute then
[14:09:05] <frandieguez> ok, it's fine... but write an alert on the other channel
[14:09:08] <mok0> RainCT_: unmute?
[14:09:22] <AntoineLeclair> (is there an IRC log for the "-chat" channel also?)
[14:09:33] <RainCT_> mok0: so you can talk and get answers in #ubuntu-classroom ;)
[14:09:42] <c_korn> where should our questions go to then ?
[14:09:49] <mok0> Here
[14:09:56] <mok0> I can read
[14:10:00] <openweek1> :)
[14:10:10] <AntoineLeclair> haha, go ahead ;)
[14:10:43] <mok0> Heh, right, so another place to get information is the MOTU packaging guide:
[14:11:23] <mok0> Which is basically a practical hash-up of what's in the Policy Manual
[14:12:06] <mok0> Let's go to REVU and select some package to look at. REVU allows everyone to leave comments, but you need to lock in with your Launchpad ID using the OpenID system.
[14:12:21] <mok0>
[14:12:45] <mok0> First, let us look at a nifty feature of REVU
[14:13:06] <mok0> At the top of the page, see a link called "Tags Cloud"
[14:13:51] <mok0> That allows us to select packages according to category. So, if you are interested in chemistry, you can see packages about that
[14:14:18] <mok0> However, the tag system only works if someone has put in some tags
[14:14:25] <mok0> So lets try that now
[14:15:09] <mok0> For example, let's take the last package on the NEW page, php5
[14:15:35] <mok0> (I don't know what that package has been uploaded to REVU, but that's another matter)
[14:16:28] <mok0> Anyway, when you go into the project page, look near the bottom where it says "Tags:"
[14:16:40] <mok0> You can edit them
[14:16:57] <mok0> Everyone with me?
[14:17:09] <frandieguez> please paste the url
[14:17:12] <AntoineLeclair> hum, are you at ?
[14:17:41] <efm> you're supposed to give the lectue in -classroom, so it is logged
[14:18:03] <ScottTesterman> I'm saving the log; it's fine.
[14:18:08] <efm> ok
[14:18:20] <mok0> efm, yes, but I didn't like the way the session was set up
[14:18:32] <mok0> efm: I need everyone to be in the same channel
[14:19:07] <jacob> (mok0: you can actually run /mode #ubuntu-classroom -m and everyone will be able to talk, but continue anyway)
[14:19:36] <mok0> jacob: ok let me do that then
[14:19:43] <mok0>  /mode #ubuntu-classroom -m
(02:09:00 PM) mok0: Please go to #ubuntu-classroom-chat
(02:19:27 PM) mok0: there
(02:19:40 PM) mok0: OK, so we'll carry on here, sorry for the confusion
(02:19:50 PM) AntoineLeclair: are you at ?
(02:19:54 PM) frandieguez_: yes
(02:20:03 PM) c_korn: yes
(02:20:16 PM) AntoineLeclair: I was asking to mok0, hehe
(02:20:37 PM) mok0: So, let's combine this tutorial with the useful and let uploaders benefit by getting their packages reviewed. Therefore, we will leave our comments once we have reviewed the package(s).
(02:20:54 PM) mok0: I'm open to suggestions :-)
(02:21:24 PM) mok0: It should be a NEW package, i.e. one that hasn't been reviewed before
(02:22:26 PM) c_korn: celtx
(02:22:39 PM) mok0: OK, I found that too...
(02:22:40 PM) frandieguez_: yes celtx
(02:23:01 PM) mok0: The first thing I generally do before spending time on a package, is to check if it's already been uploaded to Debian, or if there's been an ITP bug filed. That indicates that someone else might be working on the package, in which case there might be a conflict of interest and/or a duplicate effort == waste of someones time.
(02:23:23 PM) mok0: So let's look here: and and just do a search for the package name in the browser.
(02:24:03 PM) mok0: What's the verdict?
(02:24:12 PM) frandieguez_: isn't there
(02:24:24 PM) c_korn: nothing found
(02:24:27 PM) mok0: :)
(02:24:29 PM) ScottTesterman: not found
(02:24:46 PM) mok0: The next thing I do is to du a cursory check if this software can be distributed at all. Otherwise, there's no need spending time on it. We absolutely need a file -- normally called COPYING -- that grants Ubuntu permission to distribute the software. So let's browse down REVU's html pages into the directory and see if this permission is present.
(02:26:10 PM) mok0: Alright, so this code is derived from mozilla it seems
(02:26:17 PM) frandieguez_: yes
(02:26:21 PM) frandieguez_:
(02:26:44 PM) ScottTesterman: It's under the CePL
(02:26:51 PM) mok0: There's a file called LICENSE
(02:27:06 PM) mok0: ScottTesterman: GPL?
(02:27:16 PM) mok0: I see Mozilla Public License in there
(02:27:17 PM) frandieguez_: MPL!
(02:27:38 PM) ScottTesterman: Under debian/ there's a copyright file.
(02:27:43 PM) ScottTesterman: It's says CePL.
(02:27:52 PM) ScottTesterman: The Celtix Public License.
(02:28:16 PM) mok0: ScottTesterman: Ah, you're  way ahead of me :-)
(02:28:25 PM) ScottTesterman: Woops, sorry!
(02:28:32 PM) mok0: But let's look at debian/copyright, then
(02:29:53 PM) mok0: Well, this is a complicated license situation
(02:30:01 PM) frandieguez_: of course...
(02:30:33 PM) ScottTesterman: It looks like it's fine to use as long as Ubuntu a) changes the name of the product, and b) rips out all the Celtx logos and names from the product.
(02:30:35 PM) mok0: It is necessary to spend time reading all this stuff to figure out if the copyright file is OK
(02:31:37 PM) mok0: ScottTesterman: Right, so one job for the reviewing is to see that this is done
(02:32:14 PM) mok0: My next step is generally to download the package. I find the link to the relevant .dsc file, right click -> copy the link. Then I move into a terminal, and type "dget -ux " + right-click -> paste.
(02:33:10 PM) mok0: Yuc, it's huge
(02:33:23 PM) mok0: :-)
(02:33:54 PM) mok0: So... have you guys got a pbuilder or something like that?
(02:34:07 PM) frandieguez_: yes
(02:34:18 PM) dinxter_: yep
(02:34:18 PM) frandieguez_: from previous lectures
(02:34:19 PM) ScottTesterman: yes
(02:34:23 PM) AntoineLeclair: same here
(02:34:30 PM) mok0: Cool, so let's see if it builds
(02:34:30 PM) c_korn: yes
(02:35:26 PM) mok0: ... If the build fails, the review is usually quite short :-P
(02:36:43 PM) mok0: As you probably know, a source package is mainly composed of the pristine tarball and a diff.gz file containing the work of the packager. While it is possible for the diff.gz file to patch everything in the source tree, the current paradigm is that nothing outside the debian/ directory must be touched.
(02:37:10 PM) mok0: So, while this is building, let's check to see that nothing else is in the .diff.gz file:
(02:37:10 PM) mok0: lsdiff -z <package>.diff.gz
(02:37:39 PM) c_korn: fail
(02:38:08 PM) mok0: c_korn: you mean the two files in $topdir?
(02:38:24 PM) c_korn: and the files in the mozilla directory
(02:38:34 PM) c_korn: config.log e.g.
(02:38:35 PM) mok0: c_korn: oh yes didn't see those at first
(02:38:42 PM) mok0: tsk tsk tsk
(02:39:06 PM) mok0: I need a volunteer to write the review... ;-)
(02:39:26 PM) mok0: I generally do it in a text file and copy/paste that later into REVU
(02:40:07 PM) mok0: Another thing is that celtx.desktop is duplicated, it's also in debian/
(02:40:11 PM) c_korn: well, I am logged in...
(02:40:43 PM) mok0: c_korn: thanks
(02:40:59 PM) mok0: So, how do you like celtx.1 ?
(02:41:19 PM) c_korn: should propably in debian/ too ?
(02:41:25 PM) c_korn: +be
(02:41:25 PM) mok0: c_korn: yes
(02:41:37 PM) mok0: I am talking about the content of it
(02:41:42 PM) mok0: ?
(02:41:48 PM) mok0: Pretty useless if you ask me.
(02:42:22 PM) c_korn: oh, yes. that too :)
(02:43:01 PM) mok0: I generally refer people to the Linux Man page Howto:
(02:43:18 PM) mok0: Citation: "The DESCRIPTION section ...eloquently explains why your sequence of 0s and 1s is worth anything at all. Here's where you write down all your knowledge. This is the Hall Of Fame. Win other programmers' and users' admiration by making this section the source of reliable and detailed information. Explain what the arguments are for, the file format, what algorithms do the dirty jobs."
(02:43:59 PM) mok0: The man page is for people wanting a bit more information than is given in the package description
(02:44:42 PM) mok0: Next we do a cursory check of the files in debian/. We need at least five files to be present there: control, changelog, copyright, config and rules. Otherwise, the package won't build!
(02:45:21 PM) c_korn: config ?
(02:45:28 PM) mok0: We are reviewing for karmic+1 at this point, and StandarsdVersion is 3.8.3
(02:45:36 PM) mok0: compat
(02:45:37 PM) mok0: sorry
(02:45:42 PM) c_korn: conok
(02:45:44 PM) c_korn: -con
(02:46:18 PM) c_korn: everything there
(02:46:22 PM) mok0: Yes
(02:46:43 PM) mok0: control looks good
(02:47:03 PM) mok0: except for Standards-Version: 3.8.3
(02:47:24 PM) mok0: What about changelog?
(02:48:07 PM) c_korn: hm, shouldn't the revision be 0ubuntu1 or whatever ?
(02:48:24 PM) mok0: YES!
(02:48:29 PM) frandieguez_: c_korn i think the same
(02:48:38 PM) c_korn: good :)
(02:48:53 PM) mok0: And all changelog entries should be collapsed to 1
(02:48:59 PM) frandieguez_: and there is a lot of no usefull change lines
(02:49:05 PM) frandieguez_: xD
(02:49:34 PM) mok0: And this is important:
(02:50:11 PM) mok0: The changelog should document EVERYTHING the packager has done that makes the package different from upstreams tarball
(02:50:23 PM) mok0: In this case, he has written a manpage
(02:51:16 PM) frandieguez_: mok0 the pattern of file_changed: explication will be great for all the changes
(02:51:21 PM) mok0: Another thing you would normally document in changelog is patches needed to get the thing to compile or customized for Ubuntu
(02:51:31 PM) frandieguez_:  * file_changed: explication
(02:51:50 PM) mok0: frandieguez, what do you mean?
(02:52:22 PM) frandieguez_: the explications of changes on the source, this is better with the lines formated
(02:52:26 PM) frandieguez_:  * file_changed: explication
(02:52:40 PM) mok0: frandieguez, ah, yes
(02:53:09 PM) mok0: So, what about debian/dirs ??
(02:53:24 PM) frandieguez_: pufff, is the default file
(02:53:26 PM) c_korn: looks like the sample
(02:53:37 PM) mok0: yes, it should go
(02:53:39 PM) c_korn: usr/sbin usually isn't touched
(02:53:46 PM) mok0: exactly
(02:54:08 PM) mok0: dirs is only for creating empty dirs that the app needs
(02:54:19 PM) mok0: for example for plugins or something
(02:55:07 PM) mok0: If you look at prerm, it looks like the program needs something called /usr/lib/celtx/updates
(02:55:22 PM) mok0: I wonder if that dir is created or not...
(02:56:01 PM) mok0: It's not
(02:56:27 PM) c_korn: already built the package ?
(02:56:27 PM) mok0: So, that directory actually should be in dirs instead of what's there now
(02:56:44 PM) mok0: c_korn: yes I have a fast machine :-)
(02:57:09 PM) frandieguez_: yes
(02:57:27 PM) c_korn: ok :)
(02:57:27 PM) mok0: ... and /usr/sbin is an empty dir in the .deb :-(
(02:58:06 PM) mok0: The the de-facto requirement is to have a debian/watch file also. I require it when advocating :-) ... the exception being when upstream's sources are only available from a VCS.
(02:58:06 PM) mok0: Let's see if this watch file works... uscan --report-status
(02:59:02 PM) c_korn: Newest version on remote site is 201, local version is 2.0.1
(02:59:04 PM) frandieguez_: works
(02:59:11 PM) c_korn: => Package is up to date
(02:59:37 PM) mok0: I'm puzzled about the mangling
(02:59:48 PM) mok0: (mangled local version number 201)
(02:59:56 PM) mok0: Why does he do that?
(03:00:50 PM) mok0: Oh
(03:01:14 PM) ScottTesterman: Shouldn't the watch file return the newest version available for download?
(03:01:21 PM) mok0: ScottTesterman: yes
(03:01:49 PM) ScottTesterman: OK, then the watch doesn't work.  Newest version on remote site is 2.0.2.
(03:01:52 PM) mok0: But the name of the tarball is celtx-201-src.tar.gz
(03:02:33 PM) ScottTesterman: ah, I see
(03:02:48 PM) mok0: so the watch file needs to remove the '.' from the ubuntu version to be able to compare
(03:02:54 PM) ***jcastro points at the clock
(03:03:13 PM) mok0: Uhuh
(03:03:27 PM) c_korn: oh, time is running short
(03:04:00 PM) mok0: Well, thanks for coming guys

MeetingLogs/devweek0909/REVU_Practices (last edited 2009-09-03 19:11:26 by adsl-235-67-198)