See File::Path::mkpath, and also perlfunc for the -d and -e operators.

The first problem is that unless(-d "Automation") is checking for an Automation directory in the current working directory. It needs to be changed to -d "/etc/Automation". Likewise for the mkdir.

You do not need to use opendir to do this work. A couple of other things... always use 3-arg open, always use lexical file handles (not bareword ones), check if things like open and mkdir succeeded, and always use strict; and use warnings;.

Also, your crontab entry gets written to a file named crontab in the current working directory, same goes for the aptscript.pl file. You'll need to specify the paths to those files (in the open call) if that wasn't your intention.

Here's an untested rewrite that will hopefully get you closer.

use warnings;
use strict;

my $username = getpwuid( $< );

my $dir = '/etc';

if($username ne "root"){
    print "please run this script as root...\n";
    exit;
}
else{
    unless (-d "$dir/Automation"){
        mkdir"$dir/Automation" or die $!;

        my $cronjob 
          = "0 12 * * * /usr/bin/perl /etc/Automation/aptscript.pl";

        open my $cron_fh, '>', 'crontab';

        print $cron_fh $cronjob;
        close $cron_fh;

        open my $script_fh, '>', 'aptscript.pl' or die $!;

        print $script_fh '' .
          "#!/usr/bin/perl\n" .
          "print 'replace this script ...'";
        close $script_fh;

    }
}
[download]

This operation:

open CRONTABFILE, '>crontab';
flock CRONTABFILE, 2;
[download]

...is a potentially catastrophic bug waiting to happen, and it's repeated on a couple of different files in your sample script. You are opening crontab in write mode. More specifically, write-and-truncate mode. Imagine this sequence of events:

1. Someone opens the crontab file and gets a lock on it.
2. You open the file in clobber mode, clobbering the file.
3. You fail to get a lock because someone else already had one.
4. You've now clobbered a file someone else has a lock on.

MJD likens this scenario to taking a dump on someone's front porch and then ringing the doorbell to ask for tissue.

If you wish to obtain a lock on the crontab file, you will need to open it in a mode that doesn't clobber it before you ever get the lock. < or +< would be a reasonable approach if you are certain the file already exists. If you do not have such a guarantee, and you don't mind not knowing for sure whether or not you created the file, you could use +>> and then truncate and seek after obtaining the lock. A common idiom would also be to open and lock the target, then open a tempfile with a sufficiently randomized name (see File::Temp, do your writing there, and then rename or move the tempfile into place over the output file. The move becomes an atomic operation, which is about the least risky approach if you aren't guaranteed that other actors aren't occasionally touching the same file without proper locking.

Updated to reflect the order I intended to describe in the first place but managed to still present out of order when I typed in my thoughts (thanks MidLifeXis).

Also see http://www.plover.com/~mjd/perl/yak/flock/

A common idiom would also be to open a tempfile with a sufficiently randomized name (see File::Temp, do your writing there, and then after obtaining a lock on the output file, rename or move the tempfile into place over the output file. (emphasis added)

Race condition waiting to happen (at least as I am reading your statement). Have the lock signal the ability to modify the file (lock first, write second). If you don't lock first, you risk clobbering someone else's changes:

• You create a tmp file
• They create a tmp file
• They lock
• They overwrite
• They unlock
• You lock
• You overwrite
• You unlock
The changes made by "They" are now gone. Lock before read just to ensure that you have the newest data. Otherwise, I agree.

--MidLifeXis

You lock the target before creating a temp, and as a final step move the temp in place over the target. If you fail to get a lock on the target to begin with, stop there.

After re-reading my post I see that my words didn't reflect the process I had in my head when I wrote them. I've updated. Thanks for noticing and bringing it to my attention! :)




Dave