Bug 1289634 - Review Request: libchardet - Mozilla's universal character set detector
Review Request: libchardet - Mozilla's universal character set detector
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Petr Pisar
Fedora Extras Quality Assurance
http://mirror.oops.org/pub/oops/libch...
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2015-12-08 11:00 EST by MartinKG
Modified: 2016-08-14 12:27 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2015-12-11 10:45:24 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
ppisar: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description MartinKG 2015-12-08 11:00:56 EST
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/libchardet.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/libchardet-1.0.4-1.fc23.src.rpm

Description: libchardet provides an interface to Mozilla's universal charset detector,
which detects the charset used to encode data.

Fedora Account System Username: martinkg

Checking: libchardet-1.0.4-1.fc24.x86_64.rpm
          libchardet-devel-1.0.4-1.fc24.x86_64.rpm
          libchardet-debuginfo-1.0.4-1.fc24.x86_64.rpm
          libchardet-1.0.4-1.fc24.src.rpm
libchardet.x86_64: W: spelling-error %description -l en_US charset -> char set, char-set, catharses
libchardet-devel.x86_64: W: only-non-binary-in-usr-lib
libchardet-devel.x86_64: W: no-manual-page-for-binary chardet-config
libchardet.src: W: spelling-error %description -l en_US charset -> char set, char-set, catharses
4 packages and 0 specfiles checked; 0 errors, 4 warnings.
Comment 1 MartinKG 2015-12-08 11:03:05 EST
Bomi, formerly known as CMPlayer, is a graphical user interface (GUI) player based on mpv for Linux, depends on libchardet.
Comment 2 Upstream Release Monitoring 2015-12-10 07:46:09 EST
ppisar's scratch build of libchardet-1.0.4-1.fc24.src.rpm for f24 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12137601
Comment 3 Petr Pisar 2015-12-10 07:59:07 EST
FIX: The URL is not about libchardet. Please provide better one. Maybe a link to directory with the releases would be enough. If there is no better one, add the trailing slash. Each URL must have a path.
FIX: The Source0 does not work for me. I'm getting TCP connection refused from 14.0.82.80:80. If it does not work, add a comment about it, or better change the URL to some another location where you obtained the file from.

Source archive looks original (MD5: 93edadd9353325405d6e092127339f33). Verified from AUR <https://aur.archlinux.org/cgit/aur.git/snapshot/libchardet.tar.gz>.

TODO: The libchardet-man.patch is insufficient. The dot is still not rendered. You have to change the .B into .BR and keep the dot at the same line as the bold word like this:

.BR inbuf .
The structure of

TODO: Rename the libchardet-man.patch to libchardet-1.0.4-man.patch, so it's visible against what version the patch was developed.

FIX: The summary does not explain what is it about. For example the first line from README is better. I recommend you this wording: "Mozilla's universal character set detector".

Description verified from README. Ok.

FIX: I think correct license tag is "MPLv1.1 or LGPLv2+ or GPLv2+".
The autotools files are usual mix of FSFUL, FSFULR, BSD, GPLv+. tools/install-sh is MIT. But these files are not part of the binary RPMs what License tag is used for. Other files declare the choice of the three licenses: MPLv1.1 or LGPLv2+ or GPLv2+.

TODO: Instead of declaring %path_man_en macro, I recommend to "pushd man/en" before documentation recoding loop and "popd" after that.

TODO: Package README using %doc macro.

FIX: Do not package LICENSE file twice. Remove it from %{_datadir}/doc/%{name}/ and use "%license LICENSE" only. If you install the current package with stripping documentation (rpm -ivh --excludedocs), the LICENSE will be not be installed. That's the reason for %license macro. The problem is files under %_docdir are automatically marked as documentation.

FIX: Build-require `glibc-common' (libchardet.spec:33).
FIX: Build-require `coreutils' (libchardet.spec:34).
FIX: Build-require `make' (libchardet.spec:40).
FIX: Build-require `findutils' (libchardet.spec:48).
FIX: Build-require `gcc' for including various standard library headers and having toolchain for compilation.
FIX: Build-require `gcc-c++' for having compiler for the C++ code.
FIX: Build-require `sed' (configure.ac:24).
FIX: Build-require `perl' (configure.ac:46).

Package builds. Ok.
Not tests. Ok.

