[WIP][Functional] Add example Siri Shortcut #86
Conversation
bachand
force-pushed
the
mb--add-siri-shortcut
branch
from
7013d36 to
812c83f
Compare
Config/buck_rule_macros.bzl
Outdated
| srcs = [definition_directory + definition_name + ".intentdefinition"], | ||
| bash = """ | ||
| developer_dir=`xcode-select -p` | ||
| "$developer_dir/usr/bin/intentbuilderc" $SRCS $TMP "" |
There was a problem hiding this comment.
It's easier to just call xcrun intentbuilderc.
bachand
force-pushed
the
mb--add-siri-shortcut
branch
2 times, most recently
from
b7e2d52 to
1f64b99
Compare
bachand
changed the title
| ] | ||
| + first_party_library_dependencies | ||
| + build_phase_scripts, | ||
| frameworks = [ | ||
| "$SDKROOT/System/Library/Frameworks/Intents.framework", |
There was a problem hiding this comment.
I'm not sure that Intents.framework is strictly necessary, but I have added it to reduce the potential causes of unexpected failure.
There was a problem hiding this comment.
Nevermind. This is used by the generated intent source code.
There was a problem hiding this comment.
So it's good we keep Intents.framework 👍
| <!--Intent View Controller--> | ||
| <scene sceneID="7MM-of-jgj"> | ||
| <objects> | ||
| <viewController id="ObA-dk-sSI" customClass="IntentViewController" customModule="ExampleIntentUIBinary" sceneMemberID="viewController"> |
There was a problem hiding this comment.
customModule must be explicitly set in order to get Buck CLI to work. This is noted in a comment in App/Buck.
| @@ -0,0 +1,329 @@ | |||
| // !$*UTF8*$! | |||
There was a problem hiding this comment.
This Xcode project exists for running code generation on the .intentdefinition. The README.md explains more.
bachand
changed the title
bachand
changed the title
bachand
force-pushed
the
mb--add-siri-shortcut
branch
from
4832824 to
3a0e4d8
Compare
| ) | ||
|
|
||
| apple_bundle( | ||
| name = intent_bundle_name, |
There was a problem hiding this comment.
If necessary you can also set product_name to name the .appex file and its associated binary, but that's only necessary if you want name to be different.
There was a problem hiding this comment.
When unspecified product_name == name.
| fi | ||
| else | ||
| intent_interface="$TMP/Dummy.swift" | ||
| echo "class Dummy { }" > "$intent_interface" |
There was a problem hiding this comment.
I think I should update this to copy a pre-built version of BuckPhotoIntent.swift. If I do that I think the PR will build.
App/AppDelegate.swift
Outdated
| } | ||
| #else | ||
| // Xcode 9 | ||
| func application( |
There was a problem hiding this comment.
Can't wait to upgrade CI to get rid of this stuff.
| // This file was automatically generated and should not be edited. | ||
| // | ||
|
|
||
| import Foundation |
There was a problem hiding this comment.
Temporary until we upgrade CI.
| @@ -8,7 +8,7 @@ | |||
|
|
|||
| [swift] | |||
| version = 4.0 | |||
| compiler_flags = -DBUCK -enable-testing -g -Onone -whole-module-optimization $(config custom.other_swift_compiler_flags) | |||
| compiler_flags = -DBUCK -enable-testing -g -Onone -whole-module-optimization $(config custom.other_swift_compiler_flags) -DDISABLE_SIRI_SHORTCUT | |||
There was a problem hiding this comment.
It's behind a feature flag.
There was a problem hiding this comment.
This works around having Xcode 9 in CI.
bachand
changed the title
bachand
force-pushed
the
mb--add-siri-shortcut
branch
from
684879f to
5117a1d
Compare
|
Almost done...
I may need to map different entitlements to different binaries. |
bachand
force-pushed
the
mb--add-siri-shortcut
branch
from
c575ae1 to
e40af55
Compare
| logging_genrule( | ||
| name = interface_source_name, | ||
| srcs = [ | ||
| compiler_xcodeproj, |
There was a problem hiding this comment.
TODO: we'll need to pass the .intentdefinition here too in order to invalidate the rule when we make changes to that file.
| # missing _main "-Xlinker -rpath -Xlinker @executable_path/../../Frameworks" tells the | ||
| # executable binary to look for frameworks in ExampleApp.app/Frameworks instead of PlugIns, so | ||
| # that we don't need to have the libSwift*.dylib in ExampleApp.app/PlugIns/*.appex/Frameworks | ||
| linker_flags = [ |
There was a problem hiding this comment.
I think you need to add /usr/lib/swift as a search path to make it work on iOS 12.2 forward. It was the case for me:
linker_flags = [
"-e",
"_NSExtensionMain",
"-Xlinker",
"-rpath",
"-Xlinker",
"/usr/lib/swift"
"-Xlinker",
"-rpath",
"-Xlinker",
"@executable_path/../../Frameworks",
],
There was a problem hiding this comment.
Interesting. Thanks for sharing @rockbruno. Are you on Swift 5?
There was a problem hiding this comment.
I’m trying to understand if this is because the Swift libraries packaged with the app are now being ignored. If I understand correctly this occurs with Swift 5+ and iOS 12.2+.
There was a problem hiding this comment.
I'm on 4.2, but I'm using Xcode 10.2 which IIRC uses the Swift 5 compiler regardless of the version.
I remember they changed Buck to add this import by default so I'm surprised I had to do this, so maybe it's just the extension bundles that weren't covered by the change...
bachand
changed the title
Status
We are seeing the following in the CI logs, even though CI passes. We should resolve this before merging.
This PR adds an example of a custom Siri Intent to BuckSample.
To make this PR, I followed this great guide. I performed each step in a manually managed Xcode project and then ported the step over to Buck. You can find the complete manually managed Xcode project (with Git history) here: XcodeExampleApp.zip
Localization
This intent is localized for Spanish.
One gotcha is that Siri's language does not seem to change to Spanish even after you set the simulator's language to Spanish. You may have to set Siri's language to Spanish manually in Settings.
Generating Swift interface for intents
There is no standalone command for this.
IntentDefinitionCodegenis built into Xcode. As such, we runxcodebuildin a genrule..intentdefinition processing
/Applications/Xcode.app/Contents/Developer/usr/bin/intentbuilderc SOURCE_DEFINITION \ DESTINATION_DIR ""I diff'ed the source .intentdefinition file against the output of running
intentbuilderc. The file outputted byintentbuildercis the same. Nonetheless, it seems worth running this command for the purposes of future proofing.After being processed, this file should be copied into any target that uses the intent.
Verification / Testing
Entitlements
I tried running
codesign -d --entitlements OUTPUT_PATH BINARY_PATHon theExampleAppbinary produced by Buck CLI and the Buck-generated Xcode project. Both times this command produced an empty set of entitlements. I verified that this behavior is the same for a manually managed Xcode test project.This command also produced an empty set of entitlements when run the
Airbnbbinary produced by a Buck CLI build. Running this command on theAirbnbbinary produced by a Buck-generated Xcode project produced a non-empty file with one entitlement entry, which did not represent the specified entitlements that had many more entries.I verified that if I passed
apple_binary()a path to a non-existent entitlements file, it failed. So it is in fact doing something with this entitlements file (or at least validating its existence).Since
BuckSampledoes not yet support creating a release build, this validation makes me sufficiently convinced that I'm setting entitlements correctly.Launching the app by clicking on the Intent UI shown by Siri
I verified that the code that I added to
AppDelegate.swiftworks correctly.n.b.
When creating a Siri Shortcut in a manually managed project, I flipped on the Siri capability for the application target.
That resulted in this this change to the project file.
I don't know how to reproduce this change directly in the Buck-generated project, but I am seeing the capability checked one seemingly from the entitlements file. As such, I'm not going to investigate this further.
Running this PR
This feature is disabled by default since CI still runs Xcode 9. If you are running Xcode 10, remove
-DDISABLE_SIRI_SHORTCUTfrom this line to use the feature.