Skip to content

Conversation

NingLi670
Copy link

blob=adb_pb2.AdbRequest.InstallApk.Blob is not support in Windows System. Change it to install the apk from the file system.

@NingLi670 NingLi670 changed the title Update a11y_grpc_wrapper.py Install a11y forwarding apk from filesystem Mar 2, 2025
@kenjitoyama
Copy link
Collaborator

Hello, can you elaborate on the actual problems you're having? Blob uses tempfile.NamedTemporaryFile() which is supported on Windows. Instead of hacking A11yGrpcWrapper, the right thing to do is to fix the core issue in adb_call_parser if there is something wrong there. Also we need unit tests for these changes, and again, this will be much easier to isolate in adb_call_parser than in A11yGrpcWrapper. Cheers!

@NingLi670
Copy link
Author

Hello, can you elaborate on the actual problems you're having? Blob uses tempfile.NamedTemporaryFile() which is supported on Windows. Instead of hacking A11yGrpcWrapper, the right thing to do is to fix the core issue in adb_call_parser if there is something wrong there. Also we need unit tests for these changes, and again, this will be much easier to isolate in adb_call_parser than in A11yGrpcWrapper. Cheers!

Hi, thanks for your feedback! This is a bug I encountered while using AndroidWorld in Windows Platform. The specific error is:

