Bug 184761

Summary: [WinCairo][EWS] Build bot should clean user temp directory each time.
Product: WebKit Reporter: Ross Kirsling <ross.kirsling>
Component: New BugsAssignee: Ross Kirsling <ross.kirsling>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, commit-queue, don.olmstead, jbedard, lforschler, pvollan, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=182758
https://bugs.webkit.org/show_bug.cgi?id=187725
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Ross Kirsling 2018-04-18 16:25:31 PDT
[WinCairo][EWS] Build bot should clean user temp directory each time.
Comment 1 Ross Kirsling 2018-04-18 16:26:15 PDT
Created attachment 338276 [details]
Patch
Comment 2 Ross Kirsling 2018-04-18 17:40:03 PDT
Created attachment 338288 [details]
Patch
Comment 3 Ross Kirsling 2018-04-18 17:47:34 PDT
*** Bug 182371 has been marked as a duplicate of this bug. ***
Comment 4 Ross Kirsling 2018-04-18 17:47:45 PDT
*** Bug 182758 has been marked as a duplicate of this bug. ***
Comment 5 Lucas Forschler 2018-04-18 18:54:09 PDT
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?
Comment 6 Ross Kirsling 2018-04-18 21:18:48 PDT
(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.
Comment 7 Ross Kirsling 2018-04-18 21:21:38 PDT
Created attachment 338305 [details]
Patch
Comment 8 WebKit Commit Bot 2018-04-18 22:38:58 PDT
Comment on attachment 338305 [details]
Patch

Clearing flags on attachment: 338305

Committed r230796: <https://trac.webkit.org/changeset/230796>
Comment 9 WebKit Commit Bot 2018-04-18 22:39:00 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2018-04-18 22:39:21 PDT
<rdar://problem/39554382>