modules/android-integration: add am command and many termux-tools ones #382
Conversation
t184256
changed the title
| Provide an `am` (activity manager) command. | ||
| Is not guaranteed to be a real deal, could be of limited compatibility | ||
| with real `am` (like `termux-am`). | ||
| ''; |
There was a problem hiding this comment.
This is phrased weirdly, the "is not garunteed/could be" language is unnecessary. A link to more details on the limitations would be great (if they exist). I would suggest:
Provide an `am` (activity manager) command.
It is not the real deal, and has limited capabilities compared to the real `am`, similar to `termux-am`. See [here](insert link here) for more details.
There was a problem hiding this comment.
Thing is, I'm not well-versed in Android and don't know its exact limitations. It's very different from the real deal in /system/bin/am, which is a shell wrapper against some java thing, in that it sends the command-line over a socket and something in Termux app handles that. The help message is very different, I wouldn't be surprised if it had different error reporting or differed in exit codes, etc., etc., but I don't know the exact details. All I know it's the best am we can currently have because it works.
| --replace @TERMUX_APP_PACKAGE@ com.termux.nix | ||
| substituteInPlace scripts/termux-reload-settings.in \ | ||
| --replace @TERMUX_APP_PACKAGE@ com.termux.nix | ||
| ${gnused}/bin/sed -i 's|^am |${termux-am}/bin/am |' scripts/* |
There was a problem hiding this comment.
Why do you use substituteInPlace above but gnused here?
There was a problem hiding this comment.
because substituteInPlace is the recommended Nix way, being more explicit and less surprising and everything...
... and then there are 6 calls to am that need to be patched to use the real path, but you can't just replace all am, it has to be at the beginning of the line, so I whip up a sed and use a regexp.
t184256
force-pushed
the
android-integration
branch
from
dd13e92
to
2d22124
Compare
|
Thank you both for the reviews! |
(sorry, there will be a CI failure in test_channels_uiautomator because a warning pushes success message off-screen. will deal with it tomorrow)fixed