[Bug 907007] Review Request: unittest-cpp.spec - Lightweight unit testing framework for C++

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=907007

--- Comment #16 from Björn Esser <bjoern.esser@xxxxxxxxx> ---
Created attachment 755157
  --> https://bugzilla.redhat.com/attachment.cgi?id=755157&action=edit
Suggested fixes for rpm-spec

Hello François!

A quick view over the spec-file:

 %global         _hardened_build 1
-%global         gitversion      20130530gitc42e68bb
-%global         oldname UnitTest++
+%global         commit      c42e68bb999d01da9ec71b67ff1a2cbd6ec1b6a6
+%global         shortcommit %(c=%{commit}; echo ${c:0:7})
+%global         commitdate  20130509
+%global         gitversion  .%{commitdate}git%{shortcommit}
+%global         oldname     UnitTest++

see: http://fedoraproject.org/wiki/Packaging:SourceURL#Github

#####

 Name:           unittest-cpp
-Version:        1.4
-Release:        8%{?dist}.%{gitversion}
+Version:        1.5
+Release:        0%{gitversion}%{?dist}
 Summary:        Lightweight unit testing framework for C++
 License:        MIT
 Group:          Development/Libraries
-
-# the latest version from the old project at sf.net is 1.4
-# the new project on github has no release yet, so...
-##### creating the archive from git #####
-# git clone https://github.com/unittest-cpp/unittest-cpp.git
-# cd unittest-cpp/
-# git log
-# cd ..
-# tar cjf unittest-cpp-20130530gitc42e68bb.tbz2 unittest-cpp/
-#########################################
-
-# resulting tarball:
-Source0:        %{name}-%{gitversion}.tbz2
+URL:            https://github.com/%{name}/%{name}
+Source0:       
https://github.com/%{name}/%{name}/archive/%{commit}/%{name}-%{version}-%{shortcommit}.tar.gz

see:
https://fedoraproject.org/wiki/Packaging:NamingGuidelines?rd=Packaging/NamingGuidelines#Pre-Release_packages
and http://fedoraproject.org/wiki/Packaging:SourceURL#Github

#####

 # documentation from 1.4 tarball: docs/UnitTest++.html
 Source2:        %{name}.html

-# old Url: http://sf.net/projects/unittest-cpp
-Url:            https://github.com/unittest-cpp/unittest-cpp
-
 BuildRequires:  autoconf
 BuildRequires:  libtool

just removed some obsoleted stuff...

#####

 %package static
 Summary:        Static library for %{name}
 Group:          Development/Libraries
+Requires:       %{name}%{?_isa} = %{version}-%{release}

see:
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Duplicate_Files

Or put docs in noarch-subpkg and require this, if you'd prefer...

#####

 %prep
-%setup -q -n %{name}
+%setup -q -n %{name}-%{commit}
 cp -p %SOURCE1 %SOURCE2 .
-
-%build
 # autoreconf will complain about missing NEWS and README files
 touch NEWS
-ln README.md README
+ln -f README.md README
 # autoreconf will add a GPLv3 license text in COPYING
-ln LICENSE COPYING
-autoreconf -i
+ln -f LICENSE COPYING
+autoreconf -vfi
+
+%build
 %configure

It's prefered to have autoreconf in %prep, because it prepares the sources for
being build...

#####

+%check
+make check
+
 %install
 mkdir -p %{buildroot}%{_includedir}/%{name}
 mkdir -p %{buildroot}%{_libdir}
 mkdir -p %{buildroot}%{_datadir}/pkgconfig
-make DESTDIR=%{buildroot} install
+%make_install
 install -p -m 644 %{name}.pc %{buildroot}%{_datadir}/pkgconfig/
-rm -f %{buildroot}%{_libdir}/lib%{oldname}.la

%check-target is, in IMHO, mandatory, if Makefile has a check-rule, see:
http://fedoraproject.org/wiki/How_to_create_an_RPM_package#.25check_section
Using %make_install-macro is prefered to make install, but not required...
la-file is needed by -static subpkg for static-linking...

#####

 %files
-%doc AUTHORS LICENSE README.md 
+%doc AUTHORS LICENSE README
 %{_libdir}/lib%{oldname}.so.*

 %files devel
-%doc %{name}.html LICENSE
+%doc %{name}.html
 %{_includedir}/%{oldname}
 %{_libdir}/lib%{oldname}.so
 %{_datadir}/pkgconfig/%{name}.pc

 %files static
-%doc LICENSE
 %{_libdir}/lib%{oldname}.a
+%{_libdir}/lib%{oldname}.la

see:
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Duplicate_Files
la-file is needed by -static subpkg for static-linking...

#####

 %changelog
+* Fri May 31 2013 François Cami <fcami@xxxxxxxxxxxxxxxxx> -
1.5-0.20130509gitc42e68bb
+- improvements: globals for git-tarball/version, fixed version/release,
+  la-file goes to -static, autoreconf goes to prep-target, add check-target,
+  removed double-packaged files
+
 * Thu May 30 2013 François Cami <fcami@xxxxxxxxxxxxxxxxx> -
1.4-8.20130530gitc42e68bb

Create some clog-entry...

#####

rpmlint shows:

Rpmlint (installed packages)
----------------------------
# rpmlint unittest-cpp-devel unittest-cpp-static unittest-cpp
unittest-cpp.x86_64: W: unused-direct-shlib-dependency
/usr/lib64/libUnitTest++.so.1.4.0 /lib64/libm.so.6
3 packages and 0 specfiles checked; 0 errors, 1 warnings.

Please fix, as suggested here:
https://fedoraproject.org/wiki/Common_Rpmlint_issues#unused-direct-shlib-dependency

#####

I'll take another review-run after updated spec/srpm.

Cheers,
  Björn

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=bCEwzvHXXZ&a=cc_unsubscribe
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]