Skip to content
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

Add support for new-style SecAccessControlCreateFlags constants #173

Merged
merged 1 commit into from May 30, 2019

Conversation

LinusU
Copy link
Contributor

@LinusU LinusU commented May 18, 2019

Fixes #172

@CLAassistant
Copy link

CLAassistant commented May 18, 2019

CLA assistant check
All committers have signed the CLA.

@dfed
Copy link
Collaborator

dfed commented May 19, 2019

🤔. Looks like Xcode 9 (our minimum-supported Xcode version) doesn't like the new fields. Looks like there is no good way to handle renames of option set values over multiple Xcode version.

You might want to consider (and I know this sucks) using SecAccessControlCreateFlags.init(rawValue:) to avoid the warning. Consider instead of

if #available(macOS 10.12.1, *) {
    return .touchIDAny
} else {
    ErrorHandler.assertionFailure(".biometricAny requires macOS 10.12.1.")
    return .userPresence
}

We could write

if #available(macOS 10.12.1, *) {
    return .init(rawValue: 2) // .biometryAny with Xcode 9 compatibility.
} else {
    ErrorHandler.assertionFailure(".biometricAny requires macOS 10.12.1.")
    return .userPresence
}

biometryCurrentSet's raw value is 8, so you could do the same thing there.

Thoughts? I want to provide a warning-free Valet for you, but with Swift's limitations (and my not wanting to do a major version bump to drop Xcode 9), I'm not seeing better options.

@dfed
Copy link
Collaborator

dfed commented May 28, 2019

@LinusU let me know what you think of the above. I'd love to see a fix for our warnings land on master with you as the author.

That said, I don't want to wait forever on this – warnings are annoying, and I don't want to burden our consumers with them for longer than I must. If I don't hear from you by Monday June 3rd, I'll likely put up a PR to remove these warnings.

@LinusU
Copy link
Contributor Author

LinusU commented May 30, 2019

So sorry for the delay on this!

Took your approach and also rebased on master, thanks! ☺️

Copy link
Collaborator

@dfed dfed left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for making the fix backwards-compatible. Excited to release this and get back to zero warnings 🚀

@dfed dfed merged commit b5501be into square:master May 30, 2019
@LinusU LinusU deleted the avoid-deprecation branch May 31, 2019 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deprecation warnings with Xcode 10.2 / Swift 5 / iOS 12
3 participants