$ rpmlint libchardet.spec ../SRPMS/libchardet-1.0.4-1.fc24.src.rpm ../RPMS/x86_64/libchardet-*
libchardet.spec: W: invalid-url Source0: http://mirror.oops.org/pub/oops/libchardet/libchardet-1.0.4.tar.bz2 <urlopen error [Errno 111] Connection refused>
libchardet.src: W: invalid-url Source0: http://mirror.oops.org/pub/oops/libchardet/libchardet-1.0.4.tar.bz2 <urlopen error [Errno 111] Connection refused>
libchardet-devel.x86_64: W: only-non-binary-in-usr-lib
libchardet-devel.x86_64: W: no-manual-page-for-binary chardet-config
4 packages and 1 specfiles checked; 0 errors, 4 warnings.
rpmlint is Ok.

$ rpm -qlvp ../RPMS/x86_64/libchardet-1.0.4-1.fc24.x86_64.rpm 
lrwxrwxrwx    1 root    root                       19 Dec 10 13:34 /usr/lib64/libchardet.so.1 -> libchardet.so.1.0.0
-rwxr-xr-x    1 root    root                   125440 Dec 10 13:34 /usr/lib64/libchardet.so.1.0.0
drwxr-xr-x    2 root    root                        0 Dec 10 13:34 /usr/share/doc/libchardet
-rw-r--r--    1 root    root                      884 Feb 14  2014 /usr/share/doc/libchardet/Changelog
-rw-r--r--    1 root    root                    25755 Dec 10 13:34 /usr/share/doc/libchardet/LICENSE
-rw-r--r--    1 root    root                     1118 Dec 10 13:34 /usr/share/man/ko/man3/detect.3.gz
-rw-r--r--    1 root    root                      819 Dec 10 13:34 /usr/share/man/ko/man3/detect_destroy.3.gz
-rw-r--r--    1 root    root                     1326 Dec 10 13:34 /usr/share/man/ko/man3/detect_handledata.3.gz
-rw-r--r--    1 root    root                     1021 Dec 10 13:34 /usr/share/man/ko/man3/detect_init.3.gz
-rw-r--r--    1 root    root                      931 Dec 10 13:34 /usr/share/man/ko/man3/detect_obj_free.3.gz
-rw-r--r--    1 root    root                     1017 Dec 10 13:34 /usr/share/man/ko/man3/detect_obj_init.3.gz
-rw-r--r--    1 root    root                     1072 Dec 10 13:34 /usr/share/man/ko/man3/detect_reset.3.gz

$ rpm -qlvp ../RPMS/x86_64/libchardet-devel-1.0.4-1.fc24.x86_64.rpm 
-rwxr-xr-x    1 root    root                     1314 Dec 10 13:34 /usr/bin/chardet-config
-rw-r--r--    1 root    root                     2944 Dec 10 13:34 /usr/include/chardet/chardet-config.h
-rw-r--r--    1 root    root                     3458 Dec 10 13:34 /usr/include/chardet/chardet.h
-rw-r--r--    1 root    root                     2767 Dec 10 13:34 /usr/include/chardet/nsUniversalDetector.h
-rw-r--r--    1 root    root                      367 Dec 10 13:34 /usr/include/chardet/nscore.h
-rw-r--r--    1 root    root                      163 Dec 10 13:34 /usr/include/chardet/version.h
lrwxrwxrwx    1 root    root                       19 Dec 10 13:34 /usr/lib64/libchardet.so -> libchardet.so.1.0.0
-rw-r--r--    1 root    root                      502 Dec 10 13:34 /usr/lib64/pkgconfig/chardet.pc
-rw-r--r--    1 root    root                      998 Dec 10 13:34 /usr/share/man/man3/detect.3.gz
-rw-r--r--    1 root    root                      782 Dec 10 13:34 /usr/share/man/man3/detect_destroy.3.gz
-rw-r--r--    1 root    root                     1221 Dec 10 13:34 /usr/share/man/man3/detect_handledata.3.gz
-rw-r--r--    1 root    root                      821 Dec 10 13:34 /usr/share/man/man3/detect_init.3.gz
-rw-r--r--    1 root    root                      725 Dec 10 13:34 /usr/share/man/man3/detect_obj_free.3.gz
-rw-r--r--    1 root    root                      802 Dec 10 13:34 /usr/share/man/man3/detect_obj_init.3.gz
-rw-r--r--    1 root    root                      873 Dec 10 13:34 /usr/share/man/man3/detect_reset.3.gz

FIX: Move the Korean manual pages to devel sub-package.

