Bug 226124

Adding current maintainer to CC list.

 [x] Spec file name must match the base package %{name}, in the format %{name}.spec.
 [!] Package meets the Packaging Guidelines.
 [x] Package successfully compiles and builds into binary rpms on at least one
supported architecture.
     Tested on: f8-i386
 [!] Rpmlint output:
     Attached to this review because it is too long.
 [x] Package is not relocatable.
 [!] Buildroot is correct
 [x] Package is licensed with an open-source compatible license and meets other
legal requirements as defined in the legal section of Packaging Guidelines.
 [!] License field in the package spec file matches the actual license.
     License type: Distributable
 [x] If (and only if) the source package includes the text of the license(s) in
its own file, then that file, containing the text of the license(s) for the
package is included in %doc.
 [x] Spec file is legible and written in American English.
 [x] Sources used to build the package matches the upstream source, as provided
in the spec URL.

b71f701dcae827f2f5e4e848c66321fc  man-pages-es-1.55.tar.bz2
5827f41f77658df17f550b7f8e831432  man-pages-es-extra-0.8a.tar.gz

 [x] Package is not known to require ExcludeArch
 [x] All build dependencies are listed in BuildRequires, except for any that are
listed in the exceptions section of Packaging Guidelines.
 [-] The spec file handles locales properly.
 [-] ldconfig called in %post and %postun if required.
 [x] Package must own all directories that it creates.
 [!] Package requires other packages for directories it uses.
 [x] Package does not contain duplicates in %files.
 [x] Permissions on files are set properly.
 [x] Package has a %clean section, which contains rm -rf %{buildroot} (or
$RPM_BUILD_ROOT).
 [!] Package consistently uses macros.
 [x] Package contains code, or permissable content.
 [-] Large documentation files are in a -doc subpackage, if required.
 [-] Package uses nothing in %doc for runtime.
 [-] Header files in -devel subpackage, if present.
 [-] Static libraries in -devel subpackage, if present.
 [-] Package requires pkgconfig, if .pc files are present.
 [-] Development .so files in -devel subpackage, if present.
 [-] Fully versioned dependency in subpackages, if present.
 [-] Package does not contain any libtool archives (.la).
 [-] Package contains a properly installed %{name}.desktop file if it is a GUI
application.
 [x] Package does not own files or directories owned by other packages.

=== SUGGESTED ITEMS ===
 [x] Latest version is packaged.
 [x] Package does not include license text files separate from upstream.
 [x] Description and summary sections in the package spec file contains
translations for supported Non-English languages, if available.
 [x] Reviewer should test that the package builds in mock.
     Tested on: f8-i386
 [x] Package functions as described.
 [-] Scriptlets must be sane, if used.
 [-] The placement of pkgconfig(.pc) files are correct.
 [-] File based requires are sane.


=== Issues ===

1. Summary ends with a period.

2. "Distributable" is not a valid license:
http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#head-0e7556b4f56a6612e7334b142107e709b3886449

3. BuildRoot is not good:
http://fedoraproject.org/wiki/Packaging/Guidelines#head-b4fdd45fa76cbf54c885ef0836361319ab962473

4. Source files should use %{name} macro.

5. This comment is false and should be removed:  
# the source URL contains a tar.gz file - which has been recompressed 
# with bzip2

6. Extra package has a Summary entry in Spanish and not in Italian:
Summary(it):     Páginas de manual extras en castellano

