Bug 551911

I've roughly scanned over the package and I've seen a couple of issues. It would be good if they could be fixed first before I do the full complete review:

- source files differ from upstream:
sources in package:
25abd742dc4a0ffcf17c537dea6d791f  SOURCES/monodevelop-boo-2.2.tar.bz2
sources from upstream:
56bf610e072030274309c94de1079b8e  monodevelop-boo-2.2.tar.bz2

- the source URL does not match the link on the download page:
http://ftp.novell.com/pub/mono/sources/monodevelop-boo/monodevelop-boo-2.2.tar.bz2

- some rpmlint warnings:
SPECS/monodevelop-boo.spec:41: W: configure-without-libdir-spec
SPECS/monodevelop-boo.spec: W: mixed-use-of-spaces-and-tabs (spaces: line 4, tab: line 14)
monodevelop-boo-devel.i686: E: description-line-too-long The monodevelop-boo-devel package contains development files for monodevelop-boo.

- directory %{_libdir}/monodevelop/AddIns/BooBinding should be included in this package:
e.g. use:
%files -f %{name}.lang
%defattr(-,root,root,-)
%{_libdir}/monodevelop/AddIns/BooBinding

- License:
It looks like that the License should be rather GPLv2+ instead of MIT - please can you have a look?

SRPM URL:
http://www.all-the-johnsons.co.uk/fedora/monodevelop-boo-2.2-1.fc13.src.rpm
SPEC
http://www.all-the-johnsons.co.uk/fedora/monodevelop-boo.spec

Fixes above issues

Hello Paul,

thanks for the new package - here is the complete review:

I have tested the following src.rpm:
http://www.all-the-johnsons.co.uk/fedora/monodevelop-boo-2.2-2.fc13.src.rpm


* rpmlint: TODO
rpmlint RPMS/i686/monodevelop-boo-* SRPMS/monodevelop-boo-2.2-2.fc13.src.rpm SPECS/monodevelop-boo.spec
monodevelop-boo.i686: W: spelling-error-in-summary en_US Addin
monodevelop-boo.i686: W: spelling-error-in-description en_US Addin
monodevelop-boo.i686: E: no-binary
monodevelop-boo.i686: W: only-non-binary-in-usr-lib
monodevelop-boo.i686: W: no-documentation
monodevelop-boo-devel.i686: W: spelling-error-in-summary en_US Addin
monodevelop-boo-devel.i686: W: spelling-error-in-description en_US Addin
monodevelop-boo-devel.i686: W: no-documentation
monodevelop-boo.src: W: spelling-error-in-summary en_US Addin
monodevelop-boo.src: W: spelling-error-in-description en_US Addin
monodevelop-boo.src:42: W: configure-without-libdir-spec
monodevelop-boo.src: W: mixed-use-of-spaces-and-tabs (spaces: line 5, tab: line 4)
SPECS/monodevelop-boo.spec:42: W: configure-without-libdir-spec
SPECS/monodevelop-boo.spec: W: mixed-use-of-spaces-and-tabs (spaces: line 5, tab: line 4)
3 packages and 1 specfiles checked; 1 errors, 13 warnings.

- no-binary, no-documentation and only-non-binary-in-usr-lib are false positives
- mixed-use-of-spaces-and-tabs should be fixed
- spelling errors: IMHO "Addin" should be written "Add-in" (as it is done in
monodevelop as well as on its web page)
- configure-without-libdir-spec is also a false positive, since the packages
contains a custom-made configure script which doesn't need or handle
the --libdir parameter

* naming: OK
- name matches upstream
- spec file name matches package name

* sources: OK
- md5sum: 25abd742dc4a0ffcf17c537dea6d791f  monodevelop-boo-2.2.tar.bz2
- sources matches upstream
- Source0 tag ok
- spectool -g  works

* binaries in upstream sources: OK (n/a)

* License: OK
- License in spec file does match the actual license
- GPLv2+ license acceptable for Fedora

* package containing *.pc files must "Requires: pkgconfig": OK

* spec file written in English and legible: minor TODOs
- please split the very long line of the BuildRequires so that it fit into
80 characters for better readability
- please append a "/" to the URL to get a 100% correct URL ;-)

* compilation: TODO (minor)
- please check whether the package does support a parallel build, if not please add a short comment in the %build section
- builds fine in koji: F13

* BuildRequires: OK

* locales handling: OK

* ldconfig in %post and %postun: OK (n/a)

* package owns all directories that it creates: OK

* %files section: TODO
Please delete the empty locale directory in the %install section:
/usr/lib/monodevelop/AddIns/BooBinding/locale

* no files listed twice in %files: OK

* file permissions: OK
- %defattr used
- actual permissions in packages ok

