Bug 1293100

Any news?

Don't be so impatient, please?

The Review Process depends on volunteers doing lots of work. There are hundreds of packages waiting to be reviewed:

 * http://fedoraproject.org/PackageReviewStatus/
 * https://fedoraproject.org/wiki/Package_Review_Process
 * https://fedoraproject.org/wiki/Category:Package_Maintainers

Lots of them are full of mistakes. And while a multitude of packaging mistakes won't ever be noticed by users, there are many that result in actual problems at runtime or at upgade-time, for example.


One way to speed up reviewing is to swap reviews.


Also try doing a self-review of this particular package. Highly recommended! It that leads to questions, post them here where potential reviewers may see them.

Start with pointing the fedora-review tool at this ticket:

    fedora-review -b 1293100

It will fetch the latest src.rpm and spec file found in the "Spec URL:" and "SRPM URL:" lines, perform a local Mock build and run lots of helpful tests. Other tests are to be performed by you manually. Unfortunately, in Fedora 23 the tool suffers from the migration to DNF, so some checks take ages compared with Yum.

While good reviewers perform lots of checks without using that tool, the tool is very helpful with some of its checks (such as the source files licensing checks).


Skim over the packaging guidelines pages and watch out for stuff that's relevant to your package, such as the Systemd guidelines.


This package won't be easy to review and certainly won't pass review without a lot of work.

The spec alone contains lots of questionable/unusual things not found in thousands of Fedora packages. And the first test-builds could lead to discovering further issues. Comments are missing in the spec file. For example, there is no rationale for disabling -debuginfo package generation. Why is that done? Why doesn't the package use %attr to set file access permissions and ownership? Why doesn't it included all needed directories but runs mkdir at post-install time?


> Source1: VERSION
> %global build_version %(( cat %{SOURCE1} || git describe --long) | sed "s/[0-9]*\.[0-9]*\.[0-9]*-//" | sed "s/-[a-z 0-9]*//")
> %global git_hash %((cat %{SOURCE1} || git describe --long) | sed "s/.*-//")
> %global prod_version %((cat %{SOURCE1} || git describe --long) | sed "s/-[0-9]*-.*//")
> Version: %{prod_version}
> Release: %{build_version}

Amazing. All that to end up with a very simple %version-%release,

  tarantool-1.6.8-244.src.rpm

and additional macros that make it more difficult to use them consistently throughout the spec file. How do you bump release during a minor update or an automated mass-rebuild? I hope you are aware of the implicitly defined %version and %release macros as set up by the "Version:" and "Release:" tags. In your case, bumping the Release tag would lead to something different from %build_version, and since a macro expansion of %build_version is involved, in %release it's not an increment but either a prepended or appended value. There is no way out as long as %build_version is at the most-significant left side of %release and even defines what the source topdir is:

> cd tarantool-%{version}-%{build_version}-%{git_hash}-src

Upstream build != Fedora package build, so this is asking for trouble. If you ever needed to specify strict versioned dependencies, it would also get more complicated than with a plain %release value.

https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Package_Versioning

If the %build_version is considered important, look for ways to move it into %version. Else move it to somewhere at the right side of the Release tag, where it doesn't have a huge influence on RPM version comparison checks.


See you in 2016.

How should the git-commands in the spec-file work from outside a git-repo ?

rtsisyk's scratch build of tarantool-1.6.8-0.282.fc24.src.rpm for rawhide failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12374312

jenslody's scratch build of tarantool-1.6.8-244.src.rpm for rawhide failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12378907

rtsisyk's scratch build of tarantool-1.6.8-0.282.fc24.src.rpm for rawhide failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12380190

rtsisyk's scratch build of tarantool-1.6.8-0.282.fc24.src.rpm for rawhide failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12380198

rtsisyk's scratch build of tarantool-1.6.8-0.284.fc24.src.rpm for rawhide completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12380233

> Don't be so impatient, please?

Sorry :)

> Amazing. All that to end up with a very simple %version-%release,
> How should the git-commands in the spec-file work from outside a git-repo ?

This hack is used to automate packaging on Travis CI + Docker + rpmbuild. We actually build packages for **each** git commit without manually changing RPM spec. Current time to deliver is less than 5 minutes. I removed this stuff from proposed spec file.

> > cd tarantool-%{version}-%{build_version}-%{git_hash}-src
> Upstream build != Fedora package build, so this is asking for trouble. If you > ever needed to specify strict versioned dependencies, it would also get more > complicated than with a plain %release value.
> If the %build_version is considered important, look for ways to move it into > %version. Else move it to somewhere at the right side of the Release tag, 
> where it doesn't have a huge influence on RPM version comparison checks.

Tarantool naming convention is ${major}.${major}.${minor}-${patch}, e.g.
1.6.7-155 or 1.7.1-140. ${patch} is the number of commits after tagging a release (git describe --long --always). We usually create and tag a new git branch for a development version after publishing a minor release.

Some fixes still can be pushed (backported) to published release if have substantial reasons for that (security fixes, regressions, several drawbacks that affects customers and so on). It is enterprise level of support which we provide for free for anybody. Any user can install crucial bugfixes without need to wait for the next minor release. Of course, this approach increments ${patch} counter for already released version. Strict dependencies on ${patch} is never needed.

I carefully checked the wiki page pointed above and found that **postrelease** conception is almost close to our naming scheme. Therefore, I changed naming to use numeric postrelease tag:

# ${major}.${major}.${minor}, e.g. 1.6.8 or 1.7.5
Version: 1.6.7
# ${fedora release}.${upstream patch level (postrelease)}
Release: 0.540%{?dist}

I'm not sure that it is possible to automatically update package in Fedora for each fix, but I would be good at least to upload security fixes and minor releases.

> Start with pointing the fedora-review tool at this ticket:
> fedora-review -b 1293100
> Unfortunately, in Fedora 23 the tool suffers from the migration to DNF, so some checks take ages compared with Yum.

Unfortunately, this tool doesn't work on my rawhide from Docker. :(

I uploaded a new version of spec.

Spec URL: https://gist.github.com/rtsisyk/8a8d2dcae143d0699e76/raw/ee9e5c49d79d797c62e19b5d57dd59ca7224a22e/tarantool_1.6.8.284.spec
SRPM URL: https://gist.github.com/rtsisyk/8a8d2dcae143d0699e76/raw/ee9e5c49d79d797c62e19b5d57dd59ca7224a22e/tarantool-1.6.8-0.284.fc24.src.rpm
Buildbot: http://koji.fedoraproject.org/koji/taskinfo?taskID=12380233

Changes:

- Change naming scheme according to #1293100 comments
- Remove SCL support
- Remove devtoolkit support
- Fix tarantool-common /usr/lib64 conflicts
- Use system libyaml

I need a review for this version of spec.
Thanks!

It is not needed to *manually* update spec files for simple updates, such as when replacing source tarballs. Tools like  rpmdev-bumpspec  handle several normal versioning schemes including Fedora's pre-release and post-release schemes, provided that the Release tag is not flooded with macros. 

  $ rpmdev-bumpspec -V test.spec
  test.spec
  -1%{?dist}
  +2%{?dist}

If a versioning scheme cannot be recognised, the tool would perform a minor release bump only at the very right side of the Release tag:
  https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Minor_release_bumps_for_old_branches


> postrelease

Releases start at 1, however.  The 0 is for pre-releases and remains untouched even during updates, since it's a prefix, and the number right of it would be increased as long as a pre-release is packaged:
  https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre-Release_packages


> Tarantool naming convention is ${major}.${major}.${minor}-${patch},
> e.g. 1.6.7-155 or 1.7.1-140. 

Mapping from upstream versioning to RPM package versioning is not always easy. A patchlevel in an upstream versioning scheme is not unusual.

However, the dash in  ${major}.${major}.${minor}-${patch}  is arbitrary and could also be replaced with a dot.

Theoretically, in this case, there's nothing wrong with a direct mapping to %version and %release and making ${patch} the most-significant %release value of the RPM package, provided that the update/upgrade path will not be violated (and newer builds will always replace older ones).

Let's not forget, though, in a package collection there are activities like automated mass-rebuilds and rebuilds done by other packagers, if compilers or build/runtime dependencies change. If there's no real release number in the spec file, it can lead to increasing ${patch}, because neither tools nor people would know that ${patch} is special and is not the build release number as in thousands of packages.

In other words, there can be multiple builds of the same patchlevel release of your software. In the package collection, it is %release (if you don't touch %epoch and %version) that decides which build is newer, and not ${patch}.


> tarantool_1.6.8.284.spec 

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

Plus, somewhat ironically, here you write 1.6.8.284 and not 1.6.8-284. ;-)


> #
> # Disable stripping of /usr/bin/tarantool to allow the debug symbols
> # in runtime. Tarantool uses the debug symbols to display fiber's stack
> # traces in fiber.info().
> #
> %global debug_package %{nil}

If I were you, I would ask about that on packaging@ list. Perhaps there's a way to disable stripping while keeping the -debuginfo package generation. Without -debuginfo packages you lose features like ABRT based backtrace generation and problem reports: https://fedoraproject.org/wiki/Packaging:Debuginfo

rtsisyk's scratch build of tarantool-1.6.8.285-1.fc24.src.rpm for rawhide completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12389392

>  $ rpmdev-bumpspec -V test.spec

It works for me:

--------
VERSION=$(sed -n 's/^\([0-9\.]*\)-\([0-9]*\).*/\1\.\2/p' VERSION)
rpmdev-bumpspec fedora.spec -n ${VERSION} -c "automated build" 

-Version: 1.6.8.0
+Version: 1.6.8.285
 Release: 1%{?dist}
 
 %changelog
+* Sun Jan 03 2016 Roman Tsisyk <roman> - 1.6.8.285-1
+- automated build
+
--------

Thank you for the advice!

> Releases start at 1, however.  The 0 is for pre-releases and remains untouched even during updates, since it's a prefix, and the number right of it would be increased as long as a pre-release is packaged:

Fixed.

> However, the dash in  ${major}.${major}.${minor}-${patch}  is arbitrary and could also be replaced with a dot.

${major}.${major}.${minor}.${patch} is good scheme. dash is not mandatory at all. 
Luckily, we already use such naming scheme for our Fedora/CentOS repositories on tarantool.org:
http://tarantool.org/dist/1.6/fedora/rawhide/x86_64/Packages/
http://tarantool.org/dist/1.6/centos/7/os/x86_64/Packages/
I think we can go with the solution.

> If I were you, I would ask about that on packaging@ list. Perhaps there's a way to disable stripping while keeping the -debuginfo package generation. Without -debuginfo packages you lose features like ABRT based backtrace generation and problem reports: https://fedoraproject.org

I've sent an email about stripping to packing@

Spec URL: https://gist.githubusercontent.com/rtsisyk/8a8d2dcae143d0699e76/raw/5e0c995bd837c625de08c0b59a3aa2b8a195ca7f/tarantool_1.6.8.285.spec
SRPM URL: https://gist.github.com/rtsisyk/8a8d2dcae143d0699e76/raw/5e0c995bd837c625de08c0b59a3aa2b8a195ca7f/tarantool-1.6.8.285-1.fc24.src.rpm
Buildbot: http://koji.fedoraproject.org/koji/taskinfo?taskID=12389392

%global __strip /bin/true might work.

Don't repeat the package name in Summary. Also don't use the article, it looks bad in listings:

Summary: In-memory database and Lua application server

Remove Vendor tag.

Why do you need explicit requirements on readline and other libraries; aren't the automatically generated dependencies enough?

Development subpackage should be called devel, that's the Fedora standard.

tarantool should requires te same version of tarantool-common:
Requires: %{name}-common = %{version}-%{release}
Most likely you do not need the Requires on tarantool in tarantool-common.

%if 1%{?systemd} → this condition is always true. Maybe you meant %if 0%{?systemd}?

What's with the quotes around filenames in %files?

Scripts in %post should never fail, add || : everywhere.

rtsisyk's scratch build of tarantool-1.6.8.294-1.fc24.src.rpm for rawhide completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12483677

