search

consolidation / site-process

A thin wrapper around the Symfony Process Component that allows applications to use the Site Alias library to specify the target for a remote call.
Other
50 stars 19 forks source link

setWorkingDirectory() controls remote execution dir #25

Closed weitzman closed 5 years ago

weitzman commented 5 years ago

Overview

This pull request:

Q A
Bug fix? yes
New feature? no
Has tests? yes (in drush)
BC breaks? no
Deprecations? no

Summary

The current implementation does a setWorkingDirectory() even when the resulting Process is targetting a remote site. Thats not desireable at all, since the directory may be meant for a remote machine.

This PR removes calls to setWorkingDirectory instead relies on the Transport to do this. LocalTransport is the only one to do so.

greg-1-anderson commented 5 years ago

I'm not fond of giving the process object to addChdir. The transport is given a chance to alter the process object in the configure method. If we called this later, then local transport could cache the cwd when it was provided, and then apply it to the process object in the configure method.

weitzman commented 5 years ago

Adjusted as suggested

weitzman commented 5 years ago

Tests are passing except for Appveyor which is known failure AFAIK.

weitzman commented 5 years ago

Inverted meaning of setWorkingDirectory() as discussed.

greg-1-anderson commented 5 years ago

I think I'd rather repair setWorkingDirectory calls in the transport rather than swap in SiteProcess, but let's merge like this, as this works, and consider maybe altering later.