History log of /external/vogar/src/vogar/SshTarget.java
Revision Date Author Comments (<<< Hide modified files) (Show modified files >>>)
411cb1fb67241125203629229600ecbd821eb9c7 19-Nov-2015 Paul Duffin <paulduffin@google.com> Fix escaping and grouping of arguments to the Target shells

Vogar allows custom arguments to be passed to the classes being
tested. If these contain special shell characters then the
command line used to run the tests may not be correct. e.g. If
the argument was "-Cvm.args=-Xmx256M -Xms256M" (which tells
Caliper to run the benchmark in a process with an initial and
maximum heap size of 256M) then when the command is actually run
by the adb shell they are split into two arguments,
"-Cvm.args=-Xmx256M" and "-Xms256M" which causes Caliper to fail
as the latter is an unrecognized Caliper option.

The solution is to simply escape any special shell characters
with a backslash (\) to ensure that they are treated as literal
characters.

Another bug is that any use of an argument "-c" will result in
all the following arguments being grouped into one, e.g.
fred -c wilma barney betty
will become
fred -c "wilma barney betty"

This behaviour is required when the -c option is part of "sh"
"-c" but not in any other case. This is currently implemented in
Command which is the wrong place as it's a generic class
independent of the Target and the grouping is dependent on the
target shell.

After some investigation it turns out that while grouping isn't
required for AdbTarget and SshTarget like it is for HostTarget
neither does it break anything. Therefore, for simplicity's sake
all targets will group their arguments into one when executing
the command line. That will be done before it is appended to the
targetProcessPrefix().

The escaping and grouping will be done by a new
Target.ScriptBuilder class, instances of which will be created
by a Target.newScriptBuilder() method that will make sure that
the ScriptBuilder returned will be appropriate for the target
shell.

It will provide support for specifying inline environment
variables, a working directory and adding tokens to the command
line. It will escape where necessary, e.g. it will not escape
the environment variable name or assignment but will escape the
value. It will escape all the tokens. When asked to construct an
appropriate command line to execute that script it will group
the script contents into one argument and append it to a target
specific prefix.

VmCommandBuilder will use the new ScriptBuilder to build the
command line that it then passes to Command. It will also be
changed to track environment variables explicitly (will use the
existing but unused env field and method) which it will then
pass onto the ScriptBuilder. Finally, it will track the
workingDirectory as well.

DeviceRuntime and HostRuntime will both change how they use
VmCommandBuilder to make use of the env(String, String) method
and the new workingDirectory(File).

Command will remove the processArgs() method and simply take a
copy of its args.

Added new tests for ScriptBuilder.escape(String) method and
a JUnit 4 TestSuite AllTests that can be used to run all the
tests that currently work within an IDE.

Change-Id: I0351c9fd3fe9d42c1b454251cf78ff74fafb08b2
/external/vogar/src/vogar/SshTarget.java
7029503206e4c89fe167c3389c1062d89cf35c52 19-Nov-2015 Paul Duffin <paulduffin@google.com> Refactoring to make more testable

Prior to making a change in the behaviour to fix an issue with
the quoting of command line arguments this refactors the
existing code and adds some tests to illustrate the existing
behaviour. This will minimize the chances that the fix will
break existing code.

The changes are explained below in more detail.

This applied a couple of common refactorings to a number of
classes:
* Where classes were creating a lot of subsidiary objects in
their constructor I switched to using the dependency injection
pattern (i.e. passing in parameters to the constructor). That
makes it easier for tests to override the behaviour.

* Where classes were being given a huge object (e.g. Run) but
only required a small number of it's fields I replaced the Run
constructor parameter with the individual parts.

LocalTarget - Replaced Run with Log, Mkdir, Rm.

Run - Passed in more parameters into the constructor, moved the
code that created those objects into Vogar.run(). Also, make the
vogarJar() method work when running from a class directory to
allow it to work from within IDEA as well as when running from
jars.

SshTarget - Removed a bit of duplication of the command line
prefix used to run a script remotely.

Target - Method defaultDeviceDir() was removed and the
implementations were made static and accessed directly from
Vogar. That is necessary to break a dependency cycle; AdbTarget
required AndroidSdk, which required DeviceFileCache, which
required runnerDir, which required Target.defaultDeviceDir().
Previously, that was broken by passing the DeviceFileCache to
the AndroidSdk after construction using the setCaches() method
but that has been removed as part of this refactoring.

Vogar - Made the constructor and parseArgs(String[]) visible for
testing. Moved some of the code from Run's constructor here to
create the values to be passed into its expanded constructor.
TargetType was added to encapsulate the defaultDeviceDir() for
each Target implementation class. That allowed the cycle
described in Target to be broken.

AdbTarget - Replaced Run with individual fields. Moved
functionality from AndroidSdk that's only used by this class
into here (waitForDevice(), ensureDirectory(File), remount(),
rm(File), forwardTcp(int), push() and pull()). Bringing in
push() also required moving the pushCache from AndroidSdk as
well. Added DeviceFilesystem as a parameter to avoid having to
have a reference to AndroidSdk.