$ rpm -q --requires -p ../RPMS/x86_64/libchardet-1.0.4-1.fc24.x86_64.rpm | sort -f | uniq -c
      2 /sbin/ldconfig
      1 libc.so.6()(64bit)
      1 libc.so.6(GLIBC_2.2.5)(64bit)
      1 libc.so.6(GLIBC_2.4)(64bit)
      1 libgcc_s.so.1()(64bit)
      1 libgcc_s.so.1(GCC_3.0)(64bit)
      1 libm.so.6()(64bit)
      1 libstdc++.so.6()(64bit)
      1 libstdc++.so.6(CXXABI_1.3)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4)(64bit)
      1 rpmlib(CompressedFileNames) <= 3.0.4-1
      1 rpmlib(FileDigests) <= 4.6.0-1
      1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
      1 rpmlib(PayloadIsXz) <= 5.2-1
      1 rtld(GNU_HASH)
$ rpm -q --requires -p ../RPMS/x86_64/libchardet-devel-1.0.4-1.fc24.x86_64.rpm | sort -f | uniq -c
      1 /bin/sh
      1 /usr/bin/pkg-config
      1 libchardet(x86-64) = 1.0.4-1.fc24
      1 libchardet.so.1()(64bit)
      1 rpmlib(CompressedFileNames) <= 3.0.4-1
      1 rpmlib(FileDigests) <= 4.6.0-1
      1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
      1 rpmlib(PayloadIsXz) <= 5.2-1
Binary requires are Ok.

$ rpm -q --provides -p ../RPMS/x86_64/libchardet-1.0.4-1.fc24.x86_64.rpm | sort -f | uniq -c
      1 libchardet = 1.0.4-1.fc24
      1 libchardet(x86-64) = 1.0.4-1.fc24
      1 libchardet.so.1()(64bit)
$ rpm -q --provides -p ../RPMS/x86_64/libchardet-devel-1.0.4-1.fc24.x86_64.rpm | sort -f | uniq -c
      1 libchardet-devel = 1.0.4-1.fc24
      1 libchardet-devel(x86-64) = 1.0.4-1.fc24
      1 pkgconfig(chardet) = 1.0.4
Binary provides are Ok.

$ resolvedeps rawhide ../RPMS/x86_64/libchardet-{,devel-}1.0.4-1.fc24.x86_64.rpm 
Binary dependencies resolvable. Ok.

