Beefy Boxes and Bandwidth Generously Provided by pair Networks
XP is just a number
 
PerlMonks  

comment on

( [id://3333] : superdoc . print w/replies, xml ) Need Help??
Ah, this reminds me of the goold old times when I used to refactor CGI-scripts for a living. See, the script ain't that bad. Apparently it works, is battle tested and the worst thing is the "lack of control" what I interpret as changing the configuration of the script easily. Here is how you refactor something like that:

First thing you do is adding use strict, run the script and cry a little. But then you boldy add the the missing my statements at the broadest possible scope (usually file global) until the script compiles again.
Now your script is strict-safe, but you still have a lots of globals. You want to limit their scope. So pick one global and add a underscore to the name. Run the script.
In all subroutines where now are errors, add the missing variable (as a reference) to the subroutines argument list. Within the subroutine change all occurances of the global variable to new reference, change code where if neccessary (e.g. $formerGlobal[0] to $nowReference->[0] ).
Now find all callers of the subroutine and change them so they pass in the global variable to the subroutine.
Rinse and repeat with all globals and all subroutines they appear in.

While you're at it change the calling style of subroutines from Perl4 &foo too foo() at several places.
After that you have subroutines that are independent of any global variables which now appear only in the main function. The subs are now "real functions". Black boxes with no side effects that are functionally separated, have a clean interface and can be unit tested.

And that is now the ideal point to get rid of the globalfest alltogether by factoring them out into a configuration file. This gives you the control you want, if you make it so the user can pass in a config file name to use.

Wow, that's a lot of text, my nodes usually are way more concise but I guess this node made a litte nostalgic. So nostalgic in fact I coded it right away for fun =)
#!/usr/bin/perl #--------------------------------------------------------------------- +------------- # Filename: deploy.pl # # This script is executed by the deploy tool as part of the standard # deploy process. This script moves batch SE work flow batch scripts +to the correct # locations where <<we>> expect to find them within the local filesyst +em. # # Date Developer Comment # ---------- ---------------- ------------------------------- +------------- # 02-06-2019 R. Eaglestone Initial creation # 05-21-2019 Brother Holli Waxing and Polishing #--------------------------------------------------------------------- +------------- use Modern::Perl; use File::Copy; use Sub::Signatures; use Config::Any; # This subroutine ensures that all files are present. sub checkDeployValidity ($installDir, $executionDir, $filesToDeploy) { # First, check that the install directory exists. if ( !(-d $installDir) ) { return "install directory $installDir was not deployed."; } # Then, check that the base execution directory exists. if ( ! (-d $executionDir) ) { return "execution directory $executionDir does not exist."; } # Next, set permissions on the installation directory. if ( ! chmod(0775, $installDir) ) { return "Setting permissions on directory $installDir failed."; } # Now, check that all files were deployed. my @scriptFileNames = sort(keys(%$filesToDeploy)); foreach my $fileName (@scriptFileNames) { my $currentFile = "$installDir/$fileName"; if ( !(-f $currentFile) ) { return "file $currentFile was not deployed."; } } # Finally, if all files are present, then return success. return ""; } # This subroutine builds any missing directories. sub setUpDirectoryStructure ($dirsToDeploy) { # Build any subdirectories, if necessary. my @directoryNames = sort(keys(%$dirsToDeploy)); foreach my $dirName (@directoryNames) { # Check whether the directory exists... if ( -d $dirName ) { # If so, then set its permissions appropriately. if ( ! chmod(oct($dirsToDeploy->{$dirName}), $dirName) ) { print "\tunable to do 'chmod $dirsToDeploy->{$dirName} $dirNam +e'.\n"; } } elsif ( ! mkdir($dirName, $dirsToDeploy->{$dirName}) ) { print "\tunable to create directory '$dirName'.\n"; } } # Finally, return success once all directories exist with proper per +missions. return ""; } # This subroutine removes carriage return characters from specific fil +es. sub dosToUnix ($installDir, $filesToConvert) { foreach my $fileName ( @$filesToConvert ) { my $file = "$installDir/$fileName"; my $tempFile = "$file.tmp_dos2Unix"; open(my $DOS_FILE, "<", $file) || return "cannot open $file f +or read: $!"; open(my $UNIX_FILE, ">", $tempFile) || return "cannot open $tempFi +le for write: $!"; # Remove all carriage return ('\r') characters. while (<$DOS_FILE>) { s/\r//g; print $UNIX_FILE || return "cannot write to $tempFile: $!"; } close($UNIX_FILE) || return "cannot close $tempFile: $!"; close($DOS_FILE) || return "cannot close $file: $!"; rename($file, "$file.orig") || return "cannot rename $file to $fil +e.orig: $!"; rename($tempFile, $file) || return "cannot rename $tempFile to +$file: $!"; unlink("$file.orig") || return "cannot delete $file.orig: +$!"; } return ""; } # This subroutine copies files from the install dir to the execution d +ir. sub copyFilesToTarget ($installDir, $pathsToDeploy, $filesToDeploy) { my @fileNames = sort(keys(%$filesToDeploy)); foreach my $fileName (@fileNames) { my $source = "$installDir/$fileName"; my $path = $pathsToDeploy->{$fileName}; my $target = "$path/$fileName"; if ( -f $target) { chmod(0777, $target) || return "chmod 777 for existing file '$ta +rget' failed."; } copy($source, $target) || return "file copy to '$target' failed."; chmod(oct($filesToDeploy->{$fileName}), $target) || return "chmod +for '$target' failed."; } return ""; } # This subroutine orchestrates the deploy process. sub deploy ( $configFile ) { die("ERROR - Cannot find config file") unless -e $configFile && -f $configFile; my $config = Config::Any->load_files({ files => [$configFile], use +_ext => 1, flatten_to_hash => 0 })->{$configFile}; # Start by setting umask to 0 to simplify mkdir commands. my $oldUMask = umask(0000); # Next, check that all of the files exist. my $errorMessage = checkDeployValidity( $config->{installDir}, $conf +ig->{executionDir}, $config->{filesToDeploy} ); if ( $errorMessage ) { umask($oldUMask); die("$0: ERROR - $errorMessage"); } # Then, set up the execution directory structure. $errorMessage = setUpDirectoryStructure( $config->{dirsToDeploy} ); if ( $errorMessage ) { umask($oldUMask); die("$0: ERROR - $errorMessage"); } # Convert files from DOS format to Unix format. $errorMessage = dosToUnix( $config->{installDir}, $config->{filesToC +onvert} ); if ( $errorMessage ) { umask($oldUMask); die("$0: ERROR - $errorMessage"); } # Now, copy the files over. $errorMessage = copyFilesToTarget( $config->{installDir}, $config->{ +pathsToDeploy}, $config->{filesToDeploy} ); if ( $errorMessage ) { umask($oldUMask); die("$0: ERROR - $errorMessage"); } # Finally, restore the old umask value (in case this process is reus +ed). umask($oldUMask); print "Copy succeeded. Job finished successfully.\n"; exit(0); } deploy( $ARGV[0] // "deploy.json" );
And then the config file. I picked JSON but with Config::Any used you can pick any (duh) format it supports like Ini and xml and so on.
{ "installDir":"/whatever/jobs/", "executionDir":"/whatever/data/output/", "dirsToDeploy":{ "/whatever/jobs/boo":"0777", "/whatever/jobs/boo/jum":"0777", "/whatever/data/output/boo/jum":"0777", "/whatever/data/output/boo":"0777" }, "filesToDeploy":{ "fileSanity.pl":"0775", "transferFile.sh":"0775", "MY_LAUNCHER_11023200099.sh":"0775" }, "filesToConvert":[ "MY_LAUNCHER_11023200099.sh", "fileSanity.pl", "transferFile.sh" ], "pathsToDeploy":{ "transferFile.sh":"/fancy/deploy/path", "fileSanity.pl":"/fancy/deploy/path", "MY_LAUNCHER_11023200099.sh":"/fancy/deploy/path" } }
The next logical step would now be to put the utility subroutines into a separate module that exports them so they can be tested easier. That will then leave only the main routine in the script file. But I leave that to you and the advice of Brother Fletch.


holli

You can lead your users to water, but alas, you cannot drown them.

In reply to Re: Modernizing a Deploy script to standard distro v5.10.1 by holli
in thread Modernizing a Deploy script to standard distro v5.10.1 by rje

Title:
Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post; it's "PerlMonks-approved HTML":



  • Are you posting in the right place? Check out Where do I post X? to know for sure.
  • Posts may use any of the Perl Monks Approved HTML tags. Currently these include the following:
    <code> <a> <b> <big> <blockquote> <br /> <dd> <dl> <dt> <em> <font> <h1> <h2> <h3> <h4> <h5> <h6> <hr /> <i> <li> <nbsp> <ol> <p> <small> <strike> <strong> <sub> <sup> <table> <td> <th> <tr> <tt> <u> <ul>
  • Snippets of code should be wrapped in <code> tags not <pre> tags. In fact, <pre> tags should generally be avoided. If they must be used, extreme care should be taken to ensure that their contents do not have long lines (<70 chars), in order to prevent horizontal scrolling (and possible janitor intervention).
  • Want more info? How to link or How to display code and escape characters are good places to start.