Bug 515049 (dvisvgm)

Hi,

I'm going to do the review :-)

Are you already sponsored and part of the packager group ?

I cannot find you.

If you are not you have to add a FE-NEEDSPONSOR in the blocks part as described there:
https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Create_Your_Review_Request

Oh, sorry. I indeed forgot to mention that this is actually my first package submitted here. So I'm looking for a sponsor (block field has been updated).

I still do the review:

+ rpmlint silent:
  3 packages and 0 specfiles checked; 0 errors, 0 warnings.
+ package name ok
+ spec file name ok
! License should be GPLv2+ as it seems to fit best the source code and the README file. You might want to contact upstream to ask him to correct the tag on the website.
! source0 is not correct, please see
    https://fedoraproject.org/wiki/Packaging/SourceURL
+ package compiles on koji :
   http://koji.fedoraproject.org/koji/taskinfo?taskID=1572748
+ BuildRequires are ok
+ No translation available
+ Package owns its files
+ No duplicated files listed
+ Permissions ok
+ %clean present
+ Package code not content
+ No large documentation
+ No headers nor static
+ No pkgconfig files
+ No .so files
+ No .la files
+ No GUI available
+ Clean at the beginning of %install

- The license file being not present in the sources you might want to ask
 upstream to include one
- Worked on the .dvi I used


This package is for sure quite clean, few minor issues that we can easily fix.
    
I unfortunately cannot sponsor you.

Thanks for your immediate review. Since I'm the developer of dvisvgm I will directly update all items you mentioned above.

Please note as well that some C files have their header as GPL without specifying the version. It might be nice to be consistent on them :-)

We like it when upstream maintainers are involved with Fedora.  Once you've made those changes, please put together a new package and post the links here.  I'll take a quick look and if everything goes OK I'll get you set up.

Spec URL: http://dvisvgm.sourceforge.net/fedora/dvisvgm.spec
SRPM URL: http://dvisvgm.sourceforge.net/fedora/dvisvgm-0.8.1-1.fc11.src.rpm

Thanks for all your comments and support. 

I've updated the spec file and added the GPL v2 LICENSE file to the tarball. However, I'm not quite sure about the license constraints because the sources contain some unmodified, third-party files that are licensed under GPL v2+. Even if I'd like to publish my own code under GPL v3, do the bundled files affect the license of the complete package, e.g. force it to be GPL v2+ too (which wouldn't be a problem for me)? For now, I stay with GPL v2+.

I didn't change the headers of the third-party files, so some of them still don't mention their GPL version.

