This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 693950 - (yagf) Review Request: yagf - Graphical front-end for cuneiform
Review Request: yagf - Graphical front-end for cuneiform
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Volker Fröhlich
Fedora Extras Quality Assurance
:
Depends On: cuneiform
Blocks: RussianFedoraRemix
  Show dependency treegraph
 
Reported: 2011-04-05 23:38 EDT by Dmitrij S. Kryzhevich
Modified: 2011-09-06 23:45 EDT (History)
4 users (show)

See Also:
Fixed In Version: yagf-0.8.6-6.fc16
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-09-06 20:25:03 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
volker27: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Dmitrij S. Kryzhevich 2011-04-05 23:38:21 EDT
Spec URL: http://krege.fedorapeople.org/yagf/yagf.spec
SRPM URL: http://krege.fedorapeople.org/yagf/yagf-0.8.6-1.fc14.src.rpm
Description:

YAGF is a graphical interface for the cuneiform text
recognition program on the Linux platform. With YAGF
you can scan images via XSane, perform images preprocessing
and recognize texts using cuneiform from a single
command center. YAGF also makes it easy to scan and
recognize several images sequentially.

$ rpmlint yagf-0.8.6-1.fc14.src.rpm yagf-0.8.6-1.fc14.x86_64.rpm 
yagf.src: W: spelling-error %description -l en_US preprocessing -> reprocessing, p reprocessing, preprocessed
yagf.x86_64: W: spelling-error %description -l en_US preprocessing -> reprocessing, p reprocessing, preprocessed
yagf.x86_64: W: no-manual-page-for-binary yagf
2 packages and 0 specfiles checked; 0 errors, 3 warnings.

Spelling error is false-possitive.
Comment 1 Volker Fröhlich 2011-06-30 18:39:38 EDT
yagf-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/yagf-0.8.6-Source/src/mainform.cpp
yagf-debuginfo.x86_64: E: wrong-script-end-of-line-encoding /usr/src/debug/yagf-0.8.6-Source/src/mainform.cpp
yagf-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/yagf-0.8.6-Source/src/mainform.h
yagf-debuginfo.x86_64: E: wrong-script-end-of-line-encoding /usr/src/debug/yagf-0.8.6-Source/src/mainform.h
yagf-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/yagf-0.8.6-Source/src/main.cpp

Please see http://fedoraproject.org/wiki/Packaging:Guidelines#Handling_Locale_Files
Comment 2 Dmitrij S. Kryzhevich 2011-07-08 01:12:22 EDT
Hm, foget to check debug package. (I think, with locale files there are all OK and the problem in permisions and line-breake symbol)

$ rpmlint yagf-0.8.6-2.fc15.src.rpm yagf-0.8.6-2.fc15.x86_64.rpm yagf-debuginfo-0.8.6-2.fc15.x86_64.rpm yagf.spec 
yagf.src: W: spelling-error %description -l en_US preprocessing -> reprocessing, p reprocessing, teleprocessing
yagf.x86_64: W: spelling-error %description -l en_US preprocessing -> reprocessing, p reprocessing, teleprocessing
yagf.x86_64: W: no-manual-page-for-binary yagf
3 packages and 1 specfiles checked; 0 errors, 3 warnings.

No new warnings.

Spec URL: http://krege.fedorapeople.org/yagf/yagf.spec
SRPM URL: http://krege.fedorapeople.org/yagf/yagf-0.8.6-2.fc15.src.rpm

Changelog:
- Fix permesions for debug package.
Comment 3 Volker Fröhlich 2011-07-08 14:29:54 EDT
And the locales?
Comment 4 Dmitrij S. Kryzhevich 2011-07-11 08:48:27 EDT
What is wrong with locales?
Qt *.qm files. Locales are work. Permitions are 644. There are not handled with %find_lang script.
Comment 5 Ralf Corsepius 2011-07-11 09:55:51 EDT
the permission fixes and dos2unix calls belong into the %prep section.

