Bug 1076863

After improving your package with the hints and suggestions given below, please run your new packages and spec file through rpmlint to verify the changes. Include the output from rpmlint in your comment when you inform me of an updated src.rpm and spec file.


Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Needs clarification

  -> = suggested action to take
  Q: = question you need to answer

Issues:
=======
- Development (unversioned) .so files in -devel subpackage, if present.
  Note: Unversioned so-files directly in %_libdir.
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#DevelPackages
  -> Put the libcmockery2.so file in the -devel subpackage.
- All build dependencies are listed in BuildRequires, except for any that are
  listed in the exceptions section of Packaging Guidelines.
  Note: These BR are not needed: gcc make
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2
  -> gcc and make are always installed in the buildroot, please remove them
     from the BuildRequires.
- ldconfig called in %post and %postun if required.
  Note: /sbin/ldconfig not called in cmockery2
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#Shared_Libraries
  -> Your package caries lib*.so* files, you need to execute ldconfig in %post
     and %postun.
- Package does not contain any libtool archives (.la)
  Note: cmockery2-devel : /usr/lib/libcmockery.la
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#StaticLibraries
  -> Please prevent building the .la file, or delete the file after
     compilation. If this really is needed (I don't think it is) for static
     compilation, include it in the -static package.
- The Source0 url is invalid.
  -> use https://github.com/lpabon/%{name}/archive/v%{version}/%{name}-%{version}.tar.gz
- The summary text for the package description should no end in a '.'.
  -> Remove the . at the end of the Summary lines.
- There is no version on the last entry in the %changelog.
  -> Please add a <version>-<revision> in the changelog.
- The %clean section is only needed for EPEL-5 packages.
  Q: Are you planning to provide this package in EPEL-5?
- Invalid BuildRoot is used.
  See: https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#BuildRoot_tag
  -> If you want to provide the package in EPEL-5, set the BuildRoot to one of
     the variants from https://fedoraproject.org/wiki/EPEL:Packaging#BuildRoot_tag
- Subpackages -devel and -static are missing a versioned dependency on the
  main package.
  -> Add "Requires: %{name}%{?_isa} = %{version}-%{release}" in
     cmockery2-devel and cmockery2-static.
- The package contains obsolete autotools m4 macros.
  See: https://fedorahosted.org/FedoraReview/wiki/AutoTools
  Q: Can be dropped, or are they required for EPEL-5?
- -devel contains unneeded files.
  -> NEWS is empty, and INSTALL does not contain any useful information. These
     files should not get installed.


Accepted warnings:
==================
- Spelling errors for 'Cmockery' and 'executables'.
- exit() call in libcmockery.so in cmockery.c:exit_test().


===== MUST items =====

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Header files in -devel subpackage, if present.
[x]: Rpath absent or only used for internal libs.

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
     Note: There is no build directory. Running licensecheck on vanilla
     upstream sources. Licenses found: "Apache (v2.0)", "Unknown or
     generated". 5 files have unknown license. Detailed output of licensecheck
     in /srv/devel/fedora-
     pkgs/cmockery2.bz1076863/1076863-cmockery2/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[!]: Changelog in prescribed format.
[?]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf %{buildroot} present but not required
[x]: Sources contain only permissible code or content.
[x]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 122880 bytes in 18 files.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least one
     supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[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]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package do not use a name that already exist
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as provided
     in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: Static libraries in -static or -devel subpackage, providing -devel if
     present.
     Note: Package has .a files: cmockery2-static.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[!]: Buildroot is not present
     Note: Invalid buildroot found: %{_tmppath}/%{name}-root
     See: http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag
[?]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
     Note: %clean present but not required
[-]: If the source package does not include license text(s) as a separate file
     from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[!]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
     cmockery2-devel , cmockery2-static
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[!]: SourceX tarball generation or download is documented.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[x]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed files.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Reviewer should test that the package builds in mock.
[x]: Dist tag is present (not strictly required in GL).
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Uses parallel make %{?_smp_mflags} macro.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[?]: Package should not use obsolete m4 macros
     Note: Some obsoleted macros found, see the attachment.
     See: https://fedorahosted.org/FedoraReview/wiki/AutoTools
[x]: Rpmlint is run on all installed packages.
[-]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: cmockery2-1.3.4-1.fc21.armv7hl.rpm
          cmockery2-devel-1.3.4-1.fc21.armv7hl.rpm
          cmockery2-static-1.3.4-1.fc21.armv7hl.rpm
          cmockery2-1.3.4-1.fc21.src.rpm
cmockery2.armv7hl: W: summary-ended-with-dot C Lightweight C unit testing framework.
cmockery2.armv7hl: W: spelling-error %description -l en_US Cmockery -> Mockery, C mockery, Crockery
cmockery2.armv7hl: W: spelling-error %description -l en_US executables -> executable, executable s, executrices
cmockery2.armv7hl: W: no-version-in-last-changelog
cmockery2.armv7hl: W: shared-lib-calls-exit /usr/lib/libcmockery.so.0.0.0 exit
cmockery2.armv7hl: E: library-without-ldconfig-postin /usr/lib/libcmockery.so.0.0.0
cmockery2.armv7hl: E: library-without-ldconfig-postun /usr/lib/libcmockery.so.0.0.0
cmockery2.armv7hl: W: devel-file-in-non-devel-package /usr/lib/libcmockery.so
cmockery2-devel.armv7hl: W: summary-ended-with-dot C Lightweight C unit testing framework.
cmockery2-devel.armv7hl: W: spelling-error %description -l en_US Cmockery -> Mockery, C mockery, Crockery
cmockery2-devel.armv7hl: W: spelling-error %description -l en_US executables -> executable, executable s, executrices
cmockery2-devel.armv7hl: W: no-version-in-last-changelog
cmockery2-devel.armv7hl: E: zero-length /usr/share/doc/cmockery2-1.3.4/NEWS
cmockery2-devel.armv7hl: W: install-file-in-docs /usr/share/doc/cmockery2-1.3.4/INSTALL
cmockery2-static.armv7hl: W: summary-ended-with-dot C Lightweight C unit testing framework.
cmockery2-static.armv7hl: W: spelling-error %description -l en_US Cmockery -> Mockery, C mockery, Crockery
cmockery2-static.armv7hl: W: spelling-error %description -l en_US executables -> executable, executable s, executrices
cmockery2-static.armv7hl: W: no-version-in-last-changelog
cmockery2-static.armv7hl: W: no-documentation
cmockery2.src: W: summary-ended-with-dot C Lightweight C unit testing framework.
cmockery2.src: W: spelling-error %description -l en_US Cmockery -> Mockery, C mockery, Crockery
cmockery2.src: W: spelling-error %description -l en_US executables -> executable, executable s, executrices
cmockery2.src: W: no-version-in-last-changelog
cmockery2.src: W: invalid-url Source0: cmockery2-1.3.4.tar.gz
4 packages and 0 specfiles checked; 3 errors, 21 warnings.


AutoTools: Obsoleted m4s found
------------------------------
  AC_PROG_LIBTOOL found in: cmockery2-1.3.4/configure.ac:31
  AM_CONFIG_HEADER found in: cmockery2-1.3.4/configure.ac:22

Yes, the spec file will also be used for EL5.  If this causes an issue, then I can have a spec file for EL6+/Fedora and one for EL5.

Here is the output of rpmlint:
cmockery2-1.3.4-1.fc19.x86_64.rpm -------------------------
cmockery2.x86_64: W: spelling-error %description -l en_US Cmockery -> Mockery, C mockery, Crockery
cmockery2.x86_64: W: spelling-error %description -l en_US cmockery -> mockery, crockery, c mockery
cmockery2.x86_64: W: shared-lib-calls-exit /usr/lib64/libcmockery.so.0.0.0 exit.5
1 packages and 0 specfiles checked; 0 errors, 3 warnings.
cmockery2-debuginfo-1.3.4-1.fc19.x86_64.rpm -------------------------
cmockery2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/cmockery2-1.3.4/src/cmockery.c
cmockery2-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/cmockery2-1.3.4/src/cmockery/cmockery.h
1 packages and 0 specfiles checked; 0 errors, 2 warnings.
cmockery2-devel-1.3.4-1.fc19.x86_64.rpm -------------------------
cmockery2-devel.x86_64: W: spelling-error %description -l en_US Cmockery -> Mockery, C mockery, Crockery
cmockery2-devel.x86_64: W: no-documentation
1 packages and 0 specfiles checked; 0 errors, 2 warnings.
cmockery2-static-1.3.4-1.fc19.x86_64.rpm -------------------------
cmockery2-static.x86_64: W: spelling-error %description -l en_US Cmockery -> Mockery, C mockery, Crockery
cmockery2-static.x86_64: W: no-documentation
1 packages and 0 specfiles checked; 0 errors, 2 warnings.


SRPM and specfile update here:
Spec URL: https://raw.githubusercontent.com/lpabon/fedora/master/cmockery2/1.3.4/cmockery2.spec
SRPM URL: https://github.com/lpabon/fedora/raw/master/cmockery2/1.3.4/cmockery2-1.3.4-1.fc19.src.rpm

%{_prefix}/share/doc --》 %{_docdir}

Reason for keeping static libraries?

(In reply to Christopher Meng from comment #3)
> %{_prefix}/share/doc --》 %{_docdir}
> 
> Reason for keeping static libraries?

I am just providing them just in case someone may want them, but I can remove it.

Updated docdir according to Christopher Meng's comment.  New files available here:

SRPM and specfile update here:
Spec URL: https://raw.githubusercontent.com/lpabon/fedora/master/cmockery2/1.3.4/cmockery2.spec
SRPM URL: https://github.com/lpabon/fedora/raw/master/cmockery2/1.3.4/cmockery2-1.3.4-1.fc19.src.rpm

1. Fedora discourages using static libraries(although, we can't avoid using it somtimes), I hope you can remove that subpackage, but since you are the upstream, I want to listen to your opinion. Do they really useful for Fedora?

2. Parallel make?

https://fedoraproject.org/wiki/Packaging:Guidelines#Parallel_make

3. -devel package:

Group:          Development/Tools

??? Are you sure ???

(In reply to Christopher Meng from comment #6)
> 1. Fedora discourages using static libraries(although, we can't avoid using
> it somtimes), I hope you can remove that subpackage, but since you are the
> upstream, I want to listen to your opinion. Do they really useful for Fedora?
No problem. I really cannot think of a situation where it is absolutely required.  I removed it from the .spec file.

> 2. Parallel make?
> 
> https://fedoraproject.org/wiki/Packaging:Guidelines#Parallel_make
Fixed.

> 
> 3. -devel package:
> 
> Group:          Development/Tools
> 
> ??? Are you sure ???
This package is very similar to libcmocka, and I based cmockery2's group on what they use which is Development/Tools.  It does make sense, since this package is a tool for C developers.

New updated packages available:
SRPM and specfile update here:
Spec URL: https://raw.githubusercontent.com/lpabon/fedora/master/cmockery2/1.3.4/cmockery2.spec
SRPM URL: https://github.com/lpabon/fedora/raw/master/cmockery2/1.3.4/cmockery2-1.3.4-1.fc19.src.rpm

For some reason, GitHub is not updating the raw version of the Spec file.  Here is the normal view to the spec file:

https://github.com/lpabon/fedora/blob/master/cmockery2/1.3.4/cmockery2.spec

> %files devel
> %{_includedir}/cmockery*
> %{_libdir}/libcmockery.so
> %exclude %{_libdir}/libcmockery.a
> %exclude %{_libdir}/libcmockery.la

The proper Group tag for such library -devel packages has always been "Development/Libraries". Don't look at bad/poor examples in the package collection.

It's "Development/Libraries" here, too:
http://pkgs.fedoraproject.org/cgit/cmocka.git/plain/cmocka.spec

The group of that base package is wrong and should be "System Environment/Libraries".

However, the Group tag may also be omitted nowadays.

(In reply to Michael Schwendt from comment #10)
> It's "Development/Libraries" here, too:
> http://pkgs.fedoraproject.org/cgit/cmocka.git/plain/cmocka.spec
> 
> The group of that base package is wrong and should be "System
> Environment/Libraries".
> 
> However, the Group tag may also be omitted nowadays.

Thank you.  I have updated the spec file and SRPM:

New updated packages available:
SRPM and specfile update here:
Spec URL: https://raw.githubusercontent.com/lpabon/fedora/master/cmockery2/1.3.4/cmockery2.spec
SRPM URL: https://github.com/lpabon/fedora/raw/master/cmockery2/1.3.4/cmockery2-1.3.4-1.fc19.src.rpm

Hi reviewers, is the package spec file ok now?

Hi Luis,

I've just checked the current .spec again, and it looks good to me. You have addressed all the comments we mentioned. From my point of view, there is nothing that prevents your current proposal to be included in the Fedora Package Collection.

With this note, I'll APPROVE the package, please continue with step 8 from the review process:
- https://fedoraproject.org/wiki/Package_Review_Process#Contributor

New Package SCM Request
=======================
Package Name: cmockery2
Short Description: Lightweight C unit testing framework
Owners: lpabon
Branches: f19 f20 el5 el6 epel7
InitialCC: ndevos

What is the purpose of this section?

| %if ( 0%{_for_fedora_koji_builds} )
| Source0:        https://github.com/lpabon/%{name}/archive/v%{version}/%{name}-%{version}.tar.gz
| %else
| Source0:        %{name}-%{version}.tar.gz
| %endif

Those two Source0 tags are equivalent (because the tarball name is the same), except that the former is preferred because it also contains the full download location.


> %check
> make check
> 
> %install

The %check section is executed _after_ %install, so it ought to be placed after %install in the spec file. Btw, there are scenarios where a custom %check section needs to operate on buildroot contents, and the buildroot is empty before %install.


> [x]: %build honors applicable compiler flags or justifies otherwise.

Not true.
https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags

Git done (by process-git-requests).

ndevos is not a valid FAS account, add later in pkgdb when it is.

Hi Michael, thank you for taking a look at the spec file again.

> What is the purpose of this section?
> 
> | %if ( 0%{_for_fedora_koji_builds} )
> | Source0:       
> https://github.com/lpabon/%{name}/archive/v%{version}/%{name}-%{version}.tar.
> gz
> | %else
> | Source0:        %{name}-%{version}.tar.gz
> | %endif
> 
> Those two Source0 tags are equivalent (because the tarball name is the
> same), except that the former is preferred because it also contains the full
> download location.
-- Fixed 

> 
> 
> > %check
> > make check
> > 
> > %install
> 
> The %check section is executed _after_ %install, so it ought to be placed
> after %install in the spec file. Btw, there are scenarios where a custom
> %check section needs to operate on buildroot contents, and the buildroot is
> empty before %install.
-- Fixed

> 
> 
> > [x]: %build honors applicable compiler flags or justifies otherwise.
> 
> Not true.
> https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags
-- Fixed

cmockery2-1.3.6-1.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/cmockery2-1.3.6-1.fc20

cmockery2-1.3.6-1.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/cmockery2-1.3.6-1.fc19

cmockery2-1.3.6-1.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/cmockery2-1.3.6-1.el6

cmockery2-1.3.6-1.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/cmockery2-1.3.6-1.el5

cmockery2-1.3.6-1.fc19 has been pushed to the Fedora 19 testing repository.

cmockery2-1.3.7-1.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/cmockery2-1.3.7-1.fc20

cmockery2-1.3.7-1.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/cmockery2-1.3.7-1.fc19

cmockery2-1.3.7-1.fc20 has been pushed to the Fedora 20 stable repository.

cmockery2-1.3.7-1.fc19 has been pushed to the Fedora 19 stable repository.