01:00:21 <nirik> #startmeeting How to Review Fedora Packages 01:00:21 <zodbot> Meeting started Wed Jun 9 01:00:21 2010 UTC. The chair is nirik. Information about MeetBot at http://wiki.debian.org/MeetBot. 01:00:21 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic. 01:00:21 <nirik> #meetingname reviewing-fedora-packages 01:00:21 <nirik> #topic Introduction/Welcome 01:00:21 <zodbot> The meeting name has been set to 'reviewing-fedora-packages' 01:00:35 <nirik> welcome everyone to another fedora irc class. ;) 01:00:57 * dhimel is back. 01:01:02 <nirik> As always, please do feel free to ask questions as the class goes on... and I will have a Q&A at the end as well. 01:01:36 <nirik> So, this class I will be talking about reviewing packages for Fedora. 01:01:54 <nirik> This is more targeted for those folks who are package maintainers or wish to become package maintainers than others... 01:02:12 <nirik> but it's still good background to know how the packages you use were reviewed and vetted before they got to your machine. 01:02:28 <nirik> #topic What is a package review? 01:02:54 <nirik> Packages you use in fedora (mostly) all have passed through a review process. 01:03:11 <nirik> This allows us to check them for a number of things and make sure they are up to providing for our users. 01:03:39 <nirik> The process checks for correctness of how the package is created as well as things like licensing 01:04:12 <nirik> Of course there could still be bugs that get past the review, but it's a good way to ensure high quality on packages we provide. 01:04:32 <nirik> #topic Roles in package review 01:04:47 <nirik> There are several folks involved in a package review: 01:05:24 <nirik> - The person who is submitting the package for review. This person could be someone who already maintains packages (a packager) or someone who is new and would like to maintain this new package for Fedora. 01:06:19 <nirik> - A reviewer: This person could be anyone. Comments on reviews are welcome from anyone at all... HOWEVER: you must be an existing packager to approve another packagers review, or a sponsor to sponsor a new person who isn't yet a packager. 01:06:28 <nirik> and to clarify: 01:06:45 <nirik> - A packager: Someone who's already in the fedora packager group. 01:06:56 <nirik> - A sponsor: Someone who can sponsor new packagers. 01:07:00 * dhimel is away: Gone away for now 01:07:02 <nirik> Any questions on all that? 01:07:06 * dhimel is back. 01:07:11 <ardchoille> How does someone get into the packager group? 01:07:16 <ardchoille> Find a sponsor? 01:07:27 <nirik> an excellent question. ;) 01:07:34 <makfinsky> Link to the join page? 01:07:45 <delhage> dhimel: could you please turn off auto-away? 01:07:48 <nirik> http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group 01:08:09 <nirik> and yes, the master list of what to do if you wish to become a packager is at: http://fedoraproject.org/wiki/PackageMaintainers/Join 01:08:34 <nirik> Its important to note that many sponsors will require you to provide reviews of other pending packages to show you know the guidelines. ;) 01:08:48 <nirik> (even though you cannot approve them yet as you would not be a packager yet) 01:08:59 <ardchoille> A sensible requirement 01:09:06 <nirik> yeah. :) 01:09:18 <nirik> #topic Information on existing reviews 01:09:48 <nirik> Reviews are done in bugzilla.redhat.com currently. In the "Package Review" component. 01:10:03 <nirik> There's several great ways to follow reviews in progress and learn from them: 01:10:09 <makfinsky> Link to example search of bugzilla? 01:10:17 <nirik> makfinsky: getting there. ;) 01:10:23 <makfinsky> Or is that asking too much? :D 01:10:24 <nirik> - https://admin.fedoraproject.org/mailman/listinfo/package-review is the package-review list. 01:10:40 <makfinsky> Sorry, don't mean to get ahead of you. 01:10:41 <nirik> It gets all the comments that happen on package review bugs. You can subscribe there and browse them as they come in. 01:10:47 <nirik> makfinsky: no worries. ;) 01:11:17 <nirik> - If you want to query bugzilla you can, but even better there are some cached queries that divide out the bugs into various types: 01:11:20 <nirik> http://fedoraproject.org/PackageReviewStatus/ 01:11:42 <nirik> So, you can see 4 'groups' of package reviews 01:12:11 <nirik> makfinsky: I can get you a raw query, but the cached one is much better, IMHO. 01:12:26 <makfinsky> The cached one is great! 01:13:00 <nirik> You can of course add your email to any existing review if you want to follow it, just like any other bug. 01:13:20 <nirik> #topic Types of reviews 01:13:33 <nirik> from the cached link you can see a number of types... 01:13:50 <nirik> there's the bulk of the reviews that are NEW (ie, no reviewer has taken them) 01:14:03 <nirik> There are reviews that need a sponsor to review them. 01:14:41 <nirik> There are merge reviews. :) These are packages that were in fedora core before the review guidelines and setup existed. They basically need to be reviewed to bring those old packages up to standard. 01:15:18 <nirik> And there are hidden. These are usually reviews where the reviewer is waiting for some fix or release to move forward or otherwise can't do the review right then. 01:15:31 <nirik> All that make sense? any questions on types? 01:15:55 <ardchoille> Question on merges typ;e 01:16:01 <nirik> ardchoille: go ahead. ;) 01:16:26 <ardchoille> Wasn't that completed? I mean are there still old packages handing about from FC6 waiting? 01:16:45 <nirik> no, there are still 246 pending. 01:16:46 <ardchoille> I thought F7 completed that merge 01:16:47 <nirik> at leasat. 01:16:51 <ardchoille> Oh, ok 01:16:58 <nirik> nope. There was a push to do so, but it didn't happen. 01:17:04 <ardchoille> ok 01:17:19 <nirik> the problem is that the packages are in and working, so maintainers find tweaking things for a merge review very low priority sometimes. 01:17:33 <ardchoille> that explains it 01:17:38 <nirik> yeah. 01:17:45 <nirik> it would sure be nice to finish them someday. ;) 01:18:03 <nirik> #topic Review Process (Basics) 01:18:12 <nirik> So, what does a review entail? 01:18:20 <nirik> http://fedoraproject.org/wiki/Package_Review_Process has a overview of the process. 01:18:34 <nirik> Basically it's a back and forth with the submitter and reviewer. 01:18:48 <nirik> the reviewer checks the package against the guidelines and points out any issues. 01:19:02 <nirik> the submitter fixes them and provides a new version of the package to check. 01:19:15 <nirik> when all issues are solved, the review is approved and the package can enter the collection. 01:19:38 <ardchoille> nice package submission setup 01:19:42 <nirik> Some basic things that are always checked: 01:20:20 <fenris02> rpmlint, 32/64b builds? 01:20:25 <nirik> - Is the review submitted correctly? it should have the right summary and format and block needsponsor if the submitter needs a sponsor. It should have links to the spec file of the package and the src.rpm of the package. 01:20:55 <nirik> yeah... rpmlint must be run on every submission and any output either fixed or noted that it doesn't matter. 01:21:29 <nirik> - A build in the buildsystem (a "scratch" build) is good to test that the package builds right and works on both 32 and 64bit. 01:21:37 <fenris02> rather, that it builds in mock under 32b/64b. not that it simply builds outside on it's own 01:21:50 <nirik> yep. 01:22:00 <nirik> - You can find a partial checklist at: http://fedoraproject.org/wiki/Packaging:ReviewGuidelines 01:22:36 <nirik> - The long form of the packaging guidelines is at: http://fedoraproject.org/wiki/Packaging/Guidelines 01:23:11 <nirik> Those go over the basic items needed. 01:23:45 * nirik will move on to talking about more advanced / specific guidelines in a bit here. 01:24:21 <nirik> #topic Licensing 01:24:52 <nirik> Another big item that must be checked on packages is that they are licensed such that they are acceptable for fedora. 01:25:16 <nirik> The http://fedoraproject.org/wiki/Packaging/LicensingGuidelines can help here. 01:25:42 <nirik> There is also a licensing mailing list for questions. 01:26:03 <nirik> As well as a way to block a review on for LEGAL review if you are unsure. 01:26:28 <nirik> A full list of acceptable and unacceptable licenses for fedora can be found at http://fedoraproject.org/wiki/Licensing 01:27:04 <nirik> It's important to check the source files for any package for licensing. Do not just trust that they note the right license in their information... 01:27:25 <nirik> In some cases you may need to contact the upstream project and ask them to clarify 01:27:37 <nirik> or provide more clear information on what license they are using. 01:28:00 <nirik> As you can see licensing can get complex. ;) 01:28:18 <ardchoille> That answers a question about emulators in Fedora 01:28:21 <nirik> However, many packages are clear and only under a single acceptable license. 01:28:43 <nirik> yep. There is a note on them there. 01:29:18 <nirik> Any general licensing questions? 01:29:21 <N3LRX> If the Copying file or LICENSE file is missing from the source I usually email the author and have them include it in the source before I submit for review. 01:29:56 <nirik> yeah, it's good to clarify. 01:30:05 <silentusr> nirik: is fonts, etc, still a mess w/ licenses? 01:30:32 <nirik> In fact one of the review checkpoints is to ask upstream to include a copying/license file if they don't do so. This isn't a blocker if the situation is clear though. 01:30:51 <nirik> silentusr: depends on the font. ;) 01:30:53 <nirik> http://fedoraproject.org/wiki/Licensing#Font_Licenses 01:31:24 <nirik> Fonts are typically a place where it's hard to find clear licensing or who made the font. 01:31:44 <nirik> ie, it's included on 20,000 sites, but who was the author? and what license is it under? 01:31:51 <nirik> but there are a number where it is clear. 01:31:53 <silentusr> nirik: yeah... I found one I was going to package and got way lost in licensing... (unfortunately, I gave up) 01:32:18 <nirik> sorry to hear it. ;( There is a fonts wishlist I think... with ones that are ok licensing wise. 01:32:40 <nirik> http://fedoraproject.org/wiki/Category:Font_wishlist 01:32:44 <ardchoille> I had no idea there were so many licenses 01:32:55 <nirik> yeah, sadly people like to keep inventing them. ;( 01:33:06 <fenris02> -etoomanylawyers 01:33:17 <nirik> oh, I will note one common gotcha that people run into before we move on: 01:34:07 <nirik> if you have a package that has no licensing info at all except a COPYING file thats the GPL (any version), the License would be "GPL+", ie any version of the GPL. It's best to ask for clarification in that case because they may intend some specific version. 01:34:33 <fenris02> the COPYING file should indicate which version, so you could at least put GPLv2+ 01:34:37 <nirik> nope. 01:35:04 <nirik> The GPL says "any version" in the COPYING file (for v1, v2, v3) 01:35:18 <fenris02> ok 01:35:32 <nirik> https://fedoraproject.org/wiki/Licensing/FAQ#How_do_I_figure_out_what_version_of_the_GPL.2FLGPL_my_package_is_under.3F 01:35:38 <nirik> yeah, seems weird, but there it is. ;) 01:36:17 <nirik> #topic Checklists 01:36:45 <nirik> the http://fedoraproject.org/wiki/Packaging:ReviewGuidelines provides somewhat of a checklist. 01:37:10 <nirik> Some folks have their own they generated. 01:37:19 <nirik> I have http://fedoraproject.org/wiki/KevinFenzi/ReviewTemplate on the wiki for anyone that wants it. 01:37:31 <nirik> I would strongly urge people new to reviewing to use a checklist. 01:37:43 <nirik> This makes sure you check each item and don't forget or pass something. 01:38:03 <nirik> It also helps show that you did a throurough review to others that are looking at the review ticket. 01:38:33 <ardchoille> Also helps if someone new chooses your review to learn from 01:38:41 <nirik> just saying "rpmlint looks ok, approved" isn't going to look very good, and might cause your review to be re-reviewed by a admin. 01:39:18 <nirik> yeah, some packages are simply better done and closer to guidelines when submitted than others. 01:39:26 <nirik> or are very simple. 01:39:41 <nirik> fonts, perl packages are 2 types that are very easy both to generate and review. 01:40:07 <nirik> any questions on checklists? 01:40:46 <nirik> #topic Specific package types guidelines 01:41:10 <nirik> So, the above was general things that all packages should be checked for. 01:41:28 <nirik> Specific types of packages have their own guidelines that should be checked. 01:41:32 <nirik> https://fedoraproject.org/wiki/Special:PrefixIndex/Packaging 01:41:39 <nirik> lists them all. 01:41:53 <nirik> Note that there are a bunch of "Draft" ones. They are not in effect. 01:42:16 <nirik> For example, if the package is a Perl package: https://fedoraproject.org/wiki/Packaging/Perl 01:42:19 <nirik> should be checked. 01:42:44 <nirik> Some folks concentrate on a particular type of package. 01:43:06 <nirik> there are about 4-5 folks that submit and review tons of perl packages. ;) 01:43:17 <nirik> some folks work on java, etc. 01:44:01 <nirik> It can be a bit daunting to look at all of them, but really it's not too hard to read up on a type when reviewing that type... 01:44:59 <nirik> it's worth just noting that Packaging guidelines are written up by the fedora packaging comittee. They work up and approve gudelines and then they are approved by FESCo. 01:45:12 <nirik> so, things can change and new guidelines can come in over time. 01:45:37 <fenris02> typically minor edits, or is is major revisions / reversal of older decisions ? 01:45:58 <nirik> typically minor, or adding some new type that appears... 01:46:24 <nirik> ie, a few years ago MinGW packages started coming in and guidelines were made for them. 01:47:02 <nirik> ok, moving along... 01:47:17 <nirik> #topic Stalled reviews 01:47:33 <nirik> it can happen that a submitter or a reviewer stops responding. 01:47:44 <nirik> We have guidelines for this case: http://fedoraproject.org/wiki/Policy_for_stalled_package_reviews 01:47:57 <nirik> This allows things to keep moving along... 01:48:21 <nirik> it's worth noting that currently the queue is quite long, so wait times could be long for finding a reviewer. 01:48:45 <nirik> Some ways to help that: offer to do a review swap... ie, review other persons submission and they review yours. 01:48:55 <nirik> find people interested in that package or type to review it. 01:49:11 <ardchoille> Review swap is a nice idea 01:49:26 <nirik> yeah, works nicely sometimes. 01:49:59 <nirik> #topic Review workflow 01:50:25 <nirik> So, you can of course do the reviewing in any way you like as long as everything is checked, but here's the workflow I typically use. 01:50:48 <nirik> - Download the spec and src.rpm locally. (this also confirms that they are valid links, still exist, etc) 01:50:59 <nirik> - Fire off a mock build of the package. 01:51:18 <nirik> - Note in the review ticket that I will be reviewing it and look for a review in a while. 01:51:43 <nirik> (This is important sometimes... it's a drag to go through a review and then find out someone else already just reviewed it while you were off reviewing) 01:51:57 <ardchoille> Good point 01:52:03 <nirik> (if you are not a packager, just mention that it's an informal review) 01:52:23 <nirik> - The first half or so of my checklist can be checked while the package is building. 01:52:57 <nirik> The naming of the package, consistent macros, etc. 01:53:32 <nirik> - I typically do a 'spectool -g package.spec' on the spec file to download a copy of the source package and check it against the source in the src.rpm. 01:53:44 <nirik> - I also take this time to unpack the source and look for licensing. 01:54:13 <nirik> Once those items are done, the package is usually build and I can run checks against it. 01:54:37 <nirik> rpmlint, rpm -qp package* --provides, and --requires 01:54:55 <nirik> check the build.log for any places it says "looking for foo... no" indicating a missing build requirement. 01:55:17 <nirik> check over devel subpackages for their stuff, etc. 01:55:40 <nirik> - I also like to note issues with numbers, so I can be clear which are fixed when the submitter comes back with a new version. 01:56:08 <nirik> - It's also nice to install and check the package (if easy) that it installs, works, etc. 01:56:22 <nirik> Then you can note those items in the review and ask the submitter to address them. 01:56:47 <nirik> Any questions on that workflow? 01:57:46 <nirik> #topic Q&A 01:57:54 <N3LRX> How critical are mixed macros? ie I see some where both %{buildroot} and $RPM_BUILD_ROOT are mixed. Does that really matter? Or is it being too critical? 01:57:56 <nirik> ok, any general questions on the review process? 01:58:10 <fenris02> N3LRX, bad form imho 01:58:29 <nirik> N3LRX: http://fedoraproject.org/wiki/Packaging/Guidelines#macros 01:58:31 <ardchoille> There's much to be said for consistency 01:58:44 <nirik> yeah, basically "Mixing the two styles, while valid, is bad from a QA and usability point of view, and should not be done in Fedora packages" 01:59:19 <nirik> while on the subject, IMHO, you should avoid the silly "%{__rm}" type macros. 01:59:48 <nirik> while valid, they are less readable, and very unlikely to change, and much more typing. ;) 02:00:50 <nirik> There are lots of little things, but should all be in the guidelines... ;) 02:01:10 <nirik> Any other questions? 02:02:15 <nirik> Well, thanks for coming everyone. :) I hope it was informative. 02:02:27 <makfinsky> Thanks nirik! 02:02:30 <N3LRX> Thanks for the class nirik lots of useful links to bookmark. :) 02:02:33 <nirik> #endmeeting