New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider `XCODE_PRODUCT_BUILD_VERSION` alongside `EXPANDED_CODE_SIGN_IDENTITY` to address Xcode 10. #2476

Merged
merged 1 commit into from Jun 26, 2018
Jump to file or symbol
Failed to load files and symbols.
+22 −6
Diff settings

Always

Just for now

Treat `EXPANDED_CODE_SIGN_IDENTITY` — when `EXPANDED_CODE_SIGN_IDENTI…

…TY` is unset and `XCODE_PRODUCT_BUILD_VERSION` is set — with the same behavior as having empty string.

Fixes <#2472>.

Xcode beta release `10L176w` (and potentially more) seem to omit environment variables set to an empty string in Run Script Phases (and potentially other places). So with the heuristic of `XCODE_PRODUCT_BUILD_VERSION` being set signifying a Run Script Phases (or other within Xcode scenario), ignore the error by returning `nil`, as is done when `codeSigningAllowed()` is false.

Credit and thanks to @clayellis for reporting the behavior change and iterating on code changes.
  • Loading branch information...
jdhealy committed Jun 6, 2018
commit 986e39569779b84b10a2bec72ee60520061257a7
@@ -101,13 +101,29 @@ private func copyBCSymbolMapsForFramework(_ frameworkURL: URL, fromDirectory dir
}
private func codeSigningIdentity() -> SignalProducer<String?, CarthageError> {
return SignalProducer { () -> Result<String?, CarthageError> in
if codeSigningAllowed() {
return getEnvironmentVariable("EXPANDED_CODE_SIGN_IDENTITY").map { $0.isEmpty ? nil : $0 }
} else {
return .success(nil)
return SignalProducer(codeSigningAllowed)
.attemptMap { codeSigningAllowed in
guard codeSigningAllowed == true else { return .success(nil) }
return getEnvironmentVariable("EXPANDED_CODE_SIGN_IDENTITY")
.map { $0.isEmpty ? nil : $0 }
.flatMapError {
// See https://github.com/Carthage/Carthage/issues/2472#issuecomment-395134166 regarding Xcode 10 betas
// … or potentially non-beta Xcode releases of major version 10 or later.
switch getEnvironmentVariable("XCODE_PRODUCT_BUILD_VERSION") {
case .success(_):

This comment has been minimized.

@kballard

kballard Jun 8, 2018

Contributor

BTW, I don't know if this is some particular Carthage style you're following here, but in general you can omit the (_) and just say case .success: if you don't want any of the associated values.

@kballard

kballard Jun 8, 2018

Contributor

BTW, I don't know if this is some particular Carthage style you're following here, but in general you can omit the (_) and just say case .success: if you don't want any of the associated values.

// See the above issue.
return .success(nil)
case .failure(_):
// For users calling `carthage copy-frameworks` outside of Xcode (admittedly,
// a small fraction), this error is worthwhile in being a signpost in what’s
// necessary to add to achieve (for what most is the goal) of ensuring
// that code signing happens.
return .failure($0)
}
}
}
}
}
private func codeSigningAllowed() -> Bool {
ProTip! Use n and p to navigate between commits in a pull request.