Code review comment for lp://staging/~vlad-lesin/percona-xtrabackup/2.1-bug-1227240

Hi Vlad,

On Mon, 11 Nov 2013 11:03:20 -0000, Vlad Lesin wrote:
> On 11/11/2013 02:05 PM, Alexey Kopytov wrote:
>> Vlad,
>>
>> "require_server_version_higher_than '5.6.10'" ensures that the test is only run with xtrabackup_56 as the XtraBackup binary. If it wasn't the case, a lot of other tests would fail too. Thus checks for xtrabackup_56 (and thus, the function) are redundant, please remove them.
>>
>
> This is true for our default use case but this is not true for common
> case. "require_server_version_higher_than" checks only server version.
> If "XB_BUILD" variable is set to "autodetect"(default value) then
> xtrabackup binary name is chosen based on server version(see
> get_version_info()). But the default value and consequently xtrabackup
> binary name can be changed with "-c" option of "run.sh".
>

The '-c' option is only required to test "unreleased" patches and
configurations. For example, the xtrabackup binary based on
innodb51.patch is never released, innobackupex uses the xtradb51-based
binary to backup MySQL 5.1 servers.

However, we still need to support and maintain innodb51.patch in 2.1. In
2.2 all patches and the -c option will go away soon.

With that in mind, I don't see how, for example, "run.sh -c xtradb55"
against a 5.6 server would be a valid "case". It's just misusing a
feature designed for something completely different. And, as I said, a
lot of tests would fail in that case, not just this specific one.