(In reply to Zbigniew Jędrzejewski-Szmek from comment #13)

Thanks you for you review!

> %global __strip /bin/true might work.

Sorry, this hack doesn't work for me :(

> Don't repeat the package name in Summary. Also don't use the article, it
> looks bad in listings:
> 
> Summary: In-memory database and Lua application server

Fixed.

> Remove Vendor tag.

Fixed.

> 
> Why do you need explicit requirements on readline and other libraries;
> aren't the automatically generated dependencies enough?

Fixed.

> 
> Development subpackage should be called devel, that's the Fedora standard.

Fixed.

> tarantool should requires te same version of tarantool-common:
> Requires: %{name}-common = %{version}-%{release}
> Most likely you do not need the Requires on tarantool in tarantool-common.

tarantool-common provides /usr/bin/tarantoolctl script which uses /usr/bin/tarantool to operate. The idea was to have common init scripts and management tools for multiple versions of tarantool. 

> %if 1%{?systemd} → this condition is always true. Maybe you meant %if
> 0%{?systemd}?

Fixed.

> What's with the quotes around filenames in %files?

This is an old shell habit. Fixed.

> Scripts in %post should never fail, add || : everywhere.

I moved %post scripts to %install and %files


Changelog:

+- Rename tarantool-dev to tarantool-devel
+- Remove Vendor
+- Remove quotes from %files
+- Disable hardening to fix backtraces
+- Fix permissions for tarantoolctl directories
+- Comply with http://fedoraproject.org/wiki/Packaging:DistTag
+- Comply with http://fedoraproject.org/wiki/Packaging:LicensingGuidelines
+- Comply with http://fedoraproject.org/wiki/Packaging:Tmpfiles.d
+- Comply with the policy for log files
+- Comply with the policy for man pages
+- Other changes according to #1293100 review

Spec URL: https://gist.githubusercontent.com/rtsisyk/8a8d2dcae143d0699e76/raw/84b92b205f60c545244d191f5931cb1195208a71/tarantool_1.6.8.294.spec
SRPM URL: https://gist.github.com/rtsisyk/8a8d2dcae143d0699e76/raw/84b92b205f60c545244d191f5931cb1195208a71/tarantool-1.6.8.294-1.fc24.src.rpm
Buildbot: http://koji.fedoraproject.org/koji/taskinfo?taskID=12483677

%defattr(-,root,root,-) is unnecessary [https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#File_Permissions].

Requires: systemd-units → systemd-units has been merged into systemd for a long while

Requires: tarantool = %{version} → Requires: tarantool%{?_isa} = %{version}-%{release}

Summary: Common files, admin tools and init scripts. →
Summary: Common files and admin tools for %{name}
(no dot)

Requires: tarantool → Requires: tarantool = %{version}-%{release}

%if 0%{?fedora} >= 21
%license %{_datadir}/doc/tarantool/LICENSE
%else
%doc %{_datadir}/doc/tarantool/LICENSE
%endif
→
%{!?_licensedir:%global license %doc}
%doc LICENSE
(see https://fedoraproject.org/wiki/EPEL:Packaging#The_.25license_tag)


OK, packaging is looking mostly OK. Now we get to the hard stuff :(

/usr/lib/tarantool/tarantool.init
Please no. All this script does is run 'lua /usr/bin/tarantool something'. Just put this directly in the systemd unit file. Things will be much simpler and more robust.

/usr/lib/systemd/system/tarantool.service:
# It's not recommended to modify this file in-place, because it will be
# overwritten during package upgrades. If you want to customize there're
# number of ways:

# Recommended way:
# 1) Use "/etc/sysconfig/tarantool" or "/etc/default/tarantool" -
# They're supported by our start-stop utility - tarantoolctl

# Usual way for RPM-based distros
# 2) Create a file "/etc/systemd/system/tarantool.service",
# containing
#   .include /usr/lib/systemd/system/tarantool.service
#   # Here're your changes
#
# For example, if you want to change CONF_DIR create
# "/etc/systemd/system/tarantool.service" containing:
#   .include /usr/lib/systemd/system/tarantool.service
#   [Service]
#   Environment=CONF_DIR=/etc/tarantool/instances.other
# This will override the settings appearing below

.include stanza as been deprecated for a long time in systemd. It has been replaced by drop-in directories [1], but it actually looks like you want something else: systemd templates (also described in [1]).

[1] http://www.freedesktop.org/software/systemd/man/systemd.unit.html#Description

Something roughly like this:
/usr/lib/systemd/system/tarantool@.service:

[Unit]
Description=Tarantool instance %I
After=network.target
Documentation=man:tarantool(1)

[Service]
Type=forking
User=tarantool

ExecStart=/usr/bin/lua /usr/bin/tarantool start %I
ExecStop=/usr/bin/lua /usr/bin/tarantool stop %I

TimeoutSec=300

(Is such a long timeout really needed?)


To summarize the init part:
- remove obsolete advice from the unit file
- do not install the useless SysVinit wrapper script
- one of two options:
  - update tarantool.service to run /usr/bin/tarantool start for each instance in /etc/tarantool/instances.enabled

  (or preferrably)
  - replace tarantool.service with a bunch of tarantool@.service instances like described above, and use systemd enable/disable to manage instance enablement.

The second option is better in the long run, because those instances are more isolated, and can be managed using standard system tools. But in the short run it's more work, and it changes the way instances are enabled.


What about msgpuck? It seems that you split it out for review in #1295217? Shouldn't it be a dependency? And lua-fun?

> %defattr(-,root,root,-) is unnecessary [https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#File_Permissions].

OK, I hope it won't break RHEL/CentOS 6/7. Probably I should drop backward-compatibility code and make a separate spec file for Fedora.

> Requires: systemd-units → systemd-units has been merged into systemd for a long while

I found these lines in postgresql package during looking for systemd-tmpfiles example.

> Requires: tarantool = %{version} → Requires: tarantool%{?_isa} = %{version}-%{release}

OK, I've just found that this {?_isa} is also mentioned in the guidelines.

> Summary: Common files, admin tools and init scripts. →
> Summary: Common files and admin tools for %{name} (no dot)

Sorry, I remember that this was mentioned in the guidelines, but I've added this dot unintentionally.

> Requires: tarantool → Requires: tarantool = %{version}-%{release}

OK.

> %if 0%{?fedora} >= 21
%license %{_datadir}/doc/tarantool/LICENSE
%else
%doc %{_datadir}/doc/tarantool/LICENSE
%endif
→
%{!?_licensedir:%global license %doc}
%doc LICENSE
(see https://fedoraproject.org/wiki/EPEL:Packaging#The_.25license_tag)

OK, thanks for the advice.

The problems above are very easy to fix and I will do it a little bit later.



> OK, packaging is looking mostly OK. Now we get to the hard stuff :(

A new version systemd unit files requires some amount of work, so I've opened a ticket in upstream to track this feature [1]
[1] https://github.com/tarantool/tarantool/issues/1264

> What about msgpuck? It seems that you split it out for review in #1295217? > Shouldn't it be a dependency? And lua-fun?

It should, but if I'm not mistaken, a new spec should not depend on packages which haven't accepted to the repository yet.

BTW, msgpuck and lua-fun are standalone libraries and are used by other projects. For instance, luafun is even more popular on GitHub than tarantool itself, because suited for more broader audience. Of course, I'll add dependencies to these packages after finishing with them.

I hope we will deal with systemd soon.

(In reply to Roman Tsisyk from comment #17)
> > %defattr(-,root,root,-) is unnecessary [https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#File_Permissions].

It seems defattr in only needed in RHEL <= 4.

> > Requires: systemd-units → systemd-units has been merged into systemd for a long while
> 
> I found these lines in postgresql package during looking for
> systemd-tmpfiles example.

See https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd
for the authoritative version. Requiring systemd-units doesn't hurt,
but shouldn't be added in new packages.

> > Requires: tarantool = %{version} → Requires: tarantool%{?_isa} = %{version}-%{release}
> 
> OK, I've just found that this {?_isa} is also mentioned in the guidelines.

Well, sometimes you need it, sometimes not. First rule
is that %{_isa} is only defined for arched packages, so noarch
packages cannot really have arched dependencies.
Second rule is that in general -devel packages should
use %{_isa} in the dependency on the main package. The
most obvious reason is that they contain a symlink
%{_libdir}/*.so to %{_libdir}/*.so.x.y.z, and %{_libdir}
depends on the architecture, so the architecture of the
-devel subpackage and the main subpackage must match.

> > Summary: Common files, admin tools and init scripts. →
> > Summary: Common files and admin tools for %{name} (no dot)
> 
> Sorry, I remember that this was mentioned in the guidelines, but I've added
> this dot unintentionally.

No biggie. I'm sure this guideline is violated in numerous places.

> > OK, packaging is looking mostly OK. Now we get to the hard stuff :(
> 
> A new version systemd unit files requires some amount of work, so I've
> opened a ticket in upstream to track this feature [1]
> [1] https://github.com/tarantool/tarantool/issues/1264
> 
> > What about msgpuck? It seems that you split it out for review in #1295217? > Shouldn't it be a dependency? And lua-fun?
> 
> It should, but if I'm not mistaken, a new spec should not depend on packages
> which haven't accepted to the repository yet.

No, there's no such rule. Just put the other packages in "Depends on"
for this one.

> BTW, msgpuck and lua-fun are standalone libraries and are used by other
> projects. For instance, luafun is even more popular on GitHub than tarantool
> itself, because suited for more broader audience. Of course, I'll add
> dependencies to these packages after finishing with them.
> 
> I hope we will deal with systemd soon.

Great.

Status update:

1. I've been sponsored to packager group by Zbigniew. Thanks a lot for that.
2. New systemd scripts have been implemented as it was suggested - "preferrably replace tarantool.service with a bunch of tarantool@.service instances and use systemd enable/disable to manage instance enablement". This patch will be merged to the upstream after review inside Tarantool Team (probably tomorrow).
3. Probably it is possible to disable backtraces entirely for Fedora packages and get rid of binutils-dev, debuginfo and problems with hardening. I'm still investigating this option.
4. I plan to make a new version of RPM spec after finishing with 3 and 4.
Please stay in touch ;)

rtsisyk's scratch build of tarantool-1.6.8.451-1.fc24.src.rpm for rawhide completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12894137

A new version.

Spec URL: https://gist.github.com/rtsisyk/8a8d2dcae143d0699e76/raw/623b434534e369bd80cd282bb5ccd8c05ca11113/tarantool.spec
SRPM URL: https://gist.github.com/rtsisyk/8a8d2dcae143d0699e76/raw/623b434534e369bd80cd282bb5ccd8c05ca11113/tarantool-1.6.8.451-1.fc24.src.rpm
Fedora-Review: https://gist.github.com/rtsisyk/8a8d2dcae143d0699e76
Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=12894139

Changelog:

- Implement proper support of multi-instance management using systemd
https://github.com/tarantool/tarantool/blob/e15bc621f1bca24c1ee61405f8511369f9489716/README.systemd.md
- Enable hardened builds and -debuginfo
- Add coreutils, make and sed to BuildRequires
- Drop binutils-devel, zlib-devel dependencies
- Drop all legacy stuff

Current problems:

1.
tarantool.x86_64: E: non-standard-dir-perm /var/log/tarantool 750
tarantool.x86_64: E: non-standard-dir-perm /var/lib/tarantool 750
I use 0750 to allow read database files only for members of tarantool group.

Please help me to resolve this problem.

2. tarantool-devel.x86_64: W: no-documentation
Documentation for /usr/inc.ude/tarantool/module.h is provided on the web-site:
http://tarantool.org/doc/reference/capi.html
I can generate a man page from Doxygen, but does it make sense?

3. tar.gz contains some JS crap in doc/sphinx
=> Upstream's buildbot will be fixed soonto generate proper tarballs.

4. I'm working on enabling %check
Tarantool has the extensive test suite, so I'm trying to get test-run.py working under mock on all platforms. https://travis-ci.org/tarantool/tarantool/jobs/107191725#L1187  It would be nice to have an access to some Fedora/RHEL armhf box to check mock...

(In reply to Roman Tsisyk from comment #21)
> Changelog:
> 
> - Implement proper support of multi-instance management using systemd
> https://github.com/tarantool/tarantool/blob/
> e15bc621f1bca24c1ee61405f8511369f9489716/README.systemd.md
> - Enable hardened builds and -debuginfo
> - Add coreutils, make and sed to BuildRequires
> - Drop binutils-devel, zlib-devel dependencies
> - Drop all legacy stuff
> 
> Current problems:
> 
> 1.
> tarantool.x86_64: E: non-standard-dir-perm /var/log/tarantool 750
> tarantool.x86_64: E: non-standard-dir-perm /var/lib/tarantool 750
> I use 0750 to allow read database files only for members of tarantool group.
> 
> Please help me to resolve this problem.
I don't think it's a problem. The permissions look reasonable.

> 2. tarantool-devel.x86_64: W: no-documentation
> Documentation for /usr/inc.ude/tarantool/module.h is provided on the
> web-site:
> http://tarantool.org/doc/reference/capi.html
> I can generate a man page from Doxygen, but does it make sense?
No, documentation is nice, but it certainly isn't required.
Basically the rule is that if upstream provides documentation that
can be built without unreasonable effort, it should be included.

> 3. tar.gz contains some JS crap in doc/sphinx
> => Upstream's buildbot will be fixed soonto generate proper tarballs.

The only thing which is forbidden, is non-distributable sources. Including some javascript that sphinx uses is allowed, even if a bit messy.
 
> 4. I'm working on enabling %check
> Tarantool has the extensive test suite, so I'm trying to get test-run.py
> working under mock on all platforms.
> https://travis-ci.org/tarantool/tarantool/jobs/107191725#L1187  It would be
> nice to have an access to some Fedora/RHEL armhf box to check mock...

See https://fedoraproject.org/wiki/Test_Machine_Resources_For_Package_Maintainers. You can use those without asking anybody.

There are also PPC machines, but I think you have to ask for access.


Review:
- license is acceptable (BSD)
- license file is present, %license is used
- latest version
- builds and installs OK
- scriptlets look OK (*)
- build and requires look OK
- description and summary and package name are OK
- rpmlint only has false positives

Package is APPROVED.


(*) The %post script currently fails, but this seems to be a bug in systemd in F23. It doesn't matter much for now because it's unlikely that anyone will be using presets to enable tarantool instances in the near future. I'll try to get this fixed upstream in systemd.

Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/tarantool

(In reply to Zbigniew Jędrzejewski-Szmek from comment #22)
> > 4. I'm working on enabling %check
> > Tarantool has the extensive test suite, so I'm trying to get test-run.py
> > working under mock on all platforms.
> > https://travis-ci.org/tarantool/tarantool/jobs/107191725#L1187  It would be
> > nice to have an access to some Fedora/RHEL armhf box to check mock...
> 
> See
> https://fedoraproject.org/wiki/
> Test_Machine_Resources_For_Package_Maintainers. You can use those without
> asking anybody.
> 
> There are also PPC machines, but I think you have to ask for access.

Also, using dedicated ppc and arm Koji instances is a way faster than using generic Koji at https://koji.fedoraproject.org/koji/ . Just issue:
  $ arm-koji build --scratch rawhide packagename.src.rpm
  $ ppc-koji build --scratch rawhide packagename.src.rpm
respectively, and see the results in:
  http://arm.koji.fedoraproject.org/koji/tasks?owner=rtsisyk&state=all
  http://ppc.koji.fedoraproject.org/koji/tasks?owner=rtsisyk&state=all

You may also request a shell as described here: https://fedoraproject.org/wiki/Architectures/PowerPC#PPC_Shell_access_for_debugging

This package uses JIT compilation and fast cooperative multitasking in userspace (fibers) [1].
Currently native implementation (asm) of fibers only exists for the following architectures:

* x86
* x86_64
* armv7hl - since 1.6.8 release
* aarch64 - since 1.6.8 release, but tested only on qemu

[1]: http://tarantool.org/doc/reference/fiber.html

It is possible to use swapcontext(3) as a fallback, but this option is very slow and unreliable and can be used only for lab testing. A new native fiber backend for PowerPC, MIPS,  s390x, etc., requires a lot of efforts. We spent about two man-month of highly-qualified C developers on armv7 port. I don't think that anybody will work on new architectures without real users on these architectures. 

I think that `ExclusiveArch %{ix86} x86_64 armv7hl armv7hnl aarch64` is suitable here. I can define it only for RHEL/EPEL (if 0%{?rhel}).

Primary architectures [2] are OK and are officially supported by upstream (but ARM is still not integrated to CI).
arm*.cloud.fedoraproject.org machine was very useful and helped us to fix test suite on Fedora armv7hl.
Thanks for the advice!

[2]: https://fedoraproject.org/wiki/Architectures#Primary_Architectures

> I think that `ExclusiveArch %{ix86} x86_64 armv7hl armv7hnl aarch64` is suitable here. 

I think that's entirely reasonable.

tarantool-1.6.8.530-2.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-98716494c3

tarantool-1.6.8.530-2.fc22 has been submitted as an update to Fedora 22. https://bodhi.fedoraproject.org/updates/FEDORA-2016-c7394b6681

tarantool-1.6.8.530-2.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-220abf23ed

tarantool-1.6.8.530-2.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-220abf23ed

tarantool-1.6.8.530-2.fc22 has been pushed to the Fedora 22 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-c7394b6681

tarantool-1.6.8.530-2.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-98716494c3

tarantool-1.6.8.530-2.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.

tarantool-1.6.8.530-2.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report.

tarantool-1.6.8.530-2.fc22 has been pushed to the Fedora 22 stable repository. If problems still persist, please make note of it in this bug report.