7. Full (and wrong) path to PAQUETES and LEEME.extra files should be removed
from the description of the extra package (see also issue #8).

8. These descriptions (taken from the README and LEEME files) are better than
the current ones and should be used for the base package:
This package contains the translation into Spanish of the English
man-pages package. It is a beta release, so you can still find a lot
of bugs.  Contributions are welcome. For any doubt or suggestion about
this release, send an e-mail to Juan Piernas Canovas <piernas at
ditec.um.es>. In order to collaborate with the project, please visit
http://es.tldp.org.

Este archivo contiene la traducción al español del paquete man-pages
en inglés. Es una versión beta, por lo que todavía puede encontrar
bastantes errores. Cualquier contribución será bienvenida. Para
cualquier duda o sugerencia sobre esta versión, envíe un correo a Juan
Piernas Cánovas (piernas en ditec.um.es). Si desea colaborar en el
proyecto, por favor, visite http://es.tldp.org.

9. These descriptions (taken from the README and LEEME.extras files) are better
than the current ones and should be used for the extra package:
This package contains the eighth release of the Linux extra man pages in Spanish.
Note it is an alpha release, so you can find a lot of bugs. These man pages
are from several packages and applications. See PAQUETES file for more
information about packages.

Esta paquete contiene la octava versión de páginas de manual extras
en español para Linux. Note que es una versión alfa, por lo que puede
encontrar bastantes errores. Estas páginas proceden de distintos paquetes y
aplicaciones. Consulte el fichero PAQUETES para conocer dichos paquetes.

10. All man pages must be in UTF-8.

11. man-pages-es should Requires man, because /usr/share/man/es belongs to that
package.

12. Buildarchitecture tag is obsolete. Use Buildarch.

13. Maybe it is a good idea to define a extra_name macro too (but without the
extra version).

14. It should be wise to define macros at the beginning of the spec file.

15. The spec file has a mixed use of spaces and tabs. Choose one style and use
it consistently.

16. You should use 

make install MANDIR=$RPM_BUILD_ROOT%{_mandir}/es

to copy man pages to thier destination. This avoid the checks to update current
installed man pages (there are no current installed man pages since they go in
the buildroot).

17. You must always use macros instead of full paths: %{_mandir} instead of
/usr/share/man


NEEDSWORK.

Created attachment 272701 [details]
rpmlint output

Hi Andrea, 

It seems that http://es.tldp.org/ does not work, shall we change it to
http://ditec.um.es/~piernas/manpages-es/ ?

Regards,
Ding-Yi Chen

(In reply to comment #4)
> It seems that http://es.tldp.org/ does not work, shall we change it to
> http://ditec.um.es/~piernas/manpages-es/ ?

I agree. IMHO it would be wise to contact upstream and telling them that this
link isn't working too.

BTW, I also reviewed man-pages-it (#226125) that share many issues with this
package.

Bye,

Andrea.

Hi,
I have another question about the License of this package:
In README, it states that:
* COPYRIGHTS

These man pages come under various copyrights.
All are freely distributable when the nroff source is included.
=================================================
Obviously rpmlint doesn't consider it as "valid" license
What should we do then?

Regards,
Ding-Yi Chen

(In reply to comment #6)
> Hi,
> I have another question about the License of this package:
> In README, it states that:
> * COPYRIGHTS
> 
> These man pages come under various copyrights.
> All are freely distributable when the nroff source is included.
> =================================================
> Obviously rpmlint doesn't consider it as "valid" license
> What should we do then?

Use "Freely redistributable without restriction":
http://fedoraproject.org/wiki/PackagingDrafts/LicenseTag#head-506804096cdc9862f5c48ca18d91753900019fbe

You can also use a two line comment above the License tag to note what is stated
in the copyright file:
# These man pages come under various copyrights.
# All are freely distributable when the nroff source is included.

Bye,

Andrea.

(In reply to comment #7)
> Use "Freely redistributable without restriction":
http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#head-0e7556b4f56a6612e7334b142107e709b3886449

Ops.... This is the correct reference.

Andrea.

Hi Andrea,
It's ready now, would you mind to have a look?

Are you sure this line is correct? 

iconv -f UTF-8 -t UTF-8 %{_builddir}/%{name}-%{version}/$i -o
%{_builddir}/%{name}-%{version}/$i.utf8

It converts from UTF-8 to UTF-8...

Unresolved issues:

16.


New issues:

18. I think I missed this one in the main review. There is no need to specify
the builddir path. You can (and really should) write the lines like the following:

mv %{_builddir}/%{name}-%{version}/%{extra_pkg_name}/README
%{_builddir}/%{name}-%{version}/README.extra

in this way:

mv %{extra_pkg_name}/README %{extra_pkg_name}/README.extra

I suggest you to leave extra package docs in %{extra_pkg_name} and then use 

%doc {extra_pkg_name}/README.extra

19. In the install section there is an old conversion to UTF-8 that is
commented. I think you should delete it.

20. There is no really %build section. Move its content to %prep. Rpmlint will
give a warning with an empty %build section, but in this case it is OK. 

man-pages-es-1.55-3.fc7 has been pushed to the Fedora 7 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update man-pages-es'

man-pages-es-1.55-3.fc8 has been pushed to the Fedora 8 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update man-pages-es'

> Are you sure this line is correct? 

> iconv -f UTF-8 -t UTF-8 %{_builddir}/%{name}-%{version}/$i -o
%{_builddir}/%{name}-%{version}/$i.utf8

> It converts from UTF-8 to UTF-8...

Yes, I do it on purpose, in order to avoid the (file-is-not-utf8) warning in
rpmlint. I think it is because they lack of UTF-8 headers or somehow.

Regards,
Ding-Yi Chen

(In reply to comment #13)
> > Are you sure this line is correct? 
> 
> > iconv -f UTF-8 -t UTF-8 %{_builddir}/%{name}-%{version}/$i -o
> %{_builddir}/%{name}-%{version}/$i.utf8
> 
> > It converts from UTF-8 to UTF-8...
> 
> Yes, I do it on purpose, in order to avoid the (file-is-not-utf8) warning in
> rpmlint. I think it is because they lack of UTF-8 headers or somehow.

Isn't it more likely that they are ISO8859-15 like the other man pages?

Bye,

Andrea.

man-pages-es-1.55-4.fc8 has been pushed to the Fedora 8 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update man-pages-es'

man-pages-es-1.55-4.fc7 has been pushed to the Fedora 7 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update man-pages-es'

Everything is fine for me. 

APPROVED.

Since this package has been already built, please close this bugzilla review
ticket as NEXTRELEASE.

> Isn't it more likely that they are ISO8859-15 like the other man pages?

Well the output of 
  iconv -f UTF-8 -t UTF-8 
looks more correct than
  iconv -f ISO8859-15 -t UTF-8

Thanks for your effort. :-0

(In reply to comment #18)
> > Isn't it more likely that they are ISO8859-15 like the other man pages?
> 
> Well the output of 
>   iconv -f UTF-8 -t UTF-8 
> looks more correct than
>   iconv -f ISO8859-15 -t UTF-8

Yes... I had time to test this too.

> Thanks for your effort. :-0

You're welcome.

Bye,

Andrea.