Bug 210446 - delete-stale-build-files: Clean up sandbox and app caches
Summary: delete-stale-build-files: Clean up sandbox and app caches
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jonathan Bedard
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-04-13 12:17 PDT by Jonathan Bedard
Modified: 2020-04-17 12:45 PDT (History)
5 users (show)

See Also:


Attachments
Patch (1.26 KB, patch)
2020-04-13 12:24 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (1.23 KB, patch)
2020-04-13 14:50 PDT, Jonathan Bedard
ap: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Bedard 2020-04-13 12:17:46 PDT
There are a number of sandboxes and app caches in /var/folders which can get quite large when repeatedly running tests. We should remove those along with stale build files.
Comment 1 Jonathan Bedard 2020-04-13 12:18:11 PDT
<rdar://problem/54613619>
Comment 2 Jonathan Bedard 2020-04-13 12:24:13 PDT
Created attachment 396315 [details]
Patch
Comment 3 Geoffrey Garen 2020-04-13 12:37:37 PDT
Comment on attachment 396315 [details]
Patch

r=me
Comment 4 Alexey Proskuryakov 2020-04-13 12:45:35 PDT
Comment on attachment 396315 [details]
Patch

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

> Tools/BuildSlaveSupport/delete-stale-build-files:80
> +        '/usr/bin/find', '/var/folders', '-type', 'd', '-name', '*WebKit*',

This regex is very permissive, chances are that it will get too much. 

Also, aren’t error messages printed to inaccessible directories?
Comment 5 Alexey Proskuryakov 2020-04-13 13:34:01 PDT
Comment on attachment 396315 [details]
Patch

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

>> Tools/BuildSlaveSupport/delete-stale-build-files:80
>> +        '/usr/bin/find', '/var/folders', '-type', 'd', '-name', '*WebKit*',
> 
> This regex is very permissive, chances are that it will get too much. 
> 
> Also, aren’t error messages printed to inaccessible directories?

about inaccessible directories
Comment 6 Alexey Proskuryakov 2020-04-13 13:41:15 PDT
Wrong radar was associated with this. Correct one:

rdar://problem/61269473
Comment 7 Jonathan Bedard 2020-04-13 13:42:27 PDT
(In reply to Alexey Proskuryakov from comment #5)
> Comment on attachment 396315 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=396315&action=review
> 
> >> Tools/BuildSlaveSupport/delete-stale-build-files:80
> >> +        '/usr/bin/find', '/var/folders', '-type', 'd', '-name', '*WebKit*',
> > 
> > This regex is very permissive, chances are that it will get too much. 

I could narrow the regex to only include WebKitTestRunner, DumpRenderTree and sandboxes.

On the other hand, everything that matched that regex on the bot seemed like something we wanted to delete. between test runs.

> > 
> > Also, aren’t error messages printed to inaccessible directories?
> 
> about inaccessible directories

Yes, that's why I'm not passing any arguments to subprocess, by default, stdout and stderr are printed to the parent process's stdout and stderr. Although interestingly, there are not inaccessible directories on the bots.
Comment 8 Alexey Proskuryakov 2020-04-13 13:49:57 PDT
Both findings are surprising to me. When I run this command on my development machine, there are a LOT of inaccessible directories, and many directories are matched that should not be. E.g. T/com.apple.Safari/WebKit, T/com.apple.Safari.CacheDeleteExtension/WebKit, T/WebKitPlugin-71Fvu9, T/com.apple.WebKit.WebContent+com.apple.Safari, C/com.apple.WebKit.WebContent+com.apple.finder/com.apple.WebKit.WebContent.

In fact, why even "sudo rm"? Directories that we want to delete are owned by the buildbot user.
Comment 9 Jonathan Bedard 2020-04-13 14:13:56 PDT
(In reply to Alexey Proskuryakov from comment #8)
> ...
> 
> In fact, why even "sudo rm"? Directories that we want to delete are owned by
> the buildbot user.

Actually, that's a good point. And to take it even further, we shouldn't run find with sudo either. The only catch (and the reason I used sudo in the first place) is that find will return with a non-zero exit code when it can't traverse a directory.
Comment 10 Jonathan Bedard 2020-04-13 14:50:15 PDT
Created attachment 396336 [details]
Patch
Comment 11 Alexey Proskuryakov 2020-04-13 15:00:00 PDT
Comment on attachment 396336 [details]
Patch

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

> Tools/BuildSlaveSupport/delete-stale-build-files:79
> +        '/usr/bin/find', '/var/folders', '-type', 'd', '-name', '*WebKit*',

I still think that this need to be more specific.
Comment 12 Jonathan Bedard 2020-04-13 15:39:32 PDT
Discussed with Alexey, this makes more sense as a Buildbot step.
Comment 13 Alexey Proskuryakov 2020-04-17 12:45:45 PDT
Comment on attachment 396336 [details]
Patch

Marking r- per the above.