Spec URL: https://www.dropbox.com/s/25cb53xdr7n7puu/foo2zjs.spec SRPM URL: https://www.dropbox.com/s/lkozw9gej6ll60s/foo2zjs-0.20130602-1.fc18.src.rpm Description: This package was originally maintained in RPM Fusion due to patent issues surrounding jbigkit as I understand it. The foo2zjs package in RPM Fusion has aged significantly, and I needed to update it myself in order to get my newer printer working. After submitting the update in RPM Fusion bugzilla, it turned out that patents on jbigkit have expired and it's now available in Fedora. Hence, foo2zjs should be moved into Fedora also. This is my first package, and I'm seeking a sponsor. An individual on the RPM Fusion bugzilla said he would do the review. Fedora Account System Username: cjatherton
*** Bug 970392 has been marked as a duplicate of this bug. ***
There's a bunch of rpmlint warnings about this. Sorry, I feel like such a hypocrite when pointing this out, since you started with a package I created myself :) The spelling-error warnings are all OK, I think, and the invalid-url is also fine given the way that upstream does (or *doesn't* do, more to the point) release management. But the incorrect-fsf-address should probably be fixed (not sure of packaging guidelines there; at least in the files that get shipped lik foo2zjs-wrapper), and manual-page-warning usually does actually mean that the man page is being misrendered. I think the dangerous-command-in-%post is probably fine too, as long as that's what other similar packages in Fedora are doing.
Ok. Corrected FSF addresses in all files that rpmlint complains about. I hope that's adequate. Also, fixed all the man page warnings, which all appeared to be simple typos. The spec and srpm are now available at the Dropbox links I posted above.
I have updated foo2zjs for Fedora 19 and given it more polish. SPEC: https://www.dropbox.com/s/vzqpit9ten7ufle/foo2zjs.spec SRPM: https://www.dropbox.com/s/c3i9i2s92bih2qb/foo2zjs-0.20130626-1.fc19.src.rpm
The SRPM URL above does not seem to work anymore.
Sorry. Forgot to update URLs after updating the package again. New links here: SPEC => https://www.dropbox.com/s/vzqpit9ten7ufle/foo2zjs.spec SRPM => https://www.dropbox.com/s/qv0s1n11t7faepa/foo2zjs-0.20130813-1.fc19.src.rpm
Informal review: 1. The package is already present in the rpmfusion-free repository. A couple of years ago I asked why this is not present in Fedora and the answer was that it's because the JBig compression is patented. I used a HP LaserJet 1020 at the time which wasn't supported, only by this software. Shortly after that hplip started supporting a lot of their "winprinters" and setting up HP printers became simply "hp-setup". But hplip doesn't drive Konica-Minolta (and other) printers using the same ZjStream protocol, so the package is useful. It seems the jbigkit library (which is also used by foo2zjs) is already included in Fedora. When I try to remove it, the dependencies lead to libtiff, then gdk-pixbuf2, then half of the installed universe, so the patent problem must have been eliminated. (IANAL) About the RPM spec file and the patches included in the SRPM: 2. Have you tried sending the patches to the upstream maintainer? 3. Group tags are not needed, remove them. 4. These Requires lines are also not needed, RPM figures it out automatically: Requires: %{name} = %{version}-%{release} 5. rpmlint only spits one warning: $ rpmlint -i -v foo2zjs.spec foo2zjs.spec: W: invalid-url Source0: foo2zjs-20130813.tar.gz The value should be a valid, public HTTP, HTTPS, or FTP URL. 0 packages and 1 specfiles checked; 0 errors, 1 warnings. The website only lists the current latest foo2zjs.tar.gz and doesn't provide any revision history. No one can known whether the tar file in the SRPM actually matches any released version of foo2zjs. 6. The correct working of this driver needs extra binary data, like firmware files for the LaserJet 1005, 1020, etc. printers or ICM data files for others. They are fetched via the "./getweb" script but it's neither run (to include the downloaded extra data files) nor included in the binary RPM. In the second case, the user should also be told to run the script and add UDEV rules to download the firmware into the printer whenever it's turned on.
(In reply to David Woodhouse from comment #2) > But the incorrect-fsf-address should probably be fixed (not sure of > packaging guidelines there https://fedoraproject.org/wiki/Common_Rpmlint_issues#incorrect-fsf-address
Currently foo2hbpl refuses to install because of conflicts with foomatic-db - should be fixed with foomatic-db-4.0-41.20131218.fc21 https://koji.fedoraproject.org/koji/buildinfo?buildID=485668
(In reply to Zoltan Boszormenyi from comment #7) > 4. These Requires lines are also not needed, > Requires: %{name} = %{version}-%{release} +1. I don't see any reason why the other sub-packages should depend on foo2zjs. I think it's been there because of the files in %doc. %doc COPYING ChangeLog INSTALL README manual.pdf I'd simply add COPYING and README (maybe Changelog) into each sub-package, I don't expect anybody to have more then one of them installed at the same time. INSTALL is useless and manual.pdf is just a copy of man pages (so useless too). > RPM figures it out automatically: does it ? how come ?
To the spec file: > Requires(post): /bin/rm /usr/bin/rm > %defattr(-,root,root,-) Can be removed, see https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions > # Xerox Phaser 6110 already in foomatic-db package > rm foomatic-db/printer/Xerox-Phaser_6110.xml > rm PPD/Xerox-Phaser_6110.ppd > # Samsung CLP-310 already included in foomatic-db package > rm foomatic-db/printer/Samsung-CLP-310.xml > rm PPD/Samsung-CLP-310.ppd > # Konica Minolta 2430DL already included in foomatic-db package > rm foomatic-db/printer/KONICA_MINOLTA-magicolor_2430_DL.xml > rm PPD/KONICA_MINOLTA-magicolor_2430_DL.ppd I've removed them from foomatic-db-4.0-42.20131218.fc21 https://koji.fedoraproject.org/koji/buildinfo?buildID=485725 So you can remove these lines and add %{_datadir}/foomatic/db/source/printer/KONICA_MINOLTA-magicolor_2430_DL.xml %{_datadir}/cups/model/KONICA_MINOLTA-magicolor_2430_DL.ppd.gz to %files and %{_datadir}/cups/model/Xerox-Phaser_6110.ppd.gz %{_datadir}/foomatic/db/source/printer/Xerox-Phaser_6110.xml to %files -n foo2qpdl
New revision based on suggestions: SPEC => https://www.dropbox.com/s/vzqpit9ten7ufle/foo2zjs.spec SRPM => https://www.dropbox.com/s/d7216fijemfd5lz/foo2zjs-0.20140126-1.fc20.src.rpm
(In reply to Christopher Atherton from comment #12) > SPEC => https://www.dropbox.com/s/vzqpit9ten7ufle/foo2zjs.spec Looks good to me. But next time please increase the Release tag (even it's during review).
have you ever tried to upload spec + src.rpm to fedorapeople instead of dropbox ? Can you do it for last version of your spec/src.rpm ?
I don't belong to any project group, so can't use fedorapeople. (I may work on that...) In the meantime , an update ... SPEC => https://www.dropbox.com/s/tp49pw6n5ycb4gc/foo2zjs.spec?dl=0 SRPM => https://www.dropbox.com/s/i8936cco5xt85jg/foo2zjs-0.20151111-1.fc23.src.rpm?dl=0
Looks good to me ! There's however one rule that's changed since my comment #10 - license file should now be included using %license and not %doc. http://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text I'd be nice if you could change that, but I don't think it's a must, so I guess this package is approved !
Whoops, I forgot I have to be a sponsor to do official review of this.
$ fedora-review -b 970393 INFO: Processing bugzilla bug: 970393 INFO: Getting .spec and .srpm Urls from : 970393 INFO: --> SRPM url: https://www.dropbox.com/s/i8936cco5xt85jg/foo2zjs-0.20151111-1.fc23.src.rpm INFO: --> Spec url: https://www.dropbox.com/s/tp49pw6n5ycb4gc/foo2zjs.spec INFO: Using review directory: /home/dwmw2/970393-foo2zjs INFO: Downloading .spec and .srpm files error: line 1: Unknown tag: <!DOCTYPE html><html lang="en" xmlns:fb="http://ogp.me/ns/fb#" xml:lang="en" class="media-desktop" xmlns="http://www.w3.org/1999/xhtml"><head><script nonce="ZZ/moT16e3Wj5hsbQ1GC"> ERROR: "Can't parse specfile: can't parse specfile\n" (logs in /home/dwmw2/.cache/fedora-review.log)
Download them manually and run $ fedora-review -n foo2zjs
I've switched COPYING from %doc to %license. SPEC/SRPM have the same location, but for convenience... SPEC => https://www.dropbox.com/s/tp49pw6n5ycb4gc/foo2zjs.spec?dl=0 SRPM => https://www.dropbox.com/s/i8936cco5xt85jg/foo2zjs-0.20151111-1.fc23.src.rpm?dl=0
[ ]: Package must own all directories that it creates. Note: Directories without known owners: /usr/share/foomatic/db/source, /usr/share/foomatic/db, /usr/share/foomatic/db/source/driver, /usr/share/cups/model, /usr/share/foomatic, /usr/share/foomatic/db/source/printer, /usr/share/foo2qpdl, /usr/share/foomatic/db/source/opt, /usr/share/cups For some of those, perhaps we want Requires: foomatic-db? /usr/share/foo2qpdl seems like a simple omission. Do we want to require cups-filesystem for the latter? Also, now I'm looking at the Requires:.... should it require /bin/rm not /usr/bin/rm? [ ]: Spec use %global instead of %define unless justified. Note: %define requiring justification: %define foo2zjs_ver 20151111 That can be changed, can't it? Finally (although not strictly a review comment) does any active user have one of the printers which need firmware loaded, and can we get that working automatically too?
(In reply to David Woodhouse from comment #21) > /usr/share/foomatic > /usr/share/foomatic/db > /usr/share/foomatic/db/source These are already part of foomatic-db-filesystem. > /usr/share/foomatic/db/source/driver > /usr/share/foomatic/db/source/opt > /usr/share/foomatic/db/source/printer We can move these from foomatic-db into foomatic-db-filesystem as well I think. > /usr/share/cups > /usr/share/cups/model Yes, cups-filesystem is the one to require here. > Also, now I'm looking at the Requires:.... should it require /bin/rm not > /usr/bin/rm? I had suggested to use /usr/bin/rm in comment #11 because: $ rpm -ql coreutils | grep rm /usr/bin/rm I agree with your other notes.
(In reply to Jiri Popelka from comment #22) > We can move these from foomatic-db into foomatic-db-filesystem as well I > think. Changed in rawhide http://pkgs.fedoraproject.org/cgit/foomatic-db.git/commit/?id=b32fefc9e978728aa96644903c77465d14a8cc70 So adding Requires: foomatic-db-filesystem cups-filesystem to each (sub)package should fix the unowned directories (except the /usr/share/foo2qpdl which should be owned by foo2qpdl).
Assuming that's all fixed, consider it reviewed then. Now I just need to sponsor Christopher... what is your FAS account name? I can't see you in the 'cla_done' group...
My account name is "cjatherton".
Done. You should now be able to make the new package request as described at https://fedoraproject.org/wiki/PackageDB_admin_requests
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/foo2zjs
foo2zjs-0.20151111-1.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2015-db15317851
foo2zjs-0.20151111-1.fc23 has been pushed to the Fedora 23 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 'dnf --enablerepo=updates-testing update foo2zjs' You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-db15317851
foo2zjs-0.20151111-2.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2015-329c3c3c51
foo2zjs-0.20151111-3.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2015-b6687a7b2d
foo2zjs-0.20151111-3.fc23 has been pushed to the Fedora 23 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 'dnf --enablerepo=updates-testing update foo2zjs' You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-b6687a7b2d
foo2zjs-0.20151111-2.fc23 has been pushed to the Fedora 23 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 'dnf --enablerepo=updates-testing update foo2zjs' You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-329c3c3c51
foo2zjs-0.20151111-3.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.