* %clean section: OK

* macro usage: OK

* code vs. content: OK

* main package should not contain development related parts: OK

* large documentation into subpackage: OK (n/a)

* header files in -devel subpackage: OK (n/a)

* static libraries in -static package: OK (n/a)

* *.so link in -devel package: OK (n/a)

* devel package requires base package using fully versioned dependency: OK

* packages must not contain *.la files: OK

* GUI applications must provide *.desktop file: OK (n/a)

* packages must not own files/dirs already owned by other packages: OK

* rm -rf $RPM_BUILD_ROOT at the beginning of %install: OK

* all filenames UTF-8: OK

* functional test: TODO
- unfortunately moving the locale data into /usr/share/locale makes monodevelop
not recognize them anymore:
- start monodevelop with "LANG=de_DE monodevelop"
- open a new project with "Datei->Neu->Projektmappe"
- select Boo and click onto the Gtk# Project
- the help text below is still "Creates a Boo/Gtk# project"
- if the locales data would be still at the old place then this text would
be "Erzeugt ein Boo/Gtk# Projekt"
- there may be two fixes:
a) leave the locale where they were put by "make install", but note that they still have to be packaged correctly using %lang(xx) macros
b) make the boo addin/monodevelop aware of the new location

* debuginfo sub-package: OK (n/a)

Best regards,
Christian

SRPM URL:
http://www.all-the-johnsons.co.uk/fedora/monodevelop-boo-2.2-3.fc13.src.rpm
SPEC
http://www.all-the-johnsons.co.uk/fedora/monodevelop-boo.spec

Gettext fixes

(In reply to comment #4)
> SRPM URL:
> http://www.all-the-johnsons.co.uk/fedora/monodevelop-boo-2.2-3.fc13.src.rpm
> SPEC
> http://www.all-the-johnsons.co.uk/fedora/monodevelop-boo.spec
> 
> Gettext fixes    

I've just tested the new package and unfortunately it looks like that your gettext fix does not work. Still the same issue as described.

Additionally I have recognized that monodevelop does now start slowlier, somehow I have the feeling that it scans all installed locales files. Probably it would be better to go with option a).

Please can you also have a look at the other minor issues I've marked as "TODO" in my review? Thanks!

I have a feeling that it is an incomplete language problem more than anything else as if I go Datei->Neu->Projektmappe, it does as you say. However, under that as a sub menu is ASP.NET which is in German.

If it is a case of option "a", how do I direct %lang to look in the incorrect place?

Fixed the other TODOs but have not uploaded the incredibly minor altered spec file

Yes, it looks like that your fix partially fixed it, but probably e.g. md itself looks for the other location...

I have used something like this in a different package to mark the language files properly even if there are at an unusual place:

%install
make DESTDIR=... install

find %{buildroot} -type f -o -type l|sed '
s:'"%{buildroot}"'::
s:\(.*/lib/python2.6/site-packages/ankiqt/locale/\)\([^/_]\+\)\(.*\.mo$\):%lang(\2) \1\2\3:
s:\(.*/lib/python2.6/site-packages/anki/locale/\)\([^/_]\+\)\(.*\.mo$\):%lang(\2) \1\2\3:
s:^\([^%].*\)::
s:%lang(C) ::
/^$/d' > anki.lang

%files -f anki.lang

SRPM URL:
http://www.all-the-johnsons.co.uk/fedora/monodevelop-boo-2.2-4.fc13.src.rpm
SPEC
http://www.all-the-johnsons.co.uk/fedora/monodevelop-boo.spec

Sorts out the lang problem, removed the patch file

I've looked at the new package:

1. the lang problem is still there:
using rpmlint on the binary rpm reveals lots of the following messages:

monodevelop-boo.i686: W: file-not-in-%lang /usr/lib/monodevelop/AddIns/BooBinding/locale/ca/LC_MESSAGES/monodevelop-boo.mo
monodevelop-boo.i686: W: file-not-in-%lang /usr/lib/monodevelop/AddIns/BooBinding/locale/cs/LC_MESSAGES/monodevelop-boo.mo

The generated %{name}.lang file is empty.

The root cause is, that the sed command does not fully work. Actually the following changes are necessary:

--- monodevelop-boo.spec.2      2010-02-13 15:32:32.000000000 +0100
+++ SPECS/monodevelop-boo.spec  2010-02-17 22:11:07.000000000 +0100
@@ -50,8 +50,7 @@
 
 find %{buildroot} -type f -o -type l|sed '
 s:'"%{buildroot}"'::
