bergman on 26 Apr 2012 14:23:40 -0700 |
[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]
Re: [PLUG] First shell script for deployment... |
In the message dated: Thu, 26 Apr 2012 16:34:04 EDT, The pithy ruminations from Paul Walker on <[PLUG] First shell script for deployment...> were: => --===============1493056227== => Content-Type: multipart/alternative; boundary=047d7b15b09ba33d6b04be9ae88b Ick. Why use HTML multipart MIME for plain ASCII text? => => --047d7b15b09ba33d6b04be9ae88b => Content-Type: text/plain; charset=ISO-8859-1 => => This is the first shell script I've written... pretty basic but nice for => now since I need to maintain some consistency across installations.. (OT => but I'll use .gitignore to avoid having to do this in the future... for now => I want to eliminate as much inconsistency as possible). Any thoughts or => constructive criticism appreciated: Well, since you asked... => => <code> => #!/bin/bash => => # Run this from the directory **above** the public_html directory for the => site you are deploying Missing info. re version, date, author. I find this ironic, as the script is meant to manipulate info from a version control system. => => #create dir and clone upstream repo You should check if the directory exists before trying to do the mkdir...that would be good indication that you're in the wrong parent dir, or that a previous run of the script failed. Does there need to be a check of what user is running the script (root? apache? nobody? JoeRandomHacker?) before proceeding? => mkdir public_html_deploy1 There's an assumption about the directory where the script is being run. It's much safer to cd to an /absolute/path/to/the/working/dir at the beginning. The script is missing checks for the return value of each step. That's a common--and serious--failure. I usually deal with that with a shell function, such as: ###################################### checkandexit() { if [ $1 != 0 ] ; then # There was an error, bail out! /bin/echo "$2" 1>&2 exit $1 fi } ###################################### That also makes a convenient place to remove temp files, lock files, send messages to syslog, etc. The function is used as follows: mkdir public_html_deploy1 checkandexit $? "mkdir of public_html_deploy1 failed" => cd public_html_deploy1 checkandexit $? "Could not cd to public_html_deploy1" insert as needed after every command => git init => git clone git@foo.com:project/repo.git => git remote add foo git@foo.com:project/repo.git => git config remote.foo.push refs/heads/master:refs/heads/master => mv repo ../public_html_deploy The naming (public_html_deploy vs public_html_deploy1) is confusing--the names are so similar that it's difficult to tell what operations are being done...and all to easy to rm the wrong dir. I'd suggest changing "public_html_deploy" to something like "staging" or "git_checkout" or "Fred". => => #move untracked files from current webroot into deployment directory tree => cd ../public_html_deploy => cp -r ../public_html/articleImport . => cp -r ../public_html/themes . => cp -r ../public_html/submissions . => cp -r ../public_html/sites/all/themes/ sites/all/themes => cp ../public_html/sites/default/settings.php sites/default/settings.php => cp -r ../public_html/sites/default/files/ sites/default/files That's too hard for me to parse, given the lack of comments. Will this make sense to you in 6 months, or to your replacement in a year, when the directory structure changes? => => #get rid of the stepping stone directory => rm -rf ../public_html_deploy_1 ^^^^^^^^^^^^^^^^^^^^^^^ Critical error! You use "public_html_deploy1" when creating the directory and use "public_html_deploy_1" when removing a different directory. => => #set file permission => chmod -R 777 sites/default/files Why 777? Why set that permission on files? If I understand the intent of the script (difficult, given the lack of comments), it looks like this has something to do with web content. In that case, I understand that files and directories may need to be world readable, but there's probably no need to make them world writeable & executable. This is probably sufficient: find sites/default/files -type f -print0 | xargs -0 chmod o+r find sites/default/files -type d -print0 | xargs -0 chmod o+rx and maybe add this, if stictly necessary (but unlikely): find sites/default/files -type f -iname "*php" -print0 | xargs -0 chmod o+rx The script should probably exit with an explicit return value, so that other processes can check the status: exit 0 Overall, the script will probably work (once the critical error is fixed), but I'd consider it to be very fragile--there are way too many undocumented assumptions about directory paths, no error checking, virtually no documentation, no consideration of security or possible failure modes, no logging (optional or default) to any destination (stderr or syslog), etc. There are lots of good books and on-line examples about shell scripting. Perhaps you should check some of them. Mark "now back to writing my own script" Bergman => </code> => ___________________________________________________________________________ Philadelphia Linux Users Group -- http://www.phillylinux.org Announcements - http://lists.phillylinux.org/mailman/listinfo/plug-announce General Discussion -- http://lists.phillylinux.org/mailman/listinfo/plug