Conversation
|
The one unfortunate thing is that I've been unable to get it to build for older iOS versions (though it builds fine for newer ones). Changing It seems to be related to this issue in the Go repo, which mentions this change where the crypto library moves to a newer API, that according to Apple's documentation is available in iOS 15.0+, which would restrict compilation to those versions. I've tried downgrading Go and the crypto version, but I still run into the same issue. |
mbevc1
left a comment
There was a problem hiding this comment.
Thanks for your contribution and I'm glad to hear you got it going.
Just a side note - Darwin keychain backend has tests. The iOS file adds new backend logic with no corresponding test file. Even a stub that only compiles and skips on non-iOS would help CI catch regressions in the future.
| // Set the isAccessibleWhenUnlocked to the boolean value of | ||
| // KeychainAccessibleWhenUnlocked is a shorthand for setting the accessibility value. |
There was a problem hiding this comment.
Is this part of the same sentance, reads a bit weird 🤔
| passwordFunc PromptFunc | ||
|
|
||
| isSynchronizable bool | ||
| isAccessibleWhenUnlocked bool |
There was a problem hiding this comment.
This doesn't seem to be initialised?
The keychain struct includes isSynchronizable, and Set does check it — but the init() function that constructs the struct never sets it from cfg. Looking at the Darwin implementation, cfg.KeychainSynchronizable is presumably used to populate this field. The iOS init currently omits this, meaning isSynchronizable will always be false even if a caller configures it. This looks like an oversight.
| @@ -0,0 +1,191 @@ | |||
| //go:build ios && cgo | |||
There was a problem hiding this comment.
You comment notes that iOS 15.0+ is effectively required due to a Go runtime dependency on SecTrustCopyCertificateChain. This is a meaningful constraint for downstream users, but it's not captured anywhere in the code (no comment, no doc, no README update). It would be worth at minimum adding a comment near the build tag, e.g.:
//go:build ios && cgo
// NOTE: Due to Go's crypto library requiring SecTrustCopyCertificateChain,
// this requires iOS 15.0 or later.
I finally got iOS builds working! It turns out that only Keyring needed changes, and that go-keychain didn't need to be touched. I tested it on an iOS device and it works.
For posterity, my build command (against the basic example in the README) is:
The one thing in this PR I'm not certain about is is the change I made in
keychain_darwin.go. Strangely, that changed is needed for compilation, as despite compiling for iOS, the Darwin file would seemingly be evaluated too, and Go would be unable to find the macOS-only symbols present there and error.