Bug 1293735 (boomaga)

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.fc23.src.rpm

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> - 0.7.1-2.git8ca78b2
- Rebuilt for new git release

(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

(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#n12

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

(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 ?

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.

(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

(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.fc23.src.rpm

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> - 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

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.

(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

in which forum can i ask for a solution regarding this issue ?

Can you show you latest SRPM with fix applied?

Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/boomaga.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/boomaga-0.7.1-4.git2928eef.fc23.src.rpm

changelog
* Sat Jan 09 2016 Martin Gansser <martinkg> - 0.7.1-4.git2928eef
- used %%{_cups_serverbin} macro provided by cups-devel
- Update to new git version

It works after rebuild on my system. But you posted link to old spec file.

please can you send me a link to the working boomaga.spec file.

boomagamerger is an executable file and belongs not into /usr/lib64 but into /usr/bin

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.

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.fc23.src.rpm

%changelog
* Thu Jan 28 2016 Martin Gansser <martinkg> - 0.7.1-5.git2928eef
- Dropped link for %%{_bindir}/boomagamerger
- Added %%{name}-0.7.1-NONGUI_DIR.patch

All issues fixed. rpmlint reports no errors/warnings.

Package is APPROVED.

@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:

Martin, you need to do https://admin.fedoraproject.org/pkgdb/request/package/ instead.

Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/boomaga

package has been built successfully on fc23 and rawhide.