Package builds in F24 (http://koji.fedoraproject.org/koji/taskinfo?taskID=12137601) OK.

FIX: The pkg-config module hardcodes CFLAGS used when building the package:
$ pkg-config --cflags chardet
-O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -fvisibility=hidden -I/usr/include/chardet 
There should only be -I/usr/include/chardet.

Otherwise the package is in line with Fedora packaging guidelines.

Please correct all `FIX' items, consider fixing `TODO' items, and provide new spec file.
Resolution: Package NOT approved.
Comment 4 MartinKG 2015-12-10 14:12:04 EST
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/libchardet.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/libchardet-1.0.4-2.fc23.src.rpm

%changelog
* Thu Dec 10 2015 Martin Gansser <martinkg@fedoraproject.org> - 1.0.4-2
- fixed url address
- fixed license tag
- removed %%path_man_en macro, used %%pushd %%popd instead
- added README to using %%doc macro.
- corrected libchardet-1.0.4-man.patch
- renamed libchardet-man.patch to libchardet-1.0.4-man.patch
- renamed summary descripton
- removed LICENSE file from %%_docdir
- added patch to remove hardcodes CFLAGS from chardet.pc.in
Comment 5 Upstream Release Monitoring 2015-12-11 03:06:16 EST
ppisar's scratch build of libchardet-1.0.4-2.fc24.src.rpm for f24 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12148166
Comment 6 Petr Pisar 2015-12-11 03:11:00 EST
Spec file changes:

--- libchardet.spec.old 2015-12-09 10:32:02.000000000 +0100
+++ libchardet.spec     2015-12-10 20:09:31.000000000 +0100
@@ -1,14 +1,14 @@
-%global         path_man_en %{_builddir}/%{name}-%{version}/man/en
-
 Name:           libchardet
 Version:        1.0.4
-Release:        1%{?dist}
-Summary:        Mozilla Universal Chardet library
+Release:        2%{?dist}
+Summary:        Mozilla's universal character set detector
 Group:          System Environment/Libraries
-License:        MPLv1.1 or GPLv2+ and MIT
-URL:            http://oops.org
-Source0:        http://mirror.oops.org/pub/oops/%{name}/%{name}-%{version}.tar.bz2
-Patch0:         libchardet-man.patch
+License:        MPLv1.1 or LGPLv2+ or GPLv2+
+URL:            http://ftp.oops.org/pub/oops/libchardet/
+# wget http://ftp.oops.org/pub/oops/libchardet/libchardet-1.0.4.tar.bz2
+Source0:        %{name}-%{version}.tar.bz2
+Patch0:         %{name}-%{version}-man.patch
+Patch1:         %{name}-%{version}-pc.in.patch
 BuildRequires:  libstdc++-devel

 %description
@@ -27,12 +27,15 @@
 %prep
 %setup -q
 %patch0 -p1
+%patch1 -p1

 # Fix rpmlint file-not-utf8
+pushd man/en
 for i in detect_init.3 detect_obj_free.3 detect_obj_init.3 detect_reset.3 ; do
-  iconv --from=ISO-8859-1 --to=UTF-8 %{path_man_en}/$i > %{path_man_en}/$i.conv
-  mv %{path_man_en}/$i.conv %{path_man_en}/$i
+  iconv --from=ISO-8859-1 --to=UTF-8 $i > $i.conv
+  mv $i.conv $i
 done
+popd

 %build
 %configure --disable-static --enable-shared
@@ -47,13 +50,16 @@
 # remove all '*.la' files
 find %{buildroot} -name '*.la' -exec rm -f {} ';'

+# remove LICENSE file from %%_docdir
+rm -rf %{buildroot}%{_datadir}/doc/%{name}/LICENSE
+
 %post -p /sbin/ldconfig

 %postun -p /sbin/ldconfig

 %files -f %{name}.lang
-%doc Changelog
-%license %{_datadir}/doc/%{name}/LICENSE
+%doc Changelog README
+%license LICENSE
 %{_libdir}/%{name}.so.*

 %files devel
@@ -64,6 +70,17 @@
 %{_mandir}/man3/*

 %changelog
+* Thu Dec 10 2015 Martin Gansser <martinkg@fedoraproject.org> - 1.0.4-2
+- fixed url address
+- fixed license tag
+- removed %%path_man_en macro, used %%pushd %%popd instead
+- added README to using %%doc macro.
+- corrected libchardet-1.0.4-man.patch
+- renamed libchardet-man.patch to libchardet-1.0.4-man.patch
+- renamed summary descripton
+- removed LICENSE file from %%_docdir
+- added patch to remove hardcodes CFLAGS from chardet.pc.in
+
 * Tue Dec 08 2015 Martin Gansser <martinkg@fedoraproject.org> - 1.0.4-1
 - initial build


> FIX: The URL is not about libchardet. Please provide better one. Maybe a link to
> directory with the releases would be enough. If there is no better one, add the
> trailing slash. Each URL must have a path.
> FIX: The Source0 does not work for me. I'm getting TCP connection refused from
> 14.0.82.80:80. If it does not work, add a comment about it, or better change the
> URL to some another location where you obtained the file from.
-URL:            http://oops.org
-Source0:        http://mirror.oops.org/pub/oops/%{name}/%{name}-%{version}.tar.bz2
+URL:            http://ftp.oops.org/pub/oops/libchardet/
+# wget http://ftp.oops.org/pub/oops/libchardet/libchardet-1.0.4.tar.bz2
+Source0:        %{name}-%{version}.tar.bz2
Ok.

> TODO: The libchardet-man.patch is insufficient. The dot is still not rendered.
The new patch is good. Ok.

> TODO: Rename the libchardet-man.patch to libchardet-1.0.4-man.patch, so it's
> visible against what version the patch was developed.
-Patch0:         libchardet-man.patch
+Patch0:         %{name}-%{version}-man.patch
TODO: I recommend not to use the %version macro here. It will cause you troubles when upgrading the package to new version. If the libchardet-1.4.0-man.patch worked with a new libchardet version there would no reason for renaming the patch file.

> FIX: The summary does not explain what is it about. For example the first line from
> README is better. I recommend you this wording: "Mozilla's universal character set
> detector".
-Summary:        Mozilla Universal Chardet library
+Summary:        Mozilla's universal character set detector
Ok.

> FIX: I think correct license tag is "MPLv1.1 or LGPLv2+ or GPLv2+".
-License:        MPLv1.1 or GPLv2+ and MIT
+License:        MPLv1.1 or LGPLv2+ or GPLv2+
Ok.

> TODO: Instead of declaring %path_man_en macro, I recommend to "pushd man/en" before
> documentation recoding loop and "popd" after that.
+pushd man/en
 for i in detect_init.3 detect_obj_free.3 detect_obj_init.3 detect_reset.3 ; do
-  iconv --from=ISO-8859-1 --to=UTF-8 %{path_man_en}/$i > %{path_man_en}/$i.conv
-  mv %{path_man_en}/$i.conv %{path_man_en}/$i
+  iconv --from=ISO-8859-1 --to=UTF-8 $i > $i.conv
+  mv $i.conv $i
 done
+popd
Ok.

> TODO: Package README using %doc macro.
> FIX: Do not package LICENSE file twice.
-%doc Changelog
-%license %{_datadir}/doc/%{name}/LICENSE
+%doc Changelog README
+%license LICENSE
Ok.

> FIX: Build-require `glibc-common' (libchardet.spec:33).
> FIX: Build-require `coreutils' (libchardet.spec:34).
> FIX: Build-require `make' (libchardet.spec:40).
> FIX: Build-require `findutils' (libchardet.spec:48).
> FIX: Build-require `gcc' for including various standard library headers and having
> toolchain for compilation.
> FIX: Build-require `gcc-c++' for having compiler for the C++ code.
> FIX: Build-require `sed' (configure.ac:24).
> FIX: Build-require `perl' (configure.ac:46).
FIX: Really add the build-requires. It's necessary to specify all directly used dependencies.

$ rpmlint libchardet.spec ../SRPMS/libchardet-1.0.4-2.fc24.src.rpm ../RPMS/x86_64/libchardet-*
libchardet.spec: W: invalid-url Source0: libchardet-1.0.4.tar.bz2
libchardet.src: W: invalid-url URL: http://ftp.oops.org/pub/oops/libchardet/ <urlopen error [Errno 111] Connection refused>
libchardet.src: W: invalid-url Source0: libchardet-1.0.4.tar.bz2
libchardet.x86_64: W: invalid-url URL: http://ftp.oops.org/pub/oops/libchardet/ <urlopen error [Errno 111] Connection refused>
libchardet-debuginfo.x86_64: W: invalid-url URL: http://ftp.oops.org/pub/oops/libchardet/ <urlopen error [Errno 111] Connection refused>
libchardet-devel.x86_64: W: invalid-url URL: http://ftp.oops.org/pub/oops/libchardet/ <urlopen error [Errno 111] Connection refused>
libchardet-devel.x86_64: W: only-non-binary-in-usr-lib
libchardet-devel.x86_64: W: no-manual-page-for-binary chardet-config
4 packages and 1 specfiles checked; 0 errors, 8 warnings.
rpmlint is Ok.

$ rpm -q -lv -p ../RPMS/x86_64/libchardet-1.0.4-2.fc24.x86_64.rpm 
lrwxrwxrwx    1 root    root                       19 Dec 11 08:51 /usr/lib64/libchardet.so.1 -> libchardet.so.1.0.0
-rwxr-xr-x    1 root    root                   125440 Dec 11 08:51 /usr/lib64/libchardet.so.1.0.0
drwxr-xr-x    2 root    root                        0 Dec 11 08:51 /usr/share/doc/libchardet
-rw-r--r--    1 root    root                      884 Feb 14  2014 /usr/share/doc/libchardet/Changelog
-rw-r--r--    1 root    root                      831 Feb 10  2014 /usr/share/doc/libchardet/README
drwxr-xr-x    2 root    root                        0 Dec 11 08:51 /usr/share/licenses/libchardet
-rw-r--r--    1 root    root                    25755 Feb 10  2014 /usr/share/licenses/libchardet/LICENSE
-rw-r--r--    1 root    root                     1118 Dec 11 08:51 /usr/share/man/ko/man3/detect.3.gz
-rw-r--r--    1 root    root                      819 Dec 11 08:51 /usr/share/man/ko/man3/detect_destroy.3.gz
-rw-r--r--    1 root    root                     1326 Dec 11 08:51 /usr/share/man/ko/man3/detect_handledata.3.gz
-rw-r--r--    1 root    root                     1021 Dec 11 08:51 /usr/share/man/ko/man3/detect_init.3.gz
-rw-r--r--    1 root    root                      931 Dec 11 08:51 /usr/share/man/ko/man3/detect_obj_free.3.gz
-rw-r--r--    1 root    root                     1017 Dec 11 08:51 /usr/share/man/ko/man3/detect_obj_init.3.gz
-rw-r--r--    1 root    root                     1072 Dec 11 08:51 /usr/share/man/ko/man3/detect_reset.3.gz
FIX: Move the Korean manual pages to devel sub-package. They are from section 3, thus belongs to developmental package.

> FIX: The pkg-config module hardcodes CFLAGS used when building the package:
Ok.

Package builds in F24 (http://koji.fedoraproject.org/koji/taskinfo?taskID=12148166). Ok.

Please correct all the `FIX' items, consider fixing `TODO' items, and provide a new spec file.
Resolution: Package NOT approved.
Comment 7 MartinKG 2015-12-11 05:32:33 EST
(In reply to Petr Pisar from comment #6)

> $ rpm -q -lv -p ../RPMS/x86_64/libchardet-1.0.4-2.fc24.x86_64.rpm 
> lrwxrwxrwx    1 root    root                       19 Dec 11 08:51
> /usr/lib64/libchardet.so.1 -> libchardet.so.1.0.0
> -rwxr-xr-x    1 root    root                   125440 Dec 11 08:51
> /usr/lib64/libchardet.so.1.0.0
> drwxr-xr-x    2 root    root                        0 Dec 11 08:51
> /usr/share/doc/libchardet
> -rw-r--r--    1 root    root                      884 Feb 14  2014
> /usr/share/doc/libchardet/Changelog
> -rw-r--r--    1 root    root                      831 Feb 10  2014
> /usr/share/doc/libchardet/README
> drwxr-xr-x    2 root    root                        0 Dec 11 08:51
> /usr/share/licenses/libchardet
> -rw-r--r--    1 root    root                    25755 Feb 10  2014
> /usr/share/licenses/libchardet/LICENSE
> -rw-r--r--    1 root    root                     1118 Dec 11 08:51
> /usr/share/man/ko/man3/detect.3.gz
> -rw-r--r--    1 root    root                      819 Dec 11 08:51
> /usr/share/man/ko/man3/detect_destroy.3.gz
> -rw-r--r--    1 root    root                     1326 Dec 11 08:51
> /usr/share/man/ko/man3/detect_handledata.3.gz
> -rw-r--r--    1 root    root                     1021 Dec 11 08:51
> /usr/share/man/ko/man3/detect_init.3.gz
> -rw-r--r--    1 root    root                      931 Dec 11 08:51
> /usr/share/man/ko/man3/detect_obj_free.3.gz
> -rw-r--r--    1 root    root                     1017 Dec 11 08:51
> /usr/share/man/ko/man3/detect_obj_init.3.gz
> -rw-r--r--    1 root    root                     1072 Dec 11 08:51
> /usr/share/man/ko/man3/detect_reset.3.gz
> FIX: Move the Korean manual pages to devel sub-package. They are from
> section 3, thus belongs to developmental package.
> 
sorry don't know what is the correct command to do this ?
Comment 8 Petr Pisar 2015-12-11 07:10:37 EST
I think the problem is %find_lang --with-man gathers the /usr/share/man/ko/ files as localized stuff, list them into %{name}.lang and then "%files -f %{name}.lang" includes them into libchardet package, so they are already moved  when creating libcharet-devel.

I have no experience with it, but I think the answer is in older review <https://bugzilla.redhat.com/show_bug.cgi?id=1047647#c12>:

> If there will ever be translation files for the library, you may need to
> "grep" from the %name.lang file to extract what to include in the individual
> subpackages.

I tried it:

--- libchardet.spec.old 2015-12-10 20:09:31.000000000 +0100
+++ libchardet.spec     2015-12-11 12:55:52.188000000 +0100
@@ -10,6 +10,7 @@
 Patch0:         %{name}-%{version}-man.patch
 Patch1:         %{name}-%{version}-pc.in.patch
 BuildRequires:  libstdc++-devel
+BuildRequires:  sed
 
 %description
 libchardet provides an interface to Mozilla's universal charset detector,
@@ -45,7 +46,9 @@
 %install
 make DESTDIR=%{buildroot} install
 
-%find_lang %{name} --with-man --all-name
+%find_lang all --with-man --all-name
+< all.lang sed -e '/\/man3\//!d' > %{name}-devel.lang
+< all.lang sed -e '/\/man3\//d' > %{name}.lang
 
 # remove all '*.la' files
 find %{buildroot} -name '*.la' -exec rm -f {} ';'
@@ -62,7 +65,7 @@
 %license LICENSE
 %{_libdir}/%{name}.so.*
 
-%files devel
+%files devel -f %{name}-devel.lang
 %{_bindir}/chardet-config
 %{_libdir}/*.so
 %{_libdir}/pkgconfig/chardet.pc

But the issue is rpmbuild then fails like:

Processing files: libchardet-1.0.4-2.fc24.x86_64
error: Empty %files file /home/test/rpmbuild/BUILD/libchardet-1.0.4/libchardet.lang

because no other localized files left for the libchardet package and rpmbuild does like empty files passed by the -f option.

I think the easiest fix is to move "-f" option from libchardet to libchardet-devel package:

--- libchardet.spec.old 2015-12-10 20:09:31.000000000 +0100
+++ libchardet.spec     2015-12-11 13:07:52.898000000 +0100
@@ -45,7 +45,7 @@
 %install
 make DESTDIR=%{buildroot} install
 
-%find_lang %{name} --with-man --all-name
+%find_lang %{name}-devel --with-man --all-name
 
 # remove all '*.la' files
 find %{buildroot} -name '*.la' -exec rm -f {} ';'
@@ -57,12 +57,12 @@
 
 %postun -p /sbin/ldconfig
 
-%files -f %{name}.lang
+%files
 %doc Changelog README
 %license LICENSE
 %{_libdir}/%{name}.so.*
 
-%files devel
+%files devel -f %{name}-devel.lang
 %{_bindir}/chardet-config
 %{_libdir}/*.so
 %{_libdir}/pkgconfig/chardet.pc
Comment 9 MartinKG 2015-12-11 09:00:18 EST
thanks for your explanation, new packages again.

Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/libchardet.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/libchardet-1.0.4-3.fc23.src.rpm

%changelog
* Fri Dec 11 2015 Martin Gansser <martinkg@fedoraproject.org> - 1.0.4-3
- renamed %%version macro for patches
- added BR glibc-common
- added BR coreutils
- added BR make
- added BR findutils
- added BR gcc
- added BR gcc-c++
- added BR sed
- added BR perl
- moved Korean manual pages to devel sub-package
Comment 10 Upstream Release Monitoring 2015-12-11 09:32:28 EST
ppisar's scratch build of libchardet-1.0.4-3.fc24.src.rpm for f24 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12153893
Comment 11 Petr Pisar 2015-12-11 09:34:13 EST
Spec file changes:

--- libchardet.spec.old 2015-12-10 20:09:31.000000000 +0100
+++ libchardet.spec     2015-12-11 14:57:03.000000000 +0100
@@ -1,15 +1,24 @@
 Name:           libchardet
 Version:        1.0.4
-Release:        2%{?dist}
+Release:        3%{?dist}
 Summary:        Mozilla's universal character set detector
 Group:          System Environment/Libraries
 License:        MPLv1.1 or LGPLv2+ or GPLv2+
 URL:            http://ftp.oops.org/pub/oops/libchardet/
 # wget http://ftp.oops.org/pub/oops/libchardet/libchardet-1.0.4.tar.bz2
 Source0:        %{name}-%{version}.tar.bz2
-Patch0:         %{name}-%{version}-man.patch
-Patch1:         %{name}-%{version}-pc.in.patch
+Patch0:         %{name}-1.0.4-man.patch
+Patch1:         %{name}-1.0.4-pc.in.patch
+
 BuildRequires:  libstdc++-devel
+BuildRequires:  glibc-common
+BuildRequires:  coreutils
+BuildRequires:  make
+BuildRequires:  findutils
+BuildRequires:  gcc
+BuildRequires:  gcc-c++
+BuildRequires:  sed
+BuildRequires:  perl

 %description
 libchardet provides an interface to Mozilla's universal charset detector,
@@ -45,7 +54,7 @@
 %install
 make DESTDIR=%{buildroot} install

-%find_lang %{name} --with-man --all-name
+%find_lang %{name}-devel --with-man --all-name

 # remove all '*.la' files
 find %{buildroot} -name '*.la' -exec rm -f {} ';'
@@ -57,12 +66,12 @@

 %postun -p /sbin/ldconfig

-%files -f %{name}.lang
+%files
 %doc Changelog README
 %license LICENSE
 %{_libdir}/%{name}.so.*

-%files devel
+%files devel -f %{name}-devel.lang
 %{_bindir}/chardet-config
 %{_libdir}/*.so
 %{_libdir}/pkgconfig/chardet.pc
@@ -70,6 +79,18 @@
 %{_mandir}/man3/*

 %changelog
+* Fri Dec 11 2015 Martin Gansser <martinkg@fedoraproject.org> - 1.0.4-3
+- renamed %%version macro for patches
+- added BR glibc-common
+- added BR coreutils
+- added BR make
+- added BR findutils
+- added BR gcc
+- added BR gcc-c++
+- added BR sed
+- added BR perl
+- moved Korean manual pages to devel sub-package
+
 * Thu Dec 10 2015 Martin Gansser <martinkg@fedoraproject.org> - 1.0.4-2
 - fixed url address
 - fixed license tag


> TODO: I recommend not to use the %version macro here.
-Patch0:         %{name}-%{version}-man.patch
-Patch1:         %{name}-%{version}-pc.in.patch
+Patch0:         %{name}-1.0.4-man.patch
+Patch1:         %{name}-1.0.4-pc.in.patch
Ok.

> > FIX: Build-require `glibc-common' (libchardet.spec:33).
> > FIX: Build-require `coreutils' (libchardet.spec:34).
> > FIX: Build-require `make' (libchardet.spec:40).
> > FIX: Build-require `findutils' (libchardet.spec:48).
> > FIX: Build-require `gcc' for including various standard library headers and having
> > toolchain for compilation.
> > FIX: Build-require `gcc-c++' for having compiler for the C++ code.
> > FIX: Build-require `sed' (configure.ac:24).
> > FIX: Build-require `perl' (configure.ac:46).
> FIX: Really add the build-requires. It's necessary to specify all directly used
> dependencies.
+BuildRequires:  glibc-common
+BuildRequires:  coreutils
+BuildRequires:  make
+BuildRequires:  findutils
+BuildRequires:  gcc
+BuildRequires:  gcc-c++
+BuildRequires:  sed
+BuildRequires:  perl
Ok.

$ rpmlint libchardet.spec ../SRPMS/libchardet-1.0.4-3.fc24.src.rpm ../RPMS/x86_64/libchardet-*
libchardet.spec: W: invalid-url Source0: libchardet-1.0.4.tar.bz2
libchardet.src: W: invalid-url URL: http://ftp.oops.org/pub/oops/libchardet/ <urlopen error [Errno 111] Connection refused>
libchardet.src: W: invalid-url Source0: libchardet-1.0.4.tar.bz2
libchardet.x86_64: W: invalid-url URL: http://ftp.oops.org/pub/oops/libchardet/ <urlopen error [Errno 111] Connection refused>
libchardet-debuginfo.x86_64: W: invalid-url URL: http://ftp.oops.org/pub/oops/libchardet/ <urlopen error [Errno 111] Connection refused>
libchardet-devel.x86_64: W: invalid-url URL: http://ftp.oops.org/pub/oops/libchardet/ <urlopen error [Errno 111] Connection refused>
libchardet-devel.x86_64: W: only-non-binary-in-usr-lib
libchardet-devel.x86_64: W: no-manual-page-for-binary chardet-config
4 packages and 1 specfiles checked; 0 errors, 8 warnings.
rpmlint is Ok.

> FIX: Move the Korean manual pages to devel sub-package.

$ rpm -q -l -p ../RPMS/x86_64/libchardet-1.0.4-3.fc24.x86_64.rpm 
/usr/lib64/libchardet.so.1
/usr/lib64/libchardet.so.1.0.0
/usr/share/doc/libchardet
/usr/share/doc/libchardet/Changelog
/usr/share/doc/libchardet/README
/usr/share/licenses/libchardet
/usr/share/licenses/libchardet/LICENSE
$ rpm -q -l -p ../RPMS/x86_64/libchardet-devel-1.0.4-3.fc24.x86_64.rpm 
/usr/bin/chardet-config
/usr/include/chardet/chardet-config.h
/usr/include/chardet/chardet.h
/usr/include/chardet/nsUniversalDetector.h
/usr/include/chardet/nscore.h
/usr/include/chardet/version.h
/usr/lib64/libchardet.so
/usr/lib64/pkgconfig/chardet.pc
/usr/share/man/ko/man3/detect.3.gz
/usr/share/man/ko/man3/detect_destroy.3.gz
/usr/share/man/ko/man3/detect_handledata.3.gz
/usr/share/man/ko/man3/detect_init.3.gz
/usr/share/man/ko/man3/detect_obj_free.3.gz
/usr/share/man/ko/man3/detect_obj_init.3.gz
/usr/share/man/ko/man3/detect_reset.3.gz
/usr/share/man/man3/detect.3.gz
/usr/share/man/man3/detect_destroy.3.gz
/usr/share/man/man3/detect_handledata.3.gz
/usr/share/man/man3/detect_init.3.gz
/usr/share/man/man3/detect_obj_free.3.gz
/usr/share/man/man3/detect_obj_init.3.gz
/usr/share/man/man3/detect_reset.3.gz

File layout is Ok.

Package builds in F24 (http://koji.fedoraproject.org/koji/taskinfo?taskID=12153893). Ok.

Package is good.

Resolution: Package APPROVED.
Comment 12 MartinKG 2015-12-11 09:43:04 EST
Petr Thanks for the review.
Comment 13 Gwyn Ciesla 2015-12-11 10:17:55 EST
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/libchardet
Comment 14 MartinKG 2015-12-11 10:45:24 EST
package has been built successfully on fc23 and rawhide.

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