149963 2015-10-09 11:55:58 -0700 Teach webkitperl how to figure out IOS versions from the SDK 2015-10-09 15:01:47 -0700 1 1 1 Unclassified WebKit Tools / Tests WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 bburg bburg dbates ddkilzer lforschler oldest_to_newest 1131972 0 bburg 2015-10-09 11:55:58 -0700 . 1131973 1 262784 bburg 2015-10-09 12:02:17 -0700 Created attachment 262784 Proposed Fix 1131977 2 262784 dbates 2015-10-09 12:09:27 -0700 Comment on attachment 262784 Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=262784&action=review > Tools/Scripts/copy-webkitlibraries-to-product-directory:235 > + my $majorSDKVersion = IOSVersion()->{"major"}; We tend to follow the Webkit Code Style Guidelines for Perl code. So, this should be named iosVersion(). > Tools/Scripts/webkitdirs.pm:118 > +my $IOSVersion; Similarly this should be name iosVersion. > Tools/Scripts/webkitdirs.pm:1294 > + $IOSVersion = -1; Ditto. > Tools/Scripts/webkitdirs.pm:1301 > + $IOSVersion = { Ditto. > Tools/Scripts/webkitdirs.pm:1306 > + my $version = xcodeSDKVersion(); > + my @splitVersion = split(/\./, $version); > + @splitVersion >= 2 or die "Invalid version $version"; > + $IOSVersion = { > + "major" => $splitVersion[0], > + "minor" => $splitVersion[1], > + "subminor" => (defined($splitVersion[2]) ? $splitVersion[2] : 0), > + }; > +} This duplicates code in function osXVersion. Would it make sense to extract the common code into a shared function, say splitVersionString(), and call it from both this function and determineOSXVersion()? > Tools/Scripts/webkitdirs.pm:1308 > +sub IOSVersion() This should be named iosVersion. > Tools/Scripts/webkitdirs.pm:1311 > + return $IOSVersion; This should be named iosVersion. 1131978 3 dbates 2015-10-09 12:10:36 -0700 (In reply to comment #2) > [...] > This duplicates code in function osXVersion. I meant to write: This duplicates code in function determineOSXVersion(). 1131979 4 262784 dbates 2015-10-09 12:15:28 -0700 Comment on attachment 262784 Proposed Fix Also, we should add iosVersion() to the list of exported functions @EXPORT. 1131980 5 bburg 2015-10-09 12:23:17 -0700 (In reply to comment #2) > Comment on attachment 262784 [details] > > Tools/Scripts/webkitdirs.pm:1306 > > + my $version = xcodeSDKVersion(); > > + my @splitVersion = split(/\./, $version); > > + @splitVersion >= 2 or die "Invalid version $version"; > > + $IOSVersion = { > > + "major" => $splitVersion[0], > > + "minor" => $splitVersion[1], > > + "subminor" => (defined($splitVersion[2]) ? $splitVersion[2] : 0), > > + }; > > +} > > This duplicates code in function osXVersion. Would it make sense to extract > the common code into a shared function, say splitVersionString(), and call > it from both this function and determineOSXVersion()? I could, though I'm not sure that subminors are even used by iOS SDKs. 1132019 6 bburg 2015-10-09 15:01:47 -0700 Committed r190821: <http://trac.webkit.org/changeset/190821> 262784 2015-10-09 12:02:17 -0700 2015-10-09 12:09:27 -0700 Proposed Fix bug-149963-20151009120144.patch text/plain 2861 bburg U3VidmVyc2lvbiBSZXZpc2lvbjogMTkwODA2CmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggYzA2YTRiNmJlMGIxZjFkOGY4NmJiNmE3MDk3NGFmYzg5 ZDljYmNkYy4uYmZkYTA3MzllOGVlYjliYmM4YmViMDUzMDE4ZTI4MzNkYmM1NjUwYyAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3 IEBACisyMDE1LTEwLTA5ICBCcmlhbiBCdXJnICA8YmJ1cmdAYXBwbGUuY29tPgorCisgICAgICAg IFRlYWNoIHdlYmtpdHBlcmwgaG93IHRvIGZpZ3VyZSBvdXQgSU9TIHZlcnNpb25zIGZyb20gdGhl IFNESworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTQ5 OTYzCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgSXQg c2hvdWxkIGJlIGVhc2llciB0byBjaGVjayB0aGUgbWFqb3IgYW5kIG1pbm9yIHZlcnNpb24gb2Yg SU9TIFNES3MuCisKKyAgICAgICAgKiBTY3JpcHRzL2NvcHktd2Via2l0bGlicmFyaWVzLXRvLXBy b2R1Y3QtZGlyZWN0b3J5OiBVc2UgSU9TVmVyc2lvbigpLgorICAgICAgICAqIFNjcmlwdHMvd2Vi a2l0ZGlycy5wbToKKyAgICAgICAgKGRldGVybWluZUlPU1ZlcnNpb24pOiBBZGRlZC4gTG9vayBh dCB0aGUgU0RLIHZlcnNpb24gYW5kIG1ha2UgYSB0dXBsZS4KKyAgICAgICAgKElPU1ZlcnNpb24p OiBBZGRlZC4gCisKIDIwMTUtMTAtMDkgIENzYWJhIE9zenRyb2dvbsOhYyAgPG9zc3lAd2Via2l0 Lm9yZz4KIAogICAgICAgICBSZW1vdmUgRU5BQkxFX1NVSURfU0FOREJPWF9MSU5VWCBjcnVmdApk aWZmIC0tZ2l0IGEvVG9vbHMvU2NyaXB0cy9jb3B5LXdlYmtpdGxpYnJhcmllcy10by1wcm9kdWN0 LWRpcmVjdG9yeSBiL1Rvb2xzL1NjcmlwdHMvY29weS13ZWJraXRsaWJyYXJpZXMtdG8tcHJvZHVj dC1kaXJlY3RvcnkKaW5kZXggNDRjNGNjZjViMjRiMjJlYmNmZTY3YTAyZDJhNjc1ODc3NzMwZTk4 NC4uNGNhYWIwZmNmODZhOTUxOWYxMDM0MTBkYTM4MTczNjkwOGQwYzM3MyAxMDA3NTUKLS0tIGEv VG9vbHMvU2NyaXB0cy9jb3B5LXdlYmtpdGxpYnJhcmllcy10by1wcm9kdWN0LWRpcmVjdG9yeQor KysgYi9Ub29scy9TY3JpcHRzL2NvcHktd2Via2l0bGlicmFyaWVzLXRvLXByb2R1Y3QtZGlyZWN0 b3J5CkBAIC0yMzIsNyArMjMyLDcgQEAgaWYgKCRsbHZtKSB7CiAgICAgJGxsdm1JbmNsdWRlUGFj a2FnZSA9ICRFTlZ7TExWTV9JTkNMVURFX1BBQ0tBR0V9IHVubGVzcyAkbGx2bUluY2x1ZGVQYWNr YWdlOwogICAgIGlmICghJGxsdm1MaWJyYXJ5UGFja2FnZSB8fCAhJGxsdm1JbmNsdWRlUGFja2Fn ZSkgewogICAgICAgICBpZiAoaXNJT1NXZWJLaXQoKSkgewotICAgICAgICAgICAgbXkgJG1ham9y U0RLVmVyc2lvbiA9ICQxIGlmIHhjb2RlU0RLVmVyc2lvbigpID1+IC9eKFxkKykvOworICAgICAg ICAgICAgbXkgJG1ham9yU0RLVmVyc2lvbiA9IElPU1ZlcnNpb24oKS0+eyJtYWpvciJ9OwogICAg ICAgICAgICAgJGxsdm1MaWJyYXJ5UGFja2FnZSA9ICJXZWJLaXRMaWJyYXJpZXMvTExWTUxpYnJh cmllc0lPUyRtYWpvclNES1ZlcnNpb24udGFyLmJ6MiI7CiAgICAgICAgICAgICAkbGx2bUluY2x1 ZGVQYWNrYWdlID0gIldlYktpdExpYnJhcmllcy9MTFZNSW5jbHVkZXNJT1MkbWFqb3JTREtWZXJz aW9uLnRhci5iejIiOwogICAgICAgICB9IGVsc2lmIChpc0FwcGxlTWFjV2ViS2l0KCkpIHsKZGlm ZiAtLWdpdCBhL1Rvb2xzL1NjcmlwdHMvd2Via2l0ZGlycy5wbSBiL1Rvb2xzL1NjcmlwdHMvd2Vi a2l0ZGlycy5wbQppbmRleCA3NWYxZGQ1NGM3NGUyMjM2Nzc3MWVmMDI2MTEzOTJjMTc0YzBlMjc2 Li5mN2Y3MmRlY2EyZTUwM2MzMzJmNjA1YThjMmUyMWRhYzE4NzNiMjM4IDEwMDc1NQotLS0gYS9U b29scy9TY3JpcHRzL3dlYmtpdGRpcnMucG0KKysrIGIvVG9vbHMvU2NyaXB0cy93ZWJraXRkaXJz LnBtCkBAIC0xMTUsNiArMTE1LDcgQEAgbXkgJGRlYnVnZ2VyOwogbXkgJGRpZExvYWRJUGhvbmVT aW11bGF0b3JOb3RpZmljYXRpb247CiBteSAkbm1QYXRoOwogbXkgJG9zWFZlcnNpb247CitteSAk SU9TVmVyc2lvbjsKIG15ICRnZW5lcmF0ZURzeW07CiBteSAkaXNHdGs7CiBteSAkaXNXaW5DYWly bzsKQEAgLTEyODUsNiArMTI4NiwzMSBAQCBzdWIgb3NYVmVyc2lvbigpCiAgICAgcmV0dXJuICRv c1hWZXJzaW9uOwogfQogCitzdWIgZGV0ZXJtaW5lSU9TVmVyc2lvbigpCit7CisgICAgcmV0dXJu IGlmICRJT1NWZXJzaW9uOworCisgICAgaWYgKCFpc0lPU1dlYktpdCgpKSB7CisgICAgICAgICRJ T1NWZXJzaW9uID0gLTE7CisgICAgICAgIHJldHVybjsKKyAgICB9CisKKyAgICBteSAkdmVyc2lv biA9IHhjb2RlU0RLVmVyc2lvbigpOworICAgIG15IEBzcGxpdFZlcnNpb24gPSBzcGxpdCgvXC4v LCAkdmVyc2lvbik7CisgICAgQHNwbGl0VmVyc2lvbiA+PSAyIG9yIGRpZSAiSW52YWxpZCB2ZXJz aW9uICR2ZXJzaW9uIjsKKyAgICAkSU9TVmVyc2lvbiA9IHsKKyAgICAgICAgICAgICJtYWpvciIg PT4gJHNwbGl0VmVyc2lvblswXSwKKyAgICAgICAgICAgICJtaW5vciIgPT4gJHNwbGl0VmVyc2lv blsxXSwKKyAgICAgICAgICAgICJzdWJtaW5vciIgPT4gKGRlZmluZWQoJHNwbGl0VmVyc2lvblsy XSkgPyAkc3BsaXRWZXJzaW9uWzJdIDogMCksCisgICAgfTsKK30KKworc3ViIElPU1ZlcnNpb24o KQoreworICAgIGRldGVybWluZUlPU1ZlcnNpb24oKTsKKyAgICByZXR1cm4gJElPU1ZlcnNpb247 Cit9CisKIHN1YiBpc1dpbmRvd3NOVCgpCiB7CiAgICAgcmV0dXJuICRFTlZ7J09TJ30gZXEgJ1dp bmRvd3NfTlQnOwo=