by aviasddsa at 2012-11-04 02:54:26
Hi , I’m n00b in powershellby nohandle at 2012-11-04 04:39:24
can anyone suggest an improvement on my first script ?
thx<#
Download sysinternals + nirsoft packages from the web
#>
#globals#
$downloads= @{}
$target = 'c:\utils'
$DebugPreference = 2
$VerbosePreference = 2
$WarningPreference = 2
$log = $env:TEMP + '' + $MyInvocation.MyCommand.name + '.log'
#function#
function dl_and_extract($what) {
$webClient = New-Object System.Net.WebClient
$downloadFile = $env:TEMP + '\New.zip'
write-output "downloading $what"
$webClient.DownloadFile($what ,$downloadFile)
write-output "Extracting $downloadFile"
7za e -y $downloadfile -x!Nirlauncher* -x!autorun*
Remove-Item -Force $downloadFile
}
######################################################################################################
#main
write-output "new log started - $(Get-Date -format "yyyy/MM/d hh:mm")" | tee -Append $log
#nirsoft
#get the new dl url
$what = wget -q http://launcher.nirsoft.net/download.html -O - | grep "http.pack..zip" | sed -n "s/>.//;s/.http/http/;s/.$//;p;q"
$downloads['nirsoft'] = $what
#sysinternals
$downloads['sysint'] = 'http://download.sysinternals.com/files/SysinternalsSuite.zip'
#dl and extract
cd $target
write-output "Logfile: $log"
$downloads.keys | %{ dl_and_extract($downloads[$_]) } | tee -Append $log
write-output "finished running script - $(Get-Date -format "yyyy/MM/d hh:mm")" | tee -Append $log
Hi,by aviasddsa at 2012-11-04 06:08:10
just by quick look:
1) the code gives me errors straight away.Bad numeric constant: 7.
At C:\Users\nohandle\AppData\Local\Temp\d9a71423-a8d9-40b0-b6a6-932536361b7d.ps1:24 char:6
+ 7 <<<< za e -y $downloadfile -x!Nirlauncher-x!autorun
+ CategoryInfo : ParserError: (7:String) , ParseException
+ FullyQualifiedErrorId : BadNumericConstant
(I dont have 7zip on my station, but this is syntax not a runtime error. What version of powershell do you use?)The ‘<’ operator is reserved for future use.
At C:\Users\nohandle\AppData\Local\Temp\d9a71423-a8d9-40b0-b6a6-932536361b7d.ps1:37 char:18
+ $what = wget -q < <<<< !-- m –><a class="postlink" href="http://launcher.nirsoft.net/download.html">htt
p://launcher.nirsoft.net/download.html</a><!-- m –> -O - | grep "http.pack..zip" | sed -n "s/>.//;s/
.http/http/;s/.$//;p;q"
+ CategoryInfo : ParserError: (<:OperatorToken) [], ParseException
+ FullyQualifiedErrorId : RedirectionNotSupported
Tee-Object : A parameter cannot be found that matches parameter name ‘Append’.
At C:\Users\nohandle\AppData\Local\Temp\d9a71423-a8d9-40b0-b6a6-932536361b7d.ps1:48 char:93
+ write-output "finished running script - $(Get-Date -format "yyyy/MM/d hh:mm")" | tee -Append <<<< $log
+ CategoryInfo : InvalidArgument: ([Tee-Object], ParameterBindingException
+ FullyQualifiedErrorId : NamedParameterNotFound,Microsoft.PowerShell.Commands.TeeObjectCommand
2) I would make the download variable an array not a hashtable
3) using join-path when creating paths less error prone than using + concatenation
4) add error handling
Hope you repair it and give it another turn .)
Thanks for the input david,by nohandle at 2012-11-05 03:08:40
I’m using powershell 3.0
[quote]Bad numeric constant: 7.
At C:\Users\nohandle\AppData\Local\Temp\d9a71423-a8d9-40b0-b6a6-932536361b7d.ps1:24 char:6
+ 7 <<<< za e -y $downloadfile -x!Nirlauncher-x!autorun
+ CategoryInfo : ParserError: (7:String) , ParseException
+ FullyQualifiedErrorId : BadNumericConstant
[/quote]
I don’t know why that error happens on your station, but i noticed some other parts were not pasted correctly to the post. I’ll repaste
this script requires wget , 7za (7zip archiver ) and sed -so i guess you’ll have to comment those lines if you don’t have these…
Btw - any good alternative for sed in ps ?
Anyway I’ve reworked the script , added join-path ,removed the download array added some error trapping and tiding. it’s working over here so without issues ,give it another go.
( hope i did not over complicate - i use it to learn a bit PS …)<#
Download sysinternals + nirsoft packages from the web
Requirements (probably will change sometime, so just for reference) :
* 7za ( \http://downloads.sourceforge.net/sevenzip/7za920.zip )
* sed ( \http://garr.dl.sourceforge.net/project/gnuwin32/sed/4.2.1/sed-4.2.1-bin.zip )
* wget ( \http://users.ugent.be/~bpuype/cgi-bin/fetch.pl?dl=wget/wget-1.10.2.exe )
#>
#################################################globals############################################
$my_err = @{
ERR_DL = '-240'
ERR_EXT = '-200'
}
$packages= @{
nirsoft = @{
url = 'http://launcher.nirsoft.net/download.html'
sed = "/http.pack.zip/ {s/.(http[^""]+)""./\1/;p;q}"
}
sysint = @{
url = 'http://download.sysinternals.com/files/SysinternalsSuite.zip'
}
}
$targetDir = 'c:\utils'
$scr = get-item $MyInvocation.MyCommand.Definition
$log = join-path $env:TEMP -childpath "$($scr.BaseName).log"
$downloadFile = join-path $env:TEMP -childpath "$($scr.basename).zip"
#################################################functions############################################
#init script
function init_scr() {
$DebugPreference = 2
$VerbosePreference = 2
$WarningPreference = 2
Write-Verbose "Downloadfile: $downloadFile , log: $log "
write-verbose "Logfile: $log"
write-output "new log started - $(Get-Date -format "yyyy/MM/d hh:mm")" | tee -Append $log
$reqs = "sed.exe","7za.exe","wget.exe"
$reqs | %{
if ( -not $(Get-Command($) -ErrorAction SilentlyContinue) )
{
Write-Warning "$ not found on your system , this script may not run correctly"
}
}
}
#download package and extract to target dir
function dl_and_extract {
param(
[parameter(Mandatory=$true)]
[string]
$url,
[parameter(Mandatory=$true)]
[string]
$downloadFile
)
process {
$webClient = New-Object System.Net.WebClient
write-output "downloading $url"
try
{
$webClient.DownloadFile($url ,$downloadFile)
}
catch [System.Exception]
{
#Todo: Check why tee locks the file causing a crash
#$(
# write-output "Error downloading $url,cannot continue."
# Write-output "ERROR: " + $.Exception.ToString()
# ) | tee -a $log
write-output "Error downloading $url,cannot continue."
Write-output "ERROR: " + $.Exception.ToString()
return $my_err['ERR_DL']
}
write-output "Extracting $downloadFile to $(get-location)"
try
{
7za e -y $downloadfile #-x!Nirlauncher* -x!autorun*
}
catch [System.Exception]
{
$(
write-output "Error extracting $downloadFile"
Write-output "ERROR: " + $.Exception.ToString()
) | tee -a $log
return $my_err['ERR_EXT']
}
if (Test-Path $downloadFile )
{
Remove-Item -Force $downloadFile
}
}
}
#if retrieving the package requires additional proccessing , do it. add to hash
function reparse_package_url() {
$packages.keys | %{
if ($packages[$]['sed'])
{
$packages[$]['dl_url'] = wget -q $packages[$]['url'] -O - | sed -n $packages[$]['sed']
}
else
{
$packages[$]['dl_url'] = $packages[$]['url']
}
}
}
################################################main######################################################
init_scr
#detect if sedding is needed
reparse_package_url
#dl and extract
cd $targetDir
$packages.keys | %{
dl_and_extract -url $packages[$]['dl_url'] -downloadFile $downloadFile
} | tee -Append $log
write-output "finished running script - $(Get-Date -format "yyyy/MM/d hh:mm")" | tee -Append $log
To me it seems like you are overcomplicating it a bit. try to break the problem to smaller bits. Imagine how it should work, before you start writing first line of script you should already know how the basic parts of the app are going to be implemented and connected. start from the core functionality. test it properly, do exception handling. and then add the specialities. to be able to achieve the goal you should have at least basic understanding of how the language works so take your time, some powershell book and start with the simple examples and work your way up to the complicated ones.by aviasddsa at 2012-11-05 08:15:48
But remember the golden rule in programming - if it is too complicated you are probably doing it wrong :).
And if you can get a copy of Powershell in action second edition book the first part explaining the language is gonna give you tons of useful information. Some general OOP book can also help.
The "intialize" function (like every function) is setting the variables only in its scope if not explicitly told otherwise. meaning you get the debug visible only in the function not in the whole script.
Your exception handling doesn’t do more than the standard.
Hope this does not demotivate you. it just takes time and practice and reading
I’m not demotiviated , Actually i appericate criticismby nohandle at 2012-11-06 00:43:13
The core functionality is :
1. Downloading zipped packages from the web.
2. After download completed , extract them to target location
1 special case - some package link are coming from dynamic pages and their links change periodically so to download them, I have to download the dynamic page ,parse the content and locate the package link from the page , only afterwords download the package. (in this script - nirsoft is the example)
Could you give your take on how would you build this script ? not the whole thing, just an approach/outline …
I’ll take a look on Powershell in action second edition so thanks for that.
I believe i have some basic knowledge in oop , are you refering to something you saw in the script ? ( if it’s the nested hash - that’s the way I do small things in javascript and php though it could probably be an object too)
I missed the $DebugPreference in function thing ( thought those were globals ). the exception handling is lacking i know , just wanted to test it out…
BTW - I could make the whole thing an object with some methods to download but wouldn’t that be an overcomplication for this task ?
Thanks for the reply. critic helps me learn.
Ok. I am constructing a script that is gonna be used by me or my tech colleagues - so the exceptions can be "raw".
I want the script to help us download the packages because we do it on regular basis. No production system relies on it, so the worst thing that can happen if the script fails is that the have to be downloaded manually.
I think my colleagues are going to like it, so I anticipate there are going to be few more packages added to the job.I t should be quick to write because it saves us only about 20 minutes of work a week. So I should not waste a whole manday on it. Also the files downloaded are small and my network is fast so i dont worry too much about download the files one after another etc.
OK,lets break it down to smaller pieces:
As you said the core functionality is to download and to extract the file. two different problems so I am going to address them in (at least two) different functions.
The basic parameters that a donwload function should accept is probably a link to the file and the destination path (defaulting to users download folder). If the operation is succesfull, file object is returned, if it fails nothing is returned in the output (mimicking behaviour of the standard cmdlets like get-item), errors are output to the error output. I am not familiar with the exact behavior of the web client so teke it with grain of salt .)
I refer here for the info on exceptions http://msdn.microsoft.com/cs-cz/library/ez801hhe.aspx . Seems like there is not many exceptions we can do something about - If the address provided is wrong there is no way for this function to fix it.define script parameters [string]$url, destination, force
validate everything you can (external commands etc) thaht would prevent the
script from running. if something is wrong and you cannot fix it (or it would take
too much effort) throw terminating error
if (-not (7zip is available)) {"no 7zip on this station cannot continue"}
etc.
function downloadFile ([string]$url, [string]$Destination = <userprofile>\downloads)
{
check if the destination is existing folder,
Foreach item in $url
{
download file
void all unnecessary output
if the file object is not returned by the web
client function by default, get-item the resulting path
to return the file object
if the download ar saving the file
fails return nothing to the standard, non terminating
errors are output to error output
}
}
function extractFile ([string]$Path, [string]$Destination=<userprofile>\downloads, [switch]$Force)
{
check if the destination path exists and if it is a container (folder)
if force is specified and the path does not exist create
the directory
foreach item in path
{
check if the file exists. check if it has the right extension
if force is specified rewrite the files in the destination directory
function body
return appropriate objects
}
$destination
#this is not ideal approach, it makes the download run first and after
#all files are downloaded the extarct is run
#using jobs is better option especially for big files
$downloadedFiles = DownloadFile @psBoundParameters
extractFiles @psBoundParameters -path $downloadedFiles
}
hope this approach works