66361 2011-08-16 21:26:28 -0700 Pass --makeargs of build-webkit to chromium linux builder 2011-08-22 10:49:43 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 0 wangxianzhu webkit-unassigned aroben eric evan ossy webkit.review.bot oldest_to_newest 452107 0 wangxianzhu 2011-08-16 21:26:28 -0700 Sometimes we need to override the default -j$numCpus with --makeargs, for example, in a distributed-building environment. 452110 1 104145 wangxianzhu 2011-08-16 21:29:38 -0700 Created attachment 104145 patch 452372 2 104145 eric 2011-08-17 11:06:45 -0700 Comment on attachment 104145 patch Why? Normally all args not understood by build-webkit are passed along to the build tool. For example build-webkit --various-xcode-arguments --another-xcode-arg WHATEVER_XCODE_ARG is very common 452799 3 wangxianzhu 2011-08-17 18:50:13 -0700 (In reply to comment #2) > (From update of attachment 104145 [details]) > Why? Normally all args not understood by build-webkit are passed along to the build tool. > > For example > > build-webkit --various-xcode-arguments --another-xcode-arg WHATEVER_XCODE_ARG > > is very common Currently there are different ways to pass additional builder parameters for the buildXXX functions in webkitdirs.pm: 1. buildAutoToolsProject, buildQMakeProject, buildCMakeProjectOrExit use --makeargs to accept additional parameters to make, while pass other unknown parameters (and also --qmakeargs for buildQMakeProject) to configuration tools like autotools, qmake or cmake. 2. buildXCodeProject, buildWafProject directly pass unknown parameters to the build tool. 3. Other buildXXXs just ignore --makeargs or unknown parameters, like buildChromiumMakefile, buildChromiumVisualStudioProject, buildVisualStudioProject. It seems reasonable to me to follow the convention of the 'make' family (above 1) for buildChromiumMakefile, because it also uses 'make'. 453091 4 104145 tony 2011-08-18 09:47:07 -0700 Comment on attachment 104145 patch Whenever I need to pass args to make, I just run make directly. make -fMakefile.chromium <other args go here> doesn't seem like that much work to me. 453526 5 wangxianzhu 2011-08-18 18:06:30 -0700 (In reply to comment #4) > (From update of attachment 104145 [details]) > Whenever I need to pass args to make, I just run make directly. make -fMakefile.chromium <other args go here> doesn't seem like that much work to me. Yes, I'm also doing this. However, 'build-webkit --help' shows --makeargs=<arguments> Optional Makefile flags buildChromiumMakefile is the only buildXXX that uses Makefile but doesn't support the option, so I suggest to fix it. 453883 6 104145 tony 2011-08-19 09:47:32 -0700 Comment on attachment 104145 patch View in context: https://bugs.webkit.org/attachment.cgi?id=104145&action=review It seems fine to take this patch since many other ports use --makeargs. > Tools/Scripts/webkitdirs.pm:1834 > + my $makeArgs = "-j$numCpus"; Nit: We end up add -j$numCpus below so you don't need to set the value here. > Tools/Scripts/webkitdirs.pm:1839 > + for my $i (0 .. $#options) { > + if ($options[$i] =~ /^--makeargs=(.*)/i ) { > + $makeArgs = $1; > + } > + } This could be: for (@options) { $makeArgs = $1 if /^--makeargs=(.*)/; } 454305 7 104614 wangxianzhu 2011-08-19 21:17:41 -0700 Created attachment 104614 new patch View in context: https://bugs.webkit.org/attachment.cgi?id=104145&action=review >> Tools/Scripts/webkitdirs.pm:1834 >> + my $makeArgs = "-j$numCpus"; > > Nit: We end up add -j$numCpus below so you don't need to set the value here. Sorry this was my mistake that I forgot to remove the latter assignment. >> Tools/Scripts/webkitdirs.pm:1839 >> + } > > This could be: > for (@options) { > $makeArgs = $1 if /^--makeargs=(.*)/; > } Thanks. Done. 454684 8 104614 webkit.review.bot 2011-08-22 10:49:37 -0700 Comment on attachment 104614 new patch Clearing flags on attachment: 104614 Committed r93515: <http://trac.webkit.org/changeset/93515> 454686 9 webkit.review.bot 2011-08-22 10:49:43 -0700 All reviewed patches have been landed. Closing bug. 104145 2011-08-16 21:29:38 -0700 2011-08-19 21:17:41 -0700 patch patch66361 text/plain 2715 wangxianzhu SW5kZXg6IFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBUb29scy9DaGFuZ2VMb2cJKHJl dmlzaW9uIDkzMTkzKQorKysgVG9vbHMvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMg KzEsMTUgQEAKKzIwMTEtMDgtMTYgIFhpYW56aHUgV2FuZyAgPHdhbmd4aWFuemh1QGNocm9taXVt Lm9yZz4KKworICAgICAgICBQYXNzIC0tbWFrZWFyZ3Mgb2YgYnVpbGQtd2Via2l0IHRvIGNocm9t aXVtIGxpbnV4IGJ1aWxkZXIKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTY2MzYxCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisK KyAgICAgICAgKiBTY3JpcHRzL2J1aWxkLXdlYmtpdDoKKyAgICAgICAgKiBTY3JpcHRzL3dlYmtp dGRpcnMucG06CisgICAgICAgIChidWlsZENocm9taXVtTWFrZWZpbGUpOgorICAgICAgICAoYnVp bGRDaHJvbWl1bSk6CisKIDIwMTEtMDgtMTYgIEFkYW0gQmFydGggIDxhYmFydGhAd2Via2l0Lm9y Zz4KIAogICAgICAgICBBZGQgYSBDRyBxdWFsaWZpZXIgc2ltaWxhciB0byB0aGUgR1BVIHF1YWxp ZmllcgpJbmRleDogVG9vbHMvU2NyaXB0cy9idWlsZC13ZWJraXQKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gVG9v bHMvU2NyaXB0cy9idWlsZC13ZWJraXQJKHJldmlzaW9uIDkzMTIxKQorKysgVG9vbHMvU2NyaXB0 cy9idWlsZC13ZWJraXQJKHdvcmtpbmcgY29weSkKQEAgLTU3OCw2ICs1NzgsNyBAQCBpZiAoaXND aHJvbWl1bSgpKSB7CiAgICAgQG9wdGlvbnMgPSBAQVJHVjsKICAgICAjIENocm9taXVtIGRvZXNu J3QgYnVpbGQgYnkgcHJvamVjdCBkaXJlY3Rvcmllcy4KICAgICBAcHJvamVjdHMgPSAoKTsKKyAg ICBwdXNoIEBvcHRpb25zLCAiLS1tYWtlYXJncz0iIC4gJG1ha2VBcmdzIGlmICRtYWtlQXJnczsK ICAgICBteSAkcmVzdWx0ID0gYnVpbGRDaHJvbWl1bSgkY2xlYW4sIEBvcHRpb25zKTsKICAgICBl eGl0IGV4aXRTdGF0dXMoJHJlc3VsdCkgaWYgZXhpdFN0YXR1cygkcmVzdWx0KTsKIH0KSW5kZXg6 IFRvb2xzL1NjcmlwdHMvd2Via2l0ZGlycy5wbQo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBUb29scy9TY3JpcHRz L3dlYmtpdGRpcnMucG0JKHJldmlzaW9uIDkzMTIxKQorKysgVG9vbHMvU2NyaXB0cy93ZWJraXRk aXJzLnBtCSh3b3JraW5nIGNvcHkpCkBAIC0xODIzLDE3ICsxODIzLDI0IEBAIHN1YiBidWlsZEd0 a1Byb2plY3QKICAgICByZXR1cm4gYnVpbGRBdXRvdG9vbHNQcm9qZWN0KCRwcm9qZWN0LCAkY2xl YW4sIEBidWlsZEFyZ3MpOwogfQogCi1zdWIgYnVpbGRDaHJvbWl1bU1ha2VmaWxlKCQkKQorc3Vi IGJ1aWxkQ2hyb21pdW1NYWtlZmlsZSgkJEApCiB7Ci0gICAgbXkgKCR0YXJnZXQsICRjbGVhbikg PSBAXzsKKyAgICBteSAoJHRhcmdldCwgJGNsZWFuLCBAb3B0aW9ucykgPSBAXzsKICAgICBpZiAo JGNsZWFuKSB7CiAgICAgICAgIHJldHVybiBzeXN0ZW0gcXcocm0gLXJmIG91dCk7CiAgICAgfQog ICAgIG15ICRjb25maWcgPSBjb25maWd1cmF0aW9uKCk7CiAgICAgbXkgJG51bUNwdXMgPSBudW1i ZXJPZkNQVXMoKTsKLSAgICBteSBAY29tbWFuZCA9ICgibWFrZSIsICItZk1ha2VmaWxlLmNocm9t aXVtIiwgIi1qJG51bUNwdXMiLCAiQlVJTERUWVBFPSRjb25maWciLCAkdGFyZ2V0KTsKLSAgICBw cmludCBqb2luKCIgIiwgQGNvbW1hbmQpIC4gIlxuIjsKLSAgICByZXR1cm4gc3lzdGVtIEBjb21t YW5kOworICAgIG15ICRtYWtlQXJncyA9ICItaiRudW1DcHVzIjsKKyAgICBmb3IgbXkgJGkgKDAg Li4gJCNvcHRpb25zKSB7CisgICAgICAgIGlmICgkb3B0aW9uc1skaV0gPX4gL14tLW1ha2Vhcmdz PSguKikvaSApIHsKKyAgICAgICAgICAgICRtYWtlQXJncyA9ICQxOworICAgICAgICB9CisgICAg fQorICAgICRtYWtlQXJncyA9ICItaiRudW1DcHVzIiBpZiBub3QgJG1ha2VBcmdzOworICAgIG15 ICRjb21tYW5kID0gIm1ha2UgLWZNYWtlZmlsZS5jaHJvbWl1bSAkbWFrZUFyZ3MgQlVJTERUWVBF PSRjb25maWcgJHRhcmdldCI7CisgICAgcHJpbnQgIiRjb21tYW5kXG4iOworICAgIHJldHVybiBz eXN0ZW0gJGNvbW1hbmQ7CiB9CiAKIHN1YiBidWlsZENocm9taXVtVmlzdWFsU3R1ZGlvUHJvamVj dCgkJCkKQEAgLTE4OTQsNyArMTkwMSw3IEBAIHN1YiBidWlsZENocm9taXVtKCRAKQogICAgICAg ICAkcmVzdWx0ID0gYnVpbGRDaHJvbWl1bVZpc3VhbFN0dWRpb1Byb2plY3QoIlNvdXJjZS9XZWJL aXQvY2hyb21pdW0vV2ViS2l0LnNsbiIsICRjbGVhbik7CiAgICAgfSBlbHNpZiAoaXNMaW51eCgp KSB7CiAgICAgICAgICMgTGludXggYnVpbGQgLSBidWlsZCB1c2luZyBtYWtlLgotICAgICAgICAk IHJlc3VsdCA9IGJ1aWxkQ2hyb21pdW1NYWtlZmlsZSgiYWxsIiwgJGNsZWFuKTsKKyAgICAgICAg JHJlc3VsdCA9IGJ1aWxkQ2hyb21pdW1NYWtlZmlsZSgiYWxsIiwgJGNsZWFuLCBAb3B0aW9ucyk7 CiAgICAgfSBlbHNlIHsKICAgICAgICAgcHJpbnQgU1RERVJSICJUaGlzIHBsYXRmb3JtIGlzIG5v dCBzdXBwb3J0ZWQgYnkgY2hyb21pdW0uXG4iOwogICAgIH0K 104614 2011-08-19 21:17:41 -0700 2011-08-22 10:49:37 -0700 new patch patch66361a text/plain 2672 wangxianzhu SW5kZXg6IFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBUb29scy9DaGFuZ2VMb2cJKHJl dmlzaW9uIDkzMzg4KQorKysgVG9vbHMvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMg KzEsMTUgQEAKKzIwMTEtMDgtMTkgIFhpYW56aHUgV2FuZyAgPHdhbmd4aWFuemh1QGNocm9taXVt Lm9yZz4KKworICAgICAgICBQYXNzIC0tbWFrZWFyZ3Mgb2YgYnVpbGQtd2Via2l0IHRvIGNocm9t aXVtIGxpbnV4IGJ1aWxkZXIKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTY2MzYxCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisK KyAgICAgICAgKiBTY3JpcHRzL2J1aWxkLXdlYmtpdDoKKyAgICAgICAgKiBTY3JpcHRzL3dlYmtp dGRpcnMucG06CisgICAgICAgIChidWlsZENocm9taXVtTWFrZWZpbGUpOgorICAgICAgICAoYnVp bGRDaHJvbWl1bSk6CisKIDIwMTEtMDgtMTggIERpcmsgUHJhbmtlICA8ZHByYW5rZUBjaHJvbWl1 bS5vcmc+CiAKICAgICAgICAgbmV3LXJ1bi13ZWJraXQtdGVzdHMgaHVuZyB3aGlsZSBhY3F1aXJp bmcgaHR0cCBsb2NrIG9uIHNub3cgbGVvcGFyZCBib3RzCkluZGV4OiBUb29scy9TY3JpcHRzL2J1 aWxkLXdlYmtpdAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09Ci0tLSBUb29scy9TY3JpcHRzL2J1aWxkLXdlYmtpdAkocmV2 aXNpb24gOTMzODgpCisrKyBUb29scy9TY3JpcHRzL2J1aWxkLXdlYmtpdAkod29ya2luZyBjb3B5 KQpAQCAtNTc4LDYgKzU3OCw3IEBAIGlmIChpc0Nocm9taXVtKCkpIHsKICAgICBAb3B0aW9ucyA9 IEBBUkdWOwogICAgICMgQ2hyb21pdW0gZG9lc24ndCBidWlsZCBieSBwcm9qZWN0IGRpcmVjdG9y aWVzLgogICAgIEBwcm9qZWN0cyA9ICgpOworICAgIHB1c2ggQG9wdGlvbnMsICItLW1ha2Vhcmdz PSIgLiAkbWFrZUFyZ3MgaWYgJG1ha2VBcmdzOwogICAgIG15ICRyZXN1bHQgPSBidWlsZENocm9t aXVtKCRjbGVhbiwgQG9wdGlvbnMpOwogICAgIGV4aXQgZXhpdFN0YXR1cygkcmVzdWx0KSBpZiBl eGl0U3RhdHVzKCRyZXN1bHQpOwogfQpJbmRleDogVG9vbHMvU2NyaXB0cy93ZWJraXRkaXJzLnBt Cj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT0KLS0tIFRvb2xzL1NjcmlwdHMvd2Via2l0ZGlycy5wbQkocmV2aXNpb24gOTMz ODgpCisrKyBUb29scy9TY3JpcHRzL3dlYmtpdGRpcnMucG0JKHdvcmtpbmcgY29weSkKQEAgLTE4 MjMsMTcgKzE4MjMsMjIgQEAgc3ViIGJ1aWxkR3RrUHJvamVjdAogICAgIHJldHVybiBidWlsZEF1 dG90b29sc1Byb2plY3QoJHByb2plY3QsICRjbGVhbiwgQGJ1aWxkQXJncyk7CiB9CiAKLXN1YiBi dWlsZENocm9taXVtTWFrZWZpbGUoJCQpCitzdWIgYnVpbGRDaHJvbWl1bU1ha2VmaWxlKCQkQCkK IHsKLSAgICBteSAoJHRhcmdldCwgJGNsZWFuKSA9IEBfOworICAgIG15ICgkdGFyZ2V0LCAkY2xl YW4sIEBvcHRpb25zKSA9IEBfOwogICAgIGlmICgkY2xlYW4pIHsKICAgICAgICAgcmV0dXJuIHN5 c3RlbSBxdyhybSAtcmYgb3V0KTsKICAgICB9CiAgICAgbXkgJGNvbmZpZyA9IGNvbmZpZ3VyYXRp b24oKTsKICAgICBteSAkbnVtQ3B1cyA9IG51bWJlck9mQ1BVcygpOwotICAgIG15IEBjb21tYW5k ID0gKCJtYWtlIiwgIi1mTWFrZWZpbGUuY2hyb21pdW0iLCAiLWokbnVtQ3B1cyIsICJCVUlMRFRZ UEU9JGNvbmZpZyIsICR0YXJnZXQpOwotICAgIHByaW50IGpvaW4oIiAiLCBAY29tbWFuZCkgLiAi XG4iOwotICAgIHJldHVybiBzeXN0ZW0gQGNvbW1hbmQ7CisgICAgbXkgJG1ha2VBcmdzOworICAg IGZvciAoQG9wdGlvbnMpIHsKKyAgICAgICAgJG1ha2VBcmdzID0gJDEgaWYgL14tLW1ha2Vhcmdz PSguKikvaTsKKyAgICB9CisgICAgJG1ha2VBcmdzID0gIi1qJG51bUNwdXMiIGlmIG5vdCAkbWFr ZUFyZ3M7CisgICAgbXkgJGNvbW1hbmQgPSAibWFrZSAtZk1ha2VmaWxlLmNocm9taXVtICRtYWtl QXJncyBCVUlMRFRZUEU9JGNvbmZpZyAkdGFyZ2V0IjsKKyAgICBwcmludCAiJGNvbW1hbmRcbiI7 CisgICAgcmV0dXJuIHN5c3RlbSAkY29tbWFuZDsKIH0KIAogc3ViIGJ1aWxkQ2hyb21pdW1WaXN1 YWxTdHVkaW9Qcm9qZWN0KCQkKQpAQCAtMTg5NCw3ICsxODk5LDcgQEAgc3ViIGJ1aWxkQ2hyb21p dW0oJEApCiAgICAgICAgICRyZXN1bHQgPSBidWlsZENocm9taXVtVmlzdWFsU3R1ZGlvUHJvamVj dCgiU291cmNlL1dlYktpdC9jaHJvbWl1bS9XZWJLaXQuc2xuIiwgJGNsZWFuKTsKICAgICB9IGVs c2lmIChpc0xpbnV4KCkpIHsKICAgICAgICAgIyBMaW51eCBidWlsZCAtIGJ1aWxkIHVzaW5nIG1h a2UuCi0gICAgICAgICQgcmVzdWx0ID0gYnVpbGRDaHJvbWl1bU1ha2VmaWxlKCJhbGwiLCAkY2xl YW4pOworICAgICAgICAkcmVzdWx0ID0gYnVpbGRDaHJvbWl1bU1ha2VmaWxlKCJhbGwiLCAkY2xl YW4sIEBvcHRpb25zKTsKICAgICB9IGVsc2UgewogICAgICAgICBwcmludCBTVERFUlIgIlRoaXMg cGxhdGZvcm0gaXMgbm90IHN1cHBvcnRlZCBieSBjaHJvbWl1bS5cbiI7CiAgICAgfQo=