[Bug 1293100] Review Request: tarantool - an in-memory database and Lua application server

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1293100



--- Comment #2 from Michael Schwendt <bugs.michael@xxxxxxx> ---
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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]