Add --ios_device flag for running iOS application on a physical device #16
Conversation
|
FYI I think it's unlikely that folks would accept a new iOS specific flag into bazel itself, there's a goal of removing this type of specifics from bazel. If a flag is required it would probably be preferred to add a starlark flag in rules_apple itself |
Thanks for letting me know. I had the same assumption, but I'm curious to hear what folks will say. |
|
note that it will not get any attention from folks who can merge unless you mark ready for review |
WalkthroughA new command-line option for specifying an iOS device identifier was introduced to the Objective-C build system. This option is now accessible through the configuration API and is exposed to Starlark, allowing build rules to retrieve the selected device identifier. Corresponding changes were made to the configuration class and interface, and a new test was added to ensure that Starlark rules can access this field along with other related configuration values. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CommandLine
participant ObjcCommandLineOptions
participant ObjcConfiguration
participant StarlarkRule
User->>CommandLine: Runs build with --ios_device=UUID
CommandLine->>ObjcCommandLineOptions: Parses ios_device option
ObjcCommandLineOptions->>ObjcConfiguration: Passes iosDevice value
StarlarkRule->>ObjcConfiguration: Requests ios_device via getIosDevice()
ObjcConfiguration-->>StarlarkRule: Returns ios_device value
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🔇 Additional comments (6)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @arvi18, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request introduces a new command-line flag, --ios_device, to specify a physical iOS device for running application targets. This leverages Apple's new command-line tool in Xcode 15 for managing physical devices, allowing Bazel to install and launch applications on a specified device without relying on third-party solutions. The changes include adding the new flag to ObjcCommandLineOptions, updating ObjcConfiguration to store the flag's value, exposing the flag in Starlark, and adding a test to verify Starlark's access to the new flag.
Highlights
- New Flag:
--ios_device: Introduces a new command-line flag--ios_deviceto specify the identifier of a physical iOS device for running application targets. The flag accepts the device's identifier, ECID, serial number, UDID, user-provided name, or DNS name. - Configuration Integration: Integrates the new flag into the
ObjcConfigurationto store the specified device identifier, making it accessible during the build process. - Starlark Exposure: Exposes the
ios_deviceflag to Starlark, allowing rules to access the specified device identifier within Starlark build files. - Testing: Adds a new test case,
testStarlarkCanAccessObjcConfigurationForDevice, to verify that Starlark can correctly access the value of theios_deviceflag.
Changelog
- src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommandLineOptions.java
- Added the
ios_deviceoption with documentation and help text. (lines 51-61)
- Added the
- src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java
- Added
iosDevicefield to store the value of theios_deviceflag. (line 56) - Initialized
iosDevicein the constructor using the value fromObjcCommandLineOptions. (line 74) - Added getter method
getIosDevice()to retrieve the value ofiosDevice. (lines 103-109)
- Added
- src/main/java/com/google/devtools/build/lib/starlarkbuildapi/apple/ObjcConfigurationApi.java
- Added
getIosDevice()method to theObjcConfigurationApiinterface, exposing theios_devicevalue to Starlark. (lines 51-57)
- Added
- src/test/java/com/google/devtools/build/lib/rules/objc/ObjcStarlarkTest.java
- Added a new test case,
testStarlarkCanAccessObjcConfigurationForDevice, to verify that Starlark can access theios_deviceflag. (lines 812-864)
- Added a new test case,
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
A new flag appears,
For devices, banish fears,
Xcode's tool now guides.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a new flag --ios_device to allow running iOS applications on physical devices using the xcrun devicectl command-line tool. The changes look good overall, and the addition of a test case is appreciated. However, there are a few areas that could be improved for clarity and maintainability.
Summary of Findings
- Documentation Clarity: The help text for the
ios_deviceflag could be more explicit about the supported device identification methods and potential issues. - Error Handling: Consider adding error handling or validation for the
ios_deviceflag to ensure that the provided device identifier is valid.
Merge Readiness
The pull request is almost ready for merging. Addressing the documentation clarity and error handling suggestions would improve the robustness and user-friendliness of the new feature. I am unable to approve this pull request, and users should have others review and approve this code before merging. I recommend that the pull request not be merged until the high severity issues are addressed.
| help = | ||
| "The identifier, ECID, serial number, UDID, user-provided name, or DNS name of " | ||
| + "the device for running an iOS application. " | ||
| + "You can get a list of devices by running 'xcrun devicectl list " | ||
| + "devices'.") |
There was a problem hiding this comment.
The help text is good, but it could be improved by mentioning that the identifier can be obtained from Xcode as well, and that the tool requires Xcode 15 or later. Also, consider adding a note about potential issues with device access or permissions.
For example, the help text could be reworded as follows:
"The identifier, ECID, serial number, UDID, user-provided name, or DNS name of the device for running an iOS application. You can get a list of devices by running 'xcrun devicectl list devices' (requires Xcode 15 or later) or from Xcode's Devices and Simulators window. Ensure that the device is properly connected and authorized for development."
"The identifier, ECID, serial number, UDID, user-provided name, or DNS name of "
+ "the device for running an iOS application. "
+ "You can get a list of devices by running 'xcrun devicectl list "
+ "devices' (requires Xcode 15 or later) or from Xcode's Devices and Simulators "
+ "window. Ensure that the device is properly connected and authorized for development."
Related to this PR in the rules_apple.
In Xcode 15, Apple introduced a new command-line tool that allows management of physical devices. This makes it possible to install and launch *_application targets on a physical device without third-party solutions.
It would be nice to have a Bazel flag to pass a device identifier for this.
Summary by CodeRabbit
New Features
Tests