Merge lp:~percona-toolkit-dev/percona-toolkit/fix-942114-bad-find-usage into lp:percona-toolkit/2.1

[Brian Fraser]

[Daniel Nichter (daniel-nichter)]

Do we want -warn? The man page says "The default behaviour corresponds to -warn if standard input is a tty, and to -nowarn otherwise." pt-stalk is often run as a daemon, so STDIN won't be a tty.

[Brian Fraser (fraserbn)]

That's to trigger the warning during the tests, otherwise we could have a regression and never notice. And also, a slightly less valid and rather more paranoid reason, I've learnt not to trust defaults in shell commands : D

[Daniel Nichter (daniel-nichter)]

review: Approve

=== modified file 'bin/pt-stalk'
--- bin/pt-stalk	2012-08-14 15:11:41 +0000
+++ bin/pt-stalk	2012-08-23 06:39:00 +0000
@@ -1031,14 +1031,14 @@
    local retention_time="$2"
 
    # Delete collect files which more than --retention-time days old.
-   find "$dir" -type f -mtime +$retention_time -exec rm -f '{}' \;
+   find "$dir" -warn -type f -mtime +$retention_time -exec rm -f '{}' \;
 
    local oprofile_dir="/var/lib/oprofile/samples"
    if [ -d "$oprofile_dir" ]; then
       # "pt_collect_" here needs to match $CMD_OPCONTROL --save=pt_collect_$p
       # in collect().  TODO: fix this
-      find "$oprofile_dir" -type d -name 'pt_collect_*' \
-         -depth -mtime +$retention_time -exec rm -rf '{}' \;
+      find "$oprofile_dir" -warn -depth -type d -name 'pt_collect_*' \
+         -mtime +$retention_time -exec rm -rf '{}' \;
    fi
 }
 
 
=== modified file 't/pt-stalk/pt-stalk.t'
--- t/pt-stalk/pt-stalk.t	2012-06-04 15:53:44 +0000
+++ t/pt-stalk/pt-stalk.t	2012-08-23 06:39:00 +0000
@@ -23,9 +23,6 @@
 if ( !$dbh ) {
    plan skip_all => 'Cannot connect to sandbox master';
 }
-else {
-   plan tests => 27;
-}
 
 my $cnf      = "/tmp/12345/my.sandbox.cnf";
 my $pid_file = "/tmp/pt-stalk.pid.$PID";
@@ -282,6 +279,28 @@
    "Not stalking, pt-stalk is not running"
 );
 
+# ############################################################################
+# bad "find" usage in purge_samples gives 
+# https://bugs.launchpad.net/percona-toolkit/+bug/942114
+# ############################################################################
+
+use File::Temp qw( tempdir );
+
+my $tempdir = tempdir( CLEANUP => 1 );
+
+my $script = <<"EOT";
+. $trunk/bin/pt-stalk
+purge_samples $tempdir 10000 2>&1
+EOT
+
+$output = `$script`;
+
+unlike(
+   $output,
+   qr/\Qfind: warning: you have specified the -depth option/,
+   "Bug 942114: no bad find usage"
+);
+
 # #############################################################################
 # Done.
 # #############################################################################
@@ -289,4 +308,5 @@
 diag(`rm $log_file 2>/dev/null`);
 diag(`rm -rf $dest 2>/dev/null`);
 ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox");
-exit;
+
+done_testing;