Bug 1208695 - Review Request: liberasurecode - Erasure Code API library written in C with pluggable backends
Summary: Review Request: liberasurecode - Erasure Code API library written in C with p...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Petr Pisar
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 1212148
TreeView+ depends on / blocked
 
Reported: 2015-04-02 21:32 UTC by Pete Zaitcev
Modified: 2015-12-04 10:49 UTC (History)
10 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-11-23 22:57:22 UTC
Type: ---
ppisar: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Pete Zaitcev 2015-04-02 21:32:01 UTC
Spec URL: http://people.redhat.com/zaitcev/tmp/liberasurecode-1.0.5-1.spec
SRPM URL: http://people.redhat.com/zaitcev/tmp/liberasurecode-1.0.5-1.fc21.src.rpm
Description:
An API library for Erasure Code, written in C. It provides a number
of pluggable backends, such as Intel ISA-L library.
Fedora Account System Username: zaitcev

Comment 1 Petr Pisar 2015-04-10 15:13:07 UTC
URL is usable. Ok.

TODO: You can remove trailing `overview' from the URL tag. Even you can replace the `liberasurecode' word with `%{name}' macro.

TODO: Use real source URL <https://bitbucket.org/tsg-/liberasurecode/get/v1.0.5.tar.bz2>. Again you can replace parts with macros.

Source archive is original (SHA-256: 34327a4ababc1b11e4841cd535da073157193cfbe4ef7568d7f46977986cf1e0). Ok.
Patches are good.
Summary verified from REAMDE.md. Ok.
Description is good. Ok.

FIX: Add `Public License' to license tag (src/utils/chksum/md5.c). If you think the `heavily cut-down "BSD license"' is applies, you should add `(Public License or Public Domain)'.

FIX: Add `Copyright only' to license tag (src/utils/chksum/crc32.c).

TODO: Document by a spec comment that a following licences cover source package:
GPLv2+ (missing)
FSFULLR (m4/lt~obsolete.m4)
FSFUL (m4/libtool.m4)
GPLv3+ (m4/ax_gcc_x86_cpuid.m4)

TODO: If called `libtoolize --force --install', you would not have to edit the libtool in %build section.

TODO: Execute tests.

FIX: Package COPYING file using `%license' macro.

FIX: The `tools' sub-package has no %files section.

FIX: The `devel' sub-package must run-require exact release of the package with shared libraries. Then you don't have to duplicate COPYING and README.md.

FIX: The build script ignores Fedora's CFLAGS. (I cannot see -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong etc. among gcc arguments).

FIX: The sources does not build in F23 (http://koji.fedoraproject.org/koji/taskinfo?taskID=9456796):

libtool: link: gcc -O2 -g -Werror -D_GNU_SOURCE=1 -Wall -pedantic -std=c99 -Wl,-z -Wl,relro -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -o alg_sig_test utils/chksum/alg_sig_test-test_alg_sig.o  ../src/.libs/liberasurecode.so /home/test/rpmbuild/BUILD/tsg--liberasurecode-4e1290ea61e5/src/builtin/xor_codes/.libs/libXorcode.so -lpthread -lm -ldl
/bin/ld: utils/chksum/alg_sig_test-test_alg_sig.o: relocation R_X86_64_32 against `.rodata.str1.1' can not be used when making a shared object; recompile with -fPIC
utils/chksum/alg_sig_test-test_alg_sig.o: error adding symbols: Bad value
collect2: error: ld returned 1 exit status
Makefile:423: recipe for target 'alg_sig_test' failed

This is because not all CFLAGS are passed to the compiler.

Please correct all `FIX' items, consider fixing `TODO' items, and provide new spec file.

Comment 2 Pete Zaitcev 2015-04-16 00:00:27 UTC
Updating for 1.0.7 and addressing some of Petr's concerns (but not all!).
Will fix the CFLAGS and licensing later.

Spec URL: http://people.redhat.com/zaitcev/tmp/liberasurecode-1.0.7-1.spec
SRPM URL: http://people.redhat.com/zaitcev/tmp/liberasurecode-1.0.7-1.fc21.src.rpm
Description:
An API library for Erasure Code, written in C. It provides a number
of pluggable backends, such as Intel ISA-L library.
Fedora Account System Username: zaitcev

