Bug 1509290

>Group: System Environment/Daemons
>BuildRoot: ...
The "Group:" and "BuildRoot:" tags should not be used.
https://fedoraproject.org/wiki/Packaging:Guidelines#Tags_and_Sections

>URL: https://github.com/otto-de/lib%{name}.git
I think it's preferred for the "URL:" tag to point to a website that can be visited and read by humans. (GitHub redirects the browser to the repo's web-view, but still.)

>%if 0%{?fedora} >= 17 || 0%{?rhel} >= 7
>%license LICENSE
>%else
>%doc LICENSE
>%endif
You can replace this with just "%license LICENSE"; builders for EPEL6 handle %licence just fine.

Also, I wonder - since this is an arched package, doesn't it require for the vmod and varnish to be the same architecture?
>Requires: varnish = %varnishver
I may be wrong, but AFAIK this will pass if any arch is installed - e.g it may be possible to install vmod-uuid.x86_64, and the dependency resolver will be satisfied by varnish.i686.

If this is the case, you should use:
>Requires: varnish%{?_isa} = %varnishver

Thank you, Artur

Updated specfile and source rpm

Spec URL: https://ingvar.fedorapeople.org/varnish/vmod-uuid.spec
SRPM URL: https://ingvar.fedorapeople.org/varnish/vmod-uuid-1.3-2.fc26.src.rpm

Ingvar

copr builds for varnish-5.2 on epel6 and epel7:

https://copr.fedorainfracloud.org/coprs/ingvar/varnish52/

 - Not sure why you grab a git commit instead of the release?
   Remove:

%global commit0 ec75dddf6c2fb634972962a3b8f10fd41c59bb61
%global shortcommit0 %(c=%{commit0}; echo ${c:0:7})
%global gitowner otto-de

   Then:

Source0: https://github.com/otto-de/libvmod-uuid/archive/v%{version}/%{name}-%{version}.tar.gz

   And:

%autosetup -n lib%{name}-%{version}

 - Remove all the unnecessary commented out lines:

#Source0: vmod-uuid-1.3.tar.gz
#Patch0: vmod-uuid-1.3.fix_python_el6.patch

#setup -q -n %{name}-%{version}
#if 0%{?rhel} == 6
#patch0 -p0
#else
#endif

# Clean buildroot on older el variants
#rm -rf #{buildroot}

 - make → %make_build

 - make install DESTDIR=%{buildroot} → %make_install

 - find %{buildroot}/%{_libdir}/ -name '*.la' -exec rm -f {} ';'  → find %{buildroot}/%{_libdir}/ -name '*.la' -delete 
find %{buildroot}/%{_libdir}/ -name  '*.a' -exec rm -f {} ';' → find %{buildroot}/%{_libdir}/ -name  '*.a' -delete

 - Not needed:

%clean
rm -rf %{buildroot}

 - Not needed:

%defattr(-,root,root,-)

 - Not useful:

echo Upstream: https://github.com/%{gitowner}/lib%{name}/archive/%{commit0}/lib%{name}-%{shortcommit0}.tar.gz

 - These versions are not available on Rawhide

%global varnishver 5.2.0
%global vabi 6.1
%global vabistrict 4c4875cbf

Thus the build fails. Should be:

%global varnishver 5.2.1
%global vabi 6.1
%global vabistrict 67e562482

Though, isn't there a way to relax this to be able to build on future versions?

Thank you, Robert-André

Updated specfile and srpm:

Spec URL: https://ingvar.fedorapeople.org/varnish/vmod-uuid.spec
SRPM URL: https://ingvar.fedorapeople.org/varnish/vmod-uuid-1.3-3.fc26.src.rpm

Ingvar

 - Not needed in %files:

%defattr(-,root,root,-)

 - Use a more meaninqful name for your archive, with:

Source0: https://github.com/otto-de/lib%{name}/archive/v%{version}/%{name}-%{version}.tar.gz

 - This construct:

Requires: varnish >= %(pkg-config --modversion varnishapi)

   cannot work. varnish-devel is not installed when the SPEC is evaluated, so pkg-config returns an error:

Package varnishapi was not found in the pkg-config search path.
Perhaps you should add the directory containing `varnishapi.pc'
to the PKG_CONFIG_PATH environment variable
Package 'varnishapi', required by 'virtual:world', not found
error : line 9 : Required version : Requires: varnish >=

 - I found a way to make it work, following the example of the firefox package:

%global varnishapi_version %(pkg-config  --silence-errors --modversion varnishapi 2>/dev/null || echo 65536)

   And:

Requires: varnish >= %varnishapi_version


You'll get the correct Requires for your package in the end:

$ rpm -qp --requires vmod-uuid-1.3-3.fc28.x86_64.rpm   | sort | uniq -c
      1 libc.so.6()(64bit)
      1 libc.so.6(GLIBC_2.2.5)(64bit)
      1 libc.so.6(GLIBC_2.4)(64bit)
      1 libossp-uuid.so.16()(64bit)
      1 libpthread.so.0()(64bit)
      1 libvarnishapi.so.1()(64bit)
      1 libvarnishapi.so.1(LIBVARNISHAPI_2.0)(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)
      1 uuid
      1 varnish >= 5.2.1

Thanks again, Robert. I also figured a similar pkg-config hack. The extra macro is superflous, as we only use this value once, so the following should be enough:

Requires: varnish = %(pkg-config --silence-errors --modversion varnishapi || echo 0)

Updated specfile and srpm (I did not bump the release for this change):

Spec URL: https://ingvar.fedorapeople.org/varnish/vmod-uuid.spec
SRPM URL: https://ingvar.fedorapeople.org/varnish/vmod-uuid-1.3-3.fc26.src.rpm

An obsolete macro has been found in configure.ac:

[!]: Package should not use obsolete m4 macros
     Note: Some obsoleted macros found, see the attachment.

AutoTools: Obsoleted m4s found
------------------------------
  AM_CONFIG_HEADER found in: libvmod-uuid-1.3/configure.ac:7

You need to patch it out:


diff -up libvmod-uuid-1.3/configure.ac.fix_obsolete_m4s libvmod-uuid-1.3/configure.ac
--- libvmod-uuid-1.3/configure.ac.fix_obsolete_m4s	2017-10-30 19:05:03.000000000 +0100
+++ libvmod-uuid-1.3/configure.ac	2017-12-08 15:35:39.318045033 +0100
@@ -4,7 +4,7 @@ AC_INIT([libvmod-uuid], [1.3], [], [vmod
 AC_CONFIG_MACRO_DIR([m4])
 AC_CONFIG_SRCDIR(src/vmod_uuid.vcc)
 AC_CONFIG_AUX_DIR([build-aux])
-AM_CONFIG_HEADER(config.h)
+AC_CONFIG_HEADERS(config.h)
 
 AC_CANONICAL_SYSTEM
 AC_LANG(C)


 - I renew my recommendation to use a better name for you archive with:

Source0: https://github.com/otto-de/lib%{name}/archive/v%{version}/%{name}-%{version}.tar.gz


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

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


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

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Development (unversioned) .so files in -devel subpackage, if present.
     Note: Unversioned so-files in private %_libdir subdirectory (see
     attachment). Verify they are not in ld path.
[x]: Package does not contain any libtool archives (.la)
[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: Checking patched sources after %prep for licenses. Licenses
     found: "*No copyright* BSD (2 clause)", "BSD (2 clause)", "Apache
     (v2.0)", "Unknown or generated", "*No copyright* BSD (unspecified)".
     16 files have unknown license. Detailed output of licensecheck in
     /home/bob/packaging/review/vmod-uuid/review-vmod-uuid/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[x]: Package does not own files or directories owned by other packages.
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: 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 10240 bytes in 2 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]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[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 does not use a name that already exists.
[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]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

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

Generic:
[-]: 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).
[x]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in vmod-
     uuid-debuginfo , vmod-uuid-debugsource
[?]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: 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]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[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 debuginfo package(s).
     Note: No rpmlint messages.
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: 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: vmod-uuid-1.3-3.fc28.x86_64.rpm
          vmod-uuid-debuginfo-1.3-3.fc28.x86_64.rpm
          vmod-uuid-debugsource-1.3-3.fc28.x86_64.rpm
          vmod-uuid-1.3-3.fc28.src.rpm
vmod-uuid-debugsource.x86_64: W: no-documentation
4 packages and 0 specfiles checked; 0 errors, 1 warnings.

AutoTools: Obsoleted m4s found
------------------------------
  AM_CONFIG_HEADER found in: libvmod-uuid-1.3/configure.ac:7

Thanks again

Updated specfile and srpm:

Spec URL: https://ingvar.fedorapeople.org/varnish/vmod-uuid.spec
SRPM URL: https://ingvar.fedorapeople.org/varnish/vmod-uuid-1.3-4.fc26.src.rpm

Note the license change. The correct license for the actual source code is Apache 2.0.

I don't know what to do about missing docs in the debugsource package. It is automatically built.

License here is BSD: https://github.com/otto-de/libvmod-uuid/blob/master/LICENSE

If this license is incorrect, you shouldn't install the file and instead ask upstream to clarify and provide a correct license file.

It seems the license was changed 4 years ago: https://github.com/Sharecare/libvmod-uuid/commit/d3d8a6b4aa7b068eb0fa5d6390634983eaea91a8

I believe the correct license is BSD, the guy who changed the license is/was an employee of the Sharecare company mentionned in the Apache licensing header. They just didn't bother to update the header.

I added a github issue to the original branch, https://github.com/Sharecare/libvmod-uuid/issues/8 , and have asked my upstream at https://github.com/otto-de/libvmod-uuid to look into it.

By the way, the original Sharecare branch seems quite dead, which is why I use the otto-de fork. It is actively maintained.

Updated specfile and srpm:

Spec URL: https://ingvar.fedorapeople.org/varnish/vmod-uuid.spec
SRPM URL: https://ingvar.fedorapeople.org/varnish/vmod-uuid-1.3-4.fc26.src.rpm

(I did not update the release number for this change)

Ingvar

 - Patch should be numbered:

Patch0: vmod-uuid-1.3.fix_obsolete_m4_macro.patch

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

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


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

C/C++:
[ ]: Package does not contain kernel modules.
[ ]: Package contains no static executables.
[ ]: Development (unversioned) .so files in -devel subpackage, if present.
     Note: Unversioned so-files in private %_libdir subdirectory (see
     attachment). Verify they are not in ld path.
[x]: Package does not contain any libtool archives (.la)
[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: Checking patched sources after %prep for licenses. Licenses
     found: "*No copyright* BSD (2 clause)", "BSD (2 clause)", "Apache
     (v2.0)", "Unknown or generated", "*No copyright* BSD (unspecified)".
     16 files have unknown license. Detailed output of licensecheck in
     /home/bob/packaging/review/vmod-uuid/review-vmod-uuid/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[x]: Package does not own files or directories owned by other packages.
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: 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 10240 bytes in 2 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]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[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 does not use a name that already exists.
[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]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

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

Generic:
[-]: 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 vmod-
     uuid-debuginfo , vmod-uuid-debugsource
[?]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Patches link to upstream bugs/comments/lists or are otherwise
     justified.
[-]: 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]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

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

Generic:
[x]: Rpmlint is run on debuginfo package(s).
     Note: No rpmlint messages.
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.
[x]: Package should not use obsolete m4 macros
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: vmod-uuid-1.3-4.fc28.x86_64.rpm
          vmod-uuid-debuginfo-1.3-4.fc28.x86_64.rpm
          vmod-uuid-debugsource-1.3-4.fc28.x86_64.rpm
          vmod-uuid-1.3-4.fc28.src.rpm
vmod-uuid-debugsource.x86_64: W: no-documentation
4 packages and 0 specfiles checked; 0 errors, 1 warnings.

(In reply to Robert-André Mauchin from comment #15)
>  - Patch should be numbered:
> 
> Patch0: vmod-uuid-1.3.fix_obsolete_m4_macro.patch

It has number 0. Should that be reflected in the patch file name too? Or do you mean an upstream bug number? I added an upstream issue on that on the Sharecare repo, https://github.com/Sharecare/libvmod-uuid/issues/9,  but as that branch seems dead, I'll also notify the maintainer of the otto-de branch.

> vmod-uuid-debugsource.x86_64: W: no-documentation

I still don't know what to do about this.

Ingvar

> It has number 0.

There is no number in the SPEC you last posted.

>> vmod-uuid-debugsource.x86_64: W: no-documentation
>
>I still don't know what to do about this.

Disregard this, fedora-review doesn't play well with the new split debug packages.

(In reply to Robert-André Mauchin from comment #17)
> There is no number in the SPEC you last posted.

Ah, that was just a typo. Fixed.


Updated specfile and srpm:

Spec URL: https://ingvar.fedorapeople.org/varnish/vmod-uuid.spec
SRPM URL: https://ingvar.fedorapeople.org/varnish/vmod-uuid-1.3-4.fc26.src.rpm

(I did not update the release number for this change either.)

Ingvar

Package approuved.

Package was approved and added to Rawhide 2017-12-14.