-s:\(.*/"%{_lib}"/monodevelop/AddIns/Monodevelop.Boo/locale/\)\([^/_]\+\)\(.*\.mo$\):%lang(\2) \1\2\3:
-s:\(.*/"%{_lib}"/monodevelop/AddIns/Monodevelop.Boo/locale/\)\([^/_]\+\)\(.*\.mo$\):%lang(\2) \1\2\3:
+s:\(.*/%{_lib}/monodevelop/AddIns/BooBinding/locale/\)\([^/]\+\)\(.*\.mo$\):%lang(\2) \1\2\3:
 s:^\([^%].*\)::
 s:%lang(C) ::
 /^$/d' > %{name}.lang
@@ -61,7 +60,12 @@
 
 %files -f %{name}.lang
 %defattr(-,root,root,-)
-%{_libdir}/monodevelop/AddIns/BooBinding
+%dir %{_libdir}/monodevelop/AddIns/BooBinding
+%{_libdir}/monodevelop/AddIns/BooBinding/*.dll
+%{_libdir}/monodevelop/AddIns/BooBinding/*.mdb
+%dir %{_libdir}/monodevelop/AddIns/BooBinding/locale
+%dir %{_libdir}/monodevelop/AddIns/BooBinding/locale/*
+%dir %{_libdir}/monodevelop/AddIns/BooBinding/locale/*/LC_MESSAGES
 
- my example used two lines with the substitution rules because there were two base directories with language files (that's not the case here)
- the double-quotes around %{_lib} were still in the sed rule (since the whole block is surrounded by '')
- the \([^/_]\+\) did not work since the boo binding uses language IDs like "de_DE" (which anki didn't)
- to ensure that all directories are correctly packaged and no files are packaged twice there ware some changes in the %files section needed as well

- I have positively tested that this fixes the language problem I have described in comment #3.

2. even after these changes there are the following rpmlint warnings:
rpmlint RPMS/i686/monodevelop-boo-*-4.fc* SPECS/monodevelop-boo.spec SRPMS/monodevelop-boo-2.2-4.fc13.src.rpm 
monodevelop-boo.i686: E: no-binary
monodevelop-boo.i686: W: only-non-binary-in-usr-lib
monodevelop-boo.i686: W: no-documentation
monodevelop-boo-devel.i686: W: spelling-error Summary(en_US) Addin -> Addie, Adding, Admin
monodevelop-boo-devel.i686: W: spelling-error %description -l en_US Addin -> Addie, Adding, Admin
monodevelop-boo-devel.i686: W: no-documentation
SPECS/monodevelop-boo.spec:42: W: configure-without-libdir-spec
SPECS/monodevelop-boo.spec:12: W: mixed-use-of-spaces-and-tabs (spaces: line 12, tab: line 4)
monodevelop-boo.src:42: W: configure-without-libdir-spec
monodevelop-boo.src:12: W: mixed-use-of-spaces-and-tabs (spaces: line 12, tab: line 4)
3 packages and 1 specfiles checked; 1 errors, 9 warnings.

Sure, "no-binary", "only-non-binary-in-usr-lib", "no-documentation" and "cnofigure-without-libdir-spec" are false positives, but the others should be fixed.

I know, this may sound like nit-picking, but that's the way how I was taught doing reviews (and accepting when my packages were reviewed). Since it is usually quite hard to distinguish between "minor" and "major" issues revealed by rpmlint if there are lots of warnings, I think we should eliminate as many as possible (IMHO is this similiar to compiler warnings) in the first place... ;-)

SRPM URL:
http://www.all-the-johnsons.co.uk/fedora/monodevelop-boo-2.2-4.fc13.src.rpm
SPEC
http://www.all-the-johnsons.co.uk/fedora/monodevelop-boo.spec

Sorts out the lang problem, spec file clean ups and generally does things nicely now :-)

SRPM URL:
http://www.all-the-johnsons.co.uk/fedora/monodevelop-boo-2.2-5.fc13.src.rpm
SPEC
http://www.all-the-johnsons.co.uk/fedora/monodevelop-boo.spec

Would help if I put the correct src rpm URL...

SRPM URL:
http://www.all-the-johnsons.co.uk/fedora/monodevelop-boo-2.2-5.fc14.src.rpm

Oopsie - wrong branch...

I've tested the latest package and it works as expected.

There are two very minor issues left:

1. the line in the spec file 
mkdir -p %{buildroot}%{_datadir}/locale
is a left-over. Althought it does not harm anyone, it is not necessary - please delete it.

2. In the description of the -devel package is one spelling error left: Addin -> Add-in

Since otherwise all important issues were addressed:
-> APPROVED

Please fix the minor issues right after the import.

New Package CVS Request
=======================
Package Name: monodevelop-boo
Short Description: Boo add-in for monodevelop
Owners: pfj
Branches: f12, f13, devel
InitialCC:

CVS done (by process-cvs-requests.py).