-
-
Notifications
You must be signed in to change notification settings - Fork 535
Android sys img picker #2843
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
base: main
Are you sure you want to change the base?
Android sys img picker #2843
Changes from all commits
a545d22
f60e924
11da731
40a4f9e
88ce516
48fccb9
2a5dc7a
11ab15c
6f79400
5644e62
c979c38
290a0d9
b0c020a
a4cb82d
6da467d
2256f48
d8cf956
78cd901
c9f0b68
ffa16a8
f03a355
7af18df
c97d6f9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Android emulator system images and image types can now be selected when creating a new emulator, with options ranging from Android 26 to the latest available version. |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -10,7 +10,7 @@ | |||||
| from datetime import datetime | ||||||
| from pathlib import Path | ||||||
|
|
||||||
| from briefcase.config import PEP508_NAME_RE | ||||||
| from briefcase.config import PEP508_NAME_RE, FinalizedAppConfig | ||||||
| from briefcase.console import Console | ||||||
| from briefcase.exceptions import ( | ||||||
| BriefcaseCommandError, | ||||||
|
|
@@ -24,6 +24,7 @@ | |||||
| from briefcase.integrations.subprocess import SubprocessArgT | ||||||
|
|
||||||
| DEVICE_NOT_FOUND = re.compile(r"^error: device '[^']*' not found") | ||||||
| ANDROID_MIN_OS_VERSION = 24 | ||||||
|
|
||||||
|
|
||||||
| def create_avd_validator(emulators): | ||||||
|
|
@@ -40,6 +41,49 @@ def _validate_avd_name(avd): | |||||
| return _validate_avd_name | ||||||
|
|
||||||
|
|
||||||
| def _parse_system_image(package: str): | ||||||
| """Parse a system image package identifier into its components. | ||||||
|
|
||||||
| :param package: e.g. ``"system-images;android-31;default;x86_64"`` | ||||||
| :returns: A tuple of (api_level, tag, abi) or None if invalid. | ||||||
| """ | ||||||
| parts = package.split(";") | ||||||
| if len(parts) != 4 or parts[0] != "system-images": | ||||||
| return None | ||||||
| return parts[1], parts[2], parts[3] | ||||||
|
|
||||||
|
|
||||||
| def _api_level_sort_key(api_level: str) -> tuple: | ||||||
| """Sort key for API level strings, ordering numeric levels (and their ext suffixes) | ||||||
| numerically. Non-numeric levels sort after all numeric ones. | ||||||
|
|
||||||
| e.g. "android-34" -> (0, 34.0, 0) "android-34-ext9" -> (0, 34.0, 9) | ||||||
| "android-36.0-Baklava" -> (0, 36.0, 0) "android-CANARY" -> (1, "android- | ||||||
| CANARY") | ||||||
| """ | ||||||
| value = api_level.split("-", 1)[ | ||||||
| 1 | ||||||
| ] # "34", "34-ext9", "36.1", "36.0-Baklava", "CANARY" | ||||||
| base, _, ext = value.partition("-ext") | ||||||
| base = base.split("-", 1)[ | ||||||
| 0 | ||||||
| ] # strip trailing "-Name" suffix, e.g. "36.0-Baklava" -> "36.0" | ||||||
| try: | ||||||
| return 0, float(base), int(ext) if ext else 0 | ||||||
| except ValueError: | ||||||
| return 1, api_level | ||||||
|
|
||||||
|
|
||||||
| def min_api_level(app: FinalizedAppConfig) -> int: | ||||||
| """The minimum API level to use for the app, as an int. | ||||||
|
|
||||||
| ``min_os_version`` may be configured as a string or an int, so it's | ||||||
| coerced to int here to avoid a TypeError when compared against | ||||||
| numeric API levels. | ||||||
| """ | ||||||
| return int(getattr(app, "min_os_version", ANDROID_MIN_OS_VERSION)) | ||||||
|
|
||||||
|
|
||||||
| class AndroidDeviceNotAuthorized(BriefcaseCommandError): | ||||||
| def __init__(self, device): | ||||||
| self.device = device | ||||||
|
|
@@ -199,9 +243,20 @@ def DEFAULT_DEVICE_TYPE(self) -> str: | |||||
| def DEFAULT_DEVICE_SKIN(self) -> str: | ||||||
| return "pixel_7_pro" | ||||||
|
|
||||||
| @property | ||||||
| def DEFAULT_API_LEVEL(self) -> str: | ||||||
| return "android-31" | ||||||
|
|
||||||
| @property | ||||||
| def DEFAULT_TAG(self) -> str: | ||||||
| return "default" | ||||||
|
|
||||||
| @property | ||||||
| def DEFAULT_SYSTEM_IMAGE(self) -> str: | ||||||
| return f"system-images;android-31;default;{self.emulator_abi}" | ||||||
| return ( | ||||||
| f"system-images;{self.DEFAULT_API_LEVEL}" | ||||||
| f";{self.DEFAULT_TAG};{self.emulator_abi}" | ||||||
| ) | ||||||
|
|
||||||
| @classmethod | ||||||
| def sdk_path_from_env(cls, tools: ToolCache) -> tuple[str | None, str | None]: | ||||||
|
|
@@ -719,6 +774,44 @@ def verify_avd(self, avd: str): | |||||
| except KeyError: | ||||||
| self.tools.console.debug(f"Device {avd!r} doesn't define a skin.") | ||||||
|
|
||||||
| def list_available_system_images(self, min_api_level: int) -> list[str]: | ||||||
| """Returns a sorted list of system image package identifiers available for the | ||||||
| current architecture and minimum Android version. | ||||||
|
|
||||||
| e.g., ``{"system-images;android-31;default;x86_64"}`` | ||||||
|
|
||||||
| :param min_api_level: The minimum Android API level to include. | ||||||
| """ | ||||||
|
|
||||||
| try: | ||||||
| output = self.tools.subprocess.check_output( | ||||||
| [self.sdkmanager_path, "--list"], | ||||||
| env=self.env, | ||||||
| ) | ||||||
| except subprocess.CalledProcessError as e: | ||||||
| raise BriefcaseCommandError( | ||||||
| "Unable to invoke the Android SDK manager" | ||||||
| ) from e | ||||||
|
|
||||||
| images = [] | ||||||
| for line in output.splitlines(): | ||||||
| package = line.split("|")[0].strip() | ||||||
| parsed = _parse_system_image(package) | ||||||
| if parsed is None: | ||||||
| continue | ||||||
| api_level, _, abi = parsed | ||||||
| if abi != self.emulator_abi: | ||||||
| continue | ||||||
| api_level_str = api_level.split("-")[1] # "android-31" -> "31" | ||||||
| try: | ||||||
| if int(api_level_str.split(".")[0]) < min_api_level: | ||||||
| continue | ||||||
| except ValueError: | ||||||
| # Non-numeric API level (e.g. CANARY, CinnamonBun) always include. | ||||||
| pass | ||||||
| images.append(package) | ||||||
| return sorted(set(images)) | ||||||
|
|
||||||
| def list_installed_system_images(self) -> set[str]: | ||||||
| """Returns a set of installed system image package identifiers. | ||||||
|
|
||||||
|
|
@@ -1099,9 +1192,10 @@ def select_target_device( | |||||
|
|
||||||
| return device, name, avd | ||||||
|
|
||||||
| def create_emulator(self) -> str: | ||||||
| def create_emulator(self, app: FinalizedAppConfig) -> str: | ||||||
| """Create a new Android emulator. | ||||||
|
|
||||||
| :param app: The config object for the app. | ||||||
| :returns: The AVD of the newly created emulator. | ||||||
| """ | ||||||
| # Get the list of existing emulators | ||||||
|
|
@@ -1132,8 +1226,49 @@ def create_emulator(self) -> str: | |||||
| device_type = self.DEFAULT_DEVICE_TYPE | ||||||
| skin = self.DEFAULT_DEVICE_SKIN | ||||||
|
|
||||||
| # TODO: Provide a list of options for system images. | ||||||
| system_image = self.DEFAULT_SYSTEM_IMAGE | ||||||
|
mhsmith marked this conversation as resolved.
|
||||||
| # Get available images, raise an error if none found. | ||||||
| available_images = self.list_available_system_images( | ||||||
| min_api_level=min_api_level(app) | ||||||
| ) | ||||||
| if not available_images: | ||||||
| raise BriefcaseCommandError( | ||||||
| f"""\ | ||||||
| No Android system images are available for your architecture | ||||||
|
Comment on lines
+1234
to
+1236
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could use the same dedent function as your other PR. |
||||||
| ({self.emulator_abi}). | ||||||
|
|
||||||
| This may be caused by a network connectivity issue or an unsupported | ||||||
| architecture. Check your network connection and re-run `briefcase run android`. | ||||||
|
Comment on lines
+1239
to
+1240
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Network connectivity seems like an unlikely cause, because if the sdkmanager failed to contact the server, it should have caused a CalledProcessError. However, it would be worth mentioning |
||||||
| """ | ||||||
| ) | ||||||
|
|
||||||
| # Parse available images once for use in both selection questions. | ||||||
| parsed_images = [_parse_system_image(img) for img in available_images] | ||||||
|
|
||||||
| # Ask the user to select an API level. | ||||||
| api_levels = sorted( | ||||||
| {api_level for api_level, _, abi in parsed_images}, | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| key=_api_level_sort_key, | ||||||
| ) | ||||||
| api_level = self.tools.console.selection_question( | ||||||
| intro="Select the API level for the emulator:", | ||||||
| description="API level", | ||||||
| options=api_levels, | ||||||
| default=self.DEFAULT_API_LEVEL, | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If |
||||||
| ) | ||||||
|
|
||||||
| # Ask the user to select a tag for the chosen API level. | ||||||
| tags = sorted( | ||||||
| {tag for level, tag, _ in parsed_images if level == api_level}, | ||||||
| key=lambda x: (0 if x == "default" else 1, x), | ||||||
| ) | ||||||
| tag = self.tools.console.selection_question( | ||||||
| intro="Select the system image tag:", | ||||||
| description="Tag", | ||||||
|
Comment on lines
+1264
to
+1266
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there any precedent for calling this a "tag"? "Type" seems like a more obvious name, and has previously been used in Android Studio (see right side of screenshot). |
||||||
| options=tags, | ||||||
| default=self.DEFAULT_TAG if self.DEFAULT_TAG in tags else tags[0], | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It doesn't look like this |
||||||
| ) | ||||||
|
|
||||||
| system_image = f"system-images;{api_level};{tag};{self.emulator_abi}" | ||||||
|
|
||||||
| self._create_emulator( | ||||||
| avd=avd, | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sorting seems redundant, since they'll be sorted again in the one place that calls this function. In which case, maybe it could return a
setinstead, which would be more consistent withlist_installed_system_images, and would make the docstring example syntax correct.