Spec URL: http://informatiq.org/files/reviewboard.spec SRPM URL: http://informatiq.org/files/ReviewBoard-1.0alpha4-1.fc10.src.rpm Description: I just packaged ReviewBoard, I'm fairly new in packaging and would appreciate a review of them it was announced on the fedora-infrastructure mailing list that it is needed, so here it is.
Examining the output of rpmlint run against your SRPM/RPMs is always a good place to start. Are you a Fedora Contributor and member of the packager group, or are you seeking sponsorship?
Review Board also has django-evolution as a dep. Here's an initial spec: http://files.mesd.k12.or.us/~dyoung/reviewboard/django-evolution.spec
Can you please open Review request for django-evolution? This is the one for ReviewBoard ;-)
There's already a separate review for django-evolution, in APPROVED state: see bug 488103.
AUTHORS, COPYING, NEWS, and README should be placed in %doc.
Adding dependency on the review request you filed for Djblets (bug 487098) since this specfile has a Requires on Djblets.
I've taken Dan's work on top of Ramez's, updated it to the latest release candidate, and fixed some issues. I renamed the "Djblets" dependency to "python-djblets" to reflect the change I proposed in bug 487098. Hope this is all OK. Updated specfile is here: http://people.redhat.com/dmalcolm/python/ReviewBoard.spec Updated SRPM is here: http://people.redhat.com/dmalcolm/python/ReviewBoard-1.0-0.4.rc1.src.rpm Output from rpmlint is clean on the SRPM, and on the built RPM gives the output: ReviewBoard.noarch: E: non-executable-script /usr/lib/python2.4/site-packages/reviewboard/scmtools/testdata/svn_repo/hooks/post-lock.tmpl 0644 ReviewBoard.noarch: E: non-executable-script /usr/lib/python2.4/site-packages/reviewboard/scmtools/testdata/svn_repo/hooks/pre-lock.tmpl 0644 ReviewBoard.noarch: E: non-executable-script /usr/lib/python2.4/site-packages/reviewboard/contrib/tools/post-commit 0644 ReviewBoard.noarch: E: non-executable-script /usr/lib/python2.4/site-packages/reviewboard/scmtools/testdata/svn_repo/hooks/post-unlock.tmpl 0644 ReviewBoard.noarch: E: non-executable-script /usr/lib/python2.4/site-packages/reviewboard/scmtools/testdata/svn_repo/hooks/post-revprop-change.tmpl 0644 ReviewBoard.noarch: E: non-executable-script /usr/lib/python2.4/site-packages/reviewboard/scmtools/testdata/svn_repo/hooks/start-commit.tmpl 0644 ReviewBoard.noarch: E: non-executable-script /usr/lib/python2.4/site-packages/reviewboard/scmtools/testdata/svn_repo/hooks/pre-commit.tmpl 0644 ReviewBoard.noarch: E: non-executable-script /usr/lib/python2.4/site-packages/reviewboard/contrib/conf/reviewboard.fcgi.in 0644 ReviewBoard.noarch: W: devel-file-in-non-devel-package /usr/lib/python2.4/site-packages/reviewboard/diffviewer/testdata/new_src/foo.c ReviewBoard.noarch: E: zero-length /usr/lib/python2.4/site-packages/reviewboard/scmtools/testdata/svn_repo/db/write-lock ReviewBoard.noarch: E: non-executable-script /usr/lib/python2.4/site-packages/reviewboard/scmtools/testdata/svn_repo/hooks/post-commit.tmpl 0644 ReviewBoard.noarch: E: non-executable-script /usr/lib/python2.4/site-packages/reviewboard/scmtools/testdata/svn_repo/hooks/pre-unlock.tmpl 0644 ReviewBoard.noarch: E: non-executable-script /usr/lib/python2.4/site-packages/reviewboard/scmtools/testdata/svn_repo/hooks/pre-revprop-change.tmpl 0644 ReviewBoard.noarch: W: devel-file-in-non-devel-package /usr/lib/python2.4/site-packages/reviewboard/diffviewer/testdata/orig_src/foo.c These appear to all be example files or test data, and thus I don't think they're real problems. Caveat: I'm doing this all on a RHEL5 box, rather than specifically Fedora. How's this looking? Ramez, do you still want to own this package? Dan? I'd be happy to co-maintain.
(In reply to comment #7) > How's this looking? Ramez, do you still want to own this package? Dan? I'd > be happy to co-maintain. Please join in, i am kinda busy at the moment.
Are you sure about the "BuildRequires: python-setuptools-devel" dependency? I was able to build the RPM without it.
By the way, Review Board 1.0 was just released (http://www.review-board.org/news/2009/06/20/review-board-10-released), so maybe a package for 1.0 should be created.
any progress on packaging latest version?
Updated for 1.0.1; specfile is here: http://people.redhat.com/dmalcolm/python/ReviewBoard.spec and SRPM here: http://people.redhat.com/dmalcolm/python/ReviewBoard-1.0.1-1.fc11.src.rpm I dropped the "BuildRequires: python-setuptools-devel" as suggested in comment #9
(FWIW, my motivation here is that I'm hoping that Fedora Infrastructure can deploy an instance of reviewboard; see: https://fedorahosted.org/fedora-infrastructure/ticket/1196 "Deploy Review Board for use by hosted projects")
Please re-add the "BuildRequires: python-setuptools-devel" to the spec. I was unable to build it cleanly in mock without this. I attempted to build on a fully up-to-date F11 x86_64 box on mock using the fedora-11-x86_64 target.
(In reply to comment #14) > Please re-add the "BuildRequires: python-setuptools-devel" to the spec. I was > unable to build it cleanly in mock without this. > > I attempted to build on a fully up-to-date F11 x86_64 box on mock using the > fedora-11-x86_64 target. I'm running an up-to-date Fedora 11 x86_64 too I was able to rebuild ReviewBoard-1.0.1-1.fc11.src.rpm after removing the python-setuptools-devel package. What errors are you getting?
Ping? Any recent action here?
There seems to be missing dependencies on python-pygments and django-evolution and a newer python-djblets is needed But beyond that, it seems to be working in some quick setup and testing.
(In reply to comment #15) > (In reply to comment #14) > > Please re-add the "BuildRequires: python-setuptools-devel" to the spec. I was > > unable to build it cleanly in mock without this. > > > > I attempted to build on a fully up-to-date F11 x86_64 box on mock using the > > fedora-11-x86_64 target. > > I'm running an up-to-date Fedora 11 x86_64 too I was able to rebuild > ReviewBoard-1.0.1-1.fc11.src.rpm after removing the python-setuptools-devel > package. What errors are you getting? A mock build barfs without a BuildRequires on python-setuptools, specifically: + /usr/bin/python setup.py build Traceback (most recent call last): File "setup.py", line 15, in <module> from setuptools import setup, find_packages ImportError: No module named setuptools I've added "BuildRequires: python-setuptools" to fix the above and "Requires: python-pygments\nRequires: django-evolution" per jkatz in comment #17: http://files.mesd.k12.or.us/~dyoung/reviewboard/ReviewBoard.spec http://files.mesd.k12.or.us/~dyoung/reviewboard/ReviewBoard-1.0.1-2.fc11.src.rpm
Created attachment 379673 [details] SRPM for ReviewBoard 1.0.5.1
Created attachment 379674 [details] Specfile for ReviewBoard 1.0.5.1 I updated Dan Young's specfile from http://files.mesd.k12.or.us/~dyoung/reviewboard/ReviewBoard.spec to build ReviewBoard 1.0.5.1, now that python-djblets and Django 1.1.1 are available in Fedora. I needed to update to Django 1.1.1, as upstream ReviewBoard requires it for security fixes.
Created attachment 379845 [details] rpmlint output for 1.0.5.1 RPM Scratch build successfully performed for Rawhide: http://koji.fedoraproject.org/koji/taskinfo?taskID=1886173 Scratch build successfully performed for Fedora 12: http://koji.fedoraproject.org/koji/taskinfo?taskID=1886175 rpmlint output attached.
I'll have a go at reviewing this
For the record, this SRPM also builds successfully against EPEL5 ( http://koji.fedoraproject.org/koji/taskinfo?taskID=1886304 ), though we would certainly need to build its dependencies as well.
This is looking good, with 4 issues that need attention (see below). As I understand things, as this review was opened by Ramez, he would be the initial owner as things stand. However it appears from comment #8 that he may be rather busy. I spoke with Stephen today and he's keen to get this into Fedora ASAP. Stephen: are you happy to maintain this? Ramez: are you still interested in maintaining this package within Fedora? Dan: are you interested in (co)maintaining it? So we may want to complete the "review" part of the review, have Stephen open a fresh review request, close this one as dup of the new, and "grandfather in" the work done here. Does that sound OK? = Issues needing attention = (i) installation issue on F-12 Stephen's F12 scratch build doesn't install on my laptop F-12 with updates enabled, but not updates-testing: Error: Missing Dependency: Django >= 1.1.1 is needed by package ReviewBoard-1.0.5.1-1.fc12.noarch (/ReviewBoard-1.0.5.1-1.fc12.noarch) Error: Missing Dependency: python-djblets >= 0.5-0.1.rc1 is needed by package ReviewBoard-1.0.5.1-1.fc12.noarch (/ReviewBoard-1.0.5.1-1.fc12.noarch) - latest version of Django in F-12 updates is Django-1.1-4.fc12 - python-djblets doesn't seem to actually be in fedora-updates for f12 yet Stephen: do you have this installed and working on a machine? (ii) Source0: URL is 404; need to be changed to: http://downloads.reviewboard.org/releases/%{name}/1.0/%{name}-%{version}.tar.gz (see http://downloads.reviewboard.org/releases/ReviewBoard/1.0/ ; I notice there's a 1.1 directory as well FWIW) (iii) desktop files: the rb-site executable has a PyGTK GUI, so would normally require us to ship a .desktop file. However it can only be run when supplied a directory as a command-line argument, hence it wouldn't be meaningful to create a .desktop file for it. So this is OK, but please add a comment about the exception to the specfile. (iv) Does the package embed all of the requirements for the various SCM backends? (How well does this work with git?) (not easy to check this without a working install) = Notes = Filesystem layout: upstream have structured this code as a library and supporting tools that can be used to create (potentially) multiple local instances of ReviewBoard on a host, each stored in an arbitrary directory on the filesystem. All information for a specific instance (e.g. config, logs, tmp) goes below a particular directory for that instance (rather than e.g. /etc). I think it's acceptable for our package to reflect how upstream have structured this. = Reviewed items = - naming: name matches that of upstream tarball - specfile name is good - packaging guidelines: - N-V-R looks good - licensing "MIT" in spec matches that of README and of setup.py - spec is legible - spec follow python norms - changelog: OK - tags: OK - buildroot path uses 2nd recommendation in guidelines - buildroot is cleaned - %clean is present and correct - buildrequirements: successfully scratch-built in Koji - textual documentation present in built RPM below /usr/share/doc/ReviewBoard-1.0.5.1 - compiler flags/debuginfo packages/devel packages: N/A - pkgconfig: N/A - shared libraries: N/A - packaging static libraries: N/A - dup of system libraries: doesn't seem to - rpath: N/A for pure python code - config files: see note about FHS above - initscripts: N/A - macros: OK - locale handling: no translations present in upstream source - scriptlets: N/A - code vs content: OK - file and dir ownership: OK - users and groups: doesn't have its own user - web app: uses /usr/lib/python for its data, which seems reasonable - /srv: OK - patches: none yet - epochs: OK - Python-specific guidelines: OK - license: OK - specfile is legible - MD5sum: OK - tarball in srpm: 16947ddda7ec9df41f243949ec83a950 ReviewBoard-1.0.5.1.tar.gz - tarball from upstream: 16947ddda7ec9df41f243949ec83a950 ReviewBoard-1.0.5.1.tar.gz - rest of the MUST items covered above - I've tested an earlier version of the rpm and it functions
Created attachment 379897 [details] Specfile for ReviewBoard 1.0.5.1 I am willing to co-maintain ReviewBoard (but I do not want to be its exclusive maintainer). I don't think there's really a need to go through the trouble of opening a second review request, but if that's The Way It's Done, sure. Issues: (i) Dgango 1.1.1-2 is in updates-testing, python-djblets I installed from this koji build: http://koji.fedoraproject.org/koji/buildinfo?buildID=147983 (I have submitted this package for updates-testing in F-12, so it should be present tomorrow) (ii) Source location fixed. For the record, the 1.1 branch is their unstable development branch, and I do not intend at the moment to import that. (iii) Desktop file comment added. (iv) The package contains all the files necessary to support git, mercurial, bazaar, clearcase, cvs, perforce and subversion at least. I am unable to test the suitability of all of them. I have a test environment working with git successfully using this RPM.
This specfile built successfully in Koji here: http://koji.fedoraproject.org/koji/taskinfo?taskID=1886848 The SRPM and built noarch RPM is also available there.
Thanks; I think several of us want to comaintain this package, so I think that should be OK. A minor issue: you didn't update the %changelog in your latest specfile Other than that, this looks good to go. ACCEPTED. Setting "fedora-review" flag Please fix the %changelog before importing the specfile - thanks!
New Package CVS Request ======================= Package Name: ReviewBoard Short Description: Web-based code review tool Owners: sgallagh dmalcolm Branches: F-12 InitialCC:
(In reply to comment #24) > Dan: are you interested in (co)maintaining it? Sure, I can co-maintain. I'll request the ACL once ReviewBoard lands in CVS/pkgdb.
(In reply to comment #25) > (i) Dgango 1.1.1-2 is in updates-testing, python-djblets I installed from this > koji build: http://koji.fedoraproject.org/koji/buildinfo?buildID=147983 (I have > submitted this package for updates-testing in F-12, so it should be present > tomorrow) I submitted python-djblets in bodhi shortly after building: https://admin.fedoraproject.org/updates/python-djblets-0.5.6-0.fc12 I assume I just did that too late in the day yesterday for RelEng to sign it for updates-testing today.
CVS Done
ReviewBoard built in Rawhide: http://koji.fedoraproject.org/koji/taskinfo?taskID=1900941
Package Change Request ====================== Package Name: ReviewBoard New Branches: EL-5 Owners: sgallagh ReviewBoard is being considered for inclusion in the Fedora Hosted infrastructure. We need to branch this package to EL-5 for this support.
cvs done.
Looking at https://admin.fedoraproject.org/pkgdb/packages/name/ReviewBoard I see that this has been imported and built; belatedly closing this review out.
ReviewBoard-1.0.5.1-2.1.el5 has been submitted as an update for Fedora EPEL 5. http://admin.fedoraproject.org/updates/ReviewBoard-1.0.5.1-2.1.el5
ReviewBoard-1.0.5.1-2.1.el5 has been pushed to the Fedora EPEL 5 stable repository. If problems still persist, please make note of it in this bug report.
Comment on attachment 379897 [details] Specfile for ReviewBoard 1.0.5.1 Clearing review flag from attachment to stop daily BZ nag emails; I believe this package was approved many years ago.