AndroidSdk - Moved logic that runs shell commands to calculate
the compilationClasspath and androidJarPath out of the
constructor and into a static method to allow tests to pass
dummy values straight into the constructor. Marked constructor
as VisibleForTesting. Removed setCaches(..) method and
pushCache, passed the HostFileCache straight into the
constructor, marking the dexCache field as final. Added explicit
exception when can't find platforms directory rather than
NullPointerException. Moved methods that are only used by either
AdbTarget or DeviceFileCache into those classes.

DeviceFileCache - Replaced AndroidSdk with DeviceFileSystem and
moved cp(File, File) and mv(File, File) from AndroidSdk into
here.

DeviceRuntime - Added a Supplier<String> parameter to provide
the device user name in order to allow tests to provide their
own. That is needed because otherwise the test will attempt
to connect to a device to determine the user name.

Command - Added method to access the args for testing, moved the
special processing of "-c" option from start() into a method
called from the constructor. That allows the test to verify the
processing of that without actually executing the command.
Cleaned up unused workingDirectory method and some minor
warnings.

Added some tests and fixed a compile issue with the
JUnitRunnerTest.

Change-Id: Ib1676ee19a4f0e7a8944b2708a6dbe3899d1d292
/external/vogar/src/vogar/SshTarget.java
3fd1cf5ff629b06b63f09b79508ec290af07267f 21-Sep-2015 Neil Fuller <nfuller@google.com> Fix vogar target ls / rm tasks in the event of errors

The latest aosp/master adbd appears to report adb shell exit codes
properly.

vogar was previously relying on this not being the case.
It is possible that recent commit 99a9b24d3c broke the clean up task
behavior, but that doesn't appear to explain all the breaks.

Older devices (e.g. L-MR1) still return exit code 0 for command
failures.

This change returns the behavior to ignoring exit codes for the adb
shell tasks. Because the behavior is device-version specific there
isn't much point relying on the new exit-code behavior because vogar
would not function on the old devices.

Bug: 24250996
Change-Id: Iae7c992320c5e2caa6bdb3473a8d2536947c3ca2
/external/vogar/src/vogar/SshTarget.java
83672b1eeb88b7b8e04126a8656ac4594f676a54 22-Apr-2015 Elliott Hughes <enh@google.com> Fix vogar's idea of quoting.

This patch switches us over to using "sh -c" on the host and
"adb shell sh -c" on the device for consistency. Before we were using
exec(2) directly on the host and relying on a bug in adb shell on the
device. This patch stops exploiting that bug (which was fixed as bug
http://b/20323053) and stops using exec(2) locally to keep us honest.

We should probably remove SshTarget, but that's a job for another patch.

Bug: http://b/19734868
Bug: http://b/20323053
Change-Id: I9a74050e10502ff062fe404f8359aecdb82afbe2
/external/vogar/src/vogar/SshTarget.java
cc4bfe8e88d686c6fc28624c2b6a7a7e434beb77 27-Mar-2015 Spencer Low <CompareAndSwap@gmail.com> Fix parsing of id command

Matcher.matches() requires that the pattern match the entire input
string, so fixup the pattern to use ^ to match the beginning of the
input string and .* for the end.

Change-Id: I77ef82cd76a6e9c4f9d3b45d355886629d02c91f
Signed-off-by: Spencer Low <CompareAndSwap@gmail.com>
/external/vogar/src/vogar/SshTarget.java
a3baab9f2576bcdebd5f16647bcb3f0f80621e58 05-Dec-2014 Neil Fuller <nfuller@google.com> Change the default location for vogar execution

ART needs to be able to write to and map executable pages from the
device-dir to run without the interpreter.

The new location used here exists on KitKat and L devices tested.
If the default path does not exist it can be overridden with
--device-dir.

Bug: 18622813
Change-Id: Ia9b6fc3cca249974fabd337d48d6025bd19605a5
/external/vogar/src/vogar/SshTarget.java
30872febb4233d767d5e1cdffa2df81bac5b0f9f 07-Dec-2011 jessewilson@google.com <jessewilson@google.com@aa685c63-decc-881d-cd2b-7fa72aad72e1> Add a hack to work around missing environment variables on a third-parth sshd. We shouldn't need this to run on openssh.

git-svn-id: http://vogar.googlecode.com/svn/trunk@267 aa685c63-decc-881d-cd2b-7fa72aad72e1
/external/vogar/src/vogar/SshTarget.java
0eb0936e90bf11463c8ee937ca996d7bd654a098 07-Dec-2011 jessewilson@google.com <jessewilson@google.com@aa685c63-decc-881d-cd2b-7fa72aad72e1> Get most of running on a device via SSH working.

Run on a device like this:
vogar --mode device --ssh 192.168.149.198:2222 SocketTest.java

Currently this is failing because dalvikvm segfaults when invoked via SSH. I suspect it might be a problem with the SSH server running with either the wrong environment or the wrong privileges. I continue to investigate

git-svn-id: http://vogar.googlecode.com/svn/trunk@266 aa685c63-decc-881d-cd2b-7fa72aad72e1
/external/vogar/src/vogar/SshTarget.java