Spec URL: http://fedorapeople.org/~dcallagh/bzr-fastimport/bzr-fastimport.spec SRPM URL: http://repos.fedorapeople.org/repos/dcallagh/bzr-fastimport/fedora-15/SRPMS/bzr-fastimport-0.10.0-1.fc15.src.rpm Description: Bazaar Fast Import is a plugin providing fast loading of revision control data into Bazaar. It is designed to be used in combination with front-end programs that generate a command/data stream for it to process. Front-ends are available for a wide range of foreign VCS tools including Subversion, CVS, Git, Mercurial, Darcs and Perforce. Using git-bzr (a script available elsewhere), this plugin makes it possible to fetch from and push to bzr branches directly in git, so that I don't have to worry about learning how to use bzr properly :-) Mock build log is here: http://fedorapeople.org/~dcallagh/bzr-fastimport/build.log Note that this depends on python-fastimport, for which I have submitted a separate review request.
Looking at the spec I see you change info.py and hg2git.py with the line sed -e '1 { /^#!/d }' -i exporters/hg2git.py info.py That should instead be a patch (bzr-fastimport-shebang.patch) and applied as such. That way it can be dropped easier (if the patch gets applied upstream) and it's more obvious that you are patching the code.
(In reply to comment #1) Spec and SRPM updated: http://fedorapeople.org/~dcallagh/bzr-fastimport/bzr-fastimport.spec http://repos.fedorapeople.org/repos/dcallagh/bzr-fastimport/fedora-15/SRPMS/bzr-fastimport-0.10.0-2.fc15.src.rpm
A few things: 1) Please fix the FSF address in these files (the correct address is here: http://www.fsf.org/about/contact/ ): $ rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/bzr-fastimport-0.10.0-1.fc17.noarch.rpm bzr-fastimport.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/bzrlib/plugins/fastimport/tests/test_generic_processor.py bzr-fastimport.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/bzrlib/plugins/fastimport/cmds.py bzr-fastimport.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/bzrlib/plugins/fastimport/cache_manager.py bzr-fastimport.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/bzrlib/plugins/fastimport/tests/test_exporter.py bzr-fastimport.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/bzrlib/plugins/fastimport/tests/test_branch_mapper.py bzr-fastimport.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/bzrlib/plugins/fastimport/helpers.py bzr-fastimport.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/bzrlib/plugins/fastimport/bzr_commit_handler.py bzr-fastimport.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/bzrlib/plugins/fastimport/tests/test_commands.py bzr-fastimport.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/bzrlib/plugins/fastimport/exporters/__init__.py bzr-fastimport.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/bzrlib/plugins/fastimport/exporter.py bzr-fastimport.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/bzrlib/plugins/fastimport/__init__.py bzr-fastimport.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/bzrlib/plugins/fastimport/branch_mapper.py bzr-fastimport.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/bzrlib/plugins/fastimport/user_mapper.py bzr-fastimport.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/bzrlib/plugins/fastimport/marks_file.py bzr-fastimport.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/bzrlib/plugins/fastimport/revision_store.py bzr-fastimport.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/bzrlib/plugins/fastimport/processors/generic_processor.py bzr-fastimport.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/bzrlib/plugins/fastimport/processors/__init__.py bzr-fastimport.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/bzrlib/plugins/fastimport/branch_updater.py 1 packages and 0 specfiles checked; 18 errors, 0 warnings. You should also send the change to the upstream maintainers of this project. 2) Several parts of this spec are no longer needed in recent Fedora releases, and should be removed: a) "%defattr(-,root,root,-)" in the %files section b) the %clean section c) "rm -rf $RPM_BUILD_ROOT" in the %install section 3) This package does not function without python-fastimport, so it should have a Requires line for this.
Another note on this: the latest version of bzr-fastimport is 0.11.0. 0.10.0 has at least one issue with current bzr due to the relocation of the KnitPackRepository module.
Thanks for taking this review on, Andy. (In reply to comment #3) > A few things: > > 1) Please fix the FSF address in these files (the correct address is here: > http://www.fsf.org/about/contact/ ): > > [...] > > You should also send the change to the upstream maintainers of this project. It looks [1] like the current recommendation is just to put an URL to the GPL rather than the FSF's mailing address, which I think makes much more sense. I've added a patch to do that, and filed it upstream [2]. > 2) Several parts of this spec are no longer needed in recent Fedora releases, > and should be removed: > > a) "%defattr(-,root,root,-)" in the %files section > b) the %clean section > c) "rm -rf $RPM_BUILD_ROOT" in the %install section Done. > 3) This package does not function without python-fastimport, so it should have > a Requires line for this. Oops, probably the most important part of the spec file. Not sure how I missed that! Fixed. (In reply to comment #4) > Another note on this: the latest version of bzr-fastimport is 0.11.0. 0.10.0 > has at least one issue with current bzr due to the relocation of the > KnitPackRepository module. Version bumped. Updated spec and SRPM are here: http://fedorapeople.org/~dcallagh/bzr-fastimport/bzr-fastimport.spec http://repos.fedorapeople.org/repos/dcallagh/bzr-fastimport/fedora-15/SRPMS/bzr-fastimport-0.11.0-1.fc15.src.rpm [1] http://www.gnu.org/licenses/gpl-howto.html [2] https://bugs.launchpad.net/bzr-fastimport/+bug/868789
Thanks, everything looks good now. APPROVED
Dan, I will look and comment on this bug tomorrow. However before sponsoring you I would like to ask you to perform a couple of pre-reviews showing your understanding of the packaging guidelines. Since the two packages you have submitted are in python, a different language might be nice. Add me in cc to these reviews please (or mention them on this bug).
Ok I looked at the sources and the files tests/__init__.py seem to be GPLv2, might be a typo from upstream but worth checking it.
(In reply to comment #8) Sorry I missed this comment. You're right, that one file is lacking the "or (at your option) any later version" wording in the copyright header. Cc'ing Jelmer Vernooij who is one of the upstream maintainers. Jelmer, can you please confirm that tests/__init__.py is supposed to be licensed GPLv2+ rather than GPLv2?
It should indeed be GPLv2 *or later* - I've committed a fix to trunk.
Ok since upstream confirm the typo, could you please update the spec and the release by adding a comment about this file with a link to the commit. I will approve the new version
(In reply to comment #11) Updated: http://fedorapeople.org/~dcallagh/bzr-fastimport/bzr-fastimport.spec http://repos.fedorapeople.org/repos/dcallagh/bzr-fastimport/fedora-15/SRPMS/bzr-fastimport-0.11.0-2.fc15.src.rpm
Package Review ============== Key: - = N/A x = Check ! = Problem ? = Not evaluated [x] : MUST - Package successfully compiles and builds into binary rpms on at least one supported architecture. [x] : MUST - Spec file lacks Packager, Vendor, PreReq tags. [x] : MUST - Package does not contain any libtool archives (.la) [x] : MUST - Package use %makeinstall only when make install DESTDIR=... doesn't work. [x] : MUST - Package is named according to the Package Naming Guidelines. [x] : MUST - Sources used to build the package matches the upstream source, as provided in the spec URL. /home/pierrey/tmp/reviewhelper/713990/bzr-fastimport-0.11.0.tar.gz : MD5SUM this package : c3ba588120f346d9f72773e512c8ef34 MD5SUM upstream package : c3ba588120f346d9f72773e512c8ef34 [x] : MUST - Spec file name must match the spec package %{name}, in the format %{name}.spec. [-] : MUST - %config files are marked noreplace or the reason is justified. [-] : MUST - Package contains a properly installed %{name}.desktop using desktop-file-install file if it is a GUI application. [-] : MUST - Fully versioned dependency in subpackages, if present. [-] : MUST - Header files in -devel subpackage, if present. [-] : MUST - ldconfig called in %post and %postun if required. [-] : MUST - License file installed when any subpackage combination is installed. [-] : MUST - The spec file handles locales properly. [-] : MUST - No %config files under /usr. [-] : MUST - Development .so files in -devel subpackage, if present. [-] : MUST - Static libraries in -static subpackage, if present. [x] : MUST - Rpmlint output is silent. rpmlint bzr-fastimport-0.11.0-1.fc17.src.rpm ================================================================================ bzr-fastimport.src: W: spelling-error Summary(en_US) plugin -> plug in, plug-in, plugging bzr-fastimport.src: W: spelling-error %description -l en_US plugin -> plug in, plug-in, plugging 1 packages and 0 specfiles checked; 0 errors, 2 warnings. ================================================================================ rpmlint bzr-fastimport-0.11.0-1.fc17.noarch.rpm ================================================================================ bzr-fastimport.noarch: W: spelling-error Summary(en_US) plugin -> plug in, plug-in, plugging bzr-fastimport.noarch: W: spelling-error %description -l en_US plugin -> plug in, plug-in, plugging 1 packages and 0 specfiles checked; 0 errors, 2 warnings. ================================================================================ These can be safely ignored [x] : MUST - Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [-] : MUST - %build honors applicable compiler flags or justifies otherwise. [x] : MUST - All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [x] : MUST - Package contains no bundled libraries. [x] : MUST - Changelog in prescribed format. [x] : MUST - Sources contain only permissible code or content. [x] : MUST - Macros in Summary, %description expandable at SRPM build time. [x] : MUST - Package requires other packages for directories it uses. [x] : MUST - Package uses nothing in %doc for runtime. [x] : MUST - Package is not known to require ExcludeArch. [x] : MUST - Permissions on files are set properly. [x] : MUST - Package does not contain duplicates in %files. [-] : MUST - Large documentation files are in a -doc subpackage, if required. [x] : MUST - If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc. [x] : MUST - License field in the package spec file matches the actual license. [x] : MUST - Package consistently uses macros. instead of hard-coded directory names. [x] : MUST - Package meets the Packaging Guidelines. [x] : MUST - Package does not generates any conflict. [x] : MUST - Package does not contains kernel modules. [x] : MUST - Package contains no static executables. [x] : MUST - Package obeys FHS, except libexecdir and /usr/target. [x] : MUST - Package must own all directories that it creates. [x] : MUST - Package does not own files or directories owned by other packages. [x] : MUST - Package installs properly. [x] : MUST - Rpath absent or only used for internal libs. [x] : MUST - Package is not relocatable. [x] : MUST - Requires correct, justified where necessary. [x] : MUST - Spec file is legible and written in American English. [-] : MUST - Package contains a SysV-style init script if in need of one. [x] : MUST - File names are valid UTF-8. [-] : MUST - Useful -debuginfo package or justification otherwise. [x] : SHOULD - Reviewer should test that the package builds in mock. [x] : SHOULD - Dist tag is present. [x] : SHOULD - SourceX / PatchY prefixed with %{name}. [x] : SHOULD - SourceX is a working URL. [x] : SHOULD - Spec use %global instead of %define. [-] : SHOULD - Uses parallel make. [-] : SHOULD - The placement of pkgconfig(.pc) files are correct. [-] : SHOULD - If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x] : SHOULD - No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x] : SHOULD - Final provides and requires are sane (rpm -q --provides and rpm -q --requires). [x] : SHOULD - Package functions as described. [x] : SHOULD - Latest version is packaged. [x] : SHOULD - Package does not include license text files separate from upstream. [-] : SHOULD - Man pages included for all executables. [x] : SHOULD - Patches link to upstream bugs/comments/lists or are otherwise justified. [-] : SHOULD - Scriptlets must be sane, if used. [-] : SHOULD - Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x] : SHOULD - Package should compile and build into binary rpms on all supported architectures. [-] : SHOULD - %check is present and all tests pass. [x] : SHOULD - Packages should try to preserve timestamps of original installed files. This package is APPROVED
New Package SCM Request ======================= Package Name: bzr-fastimport Short Description: Bzr plugin for fast loading of data from other VCS tools Owners: dcallagh Branches: f15 el6 InitialCC: pingou
Git done (by process-git-requests). Added f16 branch.
bzr-fastimport-0.11.0-2.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/bzr-fastimport-0.11.0-2.fc16
bzr-fastimport-0.11.0-2.fc16 has been pushed to the Fedora 16 testing repository.
bzr-fastimport-0.11.0-2.fc16 has been pushed to the Fedora 16 stable repository.