34822 2010-02-10 17:42:26 -0800 webkit-build-directory misuses terms 2010-02-17 12:26:32 -0800 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) PC OS X 10.5 RESOLVED FIXED P2 Normal --- 34898 1 eric webkit-unassigned commit-queue dpranke mrowe oldest_to_newest 189541 0 eric 2010-02-10 17:42:26 -0800 From an email from Mark Rowe: Added: trunk/WebKitTools/Scripts/webkit-build-directory (0 => 54634) --- trunk/WebKitTools/Scripts/webkit-build-directory (rev 0) +++ trunk/WebKitTools/Scripts/webkit-build-directory 2010-02-11 01:24:04 UTC (rev 54634) @@ -0,0 +1,65 @@ +my $programName = basename($0); +my $usage = <<EOF; +Usage: $programName [options] + --base Show the root build directory instead of one corresponding to the current target (e.g. Debug, Release) + --debug Show build directory for the Debug target + -h|--help Show this help message + --release Show build directory for the Release target +EOF The terminology used here is wrong. Debug and Release are configurations, not targets. A target is a result of a build (such as derived source file generation, or the WebKit framework). The documentation for --base is also confusing. It talks about a "root build directory" while the option name implies it is the "base build directory". Presumably they're the same thing so we should pick a single name for them. + +setConfiguration(); # Figure out from the command line if we're --debug or --release or the default. … this is why the method to process these flags uses the term configuration ;-) It would probably be clearer to explain the behavior in terms of two modes: one that shows the base build directory, and a second that shows the per-configuration build directory. I would suggest using explicit flags to select the mode of operation, since as it stands the default behavior is somewhat confusing (it will show the per-configuration build directory for the active configuration). == end email == I'm about to prepare a patch to fix the Configuration/Target confusion. I'm not sure I understand the suggestion with regards to the flags. The current behavior is designed to be "correct" for any shell script/Makefile which wants to know where to put output. But you're right, that may be confusing to users just running the script without args. Could you give an example of another interface you'd prefer? 189544 1 mrowe 2010-02-10 17:49:13 -0800 The current behavior seems to be: webkit-build-directory -> WebKitBuild/Release webkit-build-directory --debug -> WebKitBuild/Debug webkit-build-directory --base -> WebKitBuild I’m basically suggesting that we do the following: webkit-build-directory -> error, need to specify which directory you want. webkit-build-directory --configuration-build-directory -> WebKitBuild/Release webkit-build-directory --configuration-build-directory --debug -> WebKitBuild/Debug webkit-build-directory --base-build-directory -> WebKitBuild I don’t think that it is intuitive for a script named webkit-build-directory to default to giving you the per-configuration build directory. Making it explicit which directory is being requested is much clearer. The option names I suggested may be a little verbose. I just picked the most literal names possible. 189556 2 48538 eric 2010-02-10 18:06:24 -0800 Created attachment 48538 Patch 189817 3 48538 eric 2010-02-11 15:37:21 -0800 Comment on attachment 48538 Patch Does this look good to you Mark? 190123 4 48538 commit-queue 2010-02-12 12:58:00 -0800 Comment on attachment 48538 Patch Rejecting patch 48538 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--quiet']" exit_code: 1 Running build-dumprendertree Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests Testing 12157 test cases. websocket/tests/bufferedAmount-after-close.html -> crashed Exiting early after 1 failures. 12124 tests run. 500.30s total testing time 12123 test cases (99%) succeeded 1 test case (<1%) crashed 7 test cases (<1%) had stderr output Full output: http://webkit-commit-queue.appspot.com/results/261095 190160 5 48538 eric 2010-02-12 14:46:03 -0800 Comment on attachment 48538 Patch Bug 34898. 190373 6 48538 commit-queue 2010-02-14 20:42:08 -0800 Comment on attachment 48538 Patch Clearing flags on attachment: 48538 Committed r54759: <http://trac.webkit.org/changeset/54759> 190374 7 commit-queue 2010-02-14 20:42:17 -0800 All reviewed patches have been landed. Closing bug. 191149 8 eric 2010-02-17 12:26:32 -0800 Crash was bug 34898. 48538 2010-02-10 18:06:24 -0800 2010-02-14 20:42:08 -0800 Patch bug-34822-20100210180622.patch text/plain 3277 eric ZGlmZiAtLWdpdCBhL1dlYktpdFRvb2xzL0NoYW5nZUxvZyBiL1dlYktpdFRvb2xzL0NoYW5nZUxv ZwppbmRleCAyODYxOTBhLi5jZmJiYmEzIDEwMDY0NAotLS0gYS9XZWJLaXRUb29scy9DaGFuZ2VM b2cKKysrIGIvV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCkBAIC0xLDUgKzEsMTcgQEAKIDIwMTAtMDIt MTAgIEVyaWMgU2VpZGVsICA8ZXJpY0B3ZWJraXQub3JnPgogCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIHdlYmtpdC1idWlsZC1kaXJlY3RvcnkgbWlzdXNl cyB0ZXJtcworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 MzQ4MjIKKworICAgICAgICAqIFNjcmlwdHMvd2Via2l0LWJ1aWxkLWRpcmVjdG9yeToKKyAgICAg ICAgIC0gQWRkIC0tdG9wLWxldmVsIGFuZCAtLWNvbmZpZ3VyYXRpb24gb3B0aW9ucyBhbmQgbWFr ZSB1c2luZyBvbmUgb2YgdGhlbSByZXF1aXJlZC4KKyAgICAgICAgKiBTY3JpcHRzL3dlYmtpdHB5 L2xheW91dF90ZXN0cy9wb3J0L21hYy5weToKKyAgICAgICAgIC0gVXNlIC0tdG9wLWxldmVsIGlu c3RlYWQgb2YgLS1iYXNlLgorCisyMDEwLTAyLTEwICBFcmljIFNlaWRlbCAgPGVyaWNAd2Via2l0 Lm9yZz4KKwogICAgICAgICBSZXZpZXdlZCBieSBEYXZpZCBMZXZpbi4KIAogICAgICAgICBydW4t Y2hyb21pdW0td2Via2l0LXRlc3RzIC0tcGxhdGZvcm09bWFjLWxlb3BhcmQgY3Jhc2hlcyB3aGVu IHVzaW5nIGEgY3VzdG9tIGJ1aWxkIGRpcmVjdG9yeQpkaWZmIC0tZ2l0IGEvV2ViS2l0VG9vbHMv U2NyaXB0cy93ZWJraXQtYnVpbGQtZGlyZWN0b3J5IGIvV2ViS2l0VG9vbHMvU2NyaXB0cy93ZWJr aXQtYnVpbGQtZGlyZWN0b3J5CmluZGV4IGE4NWM1ODcuLmJmN2Q2NmQgMTAwNzU1Ci0tLSBhL1dl YktpdFRvb2xzL1NjcmlwdHMvd2Via2l0LWJ1aWxkLWRpcmVjdG9yeQorKysgYi9XZWJLaXRUb29s cy9TY3JpcHRzL3dlYmtpdC1idWlsZC1kaXJlY3RvcnkKQEAgLTM0LDMxICszNCwzNSBAQCB1c2Ug R2V0b3B0OjpMb25nOwogdXNlIGxpYiAkRmluZEJpbjo6QmluOwogdXNlIHdlYmtpdGRpcnM7CiAK LW15ICRzaG93QmFzZVByb2R1Y3REaXJlY3RvcnkgPSAwOworbXkgJHNob3dDb25maWd1cmF0aW9u RGlyZWN0b3J5ID0gMDsKIG15ICRzaG93SGVscCA9IDA7CitteSAkc2hvd1RvcExldmVsRGlyZWN0 b3J5ID0gMDsKKwogCiBteSAkcHJvZ3JhbU5hbWUgPSBiYXNlbmFtZSgkMCk7CiBteSAkdXNhZ2Ug PSA8PEVPRjsKIFVzYWdlOiAkcHJvZ3JhbU5hbWUgW29wdGlvbnNdCi0gIC0tYmFzZSAgICAgIFNo b3cgdGhlIHJvb3QgYnVpbGQgZGlyZWN0b3J5IGluc3RlYWQgb2Ygb25lIGNvcnJlc3BvbmRpbmcg dG8gdGhlIGN1cnJlbnQgdGFyZ2V0IChlLmcuIERlYnVnLCBSZWxlYXNlKQotICAtLWRlYnVnICAg ICBTaG93IGJ1aWxkIGRpcmVjdG9yeSBmb3IgdGhlIERlYnVnIHRhcmdldAotICAtaHwtLWhlbHAg ICBTaG93IHRoaXMgaGVscCBtZXNzYWdlCi0gIC0tcmVsZWFzZSAgIFNob3cgYnVpbGQgZGlyZWN0 b3J5IGZvciB0aGUgUmVsZWFzZSB0YXJnZXQKKyAgLS1jb25maWd1cmF0aW9uICBTaG93IHRoZSBi dWlsZCBkaXJlY3RvcnkgZm9yIGEgc3BlY2lmaWMgY29uZmlndXJhdGlvbiAoZS5nLiBEZWJ1Zywg UmVsZWFzZS4gIERlZmF1bHRzIHRvIHRoZSBhY3RpdmUgY29uZmlndXJhdGlvbiBzZXQgYnkgc2V0 LXdlYmtpdC1jb25maWd1cmF0aW9uKQorICAtaHwtLWhlbHAgICAgICAgIFNob3cgdGhpcyBoZWxw IG1lc3NhZ2UKKyAgLS10b3AtbGV2ZWwgICAgICBTaG93IHRoZSB0b3AtbGV2ZWwgYnVpbGQgZGly ZWN0b3J5CisKK0VpdGhlciAtLWNvbmZpZ3VyYXRpb24gb3IgLS10b3AtbGV2ZWwgaXMgcmVxdWly ZWQuCiBFT0YKIAogc2V0Q29uZmlndXJhdGlvbigpOyAjIEZpZ3VyZSBvdXQgZnJvbSB0aGUgY29t bWFuZCBsaW5lIGlmIHdlJ3JlIC0tZGVidWcgb3IgLS1yZWxlYXNlIG9yIHRoZSBkZWZhdWx0Lgog CiBteSAkZ2V0T3B0aW9uc1Jlc3VsdCA9IEdldE9wdGlvbnMoCi0gICAgJ2Jhc2UnID0+IFwkc2hv d0Jhc2VQcm9kdWN0RGlyZWN0b3J5LAorICAgICdjb25maWd1cmF0aW9uJyA9PiBcJHNob3dDb25m aWd1cmF0aW9uRGlyZWN0b3J5LAorICAgICd0b3AtbGV2ZWwnID0+IFwkc2hvd1RvcExldmVsRGly ZWN0b3J5LAogICAgICdoZWxwfGgnID0+IFwkc2hvd0hlbHAsCiApOwogCi1pZiAoISRnZXRPcHRp b25zUmVzdWx0IHx8ICRzaG93SGVscCkgeworaWYgKCEkZ2V0T3B0aW9uc1Jlc3VsdCB8fCAkc2hv d0hlbHAgfHwgKCEkc2hvd0NvbmZpZ3VyYXRpb25EaXJlY3RvcnkgJiYgISRzaG93VG9wTGV2ZWxE aXJlY3RvcnkpKSB7CiAgICAgcHJpbnQgU1RERVJSICR1c2FnZTsKICAgICBleGl0IDE7CiB9CiAK LWlmICgkc2hvd0Jhc2VQcm9kdWN0RGlyZWN0b3J5KSB7CitpZiAoJHNob3dUb3BMZXZlbERpcmVj dG9yeSkgewogICAgIHByaW50IGJhc2VQcm9kdWN0RGlyKCkgLiAiXG4iOwogfSBlbHNlIHsKICAg ICBwcmludCBwcm9kdWN0RGlyKCkgLiAiXG4iOwpkaWZmIC0tZ2l0IGEvV2ViS2l0VG9vbHMvU2Ny aXB0cy93ZWJraXRweS9sYXlvdXRfdGVzdHMvcG9ydC9tYWMucHkgYi9XZWJLaXRUb29scy9TY3Jp cHRzL3dlYmtpdHB5L2xheW91dF90ZXN0cy9wb3J0L21hYy5weQppbmRleCBjMDllYzM3Li5iZGNm MGU5IDEwMDY0NAotLS0gYS9XZWJLaXRUb29scy9TY3JpcHRzL3dlYmtpdHB5L2xheW91dF90ZXN0 cy9wb3J0L21hYy5weQorKysgYi9XZWJLaXRUb29scy9TY3JpcHRzL3dlYmtpdHB5L2xheW91dF90 ZXN0cy9wb3J0L21hYy5weQpAQCAtMTc5LDcgKzE3OSw3IEBAIGNsYXNzIE1hY1BvcnQoYmFzZS5Q b3J0KToKIAogICAgIGRlZiBfYnVpbGRfcGF0aChzZWxmLCAqY29tcHMpOgogICAgICAgICBpZiBu b3Qgc2VsZi5fY2FjaGVkX2J1aWxkX3Jvb3Q6Ci0gICAgICAgICAgICBzZWxmLl9jYWNoZWRfYnVp bGRfcm9vdCA9IGV4ZWN1dGl2ZS5ydW5fY29tbWFuZChbIndlYmtpdC1idWlsZC1kaXJlY3Rvcnki LCAiLS1iYXNlIl0pLnJzdHJpcCgpCisgICAgICAgICAgICBzZWxmLl9jYWNoZWRfYnVpbGRfcm9v dCA9IGV4ZWN1dGl2ZS5ydW5fY29tbWFuZChbIndlYmtpdC1idWlsZC1kaXJlY3RvcnkiLCAiLS10 b3AtbGV2ZWwiXSkucnN0cmlwKCkKICAgICAgICAgcmV0dXJuIG9zLnBhdGguam9pbihzZWxmLl9j YWNoZWRfYnVpbGRfcm9vdCwgc2VsZi5fb3B0aW9ucy50YXJnZXQsICpjb21wcykKIAogICAgIGRl ZiBfa2lsbF9wcm9jZXNzKHNlbGYsIHBpZCk6Cg==