Bug 1505304 - Review Request: libcpuid - Provides CPU identification for x86
Summary: Review Request: libcpuid - Provides CPU identification for x86
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Robert-André Mauchin 🐧
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 1505306
TreeView+ depends on / blocked
 
Reported: 2017-10-23 09:27 UTC by MartinKG
Modified: 2017-10-24 07:37 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2017-10-24 07:37:13 UTC
Type: ---
Embargoed:
eclipseo: fedora-review+


Attachments (Terms of Use)

Description MartinKG 2017-10-23 09:27:02 UTC
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/libcpuid.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/libcpuid-0.4.0-1.20170504git57298c6.fc26.src.rpm

Description: Libcpuid provides CPU identification for the x86 (and x86_64).
Fedora Account System Username: martinkg

%changelog
* Sun Oct 22 2017 Martin Gansser <martinkg> - 0.4.0-1.20170504git57298c6
- Initial build.

Comment 1 Igor Gnatenko 2017-10-23 10:48:56 UTC
* Missing BuildRequires: gcc-c++

> libtoolize
> autoreconf --install
Can be replaced with autoreconf -vfi

> %exclude %{_libdir}/%{name}.so.*
Not needed

Comment 2 MartinKG 2017-10-23 11:37:33 UTC
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/libcpuid.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/libcpuid-0.4.0-2.20170504git57298c6.fc26.src.rpm

%changelog
* Mon Oct 23 2017 Martin Gansser <martinkg> - 0.4.0-2.20170504git57298c6
- Add BR gcc-c++
- replace libtoolize and autoreconf --install with autoreconf -vfi
- remove %%exclude %%{_libdir}/%%{name}.so.* not needed

Comment 3 Robert-André Mauchin 🐧 2017-10-23 13:31:36 UTC
Hello,

 - You need to add:

BuildRequires:  doxygen

Otherwise the docs and man pages are not built.

 - You should disable the build of static lib:

%configure  --disable-static

 - You didn't actually remove %exclude %{_libdir}/%{name}.so. Just use:

%files devel
%{_bindir}/cpuid_tool
%{_includedir}/%{name}
%{_libdir}/%{name}.so
%{_libdir}/pkgconfig/%{name}.pc
%{_mandir}/man3/*.3.*

The static lib %{_libdir}/%{name}.a shouldn't be included anyway.

 - The package uses obsolete m4 macros:

[!]: Package should not use obsolete m4 macros
     Note: Some obsoleted macros found, see the attachment.
     See: https://fedorahosted.org/FedoraReview/wiki/AutoTools

You should patch them out and notify upstream about it. Here's a patch for it:

diff -up libcpuid-57298c650cd3cb326d9357cdb9b9dd51d60007bb/configure.ac.fix_obsolete_m4s libcpuid-57298c650cd3cb326d9357cdb9b9dd51d60007bb/configure.ac
--- libcpuid-57298c650cd3cb326d9357cdb9b9dd51d60007bb/configure.ac.fix_obsolete_m4s	2017-05-04 14:36:19.000000000 +0200
+++ libcpuid-57298c650cd3cb326d9357cdb9b9dd51d60007bb/configure.ac	2017-10-23 15:11:43.130346420 +0200
@@ -36,7 +36,7 @@ AC_SUBST([LIBCPUID_VERSION_INFO])
 AC_PROG_CC
 m4_ifdef([AM_PROG_AR], [AM_PROG_AR])
 AC_C_CONST
-AM_PROG_LIBTOOL
+LT_INIT
 AM_CPPFLAGS="$CPPFLAGS"
 
 AC_CHECK_HEADERS([stdint.h])

Comment 4 MartinKG 2017-10-23 13:52:02 UTC
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/libcpuid.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/libcpuid-0.4.0-3.20170504git57298c6.fc26.src.rpm

%changelog
* Mon Oct 23 2017 Martin Gansser <martinkg> - 0.4.0-3.20170504git57298c6
- Add BR doxygen
- disable build of static lib
- don't remove %exclude %{_libdir}/%{name}.so
- Add %%{name}-not-use-4m-macro.patch

Comment 5 Robert-André Mauchin 🐧 2017-10-23 14:09:30 UTC
%{_libdir}/%{name}* should be: %{_libdir}/%{name}.so

There's no other file here' otherwise it will also contain the versioned lib.

Comment 6 MartinKG 2017-10-23 14:12:37 UTC
(In reply to Robert-André Mauchin from comment #5)
> %{_libdir}/%{name}* should be: %{_libdir}/%{name}.so
> 
> There's no other file here' otherwise it will also contain the versioned lib.

done, no extra file upload.

Comment 7 Robert-André Mauchin 🐧 2017-10-23 14:44:32 UTC
All ok, package accepted.

Comment 8 MartinKG 2017-10-23 15:07:38 UTC
thanks for reviewing ticket :)

Comment 9 Gwyn Ciesla 2017-10-23 18:46:16 UTC
(fedrepo-req-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/libcpuid

Comment 10 MartinKG 2017-10-23 19:07:30 UTC
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/libcpuid.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/libcpuid-0.4.0-4.20171023git2f10315.fc26.src.rpm

%changelog
* Mon Oct 23 2017 Martin Gansser <martinkg> - 0.4.0-3.20171023git2f10315
- Update to 0.4.0-4.20171023git2f10315
- Dropped %%{name}-not-use-4m-macro.patch

Comment 11 Robert-André Mauchin 🐧 2017-10-23 19:47:06 UTC
You've introduced a typo in your %changelog: it should be 0.4.0-4, not 0.4.0-3 for the last entry.

Comment 12 MartinKG 2017-10-24 06:34:08 UTC
(In reply to Robert-André Mauchin from comment #11)
> You've introduced a typo in your %changelog: it should be 0.4.0-4, not
> 0.4.0-3 for the last entry.

fixed:
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/libcpuid.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/libcpuid-0.4.0-4.20171023git2f10315.fc26.src.rpm

Comment 13 MartinKG 2017-10-24 07:37:13 UTC
%changelog
* Mon Oct 23 2017 Martin Gansser <martinkg> - 0.4.0-4.20171023git2f10315
- Update to 0.4.0-4.20171023git2f10315
- Dropped %%{name}-not-use-4m-macro.patch
- Add ExcludeArch: aarch64 %%arm ppc64le ppc64 s390x

package has been built successfully on f25, f26, f27 and rawhide.


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