[WinCairo][EWS] Build bot should clean user temp directory each time.

[Ross Kirsling]

[WinCairo][EWS] Build bot should clean user temp directory each time.

[Ross Kirsling]

Created attachment 338276 [details]
Patch

[Ross Kirsling]

Created attachment 338288 [details]
Patch

[Ross Kirsling]

*** Bug 182371 has been marked as a duplicate of this bug. ***

[Ross Kirsling]

*** Bug 182758 has been marked as a duplicate of this bug. ***

[Lucas Forschler]

Comment on attachment 338288 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=338288&action=review

I'm not a powershell expert, but this otherwise seems fine.

> Tools/EWSTools/Start-Queue.ps1:58
> +    Write-Host 'Cleaning user temp directory';

I would output the temp folder here, just to make it clear what directory you are working on.

> Tools/EWSTools/Start-Queue.ps1:67
> +        $filesRemoved++;

if $file is a directory, it looks like you'll attempt to remove it recursively, but only increment $filesremoved once... so this count may not be accurate. Otherwise, I'm not sure what the purpose of the -Recurse is here on the Remove-Item line.

> Tools/EWSTools/Start-Queue.ps1:68
> +        Write-Host ('Removed file {0}' -f  $file.FullName)

This is going to be pretty verbose... do you actually want this list? Or was this just for debugging?

[Ross Kirsling]

(In reply to Lucas Forschler from comment #5)
> Comment on attachment 338288 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=338288&action=review
> 
> I'm not a powershell expert, but this otherwise seems fine.
> 
> > Tools/EWSTools/Start-Queue.ps1:58
> > +    Write-Host 'Cleaning user temp directory';
> 
> I would output the temp folder here, just to make it clear what directory
> you are working on.

Good idea.

> > Tools/EWSTools/Start-Queue.ps1:67
> > +        $filesRemoved++;
> 
> if $file is a directory, it looks like you'll attempt to remove it
> recursively, but only increment $filesremoved once... so this count may not
> be accurate. Otherwise, I'm not sure what the purpose of the -Recurse is
> here on the Remove-Item line.
> 
> > Tools/EWSTools/Start-Queue.ps1:68
> > +        Write-Host ('Removed file {0}' -f  $file.FullName)
> 
> This is going to be pretty verbose... do you actually want this list? Or was
> this just for debugging?

This was mostly just a copy-paste-and-tweak of Don's function from here:
https://github.com/WebKitForWindows/powershell-webkit-dev/blob/master/WebKitDev/Functions/Remove-TempFiles.ps1

But I agree that this could be simplified a bit.

[Ross Kirsling]

Created attachment 338305 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 338305 [details]
Patch

Clearing flags on attachment: 338305

Committed r230796: <https://trac.webkit.org/changeset/230796>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/39554382>