Conversation
|
@marcbonnici please take a look! |
This patch fixes following issues: * Starting from Android 11 it is not possible to use Runtime.exec to execute commands that require shell permissions like `am start`. * Engineering Android versions has su that doesn't support -c argument.
kbaladurin
force-pushed
the
fix-applaunch-workload
branch
from
d9c2ccd to
3f5707e
Compare
marcbonnici
left a comment
marcbonnici
left a comment
There was a problem hiding this comment.
Few thoughts, otherwise confirmed to work on my nexus5.
| # Android doesn't allow to writable dex files starting 14 version and | ||
| # default location (/sdcard/devlib-target) doesn't support readonly files | ||
| # so we use /data/local/tmp as asset directory for this workload. | ||
| self.asset_directory = '/data/local/tmp' |
There was a problem hiding this comment.
We tend to try and avoid hardcoding paths to support different configurations if possible.
Currently we have the default self.executables_directory set to /data/local/tmp/bin. I'm wondering if these directories would share the same requirements which we could also make use of here rather than setting this parameter in two locations?
If not perhaps this be a workload Parameter with a default so a user can understand the requirements for the directory and optionally change this for their system if required?
There was a problem hiding this comment.
The main requirement for assert_directory for applaunch workload is a ability to create readonly files there and looks like executables_directory in general doesn't require it. But adding a new parameter for applauch workload with a default value is a good idea, thank you!
| try: | ||
| self.target.execute('su root id') | ||
| except TargetError: | ||
| raise WorkloadError( |
There was a problem hiding this comment.
Missing import for WorkloadError.
| # su [WHO [COMMAND...]] | ||
| # - Targets with rooted user Android version have su that supports passing | ||
| # command via -c argument. | ||
| def su_has_command_option(self): |
There was a problem hiding this comment.
Thinking about this a bit more, do other workloads that require root work on these engineering versions if they are relying on the su -c format?
In devlib we try to detect [1] which su command (su -c or 'echo {} | su) [2] we should use for a device so I'm wondering if it would be beneficial to move this check to the common code and then we can query it from here?
[1] https://github.com/ARM-software/devlib/blob/master/devlib/utils/android.py#L466
[2] https://github.com/ARM-software/devlib/blob/master/devlib/utils/android.py#L258
There was a problem hiding this comment.
Yes, I think moving this check to devlib is a good idea, thank you! I will create a PR for it.
2 participants
This patch fixes following issues:
Starting from Android 11 it is not possible to use Runtime.exec to execute commands that require shell permissions like
am start.Engineering Android versions has su that doesn't support -c argument.