Comment 3 Alan Pevec (Fedora) 2015-04-17 10:36:59 UTC
* Fedora fails linking in tests (already mentioned in comment 1)
libtool: link: gcc -O2 -g -Werror -D_GNU_SOURCE=1 -Wall -pedantic -std=c99 -Wl,-z -Wl,relro -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -o test_xor_hd_code builtin/xor_codes/test_xor_hd_code-test_xor_hd_code.o  ../src/.libs/liberasurecode.so /builddir/build/BUILD/liberasurecode-1.0.7/src/builtin/xor_codes/.libs/libXorcode.so -lpthread -lm ../src/builtin/xor_codes/.libs/libXorcode.so -ldl
/usr/bin/ld: builtin/xor_codes/test_xor_hd_code-test_xor_hd_code.o: relocation R_X86_64_32 against `.rodata.str1.8' can not be used when making a shared object; recompile with -fPIC
builtin/xor_codes/test_xor_hd_code-test_xor_hd_code.o: error adding symbols: Bad value
collect2: error: ld returned 1 exit status
Makefile:444: recipe for target 'test_xor_hd_code' failed
make[1]: *** [test_xor_hd_code] Error 1

* on EL7 linking worked but fails with:
RPM build errors:
    File not found by glob: /builddir/build/BUILDROOT/liberasurecode-1.0.7-1.el7.centos.x86_64/usr/share/doc/liberasurecode/html/*

Comment 4 Alan Pevec (Fedora) 2015-04-17 11:37:25 UTC
> FIX: The sources does not build in F23
> (http://koji.fedoraproject.org/koji/taskinfo?taskID=9456796):

Mindless quickfix which seems to have worked:
-%configure --disable-static
+%configure --disable-static CPPFLAGS="$CFLAGS"

http://koji.fedoraproject.org/koji/taskinfo?taskID=9497169

Comment 5 Pete Zaitcev 2015-04-18 04:08:24 UTC
Instead of using the CPPFLAGS trick that Alan showed in comment #4,
I chose to apply a patch to configure.am. I'm going to submit it
upstream.

Licensing is not fixed yet.

Spec URL: http://people.redhat.com/zaitcev/tmp/liberasurecode-1.0.7-2.spec
SRPM URL: http://people.redhat.com/zaitcev/tmp/liberasurecode-1.0.7-2.fc21.src.rpm
Description:
An API library for Erasure Code, written in C. It provides a number
of pluggable backends, such as Intel ISA-L library.

Fedora Account System Username: zaitcev

Comment 7 Pete Zaitcev 2015-08-01 04:33:00 UTC
All fixed up. Upstream accepted the CFLAGS thing and flushed the GPL-ed
macro from m4/.

Spec URL: http://people.redhat.com/zaitcev/tmp/liberasurecode-1.0.8-1.spec
SRPM URL: http://people.redhat.com/zaitcev/tmp/liberasurecode-1.0.8-1.fc24.src.rpm
Description:
An API library for Erasure Code, written in C. It provides a number
of pluggable backends, such as Intel ISA-L library.

Fedora Account System Username: zaitcev

Comment 8 Petr Pisar 2015-08-03 12:23:25 UTC
This is a rebase. Complete review follows.

URL is Ok.
Source archive is original (SHA-256: c12cf5a0b181f1acc113a0f2e56a089685f63f5c9a18668b2057d7f6413b00bc).

FIX: src/utils/chksum/md5.c is not BSD-licensed:

 * This software was written by Alexander Peslyak in 2001.  No copyright is
 * claimed, and the software is hereby placed in the public domain.
 * In case this attempt to disclaim copyright and place the software in the
 * public domain is deemed null and void, then the software is
 * Copyright (c) 2001 Alexander Peslyak and it is hereby released to the
 * general public under the following terms:
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted.
 *
 * There's ABSOLUTELY NO WARRANTY, express or implied.

Obtain the license identifier from Fedora legal department and add the identifier to the License tag.

FIX: src/utils/chksum/crc32.c is not BSD-licensed:

 *  COPYRIGHT (C) 1986 Gary S. Brown.  You may use this program, or
 *  code or tables extracted from it, as desired without restriction.

Obtain the license identifier from Fedora legal department and add the identifier to the License tag.

Once you resolve these licensing issues, I will resume with the review.

Comment 9 Haïkel Guémar 2015-08-03 12:59:15 UTC
AFAIK, these pieces of code are commonly reused in many FOSS projects.
* md5.c is shipped by many packages: postgresql, cyrus-sasl, kernel, llvm
Its licensing is liberal enough to be absorbed by liberasure license BSD 2 clauses
* crc32.c is shipped in openssh package (and *BSD kernels), though openssh has dropped the mention of being a derivative work from Gary S. Brown, but you can diff the code, it's the same.


One could wait Fedora Legal dept, but the answer is likely to be: go ahead with BSD :)

Comment 10 Tom "spot" Callaway 2015-08-03 18:12:27 UTC
The md5 sum license is functionally BSD, albeit, a super minimal variant.

The CRC32 license is Free and GPL compatible, added to the list as "CRC32".

Lifting FE-Legal here, as all of these licenses are compatible.

Comment 11 Haïkel Guémar 2015-08-28 09:09:11 UTC
Any update on this review?

Comment 12 Petr Pisar 2015-08-28 11:02:56 UTC
I wait on the review submitter to provide updated package with correct License value.

Comment 13 Alan Pevec (Fedora) 2015-09-09 19:42:56 UTC
Pete, please update the package according to the previous comments. Thanks!

Comment 14 Pete Zaitcev 2015-09-11 23:15:24 UTC
I left the License: to be BSD, primarily because comment #10 allows
me to do that. I studied the wiki/Packaging:LicensingGuidelines#License:_field,
and permitted syntax with "and" and "or" does not seem to match our case,
where md5.c is subsumed into the distribution under the BSD license.

Note that this is the 1.0.9 upstream release, fresh out of the oven.

Spec URL: http://people.redhat.com/zaitcev/tmp/liberasurecode-1.0.9-1.spec
SRPM URL: http://people.redhat.com/zaitcev/tmp/liberasurecode-1.0.9-1.fc24.src.rpm
Description:
An API library for Erasure Code, written in C. It provides a number
of pluggable backends, such as Intel ISA-L library.

Fedora Account System Username: zaitcev

Comment 15 Petr Pisar 2015-09-14 09:18:02 UTC
This is a rebase. New review follows.

URL is usable. Ok.
The Source0 URL is a snapshot. Ok.

Source archive is original. liberasurecode-1.0.9.tar.gz (SHA-256: 8bca01b6abfd6dd470b910eee84715e16083c2096c851d500af62602078d6ba7) content matches v1.0.9 tag snapshot < https://bitbucket.org/tsg-/liberasurecode/get/v1.0.9.tar.gz> (SHA-256: 0ea167cfa6e6d48ba1955bf501ca3b68e040ae347bc47920fca084b42ee8df9d). Ok.
liberasurecode-1.0.5-docs.patch patch is Ok.

src/utils/chksum/md5.c has BSD-like license.
src/utils/chksum/crc32.c has CRC32 license.
Other files has BSD license.
Common license conditions meet BSD license. Ok.

TODO: Fix the license comment:
> # The src/utils/chksum/md5.c is under CRC32 license
src/utils/chksum/crc32.c should be there.

TODO: Add `CRC32' identifier to the License tag.

