Spec URL: http://peter.fedorapeople.org/riak.spec SRPM URL: http://peter.fedorapeople.org/riak-0.13.0-1.fc12.src.rpm Description: Riak is a Dynamo-inspired key/value store that scales predictably and easily. Riak also simplifies development by giving developers the ability to quickly prototype, test, and deploy their applications. A truly fault-tolerant system, Riak has no single point of failure. No machines are special or central in Riak, so developers and operations professionals can decide exactly how fault-tolerant they want and need their applications to be. ========= Please, do NOT start the review - it still needs love, and I'll clean NotReady flag when it will be ready.
Unblocking luwak - I'll add this backend later. Ok, this is finally ready for review: * http://peter.fedorapeople.org/riak.spec * http://peter.fedorapeople.org/riak-1.1.4-1.fc18.src.rpm Note - I intentionally packaged ver. 1.1.4 - I'll upgrade it to 1.2.0 this september (with luwak enabled and with several new dependencies).
I'll review this one.
Created attachment 604285 [details] Mock build.log for rawhide-x86_64 configuration. Hi Peter, For a start, it doesn't build in mock :) (Log attached) I'm in the middle of reviewing the other portions. Can you work on making it build in the mean time please? Thanks, Ankur
(In reply to comment #3) > Created attachment 604285 [details] > Mock build.log for rawhide-x86_64 configuration. > > Hi Peter, > > For a start, it doesn't build in mock :) (Log attached) > > I'm in the middle of reviewing the other portions. Can you work on making it > build in the mean time please? > > Thanks, > Ankur Ohhh, I terribly sorry for that! That's my fault (I blindly uploaded version which wasn't checked in Koji and there are lots of missing BuildRequires). Stay tuned - I'll return with updated version shortly.
Done. The same version, the same location: * http://peter.fedorapeople.org/riak.spec * http://peter.fedorapeople.org/riak-1.1.4-1.fc18.src.rpm Koji scratchbuild for F-18: * http://koji.fedoraproject.org/koji/taskinfo?taskID=4388435 A bit of story behind "rebar compile generate -v" - this compiles the only binary blob for Erlang VM ("compile" part) and builds so-called "release data" ("generate" part) - a blob describing all dependent Erlang packages, the order of starting Erlang applications, some hints for live upgrade (the prominent Erlang feature I plan to introduce as a Feature for F-19 or F-20) and so on.
Okay. Here's the review. Peter, I've never done an erlang package before, so quite a few of my comments are going to be just "please check this" rather than concrete suggestions :). [+] OK [-] NA [?] Issue [+] Package meets naming and packaging guidelines [+] Spec file matches base package name. [-] Spec has consistant macro usage. [+] Meets Packaging Guidelines. [+] License [ankur@ankur basho-riak-83ec281]$ find . -name "*" -exec licensecheck '{}' \; | sed '/UNKNOWN/ d' ./package/deb/postinst: *No copyright* GENERATED FILE ./package/deb/copyright: *No copyright* Apache (v2.0) ./doc/basic-setup.txt: *No copyright* GENERATED FILE [ankur@ankur basho-riak-83ec281]$ [+] License field in spec matches [+] License file included in package [+] Spec in American English [+] Spec is legible. [+] Sources match upstream md5sum: [ankur@ankur NN-feature-detect-trials(master #)]$ wget --content-disposition https://github.com/basho/riak/tarball/riak-1.1.4 --2012-08-14 23:26:32-- https://github.com/basho/riak/tarball/riak-1.1.4 Resolving github.com... 207.97.227.239 Connecting to github.com|207.97.227.239|:443... connected. HTTP request sent, awaiting response... 302 Found Location: https://nodeload.github.com/basho/riak/tarball/riak-1.1.4 [following] --2012-08-14 23:26:33-- https://nodeload.github.com/basho/riak/tarball/riak-1.1.4 Resolving nodeload.github.com... 207.97.227.252 Connecting to nodeload.github.com|207.97.227.252|:443... connected. HTTP request sent, awaiting response... 200 OK Length: 228239 (223K) [application/octet-stream] Saving to: `basho-riak-riak-1.1.4-0-g95c5cb6.tar.gz' <snip> 5bfc5b77f520adc48eff426bc8f4db95 basho-riak-riak-1.1.4-0-g95c5cb6.tar.gz 5bfc5b77f520adc48eff426bc8f4db95 /home/ankur/rpmbuild/SOURCES/basho-riak-riak-1.1.4-0-g95c5cb6.tar.gz [-] Package needs ExcludeArch [+] BuildRequires correct [-] Spec handles locales/find_lang [-] Package is relocatable and has a reason to be. [+] Package has %defattr and permissions on files is good. [+] Package is code or permissible content. [-] Doc subpackage needed/used. [+] Packages %doc files don't affect runtime. [-] Headers/static libs in -devel subpackage. [-] Spec has needed ldconfig in post and postun [-] .pc files in -devel subpackage/requires pkgconfig [-] .so files in -devel subpackage. [-] -devel package Requires: %{name} = %{version}-%{release} [-] .la files are removed. [-] Package is a GUI app and has a .desktop file [+] Package compiles and builds on at least one arch. [+] Package has no duplicate files in %files. [+] Package doesn't own any directories other packages own. [+] Package owns all the directories it creates. [-] No rpmlint output. [ankur@ankur SRPMS]$ rpmlint riak-1.1.4-1.fc18.src.rpm /var/lib/mock/fedora-rawhide-x86_64/result/*.rpm ../SPECS/riak.spec riak.src: W: invalid-url URL: https://wiki.basho.com/display/RIAK/Riak HTTP Error 404: Not Found ^^ ? Checked in browser. Actually reports a 404. Please recheck riak.src: W: strange-permission riak.init 0775L ^^ ? Please check this (although not applicable to fedora) riak.src:8: W: macro-in-comment %global riak.src:8: W: macro-in-comment %{eval riak.src:8: W: macro-in-comment %{VERSION} riak.src:9: W: macro-in-comment %global riak.src:9: W: macro-in-comment %{VERSION} ^^ + Harmless riak.src:16: W: mixed-use-of-spaces-and-tabs (spaces: line 16, tab: line 11) ^^ ? Tiny cosmetic change riak.x86_64: E: no-binary ^^ ? I think this is okay since it's an erlang based package? riak.x86_64: W: only-non-binary-in-usr-lib ^^ ? Again, this is okay for erlang packages? riak.x86_64: W: non-standard-uid /var/lib/riak/bitcask riak riak.x86_64: W: non-standard-gid /var/lib/riak/bitcask riak riak.x86_64: W: non-standard-uid /var/log/riak/sasl riak riak.x86_64: W: non-standard-gid /var/log/riak/sasl riak riak.x86_64: W: non-standard-uid /var/log/riak riak riak.x86_64: W: non-standard-gid /var/log/riak riak riak.x86_64: W: non-standard-uid /var/lib/riak/mr_queue riak riak.x86_64: W: non-standard-gid /var/lib/riak/mr_queue riak riak.x86_64: W: non-standard-uid /var/lib/riak/ring riak riak.x86_64: W: non-standard-gid /var/lib/riak/ring riak riak.x86_64: W: non-standard-uid /var/lib/riak/dets riak riak.x86_64: W: non-standard-gid /var/lib/riak/dets riak riak.x86_64: W: non-standard-uid /var/run/riak riak riak.x86_64: W: non-standard-gid /var/run/riak riak riak.x86_64: W: non-standard-uid /var/lib/riak/merge_index riak riak.x86_64: W: non-standard-gid /var/lib/riak/merge_index riak riak.x86_64: W: non-standard-uid /var/lib/riak/leveldb riak riak.x86_64: W: non-standard-gid /var/lib/riak/leveldb riak riak.x86_64: W: non-standard-uid /var/lib/riak riak riak.x86_64: W: non-standard-gid /var/lib/riak riak ^^ + Seems okay, since you're creating a user/group for the package in the spec. riak.x86_64: W: non-conffile-in-etc /etc/tmpfiles.d/riak.conf riak.x86_64: W: non-conffile-in-etc /etc/riak/vm.args riak.x86_64: W: non-conffile-in-etc /etc/riak/app.config ^^ ? Shouldn't these be marked as %config? riak.x86_64: W: dangling-symlink /usr/lib64/riak/lib /usr/lib64/erlang/lib riak.x86_64: W: dangling-symlink /usr/lib64/riak/erts-5.9.1 /usr/lib64/erlang/erts-5.9.1 ^^ ? Looks okay. I'm guessing the Requires will pull in the targets. riak.x86_64: W: manual-page-warning /usr/share/man/man1/riak-admin.1.gz 32: a newline character is not allowed in an escape name ^^ ? Please see if you can correct this. riak.x86_64: W: log-files-without-logrotate /var/log/riak ^^ ? Please refer to https://fedoraproject.org/wiki/User:Johannbg/Packaging/LogFiles It says "This guidelines only applies if you are going to support additional optional syslog solutions such as rsyslog or syslog-ng in your package." so I think this is OK. Please do take a look, just to be sure.. [ankur@ankur SRPMS]$ [+] final provides and requires are sane: (include output of for i in *rpm; do echo $i; rpm [-]qp --provides $i; echo =; rpm -qp --requires $i; echo; done manually indented after checking each line. I also remove the rpmlib junk and anything provided by glibc.) [ankur@ankur result]$ review-req-check == riak-1.1.4-1.fc19.src.rpm == Provides: Requires: erlang-rebar erlang-cluster_info erlang-eper erlang-riak_control erlang-riak_kv erlang-riak_search == riak-1.1.4-1.fc19.x86_64.rpm == Provides: riak = 1.1.4-1.fc19 riak(x86-64) = 1.1.4-1.fc19 Requires: /bin/bash /bin/sh /bin/sh /bin/sh /usr/bin/env erlang-cluster_info erlang-eper erlang-riak_control erlang-riak_kv erlang-riak_search shadow-utils systemd-units systemd-units systemd-units ^^ Looks okay to me. SHOULD Items: [+] Should build in mock. [+] Should build on all supported archs [-] Should function as described. [+] Should have sane scriptlets. [-] Should have subpackages require base package with fully versioned depend. [+] Should have dist tag [-] Should package latest version [-] check for outstanding bugs on package. (For core merge reviews) Thanks, Ankur
(In reply to comment #6) > riak.src: W: invalid-url URL: https://wiki.basho.com/display/RIAK/Riak HTTP > Error 404: Not Found > ^^ > ? Checked in browser. Actually reports a 404. Please recheck Fixed. > riak.src: W: strange-permission riak.init 0775L > ^^ > ? Please check this (although not applicable to fedora) Fixed. > riak.src:8: W: macro-in-comment %global > riak.src:8: W: macro-in-comment %{eval > riak.src:8: W: macro-in-comment %{VERSION} > riak.src:9: W: macro-in-comment %global > riak.src:9: W: macro-in-comment %{VERSION} > ^^ > + Harmless This is a leftover. Removed. > riak.src:16: W: mixed-use-of-spaces-and-tabs (spaces: line 16, tab: line 11) > ^^ > ? Tiny cosmetic change Fixed. > riak.x86_64: E: no-binary > ^^ > ? I think this is okay since it's an erlang based package? Yep. That's how Erlang packages are designed. Some of them (as this one) contains only texts or platform-independent data but since they must be installed into %{_libdir} (which is arch-dependent) the entire package becomes arch-dependent. > riak.x86_64: W: only-non-binary-in-usr-lib > ^^ > ? Again, this is okay for erlang packages? Yes, the same reason as in the warning above. > riak.x86_64: W: non-conffile-in-etc /etc/tmpfiles.d/riak.conf > riak.x86_64: W: non-conffile-in-etc /etc/riak/vm.args > riak.x86_64: W: non-conffile-in-etc /etc/riak/app.config > ^^ > ? Shouldn't these be marked as %config? Definitely! I overlooked it - thanks for pointing me out on this. > riak.x86_64: W: dangling-symlink /usr/lib64/riak/lib /usr/lib64/erlang/lib > riak.x86_64: W: dangling-symlink /usr/lib64/riak/erts-5.9.1 > /usr/lib64/erlang/erts-5.9.1 > ^^ > ? Looks okay. I'm guessing the Requires will pull in the targets. Yep. I plan to improve it in the future (fully autogenerated Requires/Provides). I plan to propose it as a feature for F-19 or F-20. Actually I've got a plenty of proposals for Erlang in Fedora :). > riak.x86_64: W: manual-page-warning /usr/share/man/man1/riak-admin.1.gz 32: > a newline character is not allowed in an escape name > ^^ > ? Please see if you can correct this. Fixed. > riak.x86_64: W: log-files-without-logrotate /var/log/riak > ^^ > ? Please refer to > https://fedoraproject.org/wiki/User:Johannbg/Packaging/LogFiles > It says "This guidelines only applies if you are going to support additional > optional syslog solutions such as rsyslog or syslog-ng in your package." so > I think this is OK. > Please do take a look, just to be sure.. Actually I don't know for sure what's the best way to deal with Riak log. I plan to improve it later based on a user's feedback. So far I'd rather to keep it as is. Here is the new build: * http://peter.fedorapeople.org/riak.spec * http://peter.fedorapeople.org/riak-1.1.4-2.fc18.src.rpm
Hi Peter, Looks good! Did you miss a config file though? :) riak.x86_64: W: non-conffile-in-etc /etc/tmpfiles.d/riak.conf Please correct it before you commit to SCM/build for rawhide. *** APPROVED ***
(In reply to comment #8) > Hi Peter, > > Looks good! Did you miss a config file though? :) > > riak.x86_64: W: non-conffile-in-etc /etc/tmpfiles.d/riak.conf > > Please correct it before you commit to SCM/build for rawhide. Yep indeed - this should go into /usr/lib/tmpfiles.d/ - again, thanks for spotting this! > *** APPROVED *** Thanks! New Package SCM Request ======================= Package Name: riak Short Description: Dynamo-inspired key/value store Owners: peter Branches: f17 f18 el6 InitialCC:
Git done (by process-git-requests).
I've just build it for Rawhide/F-18. F-17 and EL6 branches will follow in a couple of weeks (as soon as all dependencies will be available in updates).
riak-1.1.4-2.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/riak-1.1.4-2.fc18
riak-1.1.4-2.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/riak-1.1.4-2.fc17
riak-1.1.4-2.fc17 has been pushed to the Fedora 17 stable repository.
riak-1.1.4-2.fc18 has been pushed to the Fedora 18 stable repository.
Package Change Request ====================== Package Name: riak InitialCC: erlang-sig
Done.