Traceback (most recent call last):
  File "D:\codes\android_world\minimal_task_runner.py", line 166, in <module>
    app.run(main)
  File "D:\codes\envs\android_env\Lib\site-packages\absl\app.py", line 308, in run
    _run_main(main, args)
  File "D:\codes\envs\android_env\Lib\site-packages\absl\app.py", line 254, in _run_main
    sys.exit(main(argv))
             ^^^^^^^^^^
  File "D:\codes\android_world\minimal_task_runner.py", line 162, in main
    _main()
  File "D:\codes\android_world\minimal_task_runner.py", line 96, in _main
    env = env_launcher.load_and_setup_env(
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\codes\android_world\android_world\env\env_launcher.py", line 115, in load_and_setup_env
    env = _get_env(console_port, adb_path, grpc_port)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\codes\android_world\android_world\env\env_launcher.py", line 35, in _get_env
    controller = android_world_controller.get_controller(
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\codes\android_world\android_world\env\android_world_controller.py", line 323, in get_controller
    return AndroidWorldController(android_env_instance)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\codes\android_world\android_world\env\android_world_controller.py", line 166, in __init__
    self._env = apply_a11y_forwarder_app_wrapper(
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\codes\android_world\android_world\env\android_world_controller.py", line 139, in apply_a11y_forwarder_app_wrapper
    return a11y_grpc_wrapper.A11yGrpcWrapper(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\codes\android_env-main\android_env\wrappers\a11y_grpc_wrapper.py", line 124, in __init__
    self._install_a11y_forwarding_apk()
  File "D:\codes\android_env-main\android_env\wrappers\a11y_grpc_wrapper.py", line 212, in _install_a11y_forwarding_apk
    install_response = self._env.execute_adb_call(install_request)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\codes\android_env-main\android_env\environment.py", line 150, in execute_adb_call
    return self._coordinator.execute_adb_call(call)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\codes\android_env-main\android_env\components\coordinator.py", line 181, in execute_adb_call
    return self._adb_call_parser.parse(call)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\codes\android_env-main\android_env\components\adb_call_parser.py", line 111, in parse
    return self._handlers[command_type](request, timeout)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\codes\android_env-main\android_env\components\adb_call_parser.py", line 287, in _install_apk
    response, _ = self._execute_command(
                  ^^^^^^^^^^^^^^^^^^^^^^
  File "D:\codes\android_env-main\android_env\components\adb_call_parser.py", line 81, in _execute_command
    command_output = self._adb_controller.execute_command(
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\codes\android_env-main\android_env\components\adb_controller.py", line 147, in execute_command
    raise errors.AdbControllerError(
android_env.components.errors.AdbControllerError: Error executing adb command: [adb -P 5038 -s emulator-5554 install -r -t -g D:\Users\
<user-name>\AppData\Local\Temp\tmpw0kmd7av.apk]
Caused by: Command '['C:\\adb\\platform-tools\\adb.exe', '-P', '5038', '-s', 'emulator-5554', 'install', '-r', '-t', '-g', 'D:\\Users\\
<user-name>\\AppData\\Local\\Temp\\tmpw0kmd7av.apk']' returned non-zero exit status 1.
adb stdout: [b'adb: failed to open D:\\Users\\<user-name>\\AppData\\Local\\Temp\\tmpw0kmd7av.apk: No such file or directory\r\nPerforming 
Streamed Install\r\n']
adb stderr: [None]

Besides, the unit tests can reproduce the problem, because there isn't a real Android device or avd in the unit test. I think the problem lies in adb_call_parser:

case 'blob':
with tempfile.NamedTemporaryFile(suffix='.apk') as f:
fpath = f.name
f.write(install_apk.blob.contents)
response, _ = self._execute_command(
['install', '-r', '-t', '-g', fpath], timeout=timeout
)

If delete the temporary file after close it, the install command can execute well. So what do you think if I change the code as:

      case 'blob':
        with tempfile.NamedTemporaryFile(suffix='.apk', delete=False) as f:
          fpath = f.name
          f.write(install_apk.blob.contents)

          response, _ = self._execute_command(
              ['install', '-r', '-t', '-g', fpath], timeout=timeout
          )

        try:
          os.remove(fpath)
        except Exception as e:
          logging.warning('Failed to remove temporary file: %s', fpath)

@kenjitoyama
Copy link
Collaborator

Hello @NingLi670 , thanks for providing logs.

Can you try setting delete_on_close=false instead and removing the explicit deletion after the context manager? The whole idea with using tempfile.NamedTemporaryFile() was to avoid this manual bookkeeping.

If it works well, I'll actually make the change internally at Google if you don't mind because we haven't setup PR imports yet.

Thanks,

Daniel

@NingLi670
Copy link
Author

Hi @kenjitoyama ,

It works well when setting delete_on_close=false. I'm ok if you make the change internally at Google. AndroidEnv is a great work and I'm glad to make it better.

Best regards,

Ning

copybara-service bot pushed a commit that referenced this pull request Mar 3, 2025
The behavior of
[`NamedTemporaryFile()`](https://docs.python.org/3/library/tempfile.html#tempfile.NamedTemporaryFile)
is slightly different on Windows when the file is accessed multiple times in
the same context manager. `delete_on_close=False` allows the file to exist
until the end of the context manager, which is the behavior we want in all
platforms.

Please see PR #265 for
details (and thanks to @NingLi670 for opening the original PR).

PiperOrigin-RevId: 732946651
copybara-service bot pushed a commit that referenced this pull request Mar 3, 2025
The behavior of
[`NamedTemporaryFile()`](https://docs.python.org/3/library/tempfile.html#tempfile.NamedTemporaryFile)
is slightly different on Windows when the file is accessed multiple times in
the same context manager. `delete_on_close=False` allows the file to exist
until the end of the context manager, which is the behavior we want in all
platforms.

Unfortunately this argument was only added in Python `3.12`, so we add a switch
to support earlier versions. We may remove it in the future once `3.11` is EOL.

Please see PR #265 for
details (and thanks to @NingLi670 for opening the original PR).

PiperOrigin-RevId: 732946651
copybara-service bot pushed a commit that referenced this pull request Mar 3, 2025
The behavior of
[`NamedTemporaryFile()`](https://docs.python.org/3/library/tempfile.html#tempfile.NamedTemporaryFile)
is slightly different on Windows when the file is accessed multiple times in
the same context manager. `delete_on_close=False` allows the file to exist
until the end of the context manager, which is the behavior we want in all
platforms.

Unfortunately this argument was only added in Python `3.12`, so we add a switch
to support earlier versions. We may remove it in the future once `3.11` is EOL.

Please see PR #265 for
details (and thanks to @NingLi670 for opening the original PR).

PiperOrigin-RevId: 732946651
copybara-service bot pushed a commit that referenced this pull request Mar 3, 2025
The behavior of
[`NamedTemporaryFile()`](https://docs.python.org/3/library/tempfile.html#tempfile.NamedTemporaryFile)
is slightly different on Windows when the file is accessed multiple times in
the same context manager. `delete_on_close=False` allows the file to exist
until the end of the context manager, which is the behavior we want in all
platforms.

Unfortunately this argument was only added in Python `3.12`, so we add a switch
to support earlier versions. We may remove it in the future once `3.11` is EOL.

Please see PR #265 for
details (and thanks to @NingLi670 for opening the original PR).

PiperOrigin-RevId: 732946651
copybara-service bot pushed a commit that referenced this pull request Mar 3, 2025
The behavior of
[`NamedTemporaryFile()`](https://docs.python.org/3/library/tempfile.html#tempfile.NamedTemporaryFile)
is slightly different on Windows when the file is accessed multiple times in
the same context manager. `delete_on_close=False` allows the file to exist
until the end of the context manager, which is the behavior we want in all
platforms.

Unfortunately this argument was only added in Python `3.12`, so we add a switch
to support earlier versions. We may remove it in the future once `3.11` is EOL.

Please see PR #265 for
details (and thanks to @NingLi670 for opening the original PR).

PiperOrigin-RevId: 732961399
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.

2 participants