Bug 82996 - Chromium testers should extract builds instead of building on their own
Summary: Chromium testers should extract builds instead of building on their own
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-02 19:36 PDT by Ryosuke Niwa
Modified: 2012-04-23 00:00 PDT (History)
7 users (show)

See Also:

Attachments
Patch (13.74 KB, patch)
2012-04-02 20:03 PDT, Ryosuke Niwa 		tony: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2012-04-02 19:36:28 PDT 
Chromium testers should extract builds instead of building on their own
Comment 1 Ryosuke Niwa 2012-04-02 20:03:08 PDT 
Created attachment 135262 [details]
Patch
Comment 2 Ryosuke Niwa 2012-04-02 20:05:25 PDT 
Comment on attachment 135262 [details]
Patch

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

> Tools/BuildSlaveSupport/download-built-product:39
> +    parser.add_option("--platform", dest="platform")
> +    parser.add_option("--debug", action="store_const", const="debug", dest="configuration")
> +    parser.add_option("--release", action="store_const", const="release", dest="configuration")

Note that platform and configuration are not used at the moment. They're here for the forward compatibility.
Comment 3 Tony Chang 2012-04-03 11:35:43 PDT 
Comment on attachment 135262 [details]
Patch

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

> Tools/BuildSlaveSupport/built-product-archive:219
> +    elif platform == 'chromium':
> +        removeDirectoryIfExists(configurationBuildDirectory)
> +        os.makedirs(configurationBuildDirectory)
> +        return unzipArchive(configurationBuildDirectory, configuration)

This is the same as the qt, gtk, efl branch.  Can we merge them?  Would also be nice to do the removeDir/makedirs before the if.
Comment 4 Ryosuke Niwa 2012-04-03 11:55:16 PDT 
Thanks for the review.

(In reply to comment #3)
> This is the same as the qt, gtk, efl branch.  Can we merge them?  Would also be nice to do the removeDir/makedirs before the if.

Will do.
Comment 5 Ryosuke Niwa 2012-04-03 11:58:26 PDT 
Committed r113067: <http://trac.webkit.org/changeset/113067>
Comment 6 Csaba Osztrogonác 2012-04-03 12:28:32 PDT 
Reopen, because it broke our Qt ARM tester bot on http://build.webkit.sed.hu

http://build.webkit.sed.hu/builders/ARMv5%20Linux%20Qt%20Release%20%28Test%29/builds/6274

It's too late for me now, but I can check it tomorrow what happened.
Comment 7 Ryosuke Niwa 2012-04-03 12:48:20 PDT 
(In reply to comment #6)
> Reopen, because it broke our Qt ARM tester bot on http://build.webkit.sed.hu
> 
> http://build.webkit.sed.hu/builders/ARMv5%20Linux%20Qt%20Release%20%28Test%29/builds/6274
> 
> It's too late for me now, but I can check it tomorrow what happened.

Yeah, addressing Tony's comment (calling removeDirectoryIfExists outside if) resulted in some bug :(  It should be fixed in http://trac.webkit.org/changeset/113071.
Comment 8 Csaba Osztrogonác 2012-04-03 12:51:42 PDT 
(In reply to comment #7)
> Yeah, addressing Tony's comment (calling removeDirectoryIfExists outside if) resulted in some bug :(  It should be fixed in http://trac.webkit.org/changeset/113071.

Unfortunately it didn't fix.

Traceback (most recent call last):
  File "./Tools/BuildSlaveSupport/built-product-archive", line 218, in <module>
    sys.exit(main())
  File "./Tools/BuildSlaveSupport/built-product-archive", line 61, in main
    return extractBuiltProduct(options.configuration, options.platform)
  File "./Tools/BuildSlaveSupport/built-product-archive", line 213, in extractBuiltProduct
    os.makedirs(configurationBuildDirectory)
  File "/usr/lib/python2.6/os.py", line 157, in makedirs
    mkdir(name, mode)
OSError: [Errno 17] File exists: '/home/webkitbuildbot/slaves/armReleaseTest/buildslave/arm-qt-linux-release-arm-test/build/WebKitBuild/Release'
Comment 9 Csaba Osztrogonác 2012-04-03 12:53:47 PDT 
Oh, I think I should modify our master.cfg too after this change - http://trac.webkit.org/changeset/113067/trunk/Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg
Comment 10 Ryosuke Niwa 2012-04-03 13:16:05 PDT 
(In reply to comment #9)
> Oh, I think I should modify our master.cfg too after this change - http://trac.webkit.org/changeset/113067/trunk/Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg

Ah, yes. You may need to adjust your master.cfg if you're using WebKIt's BuildSlaveSupport scripts.
Comment 11 Csaba Osztrogonác 2012-04-03 13:36:10 PDT 
I fixed our master.cfg and it works now.