Skip to content

SDKS-4916 Fix swift build failures — platform bump and if canImport guards#157

Open
rodrigoareis wants to merge 1 commit into
developfrom
SDKS-4916
Open

SDKS-4916 Fix swift build failures — platform bump and if canImport guards#157
rodrigoareis wants to merge 1 commit into
developfrom
SDKS-4916

Conversation

@rodrigoareis
Copy link
Copy Markdown
Contributor

JIRA Ticket

SDKS-4916 Fix swift build failures — platform bump and if canImport guards

Summary

  • Bump macOS(.v10_15)macOS(.v13) in Package.swift — resolves Locale.language.languageCode availability and satisfies GoogleSignIn's macOS minimum requirement (cannot remove macOS entirely due to
    SPM platform compatibility validation)
  • Add CACHING_GUIDE.md to PingStorage exclude: in Package.swift — eliminates the SPM "unhandled file" warning
  • Add condition: .when(platforms: [.iOS]) to RecaptchaEnterprise, PingOneSignals, and FacebookLogin product dependencies — prevents SPM from resolving iOS-only XCFrameworks during native macOS builds
  • Guard all affected source files with #if canImport(X) — allows the macOS build path to compile cleanly without affecting iOS/xcodebuild builds

Root Cause

Package.swift declares macOS(.v13), so swift build on a macOS host compiles all targets for native macOS. Three external XCFramework dependencies (RecaptchaEnterpriseSDK, PingOneSignals,
FBSDKLoginKit) only ship iOS slices — causing "no such module" errors. UIKit-dependent source code in TamperDetector, Browser, ExternalIdP, and ExternalIdPGoogle also needed guards.

Test Plan

  • rm -rf .build && swift build — completes with zero errors
  • swift build --target PingReCaptchaEnterprise, PingProtect, PingExternalIdPFacebook, PingPush, PingDavinci, PingOidc, PingBrowser— all pass
  • all test passed

Comment thread Fido/Fido/Fido.swift
return window
}
#endif
fatalError("Window not set. This should never occur.")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the SDK only declares macOS as a build target for 3rd-party library compatibility, consumers on macOS should never reach this code path. However the fatalError message is now misleading — on macOS the UIKit fallback block is compiled out entirely, so there is genuinely no recovery path. Consider either adding a comment that clarifies why this is safe (// macOS: build target only — this path is unreachable in practice), or adding a macOS-specific fallback via NSApplication.shared.keyWindow to make the crash truly unreachable:

#if canImport(UIKit)
if let windowScene = UIApplication.shared.connectedScenes.first as? UIWindowScene,
   let window = windowScene.windows.first {
    return window
}
#else
if let window = NSApplication.shared.keyWindow {
    return window
}
#endif
fatalError("Window not set. This should never occur.")

@unknown default:

default:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original code used @unknown default, which makes the compiler emit a warning if Apple adds a new CLAuthorizationStatus case in a future OS. This refactor replaces it with a plain default, silently swallowing any future enum cases (mapping them to authorizationDenied with no logging). The split into #if canImport(UIKit) for authorizedWhenInUse above is correct, but the switch itself should restore exhaustiveness protection:

@unknown default:
    throw LocationError.authorizationDenied

"orientation": isPortrait ? 1 : 0
]
#else
return [:]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The macOS #else path returns [:] (empty dictionary) for display info. Since macOS is a build-only target for 3rd-party library compatibility and this code won't run in production, the empty return is fine — but a comment would help future readers distinguish intent from omission:

#else
// macOS: build target only for 3rd-party library compatibility — not a supported runtime platform
return [:]
#endif

let cameraCount = discoverySession.devices.count
return ["numberOfCameras": cameraCount]
#else
return [:]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the display info fallback above — macOS is a build-only target so the empty return is intentional. Worth adding the same comment here for consistency:

#else
// macOS: build target only for 3rd-party library compatibility — not a supported runtime platform
return [:]
#endif

self.networkCountryIso = DeviceProfileConstants.unknown
}
#else
self.carrierName = DeviceProfileConstants.unknown
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since macOS is a build-only target and this code won't run in production, returning DeviceProfileConstants.unknown is acceptable. A brief comment would clarify that this is intentional (not a missing implementation), and flag that nil would be more semantically accurate if macOS ever gets promoted to a real target — telephony doesn't exist on Mac at all, whereas "Unknown" implies data was collected but unreadable:

#else
// macOS: build target only — telephony not available on this platform
self.carrierName = DeviceProfileConstants.unknown
self.networkCountryIso = DeviceProfileConstants.unknown
#endif

self.deviceName = await UIDevice.current.name
#else
self.platform = "macOS"
self.version = ProcessInfo.processInfo.operatingSystemVersionString
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ProcessInfo.processInfo.operatingSystemVersionString returns a verbose string like "Version 13.0 (Build 22A380)", while the iOS path returns "16.0" via UIDevice.current.systemVersion. Since macOS is a build-only target this won't affect production, but if macOS is ever promoted or this data is sent to a server, the structural difference will break any version string parsing. Worth aligning the format now:

let v = ProcessInfo.processInfo.operatingSystemVersion
self.version = "\(v.majorVersion).\(v.minorVersion).\(v.patchVersion)"

return .failure(.idpCanceledException(message: error.localizedDescription))
}
#else
return .failure(.illegalStateException(message: "Browser authentication is not supported on this platform"))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since macOS is a build-only target and browser-based IdP is iOS-only, returning a failure here is correct and intentional. Worth adding a comment so it's obvious this isn't a gap to fill:

#else
// macOS: build target only — browser-based IdP authentication requires UIKit and is not a supported use case
return .failure(.illegalStateException(message: "Browser authentication is not supported on this platform"))
#endif

@george-bafaloukas-forgerock
Copy link
Copy Markdown
Contributor

Package.swift — GoogleSignIn linked unconditionally on macOS

RecaptchaEnterprise, PingOneSignals, and FacebookLogin all received .when(platforms: [.iOS]) conditions because they are iOS-only XCFrameworks. GoogleSignIn 9.0.0 does ship a macOS binary so it won't cause a link error — but since every source file in PingExternalIdPGoogle is wrapped in #if canImport(UIKit), the framework is linked into the macOS build target while none of its API is actually called. For consistency, and to guard against a future GoogleSignIn version dropping macOS support, consider:

.product(name: "GoogleSignIn", package: "GoogleSignIn-iOS", condition: .when(platforms: [.iOS]))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants