Bug 524707

> $ rpmlint -iv chronojump-0.8.10-1.fc11.i586.rpm 
> chronojump.i586: I: checking
> chronojump.i586: W: devel-file-in-non-devel-package
> /usr/lib/chronojump/libchronopic.so
> A development file (usually source code) is located in a non-devel package. If
> you want to include source code in your package, be sure to create a
> development package.

If this is not meant for other software to use, then it can be ignored, yes.

> I know it's expected to have clean rpmlint but it's weird for me to create an
> devel package for libchronopic.so only, specially considering is not expected
> to be used for developing.  
Most Mono packages don't have clean rpmlint outputs either, just because rpmlint does not consider .dll and .exe to be libraries and executables :)

Please try to follow the usual format for review tickets; we need to be able to search the ticket summary for the package name.

Looks good so far! There are several minor things to fix, and one major thing -- the launcher script is broken on 64-bit archs -- the latter is a common
problem with our Mono apps, so I guess it's a good thing it's caught during
review anyway.

Fix them and I can approve this.

Koji F-12 build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1698459


ReviewTemplate

MUST

OK rpmlint

$ rpmlint ~/rpmbuild/SRPMS/chronojump-0.8.10-1.fc11.src.rpm ./chronojump*
error checking signature of /home/michel/rpmbuild/SRPMS/chronojump-0.8.10-1.fc11.src.rpm
chronojump.x86_64: W: devel-file-in-non-devel-package /usr/lib64/chronojump/libchronopic.so
4 packages and 0 specfiles checked; 0 errors, 1 warnings.


Internal use only, warning can be ignored


OK package name
OK spec file name
OK package guideline-compliant
OK license complies with guidelines
OK license field accurate
OK license file not deleted
OK spec in US English
OK spec legible
OK source matches upstream
OK builds under >= 1 archs, others excluded: Koji
?  build dependencies complete
   over-complete: you can drop BR on mono-core since you already BR: mono-devel
OK locales handled using %find_lang, no %{_datadir}/locale
FIX library -> ldconfig
   since your package does not install any library in the normal ld.so path
   and does not extend the path (see /etc/ld.so.conf*), running ldconfig is
   unnecessary (this also gives you the exception from the .so-in-devel rule)

FIX own all directories
  must depend on hicolor-icon-theme for the installed icon
OK no dupes in %files
OK permission
OK %clean RPM_BUILD_ROOT
OK macros used consistently
  not really about macros, but about scriplets. See the guideline about how to
  

OK Package contains code
?  large docs => -doc
   -doc should be in group "Documentation"

   Also, you can make -doc noarch, starting from RPM 4.6 (i.e. F-10 and above)
   and since Mono is not available on RHEL, you can just go ahead and declare
   the subpackage to be noarch without testing for the Fedora/RHEL version first

e.g.
   %package doc
   ...
   BuildArch: noarch

OK doc not runtime dependent
NA if contains *.pc, req pkgconfig
NA if libfiles are suffixed, the non-suffixed goes to devel
OK desktop file uses desktop-file-install
OK clean buildroot before install
OK filenames UTF-8

SHOULD
OK package build in mock on all architectures
FIX package functioned as described
  the launcher script is hardcoded to look for the .exe under /usr/lib instead
  of %{_libdir}:
$ chronojump
Cannot open assembly '/usr/lib/chronojump/Chronojump.exe': No such file or directory.

Unfortunately, a common problem in Mono apps, since Novell's openSUSE is also
multilib-capable (like Fedora) and thus they intentionally hard-code /usr/lib
because they consider Mono to be noarch, and we don't make that assumption :(

FIX scriplets are sane
  icon cache not updated; see
  http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache
OK other subpackages should require versioned base
OK require package not files

(In reply to comment #3)

> Fix them and I can approve this.

Everything has been fixed. I've added the scriplets for 
update-desktop-database

I don't have an x64 system so I can't test the launcher script but I think it's working right.


Spec URL: http://olea.org/tmp/chronojump.spec
SRPM URL: http://olea.org/paquetes-rpm/fedora-11/chronojump-0.8.10-2.olea.src.rpm

Updated to 0.8.11:

Spec URL: http://olea.org/tmp/chronojump.spec
SRPM URL: http://olea.org/paquetes-rpm/fedora-11/chronojump-0.8.11-1.fc11.src.rpm

If I understood right today would be the last day for get into F12. Any news about the revision?

Sorry about the delay. And no, you can still get it into F-12, you just have to get your package manually tagged. I'll help you through that if you want.

There is one fix you need to make: in src/util.cs, GetManualDir() returns GetPrefixDir() + "share/doc/chronojump"; the latter should really be

"share/doc/chronojump-doc-%{version}"

you'd probably need to fix it with sed, in the %prep section, since the version number will change each time.

Spec URL: http://olea.org/tmp/chronojump.spec
SRPM URL: http://koji.fedoraproject.org/koji/getfile?taskID=1726288&name=chronojump-0.8.11-2.fc11.src.rpm

fixed!

Just one thing -- rather than creating a tempfile, sed-ing into it and then renaming the temp, why not use sed -i (in-place) instead?

Also, sed can use different separator tokens, so when the string you want to replace is full of slashes, use something else -- say pipe -- instead

sed -i 's|share/doc/chronojump|share/doc/chronojump-%{version}|g' src/utils.cs

Otherwise, you might want to use $(mktemp) rather than `mktemp` -- backquote is a bash-ism (I just noticed it being frowned on in the Python merge review).

The program's output is *very* noisy, but I guess that's more of an upstream problem.

But you can make these changes when you commit the files to CVS -- everything else looks good.

APPROVED

New Package CVS Request
=======================
Package Name: Chronojump
Short Description: A measurement, management and statistics sport testing tool
Owners: olea
Branches: F-10 F-11 F-12 
InitialCC: salimma

(In reply to comment #8)

I take note of your suggestion

> APPROVED  

Thanks!

You have a uppercase "Chronojump" in your request, but the package seems to be "chronojump" 
can you confirm the correct case and reset the fedora-cvs flag to ? when you are ready?

New Package CVS Request
=======================
Package Name: chronojump
Short Description: A measurement, management and statistics sport testing tool
Owners: olea
Branches: F-10 F-11 F-12 
InitialCC: salimma

(In reply to comment #11)
> You have a uppercase "Chronojump" in your request, but the package seems to be
> "chronojump" 
> can you confirm the correct case and reset the fedora-cvs flag to ? when you
> are ready?  

Sorry!

cvs done.