Beefy Boxes and Bandwidth Generously Provided by pair Networks
Just another Perl shrine

Feedback on my second script

by Jackiejarvis (Novice)
on May 15, 2013 at 09:28 UTC ( #1033648=perlquestion: print w/replies, xml ) Need Help??
Jackiejarvis has asked for the wisdom of the Perl Monks concerning the following question:

Like in my previous post I would ask you to give some feedback on my second script. This script will start a rsync process between to places. I would like to get some feedback on this script so I can improved it or maybe you see some things that can go wrong that I don't see.

Many thanks!

#!/usr/bin/perl # (c) Dieter Odeurs # This script starts rsync for a back-up job use strict; use warnings; use Getopt::Long; my $path = ""; my $destination; my $help; #Show usage sub showUsage{ print <<EOF; ---------------------------- -- Usage for rsync script -- ---------------------------- Required: --path : The source directory, this is the directory you want to back-u +p Example --path='/path/to/source' --destination This is the destination, where you want to put the back-up Example --destination='/path/to/destination' Optional:"; --exclude Use this option if you want to exclude some directories Without this option the script will exclude only .etc and lost ++found Example: --exclude='test' --exclude='demo' EOF exit; } #Without exclude options the script automatically exclude .etc and los +t+found my @exclude = ( ".etc", "lost+found" ); #You must specify a path, destination and if you want to exclude some +extra files it's possible with exclude showUsage() if (!GetOptions ( 'path=s' => \$path, 'exclude=s' => \@exclude, 'destination=s' => \$destination, 'help|?' => \$help, '<>' => \&showUsage ) or defined $help ); # Check if the path exist otherwise exit the program and give an error if(defined $path){ if (-d $path){ print $path . " is a correct path \n"; }else{ print $path . " is a incorrect path \n"; exit; } }else{ showUsage(); } # Check if a destination is defined unless (defined $destination) { showUsage (); } print "exclude : @exclude \n"; # Create the command to start rsync with all the excludes and options my $cmd = "rsync -av --iconv=iso88591,utf8 --stats"; foreach my $exclude (@exclude) { $cmd = $cmd . " --exclude '$exclude'"; } $cmd .= " $path $destination"; print $cmd . "\n"; # Run the program and show the output my @result; @result = `$cmd 2>&1`; foreach my $result (@result) { print $result; chomp $result; } #exit the program exit;

Replies are listed 'Best First'.
Re: Feedback on my second script
by Corion (Pope) on May 15, 2013 at 09:38 UTC

    Two small things:

    # Check if a destination is defined unless (defined $destination) { showUsage (); }

    I would like an improved error message here, like

    # Check if a destination is defined unless (defined $destination) { print "No destination given.\n"; showUsage (); }

    And at the place where you run rsync, you capture all of its (voluminous) output just to print it out right afterwards:

    # Run the program and show the output my @result; @result = `$cmd 2>&1`; foreach my $result (@result) { print $result; chomp $result; }

    I would change that to simply use system instead, which will do the printing directly. This would also add error checking so your script signals if the underlying rsync failed:

    # Run the program and show the output system( "$cmd 2>&1") == 0 or die "rsync failed: $! / $?";

    Of course, if you really want/need to do post-processing of the rsync output, as your chomp indicates, you will need to capture the output as you did.

      Thanks for the feedback. I have inserted the things you suggest. About the running rsync. In the future I would send a mail to the admin if there is an error with the error message included. How could i do something like this is it possible if i use system or not?


        Yes, if you want to extend the program later to send the output via mail (but only in the case of error), your current approach makes sense.

        I would send a mail... How could i do something like this

        I've traditionally used Mail::Sendmail for that; although, Mail::Send may also be worth considering.

        Note too that it's generally unsafe to allow the user to specify a destination address for mail (unless you have carefully authenticated the user). In simple cases like this, it's often best to hardcode the destination address. In more advanced cases, possible destination addresses can be stored in a configuration file or database that only system administrators can edit, and the user can be allowed to select from among them.

Re: Feedback on my second script
by karlgoethebier (Parson) on May 15, 2013 at 13:00 UTC

    BTW, if using Getopt::Long why not also Pod::Usage?

    Built-in documentation, e.g../ --help or perldoc Easy to use and very handy (I'm reseller ;-)).

    Regards, Karl

    «The Crux of the Biscuit is the Apostrophe»

Log In?

What's my password?
Create A New User
Node Status?
node history
Node Type: perlquestion [id://1033648]
Front-paged by Arunbear
and all is quiet...

How do I use this? | Other CB clients
Other Users?
Others lurking in the Monastery: (2)
As of 2017-04-26 10:25 GMT
Find Nodes?
    Voting Booth?
    I'm a fool:

    Results (471 votes). Check out past polls.