#!/usr/bin/python build-webkit should collect Visual Studio Express logs and display them Here is one example script, written in python: #!/usr/bin/python from __future__ import with_statement import codecs import os.path import os webkit_root = "/mnt/svn/webkit-win-ews" build_root = os.path.join(webkit_root, "WebKitBuild") obj_root = os.path.join(build_root, "obj") build_log_paths = [] for dirpath, dirnames, filenames in os.walk(obj_root): for file_name in filenames: if file_name == "BuildLog.htm": file_path = os.path.join(dirpath, file_name) build_log_paths.append(file_path) for build_log_path in build_log_paths: with codecs.open(build_log_path, "r", "utf-16") as build_log: print build_log.read() Ideally we could wire up a hook in build-webkit itself to have them print to the console as they're generated. But at least we need a script like the one above for the win-ews.
Created attachment 69313 [details] Patch
I've not tested this yet, but I'm about to. I'm very interested in thoughts from folks who actually use windows! We need something like this to make the win-ews useful. Yes, I know it just dumps the HTML, but that's *way* better than nothing.
Comment on attachment 69313 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=69313&action=review > WebKitTools/Scripts/print-vse-failure-logs:69 > + with codecs.open(build_log_path, "r", "utf-16") as build_log: for reals?
I have slight worries about destroying someone's build environment, but I'm not sure how to proceed except to try.
Comment on attachment 69313 [details] Patch Actually, lets leave this r? until the win-ews gets a chance. :) That's the simplest way to test this patch!
Comment on attachment 69313 [details] Patch Once the win-ews approves this patch, it can be cq+'d
Created attachment 69394 [details] patch with intentional compile failure for testing the win-ews
Attachment 69394 [details] did not build on qt: Build output: http://queues.webkit.org/results/4162036
Attachment 69394 [details] did not build on gtk: Build output: http://queues.webkit.org/results/4216037
Attachment 69394 [details] did not build on mac: Build output: http://queues.webkit.org/results/4244003
Attachment 69394 [details] did not build on chromium: Build output: http://queues.webkit.org/results/4141042
Comment on attachment 69394 [details] patch with intentional compile failure for testing the win-ews View in context: https://bugs.webkit.org/attachment.cgi?id=69394&action=review Looks great -- I'd love to try it out! > WebCore/page/Frame.cpp:-201 > -} Did you mean to include this fix for WebCore/page/Frame.cpp?
Yes, I'm trying to test the win-ews failing. :)
Attachment 69394 [details] did not build on win: Build output: http://queues.webkit.org/results/4222023
Can't exec "WebKitTools/Scripts/print-vse-failure-logs": No such file or directory at WebKitTools/Scripts/build-webkit line 482. :(
Comment on attachment 69394 [details] patch with intentional compile failure for testing the win-ews It looks like you got your intention failure that the patch was intended to provoke.
Comment on attachment 69313 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=69313&action=review r- to fix the issues in the Perl code. Otherwise it looks good. > WebKitTools/Scripts/build-webkit:479 > + system("$scriptDir/print-vse-failure-logs"); This needs to use File::Spec->catfile() now: system(File::Spec->catfile($scriptDir, "print-vse-failure-logs")); > WebKitTools/Scripts/print-vse-failure-logs:53 > + print filenames Do you really want to print all of these out? > WebKitTools/Scripts/webkitdirs.pm:1193 > + return $willUseVCExpressWhenBuilding; You need to call determineConfigurationForVisualStudio() first to guarantee that this variable is set properly: determineConfigurationForVisualStudio() return $willUseVCExpressWhenBuilding;
(In reply to comment #17) > (From update of attachment 69313 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=69313&action=review > > r- to fix the issues in the Perl code. Otherwise it looks good. > > > WebKitTools/Scripts/build-webkit:479 > > + system("$scriptDir/print-vse-failure-logs"); > > This needs to use File::Spec->catfile() now: > > system(File::Spec->catfile($scriptDir, "print-vse-failure-logs")); Thanks! > > WebKitTools/Scripts/print-vse-failure-logs:53 > > + print filenames > > Do you really want to print all of these out? The whole idea with this script is to put something minimal in place that people who actually use windows can make better. :) > > WebKitTools/Scripts/webkitdirs.pm:1193 > > + return $willUseVCExpressWhenBuilding; > > You need to call determineConfigurationForVisualStudio() first to guarantee that this variable is set properly: > > determineConfigurationForVisualStudio() > return $willUseVCExpressWhenBuilding; Fixed. Thanks.
(In reply to comment #18) > (In reply to comment #17) > > > WebKitTools/Scripts/print-vse-failure-logs:53 > > > + print filenames > > > > Do you really want to print all of these out? > > The whole idea with this script is to put something minimal in place that people who actually use windows can make better. :) But this was infact left-over debug code. Removed. Thanks!
Created attachment 71964 [details] Fix Dave's suggestions
Comment on attachment 71964 [details] Fix Dave's suggestions Thanks! r=me
Comment on attachment 71964 [details] Fix Dave's suggestions Clearing flags on attachment: 71964 Committed r70607: <http://trac.webkit.org/changeset/70607>
All reviewed patches have been landed. Closing bug.
David Kilzer already reviewed this patch and it landed. Here are some remarks I had: View in context: https://bugs.webkit.org/attachment.cgi?id=71964&action=review > WebKitTools/ChangeLog:11 > + * Scripts/build-webkit: > + * Scripts/print-vse-failure-logs: Added. > + * Scripts/webkitdirs.pm: > + I suggest that you add a comment about the added function usingVisualStudioExpress() since prepare-ChangeLog does not generate a list of added/modified Perl functions. We should consider adding Perl support to prepare-ChangeLog. > WebKitTools/Scripts/print-vse-failure-logs:64 > + print "Found %s Visual Studio Express Build Logs:%s" % (len(build_log_paths), "\n".join(build_log_paths)) Take build_log_paths = ["A", "B"]. Then this line would print: Found 2 Visual Studio Express Build Logs:A B Notice, "A" prints on the first output line. I think it would be nicer to output something of the form: Found 2 Visual Studio Express Build Logs: A B Hence, I would add a new-line character ('\n') after "Logs:", such that the line reads: print "Found %s Visual Studio Express Build Logs:\n%s" % (len(build_log_paths), "\n".join(build_log_paths)). Do you foresee this command ever being called on its own and/or when there are zero BuildLog.htm files? If so, we may want to make the colon (':') after "Logs:" conditional on there being at least one path in the list build_log_paths. When we do not find any files whose name is BuildLog.htm then we output something of the form: Found 0 Visual Studio Express Build Logs. OR No Visual Studio Express Build Logs found. > WebKitTools/Scripts/webkitdirs.pm:1191 > + determineConfigurationForVisualStudio(); Notice, the variable willUseVCExpressWhenBuilding is only set in setupCygwinEnv(). And determineConfigurationForVisualStudio() only calls setupCygwinEnv() if we are building a debug build as per line 286 of webkitdirs.pm <http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitdirs.pm?rev=70188#L286>. So, we should call setupCygwinEnv() instead of determineConfigurationForVisualStudio(). Alternatively, we could make determineConfigurationForVisualStudio() always call setupCygwinEnv() (given determineConfigurationForVisualStudio() hasn't been called before; e.g. $configurationForVisualStudio is not defined).
(In reply to comment #24) > David Kilzer already reviewed this patch and it landed. Here are some remarks I had: Sorry about that Dan. I probably shouldn't have set the cq+ flag as Eric didn't request it.
Thank you dan and david for the reviews. @dan: Honestly this script is mostly a shot in the dark. I intended it to be half-baked. Someone who actually uses windows needs to make it work nice. But even as it is it's better than the current non-existant logs for VSE builds.
I may make further improvements to the script once I have the win-ews bot up and running with it. We'll see.
Committed r70720: <http://trac.webkit.org/changeset/70720>
I did a follow-up commit to fix some issues getting this to run on the win-ews bot. (The script was originally written on the win-ews bot, but I prepared the patch on my mac, and did final testing on my mac. I had planned to do one final test on the EWS bot but got distracted yesterday.) Thanks again for the reviews!
I am getting the error with Visual studio express edition setup.