FIX: Build-require `sed' (liberasurecode.spec:47).
FIX: Build-require `make' (liberasurecode.spec:49).
FIX: Build-require `findutils' (liberasurecode.spec:56).
FIX: Build-require `coreutils' (liberasurecode.spec:56).
FIX: Build-require `gcc' for including standard library header files.

TODO: Run tests.

TODO: Package AUTHORS and ChangeLog files.

$ rpmlint liberasurecode.spec ../SRPMS/liberasurecode-1.0.9-1.fc24.src.rpm ../RPMS/x86_64/liberasurecode-*
liberasurecode.spec: W: invalid-url Source0: liberasurecode-1.0.9.tar.gz
liberasurecode.src: W: spelling-error Summary(en_US) pluggable -> plug gable, plug-gable, plugged
liberasurecode.src: W: spelling-error Summary(en_US) backends -> back ends, back-ends, backhands
liberasurecode.src: W: spelling-error %description -l en_US pluggable -> plug gable, plug-gable, plugged
liberasurecode.src: W: spelling-error %description -l en_US backends -> back ends, back-ends, backhands
liberasurecode.src: W: invalid-url Source0: liberasurecode-1.0.9.tar.gz
liberasurecode.x86_64: W: spelling-error Summary(en_US) pluggable -> plug gable, plug-gable, plugged
liberasurecode.x86_64: W: spelling-error Summary(en_US) backends -> back ends, back-ends, backhands
liberasurecode.x86_64: W: spelling-error %description -l en_US pluggable -> plug gable, plug-gable, plugged
liberasurecode.x86_64: W: spelling-error %description -l en_US backends -> back ends, back-ends, backhands
liberasurecode-devel.x86_64: W: only-non-binary-in-usr-lib
liberasurecode-devel.x86_64: W: no-documentation
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/tabs.css
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/annotated.html
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/structec__args.html
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/doxygen.png
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/open.png
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/folderopen.png
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/doc.png
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/structec__backend__op__stubs.html
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/bc_s.png
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/nav_g.png
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/dir_85e1485977b1b5c7656625e6aef9fae5.html
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/nav_f.png
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/tab_s.png
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/closed.png
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/functions_vars.html
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/functions.html
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/arrowdown.png
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/splitbar.png
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/structec__backend__common.html
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/sync_on.png
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/jquery.js
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/dynsections.js
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/tab_b.png
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/tab_h.png
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/bdwn.png
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/nav_h.png
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/dir_d44c64559bbebec7f509842c48db8b23.html
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/folderclosed.png
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/index.html
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/classes.html
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/structec__backend__desc.html
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/arrowright.png
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/structec__backend.html
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/sync_off.png
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/structec__backend__args.html
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/doxygen.css
liberasurecode-doc.x86_64: W: spurious-executable-perm /usr/share/doc/liberasurecode/html/tab_a.png
5 packages and 1 specfiles checked; 0 errors, 49 warnings.

FIX: Remove executable bits from the liberasurecode-doc files.

$ rpm -q -lv -p  ../RPMS/x86_64/liberasurecode-1.0.9-1.fc24.x86_64.rpm 
lrwxrwxrwx    1 root    root                       19 Sep 14 10:55 /usr/lib64/libXorcode.so.1 -> libXorcode.so.1.0.1
-rwxr-xr-x    1 root    root                    32032 Sep 14 10:55 /usr/lib64/libXorcode.so.1.0.1
lrwxrwxrwx    1 root    root                       23 Sep 14 10:55 /usr/lib64/liberasurecode.so.1 -> liberasurecode.so.1.0.9
-rwxr-xr-x    1 root    root                    87024 Sep 14 10:55 /usr/lib64/liberasurecode.so.1.0.9
lrwxrwxrwx    1 root    root                       31 Sep 14 10:55 /usr/lib64/liberasurecode_rs_vand.so.1 -> liberasurecode_rs_vand.so.1.0.1
-rwxr-xr-x    1 root    root                    19112 Sep 14 10:55 /usr/lib64/liberasurecode_rs_vand.so.1.0.1
lrwxrwxrwx    1 root    root                       20 Sep 14 10:55 /usr/lib64/libnullcode.so.1 -> libnullcode.so.1.0.1
-rwxr-xr-x    1 root    root                     6712 Sep 14 10:55 /usr/lib64/libnullcode.so.1.0.1
drwxr-xr-x    2 root    root                        0 Sep 14 10:55 /usr/share/doc/liberasurecode
-rw-r--r--    1 root    root                    14304 Sep  6 08:26 /usr/share/doc/liberasurecode/README.md
drwxr-xr-x    2 root    root                        0 Sep 14 10:55 /usr/share/licenses/liberasurecode
-rw-r--r--    1 root    root                     1377 Sep  6 08:26 /usr/share/licenses/liberasurecode/COPYING

$ rpm -q -lv -p  ../RPMS/x86_64/liberasurecode-devel-1.0.9-1.fc24.x86_64.rpm 
drwxr-xr-x    2 root    root                        0 Sep 14 10:55 /usr/include/liberasurecode
-rw-r--r--    1 root    root                     2343 Sep 14 10:55 /usr/include/liberasurecode/alg_sig.h
-rw-r--r--    1 root    root                     4122 Sep 14 10:55 /usr/include/liberasurecode/config_liberasurecode.h
-rw-r--r--    1 root    root                    13521 Sep 14 10:55 /usr/include/liberasurecode/erasurecode.h
-rw-r--r--    1 root    root                     6525 Sep 14 10:55 /usr/include/liberasurecode/erasurecode_backend.h
-rw-r--r--    1 root    root                     5395 Sep 14 10:55 /usr/include/liberasurecode/erasurecode_helpers.h
-rw-r--r--    1 root    root                     2159 Sep 14 10:55 /usr/include/liberasurecode/erasurecode_log.h
-rw-r--r--    1 root    root                     1821 Sep 14 10:55 /usr/include/liberasurecode/erasurecode_postprocessing.h
-rw-r--r--    1 root    root                     2244 Sep 14 10:55 /usr/include/liberasurecode/erasurecode_preprocessing.h
-rw-r--r--    1 root    root                     4397 Sep 14 10:55 /usr/include/liberasurecode/erasurecode_stdinc.h
-rw-r--r--    1 root    root                     1581 Sep 14 10:55 /usr/include/liberasurecode/erasurecode_version.h
-rw-r--r--    1 root    root                     2551 Sep 14 10:55 /usr/include/liberasurecode/liberasurecode_rs_vand.h
-rw-r--r--    1 root    root                     5301 Sep 14 10:55 /usr/include/liberasurecode/list.h
-rw-r--r--    1 root    root                     2203 Sep 14 10:55 /usr/include/liberasurecode/rs_galois.h
-rw-r--r--    1 root    root                     4072 Sep 14 10:55 /usr/include/liberasurecode/xor_code.h
-rw-r--r--    1 root    root                    10931 Sep 14 10:55 /usr/include/liberasurecode/xor_hd_code_defs.h
lrwxrwxrwx    1 root    root                       19 Sep 14 10:55 /usr/lib64/libXorcode.so -> libXorcode.so.1.0.1
lrwxrwxrwx    1 root    root                       23 Sep 14 10:55 /usr/lib64/liberasurecode.so -> liberasurecode.so.1.0.9
lrwxrwxrwx    1 root    root                       31 Sep 14 10:55 /usr/lib64/liberasurecode_rs_vand.so -> liberasurecode_rs_vand.so.1.0.1
lrwxrwxrwx    1 root    root                       20 Sep 14 10:55 /usr/lib64/libnullcode.so -> libnullcode.so.1.0.1

$ rpm -q -lv -p  ../RPMS/x86_64/liberasurecode-doc-1.0.9-1.fc24.x86_64.rpm 
-rwxr-xr-x    1 root    root                     5702 Sep 14 10:55 /usr/share/doc/liberasurecode/html/annotated.html
-rwxr-xr-x    1 root    root                      246 Sep 14 10:55 /usr/share/doc/liberasurecode/html/arrowdown.png
-rwxr-xr-x    1 root    root                      229 Sep 14 10:55 /usr/share/doc/liberasurecode/html/arrowright.png
-rwxr-xr-x    1 root    root                      676 Sep 14 10:55 /usr/share/doc/liberasurecode/html/bc_s.png
-rwxr-xr-x    1 root    root                      147 Sep 14 10:55 /usr/share/doc/liberasurecode/html/bdwn.png
-rwxr-xr-x    1 root    root                     5049 Sep 14 10:55 /usr/share/doc/liberasurecode/html/classes.html
-rwxr-xr-x    1 root    root                      132 Sep 14 10:55 /usr/share/doc/liberasurecode/html/closed.png
-rwxr-xr-x    1 root    root                     4441 Sep 14 10:55 /usr/share/doc/liberasurecode/html/dir_85e1485977b1b5c7656625e6aef9fae5.html
-rwxr-xr-x    1 root    root                     4155 Sep 14 10:55 /usr/share/doc/liberasurecode/html/dir_d44c64559bbebec7f509842c48db8b23.html
-rwxr-xr-x    1 root    root                      746 Sep 14 10:55 /usr/share/doc/liberasurecode/html/doc.png
-rwxr-xr-x    1 root    root                    25495 Sep 14 10:55 /usr/share/doc/liberasurecode/html/doxygen.css
-rwxr-xr-x    1 root    root                     3779 Sep 14 10:55 /usr/share/doc/liberasurecode/html/doxygen.png
-rwxr-xr-x    1 root    root                     3140 Sep 14 10:55 /usr/share/doc/liberasurecode/html/dynsections.js
-rwxr-xr-x    1 root    root                      616 Sep 14 10:55 /usr/share/doc/liberasurecode/html/folderclosed.png
-rwxr-xr-x    1 root    root                      597 Sep 14 10:55 /usr/share/doc/liberasurecode/html/folderopen.png
-rwxr-xr-x    1 root    root                     4335 Sep 14 10:55 /usr/share/doc/liberasurecode/html/functions.html
-rwxr-xr-x    1 root    root                     4207 Sep 14 10:55 /usr/share/doc/liberasurecode/html/functions_vars.html
-rwxr-xr-x    1 root    root                     3514 Sep 14 10:55 /usr/share/doc/liberasurecode/html/index.html
-rwxr-xr-x    1 root    root                   146338 Sep 14 10:55 /usr/share/doc/liberasurecode/html/jquery.js
-rwxr-xr-x    1 root    root                      153 Sep 14 10:55 /usr/share/doc/liberasurecode/html/nav_f.png
-rwxr-xr-x    1 root    root                       95 Sep 14 10:55 /usr/share/doc/liberasurecode/html/nav_g.png
-rwxr-xr-x    1 root    root                       98 Sep 14 10:55 /usr/share/doc/liberasurecode/html/nav_h.png
-rwxr-xr-x    1 root    root                      123 Sep 14 10:55 /usr/share/doc/liberasurecode/html/open.png
-rwxr-xr-x    1 root    root                      314 Sep 14 10:55 /usr/share/doc/liberasurecode/html/splitbar.png
-rwxr-xr-x    1 root    root                     5185 Sep 14 10:55 /usr/share/doc/liberasurecode/html/structec__args.html
-rwxr-xr-x    1 root    root                     3967 Sep 14 10:55 /usr/share/doc/liberasurecode/html/structec__backend.html
-rwxr-xr-x    1 root    root                     3977 Sep 14 10:55 /usr/share/doc/liberasurecode/html/structec__backend__args.html
-rwxr-xr-x    1 root    root                     3981 Sep 14 10:55 /usr/share/doc/liberasurecode/html/structec__backend__common.html
-rwxr-xr-x    1 root    root                     3977 Sep 14 10:55 /usr/share/doc/liberasurecode/html/structec__backend__desc.html
-rwxr-xr-x    1 root    root                     4933 Sep 14 10:55 /usr/share/doc/liberasurecode/html/structec__backend__op__stubs.html
-rwxr-xr-x    1 root    root                      853 Sep 14 10:55 /usr/share/doc/liberasurecode/html/sync_off.png
-rwxr-xr-x    1 root    root                      845 Sep 14 10:55 /usr/share/doc/liberasurecode/html/sync_on.png
-rwxr-xr-x    1 root    root                      142 Sep 14 10:55 /usr/share/doc/liberasurecode/html/tab_a.png
-rwxr-xr-x    1 root    root                      169 Sep 14 10:55 /usr/share/doc/liberasurecode/html/tab_b.png
-rwxr-xr-x    1 root    root                      177 Sep 14 10:55 /usr/share/doc/liberasurecode/html/tab_h.png
-rwxr-xr-x    1 root    root                      184 Sep 14 10:55 /usr/share/doc/liberasurecode/html/tab_s.png
-rwxr-xr-x    1 root    root                     1163 Sep 14 10:55 /usr/share/doc/liberasurecode/html/tabs.css

File layout is Ok.

FIX: Remove executable bits from liberasurecode-doc files.

$ rpm -q --requires -p ../RPMS/x86_64/liberasurecode-1.0.9-1.fc24.x86_64.rpm | sort -f | uniq -c
      2 /sbin/ldconfig
      1 libc.so.6()(64bit)
      1 libc.so.6(GLIBC_2.14)(64bit)
      1 libc.so.6(GLIBC_2.2.5)(64bit)
      1 libc.so.6(GLIBC_2.3.4)(64bit)
      1 libc.so.6(GLIBC_2.4)(64bit)
      1 liberasurecode_rs_vand.so.1()(64bit)
      1 libm.so.6()(64bit)
      1 libm.so.6(GLIBC_2.2.5)(64bit)
      1 libnullcode.so.1()(64bit)
      1 libpthread.so.0()(64bit)
      1 libpthread.so.0(GLIBC_2.2.5)(64bit)
      1 libXorcode.so.1()(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)
$ rpm -q --provides -p ../RPMS/x86_64/liberasurecode-1.0.9-1.fc24.x86_64.rpm | sort -f | uniq -c
      1 liberasurecode = 1.0.9-1.fc24
      1 liberasurecode(x86-64) = 1.0.9-1.fc24
      1 liberasurecode.so.1()(64bit)
      1 liberasurecode_rs_vand.so.1()(64bit)
      1 libnullcode.so.1()(64bit)
      1 libXorcode.so.1()(64bit)
liberasurecode binary dependencies are Ok.

$ rpm -q --requires -p ../RPMS/x86_64/liberasurecode-devel-1.0.9-1.fc24.x86_64.rpm | sort -f | uniq -c
      1 liberasurecode(x86-64) = 1.0.9-1.fc24
      1 liberasurecode.so.1()(64bit)
      1 liberasurecode_rs_vand.so.1()(64bit)
      1 libnullcode.so.1()(64bit)
      1 libXorcode.so.1()(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
$ rpm -q --provides -p ../RPMS/x86_64/liberasurecode-devel-1.0.9-1.fc24.x86_64.rpm | sort -f | uniq -c
      1 liberasurecode-devel = 1.0.9-1.fc24
      1 liberasurecode-devel(x86-64) = 1.0.9-1.fc24
FIX: Run-require `gcc' by liberasurecode-devel (/usr/include/liberasurecode/erasurecode_stdinc.h includes standard library headers).

