For the ios-ews queue and for developers who want to make sure their changes build for the iOS port of WebKit using the iphoneos SDK, we need a way to disable code-signing so that Xcode doesn't stop building if it can't find a developer certificate. This will add a --disable-code-signing switch to the configure-xcode-for-ios-development script.
Created attachment 245238 [details] Patch v1
Comment on attachment 245238 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=245238&action=review > Tools/ChangeLog:11 > + (disableCodeSigning): Add. Calls Nit: "Add" => "Added". > Tools/ChangeLog:14 > + (disableCodeSigningForProductType): Add. Does the work of Ditto. > Tools/Scripts/configure-xcode-for-ios-development:65 > + -h|--help Show this help message Nit: I suggest we list the --help option at the top of the usage message so as to be consistent with the order we use in other scripts, including build-webkit and run-safari. > Tools/Scripts/configure-xcode-for-ios-development:212 > + for ( ; ; ++$index) { > + `/usr/libexec/PlistBuddy -c "Print :$index" "$plist" > /dev/null 2>&1`; > + die "Failed to disable code signing for $productType: $!" if exitStatus($?); > + > + chomp(my $name = `/usr/libexec/PlistBuddy -c "Print :$index:Name" "$plist" 2> /dev/null`); > + next unless $name eq $productType; > + > + chomp(my $value = `/usr/libexec/PlistBuddy -c "Print :$index:DefaultBuildProperties:CODE_SIGNING_ALLOWED" "$plist" 2> /dev/null`); > + return if ($value eq "NO"); > + > + system("/usr/libexec/PlistBuddy", "-x", "-c", "Set :$index:DefaultBuildProperties:CODE_SIGNING_ALLOWED NO", $plist); > + die "Failed to disable code signing for $productType: $!" if exitStatus($?); > + print "Successfully disabled code signing for $productType.\n"; > + return; > + } Notice that we have existing parsing code for finding the index position of a <dict> with child a <key> called Identifier in a plist in readXcodeSpecificationById(). Can we make use of this function or extract the common functionality into a helper function that is used by both readXcodeSpecificationById() and disableCodeSigningForProductType()?
Comment on attachment 245238 [details] Patch v1 There is some duplicate code that Dan pointed out, and I want to add an --enable-code-signing switch to undo the change that --disable-code-signing does (in case someone needs to do that). We may also want to add a check for a developer certificate in the keychain, and bail out (or ask) before continuing with disabling code signing.
(In reply to comment #2) > Comment on attachment 245238 [details] > Patch v1 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=245238&action=review > > > Tools/ChangeLog:11 > > + (disableCodeSigning): Add. Calls > > Nit: "Add" => "Added". > > > Tools/ChangeLog:14 > > + (disableCodeSigningForProductType): Add. Does the work of > > Ditto. I prefer the imperative, present tense: <http://stackoverflow.com/questions/3580013/should-i-use-past-or-present-tense-in-git-commit-messages> > > Tools/Scripts/configure-xcode-for-ios-development:65 > > + -h|--help Show this help message > > Nit: I suggest we list the --help option at the top of the usage message so > as to be consistent with the order we use in other scripts, including > build-webkit and run-safari. Will fix. > > Tools/Scripts/configure-xcode-for-ios-development:212 > > + for ( ; ; ++$index) { > > + `/usr/libexec/PlistBuddy -c "Print :$index" "$plist" > /dev/null 2>&1`; > > + die "Failed to disable code signing for $productType: $!" if exitStatus($?); > > + > > + chomp(my $name = `/usr/libexec/PlistBuddy -c "Print :$index:Name" "$plist" 2> /dev/null`); > > + next unless $name eq $productType; > > + > > + chomp(my $value = `/usr/libexec/PlistBuddy -c "Print :$index:DefaultBuildProperties:CODE_SIGNING_ALLOWED" "$plist" 2> /dev/null`); > > + return if ($value eq "NO"); > > + > > + system("/usr/libexec/PlistBuddy", "-x", "-c", "Set :$index:DefaultBuildProperties:CODE_SIGNING_ALLOWED NO", $plist); > > + die "Failed to disable code signing for $productType: $!" if exitStatus($?); > > + print "Successfully disabled code signing for $productType.\n"; > > + return; > > + } > > Notice that we have existing parsing code for finding the index position of > a <dict> with child a <key> called Identifier in a plist in > readXcodeSpecificationById(). Can we make use of this function or extract > the common functionality into a helper function that is used by both > readXcodeSpecificationById() and disableCodeSigningForProductType()? Will reuse this code in the next patch. Thanks!
<rdar://problem/19520599>
What's the next step on this? Is this patch still necessary?
Created attachment 262463 [details] Patch
Comment on attachment 262463 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=262463&action=review > Tools/Scripts/webkitdirs.pm:2012 > +sub hasIOSDevelopmentCertificate() Although, this function takes no arguments, will forward declare this function at the top of the file so that Perl can check its prototype.
Created attachment 262468 [details] Patch Actually, moved hasIOSDevelopmentCertificate() to come before XcodeOptions() instead of forward declaring it. Added missing semicolon in hasIOSDevelopmentCertificate().
Comment on attachment 262468 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=262468&action=review Neat. EWS is broken due to a (new) unrelated issue. > Tools/Scripts/webkitdirs.pm:688 > + push @options, ("-configuration", $configuration); > + push @options, ("-xcconfig", sourceDir() . "/Tools/asan/asan.xcconfig", "ASAN_IGNORE=" . sourceDir() . "/Tools/asan/webkit-asan-ignore.txt") if $asanIsEnabled; I think that parentheses are not needed here.
(In reply to comment #10) > > Tools/Scripts/webkitdirs.pm:688 > > + push @options, ("-configuration", $configuration); > > + push @options, ("-xcconfig", sourceDir() . "/Tools/asan/asan.xcconfig", "ASAN_IGNORE=" . sourceDir() . "/Tools/asan/webkit-asan-ignore.txt") if $asanIsEnabled; > > I think that parentheses are not needed here. Yes, parentheses are not needed here. I will keep them because I feel they improves the readability of this code.
Comment on attachment 262468 [details] Patch Clearing flags on attachment: 262468 Committed r190583: <http://trac.webkit.org/changeset/190583>
All reviewed patches have been landed. Closing bug.