(In reply to comment #8)
> Spec URL: http://dvisvgm.sourceforge.net/fedora/dvisvgm.spec
> SRPM URL: http://dvisvgm.sourceforge.net/fedora/dvisvgm-0.8.1-1.fc11.src.rpm
> 
> Thanks for all your comments and support. 
> 
> I've updated the spec file and added the GPL v2 LICENSE file to the tarball.
> However, I'm not quite sure about the license constraints because the sources
> contain some unmodified, third-party files that are licensed under GPL v2+.
> Even if I'd like to publish my own code under GPL v3, do the bundled files
> affect the license of the complete package, e.g. force it to be GPL v2+ too
> (which wouldn't be a problem for me)? For now, I stay with GPL v2+.

You can license your own files as GPLv3+. Then the binary will be formed out of GPLv2+ and GPLv3+ parts, so the License: tag would be either "GPLv2+ and GPLv3+" or just "GPLv3+" since that's the license of the binary that's produced.

> I didn't change the headers of the third-party files, so some of them still
> don't mention their GPL version.  

If they don't mention a version, they're GPL+.

**

Drop
 gzip -9 ${RPM_BUILD_ROOT}%{_mandir}/*/*
as RPM will do this for you automatically.

**


I am willing to sponsor you if you show me your knowing of the Fedora  guidelines, most importantly
http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets

This you can do by making at least one other submission and performing a couple of informal reviews of packages of other people. Please review only packages *not* marked with FE-NEEDSPONSOR, as I will have to do the full formal review after you to check that you have got everything correctly. Once I have sponsored you you will be able to do formal reviews of your own.

Also, you are using the potrace library which needs to be packaged separately.
http://fedoraproject.org/wiki/Packaging/Guidelines#Duplication_of_system_libraries

(In reply to comment #10)
> Also, you are using the potrace library which needs to be packaged separately.
> http://fedoraproject.org/wiki/Packaging/Guidelines#Duplication_of_system_libraries  

I have packaged potrace in bug #515206, you need to make this package use that version.

Spec URL: http://dvisvgm.sourceforge.net/fedora/dvisvgm.spec
SRPM URL: http://dvisvgm.sourceforge.net/fedora/dvisvgm-0.8.1-1.fc11.src.rpm

Thanks for your comments and for providing the potrace-devel package. 
I've removed the gzip line from the spec file and added a patch that changes the Makefile to link the separately packaged potrace library.

Hi Jason and Jussi,

thanks for willing to sponsor me. I really appreciate that. Since I'm primarily the upstream maintainer of dvisvgm and probably won't have much time to help reviewing many other packages, I'm currently uncertain what to do now. If it's a prerequisite to do some reviews and provide further packages before getting sponsored, I'll try do that in the following weeks (I'm pretty busy at the moment). However, if there is another possibility for developers to get their utilities added to Fedora I would prefer that.
Maybe it's also possible someone else takes over this package.

For upstream maintainers I am willing to make exceptions - you don't need to make another submission, you only need to do the informal reviews. They shouldn't take too much time, as it's mostly just going through the review checklist.

For instance bug #510865 and bug #514634 should be fine.

Great, thanks for that. I've looked over bug #510865 and bug #514634 and added comments according to the review guidelines. Hopefully, they are complete and correct. 
I just found an unsubmitted Fedora package of cxxtest (a junit-like tool for C++) that I privately built few month ago. cxxtest is not yet part of Fedora. Maybe I could upload a cleaned version of this package as my second submission.

Ok, package cxxtest submitted: bug #515463

To make sure the package doesn't use the bundled potracelib, remove the directory in %prep with

 %prep
 %setup -q
 %patch0 -p 1 -b .orig
 # Remove bundled potracelib
 rm -rf potracelib

**

You haven't updated the SRPM to the newest spec.

Also, whenever you make changes to the spec you must bump the release tag and make a comment in the changelog. Otherwise it's very hard for others to keep track.

To my count the releases so far have been:
1: initial release
2: rebase to upstream with added tarball (btw, you should have made a point release upstream :P)
3: don't use bundled potrace

so the next release should be 4. Add the missing entries to the changelog.

**

rpmlint output is clean.


MUST: The package does not yet exist in Fedora. The Review Request is not a duplicate. OK
MUST: The spec file for the package is legible and macros are used consistently. OK
MUST: The package must be named according to the Package Naming Guidelines. OK
MUST: The spec file name must match the base package %{name}. OK
MUST: The package must be licensed with a Fedora approved license and meet the  Licensing Guidelines. OK
MUST: The License field in the package spec file must match the actual license. OK
MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. OK
MUST: The package MUST successfully compile and build into binary rpms. OK
MUST: The spec file MUST handle locales properly. N/A

MUST: Optflags are used and time stamps preserved. OK
- All files are generated in %build, no need to preserve time stamps upon install.

MUST: Packages containing shared library files must call ldconfig. OK
MUST: A package must own all directories that it creates or require the package that owns the directory. OK
MUST: Files only listed once in %files listings. OK
MUST: Debuginfo package is complete. OK
MUST: Permissions on files must be set properly. OK
MUST: Clean section exists. OK
MUST: Large documentation files must go in a -doc subpackage. N/A

MUST: All relevant items are included in %doc. Items in %doc do not affect runtime of application. NEEDSWORK
- Add LICENSE to %doc.

MUST: Header files must be in a -devel package. N/A
MUST: Static libraries must be in a -static package. N/A
MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'. N/A
MUST: If a package contains library files with a suffix then library files ending in .so must go in a -devel package. N/A
MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency. N/A
MUST: Packages does not contain any .la libtool archives. N/A
MUST: Desktop files are installed properly. N/A
MUST: No file conflicts with other packages and no general names. OK
MUST: Buildroot cleaned before install. OK
SHOULD: %{?dist} tag is used in release. OK
SHOULD: If the package does not include license text(s) as separate files from upstream, the packager should query upstream to include it. OK
SHOULD: The package builds in mock. OK

**

Fix the issues.

You can apply for packager group membership in FAS now and I'll sponsor you.

Spec URL: http://dvisvgm.sourceforge.net/fedora/dvisvgm.spec
SRPM URL: http://dvisvgm.sourceforge.net/fedora/dvisvgm-0.8.1-4.fc11.src.rpm

Issues hopefully fixed.


(In reply to comment #17)
> (btw, you should have made a point
> release upstream :P)

Yeah, you're right. I just added the license file upstream and was too lazy to make a separate release for that. :)


> You can apply for packager group membership in FAS now and I'll sponsor you.  

Great! Thanks for your help and patience.

As you probably have noticed, I have sponsored you. This package is good to go.

APPROVED

Proceed with 
http://fedoraproject.org/wiki/PackageMaintainers/Join#Add_Package_to_CVS_and_Set_Owner


As I said before, you are now able to do package reviews in Fedora. It's a very efficient way to improve the package collection and we're not really short on reviews to be done, so your work would be greatly appreciated. After all, reviewing a package is a one-go thing, maintaining one is much more work :)

Yes, I just noticed your sponsorship. Thank you again for that. I really appreciate your support.

Package reviewing seems to be a good opportunity to become more familiar with the details of rpm packaging. I think, I will try to review some packages from time to time. But before, I have to practice a bit privately. Otherwise my comments  probably wouldn't be of much help.

(In reply to comment #20)
> Package reviewing seems to be a good opportunity to become more familiar with
> the details of rpm packaging. I think, I will try to review some packages from
> time to time. But before, I have to practice a bit privately. Otherwise my
> comments  probably wouldn't be of much help.  

That's always an option. But you learn by doing, and if you have a problem you're certainly not alone, you can always e.g. ask me.

New Package CVS Request
=======================
Package Name: dvisvgm
Short Description: A DVI to SVG converter
Owners: mgieseki
Branches: F-10 F-11 EL-5
InitialCC:

CVS done.

dvisvgm-0.8.1-4.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/dvisvgm-0.8.1-4.fc11

dvisvgm-0.8.1-4.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/dvisvgm-0.8.1-4.fc10

dvisvgm-0.8.1-4.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update dvisvgm'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-8472

dvisvgm-0.8.1-4.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update dvisvgm'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-8499

dvisvgm-0.8.1-5.el5 has been submitted as an update for Fedora EPEL 5.
http://admin.fedoraproject.org/updates/dvisvgm-0.8.1-5.el5

dvisvgm-0.8.1-5.el5 has been pushed to the Fedora EPEL 5 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update dvisvgm'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/EL-5/FEDORA-EPEL-2009-0276

dvisvgm-0.8.1-4.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

dvisvgm-0.8.1-4.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

dvisvgm-0.8.3-1.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/dvisvgm-0.8.3-1.fc11

dvisvgm-0.8.3-1.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/dvisvgm-0.8.3-1.fc10

dvisvgm-0.8.3-1.el5 has been submitted as an update for Fedora EPEL 5.
http://admin.fedoraproject.org/updates/dvisvgm-0.8.3-1.el5

Martin: as dvisvgm has already been introduced in the stable repository, the new updates don't resolve this bug anymore as it has already been marked closed. Don't mark this bug any more in updates, OK?

That's strange. I didn't enter a bug number in the "Bugs" field and unchecked the box "Close bugs when update is stable" in the bodhi web-interface. However, the bug number from the previous build was automatically assigned after submitting the new update. I thought that's the normal behavior...

dvisvgm-0.8.3-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.

dvisvgm-0.8.3-1.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

dvisvgm-0.8.3-1.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Package Change Request
======================
Package Name: dvisvgm
New Branches: epel7
Owners: mgieseki
InitialCC:

Git done (by process-git-requests).