Revert "remote_access: enforce shell=True on shell commands"
This reverts commit 2415126c2943b21af0d9a25e2e640b4359406bfd.
2/2
BUG=chromium:776099
Change-Id: I7c13bc56398cc204afbc35eef898ce7f6f36f90d
Reviewed-on: https://chromium-review.googlesource.com/734981
Reviewed-by: Richard Barnette <jrbarnette@google.com>
Tested-by: David Haddock <dhaddock@chromium.org>
Trybot-Ready: David Haddock <dhaddock@chromium.org>
diff --git a/scripts/deploy_chrome.py b/scripts/deploy_chrome.py
index 9219879..d913e73 100644
--- a/scripts/deploy_chrome.py
+++ b/scripts/deploy_chrome.py
@@ -50,9 +50,9 @@
KILL_PROC_MAX_WAIT = 10
POST_KILL_WAIT = 2
-# Complicated commands that unittests want to mock out.
-# All others should be inlined.
-MOUNT_RW_COMMAND = ['mount', '-o', 'remount,rw', '/']
+MOUNT_RW_COMMAND = 'mount -o remount,rw /'
+LSOF_COMMAND = 'lsof %s/chrome'
+DBUS_RELOAD_COMMAND = 'killall -HUP dbus-daemon'
_ANDROID_DIR = '/system/chrome'
_ANDROID_DIR_EXTRACT_PATH = 'system/chrome/*'
@@ -60,6 +60,13 @@
_CHROME_DIR = '/opt/google/chrome'
_CHROME_DIR_MOUNT = '/mnt/stateful_partition/deploy_rootfs/opt/google/chrome'
+_UMOUNT_DIR_IF_MOUNTPOINT_CMD = (
+ 'if mountpoint -q %(dir)s; then umount %(dir)s; fi')
+_BIND_TO_FINAL_DIR_CMD = 'mount --rbind %s %s'
+_SET_MOUNT_FLAGS_CMD = 'mount -o remount,exec,suid %s'
+
+DF_COMMAND = 'df -k %s'
+
def _UrlBaseName(url):
"""Return the last component of the URL."""
@@ -98,7 +105,7 @@
self.chrome_dir = _CHROME_DIR
def _GetRemoteMountFree(self, remote_dir):
- result = self.device.RunCommand(['df', '-k', remote_dir])
+ result = self.device.RunCommand(DF_COMMAND % remote_dir)
line = result.output.splitlines()[1]
value = line.split()[3]
multipliers = {
@@ -109,7 +116,7 @@
return int(value.rstrip('GMK')) * multipliers.get(value[-1], 1)
def _GetRemoteDirSize(self, remote_dir):
- result = self.device.RunCommand(['du', '-ks', remote_dir],
+ result = self.device.RunCommand('du -ks %s' % remote_dir,
capture_output=True)
return int(result.output.split()[0])
@@ -120,9 +127,8 @@
return int(result.output.split()[0])
def _ChromeFileInUse(self):
- result = self.device.RunCommand(
- ['lsof', os.path.join(self.options.target_dir, 'chrome')],
- error_code_ok=True, capture_output=True)
+ result = self.device.RunCommand(LSOF_COMMAND % (self.options.target_dir,),
+ error_code_ok=True, capture_output=True)
return result.returncode == 0
def _Reboot(self):
@@ -142,17 +148,17 @@
# Since we stopped Chrome earlier, it's good form to start it up again.
if self.options.startui:
logging.info('Starting Chrome...')
- self.device.RunCommand(['start', 'ui'])
+ self.device.RunCommand('start ui')
raise DeployFailure('Need rootfs verification to be disabled. '
'Aborting.')
logging.info('Removing rootfs verification from %s', self.options.to)
# Running in VM's cause make_dev_ssd's firmware sanity checks to fail.
# Use --force to bypass the checks.
- cmd = ['/usr/share/vboot/bin/make_dev_ssd.sh', '--force',
- '--remove_rootfs_verification', '--partitions']
+ cmd = ('/usr/share/vboot/bin/make_dev_ssd.sh --partitions %d '
+ '--remove_rootfs_verification --force')
for partition in (KERNEL_A_PARTITION, KERNEL_B_PARTITION):
- self.device.RunCommand(cmd + [str(partition)], error_code_ok=True)
+ self.device.RunCommand(cmd % partition, error_code_ok=True)
self._Reboot()
@@ -167,7 +173,7 @@
# <job_name> <status> ['process' <pid>].
# <status> is in the format <goal>/<state>.
try:
- result = self.device.RunCommand(['status', 'ui'], capture_output=True)
+ result = self.device.RunCommand('status ui', capture_output=True)
except cros_build_lib.RunCommandError as e:
if 'Unknown job' in e.result.error:
return False
@@ -179,7 +185,7 @@
def _KillProcsIfNeeded(self):
if self._CheckUiJobStarted():
logging.info('Shutting down Chrome...')
- self.device.RunCommand(['stop', 'ui'])
+ self.device.RunCommand('stop ui')
# Developers sometimes run session_manager manually, in which case we'll
# need to help shut the chrome processes down.
@@ -189,7 +195,7 @@
logging.warning('The chrome binary on the device is in use.')
logging.warning('Killing chrome and session_manager processes...\n')
- self.device.RunCommand(['pkill', 'chrome|session_manager'],
+ self.device.RunCommand("pkill 'chrome|session_manager'",
error_code_ok=True)
# Wait for processes to actually terminate
time.sleep(POST_KILL_WAIT)
@@ -268,26 +274,23 @@
for p in self.copy_paths:
if p.mode:
# Set mode if necessary.
- self.device.RunCommand(
- ['chmod', '%o' % p.mode,
- '%s/%s' % (self.options.target_dir,
- p.src if not p.dest else p.dest)])
+ self.device.RunCommand('chmod %o %s/%s' % (
+ p.mode, self.options.target_dir, p.src if not p.dest else p.dest))
# Send SIGHUP to dbus-daemon to tell it to reload its configs. This won't
# pick up major changes (bus type, logging, etc.), but all we care about is
# getting the latest policy from /opt/google/chrome/dbus so that Chrome will
# be authorized to take ownership of its service names.
- self.device.RunCommand(['killall', '-HUP', 'dbus-daemon'],
- error_code_ok=True)
+ self.device.RunCommand(DBUS_RELOAD_COMMAND, error_code_ok=True)
if self.options.startui:
logging.info('Starting UI...')
- self.device.RunCommand(['start', 'ui'])
+ self.device.RunCommand('start ui')
def _CheckConnection(self):
try:
logging.info('Testing connection to the device...')
- self.device.RunCommand(['true'])
+ self.device.RunCommand('true')
except cros_build_lib.RunCommandError as ex:
logging.error('Error connecting to the test device.')
raise DeployFailure(ex)
@@ -315,16 +318,15 @@
logging.info('Mounting Chrome...')
# Create directory if does not exist
- self.device.RunCommand(['mkdir', '-p', '--mode', '0775',
- self.options.mount_dir])
+ self.device.RunCommand('mkdir -p --mode 0775 %s' % (
+ self.options.mount_dir,))
# Umount the existing mount on mount_dir if present first
- cmd = 'if mountpoint -q %(dir)s; then umount %(dir)s; fi'
- self.device.RunCommand(cmd % {'dir': self.options.mount_dir}, shell=True)
- self.device.RunCommand(['mount', '--rbind', self.options.target_dir,
- self.options.mount_dir])
+ self.device.RunCommand(_UMOUNT_DIR_IF_MOUNTPOINT_CMD %
+ {'dir': self.options.mount_dir})
+ self.device.RunCommand(_BIND_TO_FINAL_DIR_CMD % (self.options.target_dir,
+ self.options.mount_dir))
# Chrome needs partition to have exec and suid flags set
- self.device.RunCommand(['mount', '-o', 'remount,exec,suid',
- self.options.mount_dir])
+ self.device.RunCommand(_SET_MOUNT_FLAGS_CMD % (self.options.mount_dir,))
def Cleanup(self):
"""Clean up RemoteDevice."""