Code review comment for lp:~stewart/percona-server/pkg-5.5

Revision history for this message
Stewart Smith (stewart) wrote :

Laurynas Biveinis <email address hidden> writes:
> - Why does moving the Percona Server version suffix from Makefile to
> VERSION cause percona_innodb_version test change to
> "5.5.x-x.x-x.x", i.e. one extra "-x.x"? Or is it not just a move
> but some version string format change too caused by a more MySQL-y
> way of doing things?

For 5.5 this is mysql-innodb version, with mysql version including the
PS version now, so that it's actually correct.

> - "<email address hidden>" in mysql_version.cmake is a wrong e-mail
> contact for "Percona Engineering". I am not sure what should be the
> right one though.

Yep... we likely need something for this.

> - build/debian/rules have diverged from the old build-dpkg.sh and
> from the old/current build-binary.sh in server CMake options at
> least in one instance: -DWITH_SSL=bundled, although we have
> switched to -DWITH_SSL=system. Was that intentional? If no, how
> can we ensure that our CMake options are in sync across all the
> packaging options? If yes, how do we handle the diverging CMake
> options across platforms?

for debian, you have to use =system as per Debian policy.

> - Other suspicious build/debian/rules CMake difference vs
> build-binary.sh. I am not sure if these are real issues or
> intentional changes: -DWITH_PAM=ON missing, -DINSTALL_LAYOUT=RPM
> (sic?) extra.

okay, likely bug.

> - rev 572 adds Debian translations. How should we manage them in the
> future?

probably just import/sync them with the Debian MySQL packagses, but I'm
honestly not sure of the best way.

> - Please create a corresponding blueprint(s) and/or bug report(s) as
> necessary. The libperconaserverclient blueprint and the "PS source
> layout != MySQL one" bug report do not exhaust the set of changes
> made here.
>
> - Please also log something (or point to if they exist) for the
> follow-up work items so that we don't let slip them through the
> cracks, such as storage/HS -> plugin/HS move.

ack.

--
Stewart Smith

« Back to merge proposal