Bug 994152 (libprocess)

cross reference: https://issues.apache.org/jira/browse/MESOS-543

Issue in SPEC:

1. %changelog:

* Tue Jul 23 2013 Igor Gnatenko <i.gnatenko.brain> - 0.1.0-1.f0d47da
- Initial release

Then the version is 0.0.1-xxx, so please correct.

2. Drop these:

BuildRequires:  automake
BuildRequires:  gcc-c++

3. %description is too long.

4. Only ships %doc once, not twice.

update re: comment #2 can be found @:
https://github.com/ignatenkobrain/libprocess-rpm/blob/master/libprocess.spec

(In reply to Timothy St. Clair from comment #3)
> update re: comment #2 can be found @:
> https://github.com/ignatenkobrain/libprocess-rpm/blob/master/libprocess.spec

So where is the SRPM?

%files
%{_libdir}/libprocess.s*

Seems you've included development libraries?(.so)

And %{_datadir}/pkgconfig/* in the base package? That's suspicious. Should be %_libdir/pkgconfig. Please also check its contents (or even verify using pkg-config).

Probably:
https://github.com/ignatenkobrain/libprocess/blob/master/libprocess.pc.in

Should definitely be moved into the -devel package. And it contains a few copy'n'paste mistakes.

Not defining libdir in it is acceptable, but can backfire eventually, if someone evaluates the pkg config with common queries such as:

  $ pkg-config --variable=libdir libprocess

  $

Naturally, one could retrieve the libdir the library is stored in.

Updated: 
spec: http://tstclair.fedorapeople.org/mesos/libprocess/libprocess.spec
srpm: http://tstclair.fedorapeople.org/mesos/libprocess/libprocess-0.0.1-7.c2373fa.fc21.src.rpm

re comment #5 - it's in the proper location(s)
re comment #6 - There are 2 standard locations, I have no real preference, if it's a blocker I'll change
re comment #7 - fixed.

pkgconfig has two locations because this is related to BuildArch.

If you mark this package as noarch, put it under datadir.

*******

And what I told you in comment #5 is that:

*.so.* is used for runtime and *.so is used for development. You used glob to include all libs.

Package fails to build on f20:
...
src/process.cpp: In function 'void process::send_file(ev_loop*, ev_io*, int)':
src/process.cpp:1096:22: error: 'sendfile' is not a member of 'os'
     ssize_t length = os::sendfile(s, fd, offset, size);
...

Further issues:

* The compiler is invoked with broken arguments:
... -I.//include -I -I/src -I -I   ...

The -I.//include doesn't look cosher, but real issue here is the subsequent "-I without arguments":
# touch foo.cc
# gcc -o foo.o -c foo.cc -I
gcc: error: missing path after ‘-I’

You are only not seeing this error with this package, because these "-I"'s swallow their next argument (which happen to be further -I's):
# gcc -o foo.o -c foo.cc -I -I


* consider to append --disable-static to %configure
This avoids building the static libs and thus cuts down compilation times by ca. factor 2.

re comment #10: requires stout-0.1.2-2.c784a5e.fc20 which is in koji and has been recently been submitted to testing.

re comment #9: fixed in

spec: http://tstclair.fedorapeople.org/mesos/libprocess/libprocess.spec
srpm: http://tstclair.fedorapeople.org/mesos/libprocess/libprocess-0.0.1-9.bc412ff.fc21.src.rpm

I'll take.

Created attachment 795097 [details]
build.log - FTBFS on rawhide

FTBFS on rawhide !!!

Seems there are errors during build of testsuite...

(In reply to Timothy St. Clair from comment #11)
> re comment #10: requires stout-0.1.2-2.c784a5e.fc20 which is in koji and has
> been recently been submitted to testing.

From the build.log from comment#14:
... -I.//include -I -I/src -I -I    ...

=> compiler is still called incorrectly.

Could you please attach the test-suite.log file it contains the details on where the tests fail.  I've created a new rawhide image to verify and seems to fail around DNS issues.

(In reply to Timothy St. Clair from comment #16)
> Could you please attach the test-suite.log file it contains the details on
> where the tests fail.  I've created a new rawhide image to verify and seems
> to fail around DNS issues.

You are right testsuite fails, if build-host can't resolve it's name by DNS.  So you should append "||:" to the invocation of testsuite with the spec.  Koji-workers are not able to resolve their hostname by DNS, too.  ;)

Updated to disable tests that require DNS:

SRPM: http://tstclair.fedorapeople.org/mesos/libprocess/libprocess-0.0.1-10.4405209.fc21.src.rpm

SPEC: http://tstclair.fedorapeople.org/mesos/libprocess/libprocess.spec

rebuilt with same URL's and a very minor spec modification.

Package has severe issues.  :(

#####

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

Legend:
[x] = Pass
[!] = Fail
[-] = Not applicable
[?] = Not evaluated


Issues:
=======
- ldconfig called in %post and %postun if required.
  Note: /sbin/ldconfig not called in libprocess
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#Shared_Libraries
- strange stuff on cc invocation:  `-I.//include -I -I/src -I -I`

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

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.
[x]: Development (unversioned) .so files in -devel subpackage, if present.

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:
     "Unknown or generated". 63 files have unknown license. Detailed output of
     licensecheck in
     /home/besser82/shared/fedora/review/994152-libprocess/licensecheck.txt

     ---> License-tag is fine

[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.
[x]: 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

     --->  You want to build for el5, too?!?

[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[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 20480 bytes in 2 files.
[!]: Package complies to the Packaging Guidelines

     ---> SEVERE issues present.

[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]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Each %files section contains %defattr if rpm < 4.4
[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]: 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]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: The placement of pkgconfig(.pc) files are correct.
     Note: libprocess : /usr/lib64/pkgconfig/libprocess.pc
[-]: 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.

     ---> results get irgnoredd by "||:", because testsuite fails when
          there's no proper DNS-resolv.

[x]: Packages should try to preserve timestamps of original installed files.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[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]: Dist tag is present (not strictly required in GL).
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Fully versioned dependency in subpackages if applicable.
[x]: Uses parallel make %{?_smp_mflags} macro.
[x]: SourceX tarball generation or download is documented.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

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

Generic:
[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: libprocess-0.0.1-10.4405209.fc21.x86_64.rpm
          libprocess-devel-0.0.1-10.4405209.fc21.x86_64.rpm
          libprocess-0.0.1-10.4405209.fc21.src.rpm
libprocess.x86_64: E: library-without-ldconfig-postin /usr/lib64/libprocess.so.0.0.0
libprocess.x86_64: E: library-without-ldconfig-postun /usr/lib64/libprocess.so.0.0.0
libprocess.x86_64: W: devel-file-in-non-devel-package /usr/lib64/pkgconfig/libprocess.pc
libprocess-devel.x86_64: W: no-documentation
3 packages and 0 specfiles checked; 2 errors, 2 warnings.




Rpmlint (installed packages)
----------------------------
# rpmlint libprocess-devel libprocess
libprocess-devel.x86_64: W: no-documentation
libprocess.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libprocess.so.0.0.0 /lib64/libgtest.so.0
libprocess.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libprocess.so.0.0.0 /lib64/librt.so.1
libprocess.x86_64: E: library-without-ldconfig-postin /usr/lib64/libprocess.so.0.0.0
libprocess.x86_64: E: library-without-ldconfig-postun /usr/lib64/libprocess.so.0.0.0
libprocess.x86_64: W: devel-file-in-non-devel-package /usr/lib64/pkgconfig/libprocess.pc
2 packages and 0 specfiles checked; 2 errors, 4 warnings.
# echo 'rpmlint-done:'



Requires
--------
libprocess-devel (rpmlib, GLIBC filtered):
    libprocess(x86-64)
    libprocess.so.0()(64bit)

libprocess (rpmlib, GLIBC filtered):
    /usr/bin/pkg-config
    libc.so.6()(64bit)
    libev.so.4()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libgcc_s.so.1(GCC_4.0.0)(64bit)
    libglog.so.0()(64bit)
    libgtest.so.0()(64bit)
    libhttp_parser.so.2()(64bit)
    libm.so.6()(64bit)
    librt.so.1()(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libz.so.1()(64bit)
    rtld(GNU_HASH)



Provides
--------
libprocess-devel:
    libprocess-devel
    libprocess-devel(x86-64)

libprocess:
    libprocess
    libprocess(x86-64)
    libprocess.so.0()(64bit)
    pkgconfig(libprocess)



Source checksums
----------------
https://github.com/ignatenkobrain/libprocess/archive/4405209a4a9b4e6e094eaf16f1b8ea53899560d8/libprocess-0.0.1-4405209.tar.gz :
  CHECKSUM(SHA256) this package     : 9240360ad463313b7984abe94a150a43f18aaa4dfa6716a117cbd8421a95aea0
  CHECKSUM(SHA256) upstream package : 9240360ad463313b7984abe94a150a43f18aaa4dfa6716a117cbd8421a95aea0


Generated by fedora-review 0.5.0 (920221d) last change: 2013-08-30
Command line :/usr/bin/fedora-review -m fedora-rawhide-x86_64 -b 994152
Buildroot used: fedora-rawhide-x86_64
Active plugins: Generic, Shell-api, C/C++
Disabled plugins: Java, Python, SugarActivity, Perl, R, PHP, Ruby
Disabled flags: EPEL5, EXARCH, DISTTAG

#####

Please fix those issues and I'll take another spin.

Updated per review:

http://tstclair.fedorapeople.org/mesos/libprocess/libprocess.spec
http://tstclair.fedorapeople.org/mesos/libprocess/libprocess-0.0.1-11.d283d26.fc21.src.rpm

Notes from review: 
---------------------------------------
[!]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf %{buildroot} present but not required

*Fixed*
     --->  You want to build for el5, too?!?
*No*
---------------------------------------

rpmlint issues fixed.

FTBFS on rawhide:

Ausführung(%build): /bin/sh -e /var/tmp/rpm-tmp.cJr7q9
+ umask 022
+ cd /builddir/build/BUILD
+ cd libprocess-d283d260937c8e7fc3a5fb4ffdbdef9f21688a50
+ ./bootstrap
autoreconf: Entering directory `.'
autoreconf: configure.ac: not using Gettext
autoreconf: running: aclocal --warnings=all -I m4
autoreconf: configure.ac: tracing
configure.ac:28: warning: back quotes and double quotes must not be escaped in: unrecognized option: $[1]
configure.ac:28: Try \`$[0] --help' for more information.
aclocal.m4:625: LT_OUTPUT is expanded from...
configure.ac:28: the top level
configure.ac:28: warning: back quotes and double quotes must not be escaped in: unrecognized argument: $[1]
configure.ac:28: Try \`$[0] --help' for more information.
aclocal.m4:625: LT_OUTPUT is expanded from...
configure.ac:28: the top level
configure.ac:84: warning: The macro `AC_LANG_SAVE' is obsolete.
configure.ac:84: You should run autoupdate.
../../lib/autoconf/lang.m4:125: AC_LANG_SAVE is expanded from...
m4/acx_pthread.m4:63: ACX_PTHREAD is expanded from...
configure.ac:84: the top level
configure.ac:84: warning: The macro `AC_LANG_C' is obsolete.
configure.ac:84: You should run autoupdate.
../../lib/autoconf/c.m4:72: AC_LANG_C is expanded from...
m4/acx_pthread.m4:63: ACX_PTHREAD is expanded from...
configure.ac:84: the top level
configure.ac:84: warning: The macro `AC_TRY_LINK' is obsolete.
configure.ac:84: You should run autoupdate.
../../lib/autoconf/general.m4:2687: AC_TRY_LINK is expanded from...
m4/acx_pthread.m4:63: ACX_PTHREAD is expanded from...
configure.ac:84: the top level
configure.ac:84: warning: The macro `AC_LANG_RESTORE' is obsolete.
configure.ac:84: You should run autoupdate.
../../lib/autoconf/lang.m4:134: AC_LANG_RESTORE is expanded from...
m4/acx_pthread.m4:63: ACX_PTHREAD is expanded from...
configure.ac:84: the top level
autoreconf: running: libtoolize --copy
libtoolize: putting auxiliary files in `.'.
libtoolize: copying file `./ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIR, `m4'.
libtoolize: copying file `m4/libtool.m4'
libtoolize: copying file `m4/ltoptions.m4'
libtoolize: copying file `m4/ltsugar.m4'
libtoolize: copying file `m4/ltversion.m4'
libtoolize: copying file `m4/lt~obsolete.m4'
autoreconf: running: /usr/bin/autoconf --warnings=all
configure.ac:28: warning: back quotes and double quotes must not be escaped in: unrecognized option: $[1]
configure.ac:28: Try \`$[0] --help' for more information.
m4/libtool.m4:609: LT_OUTPUT is expanded from...
configure.ac:28: the top level
configure.ac:28: warning: back quotes and double quotes must not be escaped in: unrecognized argument: $[1]
configure.ac:28: Try \`$[0] --help' for more information.
m4/libtool.m4:609: LT_OUTPUT is expanded from...
configure.ac:28: the top level
configure.ac:84: warning: The macro `AC_LANG_SAVE' is obsolete.
configure.ac:84: You should run autoupdate.
../../lib/autoconf/lang.m4:125: AC_LANG_SAVE is expanded from...
m4/acx_pthread.m4:63: ACX_PTHREAD is expanded from...
configure.ac:84: the top level
configure.ac:84: warning: The macro `AC_LANG_C' is obsolete.
configure.ac:84: You should run autoupdate.
../../lib/autoconf/c.m4:72: AC_LANG_C is expanded from...
m4/acx_pthread.m4:63: ACX_PTHREAD is expanded from...
configure.ac:84: the top level
configure.ac:84: warning: The macro `AC_TRY_LINK' is obsolete.
configure.ac:84: You should run autoupdate.
../../lib/autoconf/general.m4:2687: AC_TRY_LINK is expanded from...
m4/acx_pthread.m4:63: ACX_PTHREAD is expanded from...
configure.ac:84: the top level
configure.ac:84: warning: The macro `AC_LANG_RESTORE' is obsolete.
configure.ac:84: You should run autoupdate.
../../lib/autoconf/lang.m4:134: AC_LANG_RESTORE is expanded from...
m4/acx_pthread.m4:63: ACX_PTHREAD is expanded from...
configure.ac:84: the top level
autoreconf: configure.ac: not using Autoheader
autoreconf: running: automake --add-missing --copy --no-force --warnings=all
configure.ac:21: installing './ar-lib'
configure.ac:21: installing './compile'
configure.ac:8: installing './config.guess'
configure.ac:8: installing './config.sub'
configure.ac:18: installing './install-sh'
configure.ac:18: installing './missing'
automake: warnings are treated as errors
Makefile.am:11: warning: source file 'src/latch.cpp' is in a subdirectory,
Makefile.am:11: but option 'subdir-objects' is disabled
automake: warning: possible forward-incompatibility.
automake: At least a source file is in a subdirectory, but the 'subdir-objects'
automake: automake option hasn't been enabled.  For now, the corresponding output
automake: object file(s) will be placed in the top-level directory.  However,
automake: this behaviour will change in future Automake versions: they will
automake: unconditionally cause object files to be placed in the same subdirectory
automake: of the corresponding sources.
automake: You are advised to start using 'subdir-objects' option throughout your
automake: project, to avoid future incompatibilities.
Makefile.am:11: warning: source file 'src/pid.cpp' is in a subdirectory,
Makefile.am:11: but option 'subdir-objects' is disabled
Makefile.am:11: warning: source file 'src/process.cpp' is in a subdirectory,
Makefile.am:11: but option 'subdir-objects' is disabled
Makefile.am:11: warning: source file 'src/statistics.cpp' is in a subdirectory,
Makefile.am:11: but option 'subdir-objects' is disabled
Makefile.am:51: warning: source file 'src/tests/decoder_tests.cpp' is in a subdirectory,
Makefile.am:51: but option 'subdir-objects' is disabled
Makefile.am:51: warning: source file 'src/tests/encoder_tests.cpp' is in a subdirectory,
Makefile.am:51: but option 'subdir-objects' is disabled
Makefile.am:51: warning: source file 'src/tests/http_tests.cpp' is in a subdirectory,
Makefile.am:51: but option 'subdir-objects' is disabled
Makefile.am:51: warning: source file 'src/tests/io_tests.cpp' is in a subdirectory,
Makefile.am:51: but option 'subdir-objects' is disabled
Makefile.am:51: warning: source file 'src/tests/main.cpp' is in a subdirectory,
Makefile.am:51: but option 'subdir-objects' is disabled
Makefile.am:51: warning: source file 'src/tests/process_tests.cpp' is in a subdirectory,
Makefile.am:51: but option 'subdir-objects' is disabled
Makefile.am:51: warning: source file 'src/tests/statistics_tests.cpp' is in a subdirectory,
Makefile.am:51: but option 'subdir-objects' is disabled
Makefile.am:51: warning: source file 'src/tests/time_tests.cpp' is in a subdirectory,
Makefile.am:51: but option 'subdir-objects' is disabled
Makefile.am:51: warning: source file '/usr/src/gmock/gmock-all.cc' is in a subdirectory,
Makefile.am:51: but option 'subdir-objects' is disabled
Makefile.am: installing './depcomp'
parallel-tests: installing './test-driver'
autoreconf: automake failed with exit status: 1
Fehler: Fehler-Status beim Beenden von /var/tmp/rpm-tmp.cJr7q9 (%build)
Fehler beim Bauen des RPM:
    Fehler-Status beim Beenden von /var/tmp/rpm-tmp.cJr7q9 (%build)
Child return code was: 1
EXCEPTION: Command failed. See logs for output.

#####

Please try to fix that.

There was a recent update to automake 1.14 which caused issues across several packages.  I'll post a more detailed update shortly.

(In reply to Timothy St. Clair from comment #23)
> There was a recent update to automake 1.14 which caused issues across
> several packages.  I'll post a more detailed update shortly.

You are just about to learn, why running the autotools in builts is dangerous and an unsafe project design.

After several conversations with upstream, they've decided to pivot and subsume both libprocess and stout as sub-packages within mesos.  

I'll be posting a updated spec and srpm shortly.