ADB root fails if already root

[douglas-raillard-arm]

with recent versions of adb, adb root can fail if the daemon is already running as root:

$ adb -s 1A261FDF6007F8 root
adbd is already running as root

This raises makes the adb_root() function raise an exception:

CalledProcessError: Command 'adb -s 1A261FDF6007F8 root' returned non-zero exit status 1.

I suppose we should parse the exception's .output and check for adbd is already running as root string in it. If we do that, we will need to run the command with LC_ALL=C to ensure the output is not translated.

[marcbonnici]

Makes sense, would you be able to try this patch out [1] and see if it does the trick?

As for running with LC_ALL=C this seems like this might be best dealt with as part of a wider change since non of the other adb commands are currently doing this at the moment either.

[1] https://github.com/marcbonnici/devlib/commit/68e56a99c43747753b38b95b55972dfb5abdd0aa

[douglas-raillard-arm]

This patch fixed the issue, and it seems to be mostly equivalent to yours:

diff --git a/external/devlib/devlib/utils/android.py b/external/devlib/devlib/utils/android.py
index 32afa96b0..05537b5ad 100755
--- a/devlib/devlib/utils/android.py
+++ b/devlib/devlib/utils/android.py
@@ -385,9 +385,16 @@ class AdbConnection(ConnectionBase):

     def adb_root(self, enable=True):
         cmd = 'root' if enable else 'unroot'
-        output = adb_command(self.device, cmd, timeout=30, adb_server=self.adb_server)
-        if 'cannot run as root in production builds' in output:
-            raise TargetStableError(output)
+        try:
+            output = adb_command(self.device, cmd, timeout=30, adb_server=self.adb_server, stderr=subprocess.STDOUT)
+        except subprocess.CalledProcessError as e:
+            if 'is already running as root' in e.output:
+                pass
+            else:
+                raise
+        else:
+            if 'cannot run as root in production builds' in output:
+                raise TargetStableError(output)
         AdbConnection._connected_as_root[self.device] = enable

     def wait_for_device(self, timeout=30):

It does not deal with LC_ALL though

[douglas-raillard-arm]

For the background, we just hit this issue with the official android image on pixel 6, and (I guess) a latest (or at least very recent) version of the adb client (LISA installs it from google's android tools zip).

[marcbonnici]

Ok thanks for confirming and for the background that's good to know. If that fixes the issue I'll submit https://github.com/ARM-software/devlib/pull/598 for now and create a new ticket for supporting LC_ALL? I think we should just be able to add that as part of the adb command generation function but would need to check.

[douglas-raillard-arm]

Sounds good, thanks

[marcbonnici]

Merged https://github.com/ARM-software/devlib/pull/598 and raised https://github.com/ARM-software/devlib/issues/599 for tracking LC_ALL. Therefore closing this issue for now.