105671 2012-12-21 17:04:29 -0800 chromium: webkit-build-directory doesn't handle ninja output dirs 2012-12-21 19:58:05 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 104434 1 webkit.review.bot webkit.review.bot dbates eric thakis oldest_to_newest 796555 0 webkit.review.bot 2012-12-21 17:04:29 -0800 chromum: webkit-build-directory doesn't handle ninja output dirs Requested by thakis_ on #webkit. 796556 1 thakis 2012-12-21 17:05:19 -0800 eseidel: BuildSlaveSupport/built-product-archive: _buildDirectory = subprocess.Popen(['perl', os.path.join(os.path.dirname(__file__), "..", "Scripts", "webkit-build-directory"), [5:00pm] eseidel: thakis_: it appears the webkit build bots seem to care about webkit-build-directory workign correctly 796559 2 thakis 2012-12-21 17:12:34 -0800 Actually, this appears to be working fine if --chromium is passed as argument. 796560 3 thakis 2012-12-21 17:13:25 -0800 …or does it. nevermind. 796561 4 thakis 2012-12-21 17:19:26 -0800 This needs a patch like this: --- a/Tools/Scripts/webkitdirs.pm +++ b/Tools/Scripts/webkitdirs.pm @@ -222,7 +222,7 @@ sub determineBaseProductDir undef $baseProductDir unless $baseProductDir =~ /^\//; } elsif (isChromium()) { - if (isLinux() || isChromiumAndroid() || isChromiumMacMake()) { + if (isLinux() || isChromiumAndroid() || isChromiumMacMake() || isChromiumNinja()) { $baseProductDir = "$sourceDir/out"; …but isChromiumNinja() calls configuration() which in turn calls the function this patch is in, leading to infinite recursion. 796563 5 180577 thakis 2012-12-21 17:29:38 -0800 Created attachment 180577 Patch 796564 6 thakis 2012-12-21 17:29:56 -0800 Patch attempt. 796568 7 180577 dbates 2012-12-21 17:36:30 -0800 Comment on attachment 180577 Patch Looks sane to me. 796572 8 thakis 2012-12-21 17:42:58 -0800 tony^work: thakis_: yes, the webkit waterfalls use a builder / tester split [5:33pm] tony^work: thakis_: http://build.webkit.org/waterfall?category=Chromium [5:33pm] TabAtkins left the chat room. (Quit: Lost terminal) [5:34pm] psolanki left the chat room. (Read error: Connection reset by peer) [5:34pm] thakis_: tony^work: looks like stuff always gets unzipped into webkitbuild, so that probably Just Works [5:34pm] tony^work: thakis_: All the code for zipping/unzipping should be in Tools/BuildSlaveSupport [5:34pm] thakis_: as both xcodebuild and out end up in the same place on the test tony^work: we probably need to teach the zipping script to zip the right files tony^work: that's the built-product-archive script, I think [5:38pm] thakis_: dydz: thanks! [5:38pm] tony^work: Tools/BuildSlaveSupport/built-product-archive [5:39pm] thakis_: tony^work: that just calls out to webkit-build-directory, which https://bugs.webkit.org/show_bug.cgi?id=105671 is fixing …sounds like this patch helps with the builder/tester split, too. 796574 9 180577 webkit.review.bot 2012-12-21 18:13:45 -0800 Comment on attachment 180577 Patch Clearing flags on attachment: 180577 Committed r138406: <http://trac.webkit.org/changeset/138406> 796575 10 webkit.review.bot 2012-12-21 18:13:49 -0800 All reviewed patches have been landed. Closing bug. 796578 11 180577 eric 2012-12-21 18:36:17 -0800 Comment on attachment 180577 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=180577&action=review > Tools/Scripts/webkitdirs.pm:1207 > + # This function can be called from baseProductDir(), which in turn is > + # called by configuration(). So calling configuration() here leads to > + # infinite recursion. Gyp writes both Debug and Release at the same time > + # by default, so just check the timestamp on the Release build.ninja file. > + my $config = "Release"; This was what had held me up before. Neat to see we only need Release here. 796600 12 eric 2012-12-21 19:58:05 -0800 I think this is a pretty elegant solution and I'm no longer worried about the build-webkit --chromium switch to ninja. I think that's OK to fire whenever. I'm happy to commit it any time after next Tuesday (since it's now vacation time). :) 180577 2012-12-21 17:29:38 -0800 2012-12-21 18:36:17 -0800 Patch bug-105671-20121221172705.patch text/plain 2001 thakis U3VidmVyc2lvbiBSZXZpc2lvbjogMTM4MzMyCmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggNGE5N2NjYWI1ZWVkN2VmOGJmOTZjNWVlODU4ZTM0MDM3 Mjk2MmY0NS4uMTZmYWRiYzc1OGYyODEwMTAxMDY0ODcyMDFmZTYwNjUyODY1YmVlOSAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2 IEBACisyMDEyLTEyLTIxICBOaWNvIFdlYmVyICA8dGhha2lzQGNocm9taXVtLm9yZz4KKworICAg ICAgICBjaHJvbWl1bTogd2Via2l0LWJ1aWxkLWRpcmVjdG9yeSBkb2Vzbid0IGhhbmRsZSBuaW5q YSBvdXRwdXQgZGlycworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9MTA1NjcxCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAg ICAgICAgVGVhY2ggd2Via2l0LWJ1aWxkLWRpcmVjdG9yeSB0byBkZXRlY3QgdGhlIG5pbmphIGJ1 aWxkIGRpcmVjdG9yeS4KKworICAgICAgICAqIFNjcmlwdHMvd2Via2l0ZGlycy5wbToKKyAgICAg ICAgKGRldGVybWluZUJhc2VQcm9kdWN0RGlyKToKKyAgICAgICAgKGRldGVybWluZUlzQ2hyb21p dW1OaW5qYSk6CisKIDIwMTItMTItMjAgIFJ5dWFuIENob2kgIDxyeXVhbi5jaG9pQHNhbXN1bmcu Y29tPgogCiAgICAgICAgIFtFRkxdIEJ1aWxkIGJyZWFrIHdpdGggbGF0ZXN0IEVGTCBsaWJyYXJp ZXMuCmRpZmYgLS1naXQgYS9Ub29scy9TY3JpcHRzL3dlYmtpdGRpcnMucG0gYi9Ub29scy9TY3Jp cHRzL3dlYmtpdGRpcnMucG0KaW5kZXggNWEyZTA0NDgwMDZhYjE1NDY0MTFiYWZjMDNjNWZjZWNh MTYzZWU1Ny4uM2E1OTZmMjQ0YzM4MTRlZDczODUwMWUzMmU0OTFhYjE3ZWFhYTIyNyAxMDA3NTUK LS0tIGEvVG9vbHMvU2NyaXB0cy93ZWJraXRkaXJzLnBtCisrKyBiL1Rvb2xzL1NjcmlwdHMvd2Vi a2l0ZGlycy5wbQpAQCAtMjIyLDcgKzIyMiw3IEBAIHN1YiBkZXRlcm1pbmVCYXNlUHJvZHVjdERp cgogCiAgICAgICAgIHVuZGVmICRiYXNlUHJvZHVjdERpciB1bmxlc3MgJGJhc2VQcm9kdWN0RGly ID1+IC9eXC8vOwogICAgIH0gZWxzaWYgKGlzQ2hyb21pdW0oKSkgewotICAgICAgICBpZiAoaXNM aW51eCgpIHx8IGlzQ2hyb21pdW1BbmRyb2lkKCkgfHwgaXNDaHJvbWl1bU1hY01ha2UoKSkgewor ICAgICAgICBpZiAoaXNMaW51eCgpIHx8IGlzQ2hyb21pdW1BbmRyb2lkKCkgfHwgaXNDaHJvbWl1 bU1hY01ha2UoKSB8fCBpc0Nocm9taXVtTmluamEoKSkgewogICAgICAgICAgICAgJGJhc2VQcm9k dWN0RGlyID0gIiRzb3VyY2VEaXIvb3V0IjsKICAgICAgICAgfSBlbHNpZiAoaXNEYXJ3aW4oKSkg ewogICAgICAgICAgICAgJGJhc2VQcm9kdWN0RGlyID0gIiRzb3VyY2VEaXIvU291cmNlL1dlYktp dC9jaHJvbWl1bS94Y29kZWJ1aWxkIjsKQEAgLTEyMDAsNyArMTIwMCwxMSBAQCBzdWIgZGV0ZXJt aW5lSXNDaHJvbWl1bU5pbmphKCkKIHsKICAgICByZXR1cm4gaWYgZGVmaW5lZCgkaXNDaHJvbWl1 bU5pbmphKTsKIAotICAgIG15ICRjb25maWcgPSBjb25maWd1cmF0aW9uKCk7CisgICAgIyBUaGlz IGZ1bmN0aW9uIGNhbiBiZSBjYWxsZWQgZnJvbSBiYXNlUHJvZHVjdERpcigpLCB3aGljaCBpbiB0 dXJuIGlzCisgICAgIyBjYWxsZWQgYnkgY29uZmlndXJhdGlvbigpLiBTbyBjYWxsaW5nIGNvbmZp Z3VyYXRpb24oKSBoZXJlIGxlYWRzIHRvCisgICAgIyBpbmZpbml0ZSByZWN1cnNpb24uIEd5cCB3 cml0ZXMgYm90aCBEZWJ1ZyBhbmQgUmVsZWFzZSBhdCB0aGUgc2FtZSB0aW1lCisgICAgIyBieSBk ZWZhdWx0LCBzbyBqdXN0IGNoZWNrIHRoZSB0aW1lc3RhbXAgb24gdGhlIFJlbGVhc2UgYnVpbGQu bmluamEgZmlsZS4KKyAgICBteSAkY29uZmlnID0gIlJlbGVhc2UiOwogCiAgICAgbXkgJGhhc1Vw VG9EYXRlTmluamFidWlsZCA9IDA7CiAgICAgaWYgKC1lICJvdXQvJGNvbmZpZy9idWlsZC5uaW5q YSIpIHsK