https://bugzilla.redhat.com/show_bug.cgi?id=1293735
Bug ID: 1293735 Summary: Review Request: <main package name here> - <short summary here> Product: Fedora Version: rawhide Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: mgansser@alice.de QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/boomaga.spec SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/boomaga-0.7.1-1.fc23.src.rpm
Description: Boomaga (BOOklet MAnager) is a virtual printer for viewing a document before printing it out using the physical printer. The program is very simple to work with. Running any program, click "print" and select "Boomaga" to see in several seconds (CUPS takes some time to respond) the Boomaga window open. If you print out one more document, it gets added to the previous one, and you can also print them out as one, and you can also print them out as one. Regardless of whether your printer supports duplex printing or not, you would be able to easily print on both sides of the sheet. If your printer does not support duplex printing, point this out in the settings, and Booklet would ask you to turn over the pages half way through printing your document.
The program can also help you get your documents prepared a bit before printing. At this stage Boomaga makes it possible to:
* Paste several documents together. * Print several pages on one sheet. * 1, 2, 4, 8 pages per sheet * Booklet. Folding the sheets in two, you'll get a book.
Fedora Account System Username: martinkg
rpmlint boomaga.spec ../SRPMS/boomaga-0.7.1-1.fc23.src.rpm ../RPMS/x86_64/boomaga-* boomaga.spec: W: invalid-url Source0: boomaga-0.7.1.tar.gz boomaga.src: W: invalid-url Source0: boomaga-0.7.1.tar.gz 1 packages and 1 specfiles checked; 0 errors, 2 warnings.
https://bugzilla.redhat.com/show_bug.cgi?id=1293735
MartinKG mgansser@alice.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Summary|Review Request: <main |Review Request: boomaga - A |package name here> - <short |virtual printer for viewing |summary here> |a document before printing
https://bugzilla.redhat.com/show_bug.cgi?id=1293735
MartinKG mgansser@alice.de changed:
What |Removed |Added ---------------------------------------------------------------------------- URL| |http://www.boomaga.org/inde | |x.html Alias| |boomaga
https://bugzilla.redhat.com/show_bug.cgi?id=1293735
--- Comment #1 from MartinKG mgansser@alice.de --- rpm package update:
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/boomaga.spec SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/boomaga-0.7.1-2.git8ca78b2.fc...
rpmlint boomaga.spec ../SRPMS/boomaga-0.7.1-2.git8ca78b2.fc23.src.rpm ../RPMS/x86_64/boomaga-* 3 packages and 1 specfiles checked; 0 errors, 0 warnings.
%changelog * Fri Dec 25 2015 Martin Gansser martinkg@fedoraproject.org - 0.7.1-2.git8ca78b2 - Rebuilt for new git release
https://bugzilla.redhat.com/show_bug.cgi?id=1293735
Sergio Monteiro Basto sergio@serjux.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |sergio@serjux.com
--- Comment #2 from Sergio Monteiro Basto sergio@serjux.com --- (In reply to MartinKG from comment #0) I run fedora-review -b 1293735 -x CheckOwnDirs I got a review for boomaga-0.7.1-1, before you update to 0.7.1-2
[!]: update-mime-database is invoked in %post and %postun if package stores mime configuration in /usr/share/mime/packages. Note: mimeinfo files in: boomaga See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#mimeinfo
sh: /usr/bin/python: No such file or directory 2 packages and 0 specfiles checked; 0 errors, 0 warnings.
I think you need check python guideline and prepare the package for python3 as default [1]
I recently have review [2] python-gammu maybe you may follow it , it is very simple all almost done with 2 or 3 macros
[1] https://fedoraproject.org/wiki/Packaging:Python
[2] https://bugzilla.redhat.com/show_bug.cgi?id=1234654
but I haven't much time
https://bugzilla.redhat.com/show_bug.cgi?id=1293735
--- Comment #3 from Sergio Monteiro Basto sergio@serjux.com --- (In reply to MartinKG from comment #1)
- Rebuilt for new git release
Please also use SourceURL guideline [1] like this [2]
[1] https://fedoraproject.org/wiki/Packaging:SourceURL#Git_Tags
[2] https://pkgs.fedoraproject.org/cgit/rawstudio.git/tree/rawstudio.spec#n19 or http://pkgs.fedoraproject.org/cgit/python-gammu.git/tree/python-gammu.spec#n...
https://bugzilla.redhat.com/show_bug.cgi?id=1293735
--- Comment #4 from Upstream Release Monitoring upstream-release-monitoring@fedoraproject.org --- martinkg's scratch build of boomaga-0.7.1-2.git8ca78b2.fc23.src.rpm for rawhide completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12314612
https://bugzilla.redhat.com/show_bug.cgi?id=1293735
--- Comment #5 from MartinKG mgansser@alice.de --- (In reply to Sergio Monteiro Basto from comment #2)
(In reply to MartinKG from comment #0) I run fedora-review -b 1293735 -x CheckOwnDirs I got a review for boomaga-0.7.1-1, before you update to 0.7.1-2
[!]: update-mime-database is invoked in %post and %postun if package stores mime configuration in /usr/share/mime/packages. Note: mimeinfo files in: boomaga See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#mimeinfo
this part is already in the spec file, it's unclear for me, what i have to do ?
%post # Install the printer to cups backends if [ $1 = 1 ]; then sh %{_datadir}/%{name}/scripts/installPrinter.sh fi /bin/touch --no-create %{_datadir}/icons/hicolor &> /dev/null || : /bin/touch --no-create %{_datadir}/mime/packages &> /dev/null || : /usr/bin/update-desktop-database &> /dev/null || :
%postun /usr/bin/update-desktop-database &> /dev/null || : if [ $1 -eq 0 ] ; then /bin/touch --no-create %{_datadir}/icons/hicolor &>/dev/null /bin/touch --no-create %{_datadir}/mime/packages &>/dev/null /usr/bin/gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || : /usr/bin/update-mime-database %{_datadir}/mime &> /dev/null || : fi
%posttrans /usr/bin/gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || : /usr/bin/update-mime-database %{?fedora:-n} %{_datadir}/mime &> /dev/null || :
sh: /usr/bin/python: No such file or directory 2 packages and 0 specfiles checked; 0 errors, 0 warnings.
I think you need check python guideline and prepare the package for python3 as default [1]
I recently have review [2] python-gammu maybe you may follow it , it is very simple all almost done with 2 or 3 macros
[1] https://fedoraproject.org/wiki/Packaging:Python
[2] https://bugzilla.redhat.com/show_bug.cgi?id=1234654
but I haven't much time
I 'am do not understanding, why the package needs python ? and it's totally unclear for me what i have to change in the spec file ?
https://bugzilla.redhat.com/show_bug.cgi?id=1293735
--- Comment #6 from Dmitry Mikhirev mikhirev@gmail.com --- An unofficial review.
Need to be fixed:
* Error opening pdf file: cannot find boomagamerger. * Cups backend and filter directories are %{_prefix}/lib/cups/{backend,filter}, not %{_libdir}/cups/{backend,filter}. * Directories under %{_datadir}/icons/hicolor must not be owned by this package. * %post and %preun scripts are inconsistent:
%post # Install the printer to cups backends if [ $1 = 1 ]; then sh %{_datadir}/%{name}/scripts/installPrinter.sh fi
%preun # Uninstall the printer lpadmin -x "Boomaga"
The printer will be removed on package update. Use 'if [ $1 = 0 ]' in %preun.
https://bugzilla.redhat.com/show_bug.cgi?id=1293735
--- Comment #7 from Sergio Monteiro Basto sergio@serjux.com --- (In reply to MartinKG from comment #5)
I 'am do not understanding, why the package needs python ? and it's totally unclear for me what i have to change in the spec file ?
Sorry python stuff was a mistake [1] fedora-review shows an error but isn't related with this .spec
About update-mime-database I pass.
About SourceURL is correct, you just improve what is wiki page , you have a more elegant solution .
my comments are all replied.
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1243292#c10
https://bugzilla.redhat.com/show_bug.cgi?id=1293735
--- Comment #8 from MartinKG mgansser@alice.de --- (In reply to Dmitry Mikhirev from comment #6)
An unofficial review.
Need to be fixed:
- Error opening pdf file: cannot find boomagamerger.
done
- Cups backend and filter directories are
%{_prefix}/lib/cups/{backend,filter}, not %{_libdir}/cups/{backend,filter}.
done
- Directories under %{_datadir}/icons/hicolor must not be owned by this
package.
done
- %post and %preun scripts are inconsistent:
%post # Install the printer to cups backends if [ $1 = 1 ]; then sh %{_datadir}/%{name}/scripts/installPrinter.sh fi
%preun # Uninstall the printer lpadmin -x "Boomaga"
The printer will be removed on package update. Use 'if [ $1 = 0 ]' in %preun.
done
rpm package update:
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/boomaga.spec SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/boomaga-0.7.1-3.git8ca78b2.fc...
rpmlint boomaga.spec ../SRPMS/boomaga-0.7.1-3.git8ca78b2.fc23.src.rpm ../RPMS/x86_64/boomaga-* boomaga.spec:59: E: hardcoded-library-path in %{_prefix}/lib/cups/backend boomaga.spec:60: E: hardcoded-library-path in %{_prefix}/lib/cups/filter boomaga.spec:119: E: hardcoded-library-path in %{_prefix}/lib/cups boomaga.spec:120: E: hardcoded-library-path in %{_prefix}/lib/cups/backend boomaga.spec:122: E: hardcoded-library-path in %{_prefix}/lib/cups/backend/%{name} boomaga.spec:125: E: hardcoded-library-path in %{_prefix}/lib/cups/filter boomaga.spec:126: E: hardcoded-library-path in %{_prefix}/lib/cups/filter/boomaga_pstopdf boomaga.src:59: E: hardcoded-library-path in %{_prefix}/lib/cups/backend boomaga.src:60: E: hardcoded-library-path in %{_prefix}/lib/cups/filter boomaga.src:119: E: hardcoded-library-path in %{_prefix}/lib/cups boomaga.src:120: E: hardcoded-library-path in %{_prefix}/lib/cups/backend boomaga.src:122: E: hardcoded-library-path in %{_prefix}/lib/cups/backend/%{name} boomaga.src:125: E: hardcoded-library-path in %{_prefix}/lib/cups/filter boomaga.src:126: E: hardcoded-library-path in %{_prefix}/lib/cups/filter/boomaga_pstopdf boomaga.x86_64: W: no-manual-page-for-binary boomagamerger 3 packages and 1 specfiles checked; 14 errors, 1 warnings.
%changelog * Sat Dec 26 2015 Martin Gansser martinkg@fedoraproject.org - 0.7.1-3.git8ca78b2 - Follow https://fedoraproject.org/wiki/Packaging:SourceURL - corrected cups backend and filter directories - take ownership of unowned directory %%{_datadir}/icons/hicolor - use if condition in %%preun script - linked missing %%{_bindir}/boomagamerger
https://bugzilla.redhat.com/show_bug.cgi?id=1293735
Dmitry Mikhirev mikhirev@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |mikhirev@gmail.com Assignee|nobody@fedoraproject.org |mikhirev@gmail.com Flags| |fedora-review?
--- Comment #9 from Dmitry Mikhirev mikhirev@gmail.com --- OK, now I can finish the review officially.
- Error opening pdf file: cannot find boomagamerger.
done
Well, symlinking to %{_bindir} works, but the proper fix should be patching gui/kernel/tmppdffile.cpp to use compile-time defined path to search boomagamerger instead hardcoded:
dirs << QApplication::applicationDirPath() + "/../lib/boomaga/";
The correct path can be passed by cmake as macro definition. It is upstream bug, because gui/pdfmerger/CMakeLists.txt respects LIB_SUFFIX, but the code does not. Please open an issue or pull request.
boomaga.spec:59: E: hardcoded-library-path in %{_prefix}/lib/cups/backend
I'm sorry, that's my mistake. The resulting path is correct now, but another macro should be used: %{_exec_prefix} instead %{_prefix}. The even better option is to use the %{_cups_serverbin} macro provided by the cups-devel package to ensure that the path will remain correct after possible changes in cups packaging.
https://bugzilla.redhat.com/show_bug.cgi?id=1293735
--- Comment #10 from MartinKG mgansser@alice.de --- (In reply to Dmitry Mikhirev from comment #9)
OK, now I can finish the review officially.
- Error opening pdf file: cannot find boomagamerger.
done
Well, symlinking to %{_bindir} works, but the proper fix should be patching gui/kernel/tmppdffile.cpp to use compile-time defined path to search boomagamerger instead hardcoded:
dirs << QApplication::applicationDirPath() + "/../lib/boomaga/";
The correct path can be passed by cmake as macro definition. It is upstream bug, because gui/pdfmerger/CMakeLists.txt respects LIB_SUFFIX, but the code does not. Please open an issue or pull request.
I reported this bug upstream: https://github.com/Boomaga/boomaga/issues/32 but the mentioned solution from the developer doesn't work.
boomaga.spec:59: E: hardcoded-library-path in %{_prefix}/lib/cups/backend
I'm sorry, that's my mistake. The resulting path is correct now, but another macro should be used: %{_exec_prefix} instead %{_prefix}. The even better option is to use the %{_cups_serverbin} macro provided by the cups-devel package to ensure that the path will remain correct after possible changes in cups packaging.
done
https://bugzilla.redhat.com/show_bug.cgi?id=1293735
--- Comment #11 from MartinKG mgansser@alice.de --- in which forum can i ask for a solution regarding this issue ?
https://bugzilla.redhat.com/show_bug.cgi?id=1293735
--- Comment #12 from Dmitry Mikhirev mikhirev@gmail.com --- Can you show you latest SRPM with fix applied?
https://bugzilla.redhat.com/show_bug.cgi?id=1293735
--- Comment #13 from MartinKG mgansser@alice.de --- Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/boomaga.spec SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/boomaga-0.7.1-4.git2928eef.fc...
changelog * Sat Jan 09 2016 Martin Gansser martinkg@fedoraproject.org - 0.7.1-4.git2928eef - used %%{_cups_serverbin} macro provided by cups-devel - Update to new git version
https://bugzilla.redhat.com/show_bug.cgi?id=1293735
--- Comment #14 from Dmitry Mikhirev mikhirev@gmail.com --- It works after rebuild on my system. But you posted link to old spec file.
https://bugzilla.redhat.com/show_bug.cgi?id=1293735
--- Comment #15 from MartinKG mgansser@alice.de --- please can you send me a link to the working boomaga.spec file.
https://bugzilla.redhat.com/show_bug.cgi?id=1293735
--- Comment #16 from MartinKG mgansser@alice.de --- boomagamerger is an executable file and belongs not into /usr/lib64 but into /usr/bin
https://bugzilla.redhat.com/show_bug.cgi?id=1293735
--- Comment #17 from Dmitry Mikhirev mikhirev@gmail.com --- I'm sorry, the rebuild backage contained a symlink in /usr/bin, and that is why it worked. The mistake is that the final slash in path is missing, so the program tries to open /usr/lib64/boomagaboomagamerger file and fails.
boomagamerger is an executable file and belongs not into /usr/lib64 but into /usr/bin
Not all executable goes to %{_bindir}. It is used for programs that user runs normally, but programs designed to be run by other programs goes to %{_libexecdir} or %{_libdir}/%{name}. See https://fedoraproject.org/wiki/Packaging:Guidelines#Libexecdir for more details.
https://bugzilla.redhat.com/show_bug.cgi?id=1293735
--- Comment #18 from MartinKG mgansser@alice.de --- Dmitry thanks for your helpfulness and Explanation.
here is the new rpm package: Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/boomaga.spec SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/boomaga-0.7.1-5.git2928eef.fc...
%changelog * Thu Jan 28 2016 Martin Gansser martinkg@fedoraproject.org - 0.7.1-5.git2928eef - Dropped link for %%{_bindir}/boomagamerger - Added %%{name}-0.7.1-NONGUI_DIR.patch
https://bugzilla.redhat.com/show_bug.cgi?id=1293735
Dmitry Mikhirev mikhirev@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |POST Flags|fedora-review? |fedora-review+
--- Comment #19 from Dmitry Mikhirev mikhirev@gmail.com --- All issues fixed. rpmlint reports no errors/warnings.
Package is APPROVED.
https://bugzilla.redhat.com/show_bug.cgi?id=1293735
--- Comment #20 from MartinKG mgansser@alice.de --- @Dmitry Thanks for the review.
New Package SCM Request ======================= Package Name: boomaga Short Description: A virtual printer for viewing a document before printing Owners: martinkg Branches: f23 rawhide InitialCC:
https://bugzilla.redhat.com/show_bug.cgi?id=1293735
Zbigniew Jędrzejewski-Szmek zbyszek@in.waw.pl changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |zbyszek@in.waw.pl
--- Comment #21 from Zbigniew Jędrzejewski-Szmek zbyszek@in.waw.pl --- Martin, you need to do https://admin.fedoraproject.org/pkgdb/request/package/ instead.
https://bugzilla.redhat.com/show_bug.cgi?id=1293735
--- Comment #22 from Jon Ciesla limburgher@gmail.com --- Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/boomaga
https://bugzilla.redhat.com/show_bug.cgi?id=1293735
MartinKG mgansser@alice.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|POST |CLOSED Resolution|--- |NEXTRELEASE Last Closed| |2016-01-31 16:58:07
--- Comment #23 from MartinKG mgansser@alice.de --- package has been built successfully on fc23 and rawhide.
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla