There doesn't appear to be anything that actually uses tmp_path in the
connection plugins so we don't need to pass that in to exec_command.
That change also means that we don't need to pass tmp_path around in
many places in the action plugins any more. there may be more cleanup
that can be done there as well (the action plugin's public run() method
takes tmp as a keyword arg but that may not be necessary).
As a sideeffect of this patch, some potential problems with chmod and
the patch, assemble, copy, and template modules has been fixed (those
modules called _remote_chmod() with the wrong order for their
parameters. Removing the tmp parameter fixed them.)
The main exec_command/put_file/fetch_file methods now _build_command and
call _run to handle input from/output to the ssh process. The purpose is
to bring connection handling together in one place so that the locking
doesn't have to be split across functions.
Note that this doesn't change the privilege escalation and connection IO
code at all—just puts it all into one function.
Most of the changes are just moving code from one place to another (e.g.
from _connect to _build_command, from _exec_command and _communicate to
_run), but there are some other notable changes:
1. We test for the existence of sshpass the first time we need to use
password authentication, and remember the result.
2. We set _persistent in _build_command if we're using ControlPersist,
for later use in close(). (The detection could be smarter.)
3. Some apparently inadvertent inconsistencies between put_file and
fetch_file (e.g. argument quoting, sftp -b use) have been removed.
Also reorders functions into a logical sequence, removes unused imports
and functions, etc.
Aside: the high-level EXEC/PUT/FETCH description should really be logged
from ConnectionBase, while individual subclasses log transport-specific
details.
jctanner
James Cammarata
Matt Clay
Toshio Kuratomi
Matt Martz
Matthew Stoltenberg
Alexey Shabalin
Brian Coca
Abhijit Menon-Sen