(You want to fix the sources before compilation, not after compilation).
Comment 6 Volker Fröhlich 2011-07-11 10:08:21 EDT
But they should be handled with %find_lang.
Comment 7 Dmitrij S. Kryzhevich 2011-07-11 23:50:12 EDT
(In reply to comment #5)
> the permission fixes and dos2unix calls belong into the %prep section.
> 
> (You want to fix the sources before compilation, not after compilation).

Fixed. As thise change does not affect the compilation, still sources change - before.

Spec URL: http://krege.fedorapeople.org/yagf/yagf.spec
SRPM URL: http://krege.fedorapeople.org/yagf/yagf-0.8.6-3.fc15.src.rpm

(In reply to comment #6)
> But they should be handled with %find_lang.

I think, you are sure that it must. Please, provide the example of %find_lang usage for Qt translation mechanism. As for me, it is impossible. I tried to ask Google, there waere no aswers.
Comment 8 Volker Fröhlich 2011-07-12 00:31:06 EDT
http://www.geofrogger.net/review/yagf.spec (Notice, my example doesn't provide a changelog entry or a release bump.)

Since RHEL 5 only has Qt 4.2 and you BR 4.3, you won't get it in there. Thus you can drop everything specific to EPEL 5:

- clean section
- rm -rf %{buildroot}
- build root definition

I also removed the defattr line, which is generally useless since EPEL 4.
Comment 9 Dmitrij S. Kryzhevich 2011-07-12 06:40:06 EDT
Very thanks!

It works. I thought there would be double entries, but with %dir all work.

Your link on locales guid does not mention qt scpecifics. Internal fedoraprject wiki search as well as google one did not show me anything. I know how to manage qt translations with find_lang now. Thanks one more time.

Spec URL: http://krege.fedorapeople.org/yagf/yagf.spec
SRPM URL: http://krege.fedorapeople.org/yagf/yagf-0.8.6-4.fc15.src.rpm

Changes: spec clean up (is it realy needed? but it is done) and find_lang usage.
Comment 10 Volker Fröhlich 2011-07-12 07:39:59 EDT
/usr/lib/rpm/find-lang.sh is where it is at.
Comment 11 Ralf Corsepius 2011-07-12 09:02:51 EDT
(In reply to comment #10)
> /usr/lib/rpm/find-lang.sh is where it is at.
I don't understand what you are trying to say.

%find_lang as being utilized in *-4 seems to do what it is supposed to:
# cat /var/lib/mock/fedora-15-x86_64/root/builddir/build/BUILD/yagf-0.8.6-Source/yagf.lang
%lang(de) /usr/share/yagf/translations/yagf_de.qm
%lang(lt) /usr/share/yagf/translations/yagf_lt.qm
%lang(uk) /usr/share/yagf/translations/yagf_uk.qm
%lang(pl) /usr/share/yagf/translations/yagf_pl.qm
%lang(ru) /usr/share/yagf/translations/yagf_ru.qm


One issue, however:
# rpm -qlp yagf-0.8.6-4.fc15.x86_64.rpm
...
/usr/share/yagf
/usr/share/yagf/translations/yagf_de.qm
...

=> /usr/share/yagf/translations/ is unowned.

Please add
%dir %{datadir}/%{name}/translations
to %files
Comment 12 Volker Fröhlich 2011-07-12 10:12:28 EDT
Oh, I wanted to point out where the actual script is and how to find out the options, like --with-qt.
Comment 13 Dmitrij S. Kryzhevich 2011-07-12 23:40:05 EDT
(In reply to comment #11)
> (In reply to comment #10)
> > /usr/lib/rpm/find-lang.sh is where it is at.
> I don't understand what you are trying to say.

I'v understood.


> One issue, however:
> # rpm -qlp yagf-0.8.6-4.fc15.x86_64.rpm
> ...
> /usr/share/yagf
> /usr/share/yagf/translations/yagf_de.qm
> ...
> 
> => /usr/share/yagf/translations/ is unowned.
> 
> Please add
> %dir %{datadir}/%{name}/translations
> to %files

Fixed.
Spec URL: http://krege.fedorapeople.org/yagf/yagf.spec
SRPM URL: http://krege.fedorapeople.org/yagf/yagf-0.8.6-5.fc15.src.rpm
Comment 14 Volker Fröhlich 2011-08-13 03:22:55 EDT
http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache is the wrong scriptlet to run, as you don't install files "into one of the subdirectories in %{_datadir}/icons/ ..."

Please drop the version constraint for qt4-devel. There is no version that old in Fedora.

Instead, your spec file meets the requirements for http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#desktop-database

"Comment[pl]=Grafinė teksto atpažinimo programos Cuneiform sąsaja" -- That line in the desktop file is wrong as well. Just delete it.

(line-break doesn't spell line-brake)
(You can cut out "on the Linux platform" from the description.)
(I also suggest to put each BuildRequirement on a separate line, as it is easier to overlook.)

I guess we can do the formal review afterwards.
Comment 15 Dmitrij S. Kryzhevich 2011-08-14 23:21:16 EDT
(In reply to comment #14)
> http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache is the
> wrong scriptlet to run, as you don't install files "into one of the
> subdirectories in %{_datadir}/icons/ ..."

It does install. /usr/share/icons/hicolor/96x96/apps/yagf.png.

> Please drop the version constraint for qt4-devel. There is no version that old
> in Fedora.

OK.


> Instead, your spec file meets the requirements for
> http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#desktop-database
> 
> "Comment[pl]=Grafinė teksto atpažinimo programos Cuneiform sąsaja" -- That line
> in the desktop file is wrong as well. Just delete it.

Missed that. Fixed as well as Name section.

> (line-break doesn't spell line-brake)
+
> (You can cut out "on the Linux platform" from the description.)
+
> (I also suggest to put each BuildRequirement on a separate line, as it is
> easier to overlook.)
+

Spec URL: http://krege.fedorapeople.org/yagf/yagf.spec
SRPM URL: http://krege.fedorapeople.org/yagf/yagf-0.8.6-6.fc15.src.rpm
Comment 16 Volker Fröhlich 2011-08-15 15:35:29 EDT
Argh, sorry for the noise.
Comment 17 Volker Fröhlich 2011-08-19 19:56:59 EDT
====================
===== APPROVED =====
====================

Comment:

http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache

The sample scriptlets use absolute paths to the binaries. You may opt to do that as well.

---------------------

Review:

[+] Good
[-] Needs work
[0] Does not apply

MUST:
=====
[+] rpmlint:
[makerpm@fedora15 rpmbuild]$ rpmlint SRPMS/yagf-0.8.6-6.fc15.src.rpm RPMS/x86_64/yagf-*0.8.6-6.fc15.x86_64.rpm 
yagf.src: W: spelling-error %description -l en_US preprocessing -> reprocessing, p reprocessing, teleprocessing
yagf.x86_64: W: spelling-error %description -l en_US preprocessing -> reprocessing, p reprocessing, teleprocessing
yagf.x86_64: W: no-manual-page-for-binary yagf
3 packages and 0 specfiles checked; 0 errors, 3 warnings.

    No additional findings with rpmlint on the installed package.

[+] Naming according to the Package Naming Guidelines
[+] Spec file matches base package name
[+] Packaging guidelines met
[+] License approved for Fedora
[+] License field in spec matches code
[+] License file included, if source package includes it
[+] Spec in American English
[+] Spec is legible
[+] Sources match upstream md5sum: 16582668d9ae34567c4f159c3208bfca
[+] Compiles and builds into binary RPMs on at least one primary architecture:
    http://koji.fedoraproject.org/koji/taskinfo?taskID=3287672

[0] ExcludeArch is specified and commented
[+] Locales are handled correctly
[+] All build dependencies listed
[0] Calls ldconfig for its shared libraries
[+] No bundled system libraries
[0] Stated as relocatable package
[+] Owns all its directories or requires package that does
[+] No file listing duplicates
[+] File permissions correct
[+] Consistent use of macros
[+] Code or permissible content
[0] Large documentation in -doc subpackage
[+] No runtime dependency of files listed as %doc
[0] Header files in -devel subpackage
[0] Static files in -static subpackage
[0] Library files without suffix in -devel subpackage
[0] Devel-package requires base package
[0] No .la libtool archives
[+] GUI application includes properly installed %{name}.desktop file
[+] No files or directories owned, that other packages own
[+] Filenames in packages are UTF-8
[0] When building a compat package, it must install using easy_install -m so it
won't conflict with the main package.
[0] When building multiple versions (for a compat package) one of the packages
must contain a default version that is usable via "import MODULE" with no prior
setup.

SHOULD:
=======

[0] Query upstream if no license text is included
[+] Package builds in mock:
    fedora-rawhide-x86_64, fedora-16-i386, fedora-15-x86_64
[+] Package works as described -- I played around a little bit.
[+] Scriptlets are sane, if used
[0] Subpackages other than -devel should require base package (versioned)
[0] pkgconfig files in -devel subpackage
[0] Dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider
requiring the package which provides the file instead of the file itself
[0] Contain man pages, where they make sense

    It is a GUI application and there is online help.

rpm -qp --requires look fine as well.
Comment 18 Dmitrij S. Kryzhevich 2011-08-22 03:51:03 EDT
Thanks a lot.


New Package SCM Request
=======================
Package Name: yagf
Short Description: Graphical front-end for cuneiform
Owners: krege
Branches: f14 f15
InitialCC:
Comment 19 Jon Ciesla 2011-08-22 05:52:31 EDT
Git done (by process-git-requests).

Added f16 branch.
Comment 20 Dmitrij S. Kryzhevich 2011-08-22 23:27:58 EDT
F16 branch already? Thanks.
Comment 21 Fedora Update System 2011-08-25 00:03:33 EDT
yagf-0.8.6-6.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/yagf-0.8.6-6.fc14
Comment 22 Fedora Update System 2011-08-25 00:03:48 EDT
yagf-0.8.6-6.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/yagf-0.8.6-6.fc15
Comment 23 Fedora Update System 2011-08-25 00:03:56 EDT
yagf-0.8.6-6.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/yagf-0.8.6-6.fc16
Comment 24 Fedora Update System 2011-08-26 10:20:13 EDT
yagf-0.8.6-6.fc16 has been pushed to the Fedora 16 testing repository.
Comment 25 Fedora Update System 2011-09-06 20:24:55 EDT
yagf-0.8.6-6.fc14 has been pushed to the Fedora 14 stable repository.
Comment 26 Fedora Update System 2011-09-06 20:28:22 EDT
yagf-0.8.6-6.fc15 has been pushed to the Fedora 15 stable repository.
Comment 27 Fedora Update System 2011-09-06 23:45:44 EDT
yagf-0.8.6-6.fc16 has been pushed to the Fedora 16 stable repository.

Note You need to log in before you can comment on or make changes to this bug.