Code review comment for lp:~percona-dev/percona-xtrabackup/bug_737462

Alexey Kopytov (akopytov) wrote :

Valentine,

1. $option_ibbackup_binary is initialized to 'autodetect', so the condition (!$option_ibbackup_binary) will never be true. You should use "if( $option_ibbackup_binary eq 'autodetect' ) " instead.

2. Alignment and closing brace placement is wrong.

3. I think it makes more sense to default to 'xtrabackup' rather than 'xtrabackup_51'. Then, in the windows case, for example, you don't even need to specify the binary explicitly.

I think it's easier to show the patch:

--- innobackupex 2011-04-14 17:59:27 +0000
+++ innobackupex 2011-04-27 08:24:18 +0000
@@ -215,7 +215,9 @@ print_version();

 # initialize global variables and perform some checks
 if ($option_copy_back) {
- $option_ibbackup_binary = 'xtrabackup_51';
+ if ( $option_ibbackup_binary eq 'autodetect' ) {
+ $option_ibbackup_binary = 'xtrabackup';
+ }
 } elsif ($option_apply_log) {
  # Read XtraBackup version from backup dir
  if (-e "$backup_dir/$xtrabackup_binary_file") {

review: Needs Fixing

« Back to merge proposal