$ rpm -q --requires -p ../RPMS/x86_64/liberasurecode-doc-1.0.9-1.fc24.x86_64.rpm | sort -f | uniq -c
      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
$ rpm -q --provides -p ../RPMS/x86_64/liberasurecode-doc-1.0.9-1.fc24.x86_64.rpm | sort -f | uniq -c
      1 liberasurecode-doc = 1.0.9-1.fc24
      1 liberasurecode-doc(x86-64) = 1.0.9-1.fc24
liberasurecode-doc binary dependencies are Ok.

$ resolvedeps rawhide ../RPMS/x86_64/liberasurecode{,-devel,-doc}-1.0.9-1.fc24.x86_64.rpm
Binary dependencies resolvable. Ok.

Package builds in F24 (http://koji.fedoraproject.org/koji/taskinfo?taskID=11077603). Ok.

Otherwise the package is in line with Fedora packaging guidelines.


Please correct all `FIX' items, consider fixing `TODO' items, and provide a new spec file.
Resolution: Package NOT approved.

Comment 16 Pete Zaitcev 2015-09-14 19:25:53 UTC
(In reply to Petr Pisar from comment #15)

> TODO: Add `CRC32' identifier to the License tag.

What is the exact syntax you have in mind? Whitespace? Comma? "And"? "Or"?

Comment 17 Petr Pisar 2015-09-15 07:44:52 UTC
(In reply to Pete Zaitcev from comment #16)
> (In reply to Petr Pisar from comment #15)
> 
> > TODO: Add `CRC32' identifier to the License tag.
> 
> What is the exact syntax you have in mind? Whitespace? Comma? "And"? "Or"?

License: BSD and CRC32

See <https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Multiple_Licensing_Scenarios>.

Comment 18 Pete Zaitcev 2015-09-15 21:22:21 UTC
Fixed all the FIX comments, some of the TODOs. The "make test" crashes,
but I reported it upstream and Kevin had it fixed (a double-free) for 1.0.10.

Spec URL: http://people.redhat.com/zaitcev/tmp/liberasurecode-1.0.9-2.spec
SRPM URL: http://people.redhat.com/zaitcev/tmp/liberasurecode-1.0.9-2.fc22.src.rpm
Description:
An API library for Erasure Code, written in C. It provides a number
of pluggable backends, such as Intel ISA-L library.

Fedora Account System Username: zaitcev

Comment 19 Petr Pisar 2015-09-17 09:17:56 UTC
FIX: The spec file has forbidden name <https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Spec_File_Naming>. Rename it to liberasurecode.spec. I believe this was only a mistake as previous SRPMs contain correct spec file name.

Spec file changes:

--- liberasurecode.spec.old     2015-09-14 10:07:55.135000000 +0200
+++ liberasurecode.spec 2015-09-17 10:54:14.158000000 +0200
@@ -1,12 +1,10 @@
 Name:           liberasurecode
 Version:        1.0.9
-Release:        1%{?dist}
+Release:        2%{?dist}
 Summary:        Erasure Code API library written in C with pluggable backends

-# This is a 2-clause BSD with clause numbers edited out for some reason.
-# The src/utils/chksum/md5.c is under CRC32 license, but it's subsumed
-# into the BSD-licensed distribution (see README.md).
-License:        BSD
+# Main license is a 2-clause BSD with clause numbers removed for some reason.
+License:        BSD and CRC32
 URL:            https://bitbucket.org/tsg-/liberasurecode/
 # Bitbucket's web export naming is like the old github (== awful), so we pull
 # the tag using git CLI. Save the current command for Source0 below.
@@ -16,8 +14,13 @@

 BuildRequires:  autoconf
 BuildRequires:  automake
-BuildRequires:  libtool
+BuildRequires:  coreutils
 BuildRequires:  doxygen
+BuildRequires:  findutils
+BuildRequires:  gcc
+BuildRequires:  libtool
+BuildRequires:  make
+BuildRequires:  sed

 %description
 An API library for Erasure Code, written in C. It provides a number
@@ -32,6 +35,7 @@
 %package devel
 Summary:        Development files for %{name}
 Requires:       %{name}%{?_isa} = %{version}-%{release}
+Requires:       gcc

 %description devel
 The %{name}-devel package contains libraries and header files for
@@ -54,6 +58,7 @@
 %install
 %make_install
 find $RPM_BUILD_ROOT -name '*.la' -exec rm -f {} ';'
+find $RPM_BUILD_ROOT%{_datadir}/doc -type f -exec chmod a-x {} ';'

 %post -p /sbin/ldconfig

@@ -62,7 +67,7 @@

 %files
 %license COPYING
-%doc README.md
+%doc AUTHORS ChangeLog README.md
 %{_libdir}/*.so.*

 %files doc
@@ -74,6 +79,9 @@


 %changelog
+* Tue Sep 15 2015 Pete Zaitcev <zaitcev@redhat.com> 1.0.9-2
+- Address review comments (#1208695)
+
 * Fri Sep 11 2015 Pete Zaitcev <zaitcev@redhat.com> 1.0.9-1
 - Release 1.0.9: true plug-in architecture


> TODO: Fix the license comment:
> > # The src/utils/chksum/md5.c is under CRC32 license
src/utils/chksum/crc32.c should be there.
> TODO: Add `CRC32' identifier to the License tag.
-# This is a 2-clause BSD with clause numbers edited out for some reason.
-# The src/utils/chksum/md5.c is under CRC32 license, but it's subsumed
-# into the BSD-licensed distribution (see README.md).
-License:        BSD
+# Main license is a 2-clause BSD with clause numbers removed for some reason.
+License:        BSD and CRC32
Ok.

> FIX: Build-require `sed' (liberasurecode.spec:47).
+BuildRequires:  sed
Ok.

> FIX: Build-require `make' (liberasurecode.spec:49).
+BuildRequires:  make
Ok.

> FIX: Build-require `findutils' (liberasurecode.spec:56).
+BuildRequires:  findutils
Ok.

> FIX: Build-require `coreutils' (liberasurecode.spec:56).
+BuildRequires:  coreutils
Ok.

> FIX: Build-require `gcc' for including standard library header files.
+BuildRequires:  gcc
Ok.

> TODO: Run tests.
> The "make test" crashes,
> but I reported it upstream and Kevin had it fixed (a double-free) for 1.0.10.
TODO: I recommend drop a comment into the spec file in order not to forget to enable the tests when upgrading the package.

> TODO: Package AUTHORS and ChangeLog files.
-%doc README.md
+%doc AUTHORS ChangeLog README.md
Ok.

$ rpmlint liberasurecode.spec ../SRPMS/liberasurecode-1.0.9-2.fc24.src.rpm ../RPMS/x86_64/liberasurecode-*
liberasurecode.spec: W: invalid-url Source0: liberasurecode-1.0.9.tar.gz
liberasurecode.src: W: invalid-license CRC32
liberasurecode.src: W: invalid-url Source0: liberasurecode-1.0.9.tar.gz
liberasurecode.x86_64: W: invalid-license CRC32
liberasurecode-debuginfo.x86_64: W: invalid-license CRC32
liberasurecode-devel.x86_64: W: invalid-license CRC32
liberasurecode-devel.x86_64: W: only-non-binary-in-usr-lib
liberasurecode-devel.x86_64: W: no-documentation
liberasurecode-doc.x86_64: W: invalid-license CRC32
5 packages and 1 specfiles checked; 0 errors, 9 warnings.
rpmlint is Ok.

Package builds in F24 (http://koji.fedoraproject.org/koji/taskinfo?taskID=11119976). Ok.

> FIX: Remove executable bits from the liberasurecode-doc files.
$ rpm -q -lv -p ../RPMS/x86_64/liberasurecode-doc-1.0.9-2.fc24.x86_64.rpm 
-rw-r--r--    1 root    root                     5702 Sep 17 11:02 /usr/share/doc/liberasurecode/html/annotated.html
-rw-r--r--    1 root    root                      246 Sep 17 11:02 /usr/share/doc/liberasurecode/html/arrowdown.png
-rw-r--r--    1 root    root                      229 Sep 17 11:02 /usr/share/doc/liberasurecode/html/arrowright.png
-rw-r--r--    1 root    root                      676 Sep 17 11:02 /usr/share/doc/liberasurecode/html/bc_s.png
-rw-r--r--    1 root    root                      147 Sep 17 11:02 /usr/share/doc/liberasurecode/html/bdwn.png
-rw-r--r--    1 root    root                     5049 Sep 17 11:02 /usr/share/doc/liberasurecode/html/classes.html
-rw-r--r--    1 root    root                      132 Sep 17 11:02 /usr/share/doc/liberasurecode/html/closed.png
-rw-r--r--    1 root    root                     4441 Sep 17 11:02 /usr/share/doc/liberasurecode/html/dir_85e1485977b1b5c7656625e6aef9fae5.html
-rw-r--r--    1 root    root                     4155 Sep 17 11:02 /usr/share/doc/liberasurecode/html/dir_d44c64559bbebec7f509842c48db8b23.html
-rw-r--r--    1 root    root                      746 Sep 17 11:02 /usr/share/doc/liberasurecode/html/doc.png
-rw-r--r--    1 root    root                    25495 Sep 17 11:02 /usr/share/doc/liberasurecode/html/doxygen.css
-rw-r--r--    1 root    root                     3779 Sep 17 11:02 /usr/share/doc/liberasurecode/html/doxygen.png
-rw-r--r--    1 root    root                     3140 Sep 17 11:02 /usr/share/doc/liberasurecode/html/dynsections.js
-rw-r--r--    1 root    root                      616 Sep 17 11:02 /usr/share/doc/liberasurecode/html/folderclosed.png
-rw-r--r--    1 root    root                      597 Sep 17 11:02 /usr/share/doc/liberasurecode/html/folderopen.png
-rw-r--r--    1 root    root                     4335 Sep 17 11:02 /usr/share/doc/liberasurecode/html/functions.html
-rw-r--r--    1 root    root                     4207 Sep 17 11:02 /usr/share/doc/liberasurecode/html/functions_vars.html
-rw-r--r--    1 root    root                     3514 Sep 17 11:02 /usr/share/doc/liberasurecode/html/index.html
-rw-r--r--    1 root    root                   146338 Sep 17 11:02 /usr/share/doc/liberasurecode/html/jquery.js
-rw-r--r--    1 root    root                      153 Sep 17 11:02 /usr/share/doc/liberasurecode/html/nav_f.png
-rw-r--r--    1 root    root                       95 Sep 17 11:02 /usr/share/doc/liberasurecode/html/nav_g.png
-rw-r--r--    1 root    root                       98 Sep 17 11:02 /usr/share/doc/liberasurecode/html/nav_h.png
-rw-r--r--    1 root    root                      123 Sep 17 11:02 /usr/share/doc/liberasurecode/html/open.png
-rw-r--r--    1 root    root                      314 Sep 17 11:02 /usr/share/doc/liberasurecode/html/splitbar.png
-rw-r--r--    1 root    root                     5185 Sep 17 11:02 /usr/share/doc/liberasurecode/html/structec__args.html
-rw-r--r--    1 root    root                     3967 Sep 17 11:02 /usr/share/doc/liberasurecode/html/structec__backend.html
-rw-r--r--    1 root    root                     3977 Sep 17 11:02 /usr/share/doc/liberasurecode/html/structec__backend__args.html
-rw-r--r--    1 root    root                     3981 Sep 17 11:02 /usr/share/doc/liberasurecode/html/structec__backend__common.html
-rw-r--r--    1 root    root                     3977 Sep 17 11:02 /usr/share/doc/liberasurecode/html/structec__backend__desc.html
-rw-r--r--    1 root    root                     4933 Sep 17 11:02 /usr/share/doc/liberasurecode/html/structec__backend__op__stubs.html
-rw-r--r--    1 root    root                      853 Sep 17 11:02 /usr/share/doc/liberasurecode/html/sync_off.png
-rw-r--r--    1 root    root                      845 Sep 17 11:02 /usr/share/doc/liberasurecode/html/sync_on.png
-rw-r--r--    1 root    root                      142 Sep 17 11:02 /usr/share/doc/liberasurecode/html/tab_a.png
-rw-r--r--    1 root    root                      169 Sep 17 11:02 /usr/share/doc/liberasurecode/html/tab_b.png
-rw-r--r--    1 root    root                      177 Sep 17 11:02 /usr/share/doc/liberasurecode/html/tab_h.png
-rw-r--r--    1 root    root                      184 Sep 17 11:02 /usr/share/doc/liberasurecode/html/tab_s.png
-rw-r--r--    1 root    root                     1163 Sep 17 11:02 /usr/share/doc/liberasurecode/html/tabs.css
File permissions are Ok.

> FIX: Run-require `gcc' by liberasurecode-devel
> (/usr/include/liberasurecode/erasurecode_stdinc.h includes standard library
> headers).
$ rpm -q --requires -p ../RPMS/x86_64/liberasurecode-devel-1.0.9-2.fc24.x86_64.rpm | sort -f | uniq -c
      1 gcc
      1 liberasurecode(x86-64) = 1.0.9-2.fc24
      1 liberasurecode.so.1()(64bit)
      1 liberasurecode_rs_vand.so.1()(64bit)
      1 libnullcode.so.1()(64bit)
      1 libXorcode.so.1()(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
Binary requires are Ok.

$ resolvedeps rawhide ../RPMS/x86_64/liberasurecode-*
Binary dependencies resolvable. Ok.

Except the spec file name, the package is good.

Please correct all FIX items and consider fixing TODO items before building the package.
Resolution: Package APPROVED.

Comment 20 Pete Zaitcev 2015-09-23 03:04:54 UTC
Final touches done:
 - fixed the spec file name inside SRPM (verified with rpm2cpio|cpio -it)
 - added TODO comment about "make test"

Spec URL: http://people.redhat.com/zaitcev/tmp/liberasurecode-1.0.9-3.spec
SRPM URL: http://people.redhat.com/zaitcev/tmp/liberasurecode-1.0.9-3.fc22.src.rpm
Description:
An API library for Erasure Code, written in C. It provides a number
of pluggable backends, such as Intel ISA-L library.

Fedora Account System Username: zaitcev

Comment 21 Pete Zaitcev 2015-09-23 04:02:26 UTC
New Package SCM Request
=======================
Package Name: liberasurecode
Short Description: Erasure Code API library written in C with pluggable backends
Upstream URL: https://bitbucket.org/tsg-/liberasurecode/
Owners: zaitcev
Branches: f24 f23 epel7
InitialCC:

Comment 22 Gwyn Ciesla 2015-09-23 15:33:58 UTC
Git done (by process-git-requests).

f24 is master and thus automatic and should not be manually requested.

Comment 23 Fedora Update System 2015-10-23 16:51:12 UTC
liberasurecode-1.0.9-3.fc22 has been submitted as an update to Fedora 22. https://bodhi.fedoraproject.org/updates/FEDORA-2015-1ab5c917e2

Comment 24 Fedora Update System 2015-10-28 20:27:17 UTC
liberasurecode-1.0.9-3.fc22 has been pushed to the Fedora 22 testing repository. If problems still persist, please make note of it in this bug report.
If you want to test the update, you can install it with
$ su -c 'dnf --enablerepo=updates-testing update liberasurecode'
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-1ab5c917e2

Comment 25 Fedora Update System 2015-11-24 22:25:09 UTC
liberasurecode-1.0.9-3.fc22 has been pushed to the Fedora 22 stable repository. If problems still persist, please make note of it in this bug report.

Comment 26 Tushar Gohad 2015-12-04 10:38:29 UTC
Hi Pete, looks like you requested a sync to epel7 in #c21 but it never went through?  Can you please check?  Thanks.

Comment 27 Tushar Gohad 2015-12-04 10:49:37 UTC
@zaitcev, epel7 build is seen here: http://koji.fedoraproject.org/koji/buildinfo?buildID=687598 but not here: https://bodhi.fedoraproject.org/updates/?packages=liberasurecode.  Request a new build?


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