New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Automate deletion of temporary files on windows #4796
Automate deletion of temporary files on windows #4796
Conversation
EWS run on previous version of this PR (hash eb48698) |
name = 'delete-stale-build-files' | ||
description = ['Deleting stale build files'] | ||
descriptionDone = ['Deleted stale build files'] | ||
command = ['python3', 'Tools/CISupport/delete-stale-build-files', WithProperties('--platform=%(fullPlatform)s')] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete-stale-build-files
looks like it's expecting --platform
to be win
. Does fullPlatform
report win
on Windows or a "fuller" double like win32-x64
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, fullPlatform is "win". It can be verified in any ews build like https://ews-build.webkit.org/#/builders/10/builds/158586 (in properties tab)
@@ -86,6 +91,21 @@ def webkitBuildDirectory(platform, fullPlatform, configuration): | |||
return subprocess.Popen(['perl', os.path.join(os.path.dirname(__file__), "..", "Scripts", "webkit-build-directory"), | |||
"--" + platform, "--" + configuration, '--top-level'], stdout=subprocess.PIPE).communicate()[0].strip() | |||
|
|||
def deleteWindowsStaleFiles(): | |||
directoriesToDelete = ['/tmp', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blindly deleting /tmp
is not a good idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does /tmp
exist on Windows? Would temp not be something like /cygdrive/c/buildbot/AppData/local/Temp
? Given there is no "root" (rather only drive mount points) on NTFS.
def deleteWindowsStaleFiles(): | ||
directoriesToDelete = ['/tmp', | ||
'/cygdrive/c/Program Files (x86)/Windows Kits/10/Debuggers/x64/sym/WebKit.pdb', | ||
'/cygdrive/c/Program Files (x86)/Windows Kits/10/Debuggers/x64/sym/WTF.pdb', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's possible to determine the Debuggers
folder with environment variable. It will make this good resilient to updates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason not to just delete all the Debuggers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No reason, I wasn't sure if it's fine to delete it totally (e.g.: not sure if those files are used as a cache to speed up subsequent builds). Those were the directories which were large (in GBs, rest all were smaller). e.g.:
buildbot@ews210 /cygdrive/c/Program Files (x86)/Windows Kits/10/Debuggers/x64/sym
$ du -hs * | grep G
21G DumpRenderTreeLib.pdb
21G JavaScriptCore.pdb
4.1G WTF.pdb
133G WebKit.pdb
eb48698
to
1c584cf
Compare
EWS run on current version of this PR (hash 1c584cf) |
Tested in https://ews-build.webkit-uat.org/#/builders/13/builds/4316 (step 5). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on a slack conversation, it may be worth investigating how we can disable the generation of unnecessary symbol files at build time, but with how frequently our Windows bots are running out of space and requiring manual intervention this seems like a good stopgap solution.
https://bugs.webkit.org/show_bug.cgi?id=245786 rdar://72849252 Reviewed by Ryan Haddad. * Tools/CISupport/delete-stale-build-files: * Tools/CISupport/ews-build/factories.py: * Tools/CISupport/ews-build/steps.py: (DeleteStaleBuildFiles): * Tools/CISupport/ews-build/steps_unittest.py: Added unit-tests. * Tools/CISupport/ews-build/factories_unittest.py: Updated unit-tests. Canonical link: https://commits.webkit.org/255045@main
1c584cf
to
a7ec0ec
Compare
Committed 255045@main (a7ec0ec): https://commits.webkit.org/255045@main Reviewed commits have been landed. Closing PR #4796 and removing active labels. |
This was reverted in https://bugs.webkit.org/show_bug.cgi?id=245911, since it breaks configurations where this script is invoked with python 2 |
Re-landing in #5037 |
a7ec0ec
1c584cf