Skip to content

Commit

Permalink
Get rid of enum for controlling where subprocess executes
Browse files Browse the repository at this point in the history
Use a bool flag instead to make it easier to pass across FFI.
  • Loading branch information
gshuflin committed Oct 23, 2019
1 parent 3ff4621 commit d7f50e5
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 49 deletions.
6 changes: 1 addition & 5 deletions src/python/pants/engine/interactive_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@
from pants.base.exception_sink import ExceptionSink



RunLocation = Enum("RunLocation", "WORKSPACE TEMPDIR")


@dataclass(frozen=True)
class InteractiveProcessResult:
process_exit_code: int
Expand All @@ -23,7 +19,7 @@ class InteractiveProcessResult:
class InteractiveProcessRequest:
argv: Tuple[str, ...]
env: Tuple[str, ...] = ()
run_location: RunLocation = RunLocation.TEMPDIR
run_in_workspace: bool = False


@dataclass(frozen=True)
Expand Down
2 changes: 0 additions & 2 deletions src/python/pants/engine/scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -600,8 +600,6 @@ def run_local_interactive_process(self, request: InteractiveProcessRequest) -> I
)
return self._scheduler._raise_or_return(result)



def materialize_directories(self, directories_paths_and_digests):
"""Creates the specified directories on the file system.
:param directories_paths_and_digests tuple<DirectoryToMaterialize>: Tuple of the path and
Expand Down
8 changes: 2 additions & 6 deletions src/python/pants/rules/core/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,7 @@
from pants.engine.addressable import BuildFileAddresses
from pants.engine.console import Console
from pants.engine.goal import Goal
from pants.engine.interactive_runner import (
InteractiveProcessRequest,
InteractiveRunner,
RunLocation,
)
from pants.engine.interactive_runner import InteractiveProcessRequest, InteractiveRunner
from pants.engine.rules import console_rule


Expand All @@ -24,7 +20,7 @@ def run(console: Console, runner: InteractiveRunner, build_file_addresses: Build
request = InteractiveProcessRequest(
argv=["/usr/bin/python"],
env=("TEST_ENV", "TEST"),
run_location=RunLocation.WORKSPACE
run_in_workspace=False,
)

try:
Expand Down
31 changes: 1 addition & 30 deletions src/rust/engine/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/rust/engine/engine_cffi/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ logging = { path = "../logging" }
rule_graph = { path = "../rule_graph" }
store = { path = "../fs/store" }
tar_api = { path = "../tar_api" }
tempfile = "3"
workunit_store = { path = "../workunit_store" }
ctrlc = "3.1.3"

[build-dependencies]
build_utils = { path = "../build_utils" }
Expand Down
12 changes: 12 additions & 0 deletions src/rust/engine/engine_cffi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ use std::os::raw;
use std::panic;
use std::path::{Path, PathBuf};
use std::time::Duration;
use tempfile::TempDir;

#[cfg(test)]
mod tests;
Expand Down Expand Up @@ -1015,6 +1016,17 @@ pub extern "C" fn run_local_interactive_process(
command.env(key, value);
}

let run_in_workspace = externs::project_bool(&value, "run_in_workspace");
let maybe_tempdir = if run_in_workspace {
None
} else {
Some(TempDir::new().map_err(|err| format!("Error creating tempdir: {}", err))?)
};

maybe_tempdir
.as_ref()
.map(|ref tempdir| command.current_dir(tempdir.path()));

let mut subprocess = command.spawn().map_err(|e| e.to_string())?;
let exit_status = subprocess.wait().map_err(|e| e.to_string())?;
let code = exit_status.code().unwrap_or(-1);
Expand Down
10 changes: 5 additions & 5 deletions src/rust/engine/src/externs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,17 +185,17 @@ pub fn project_multi_strs(item: &Value, field: &str) -> Vec<String> {
// This is intended for projecting environment variable maps - i.e. Python Dict[str, str] that are
// encoded as a Tuple of an (even) number of str's. It could be made more general if we need
// similar functionality for something else.
pub fn project_tuple_encoded_map(value: &Value, field: &str) -> Result<BTreeMap<String, String>, String> {
pub fn project_tuple_encoded_map(
value: &Value,
field: &str,
) -> Result<BTreeMap<String, String>, String> {
let mut map: BTreeMap<String, String> = BTreeMap::new();
let parts = project_multi_strs(&value, field);
if parts.len() % 2 != 0 {
return Err("Error parsing env: odd number of parts".to_owned());
}
for i in 0..(parts.len() / 2) {
map.insert(
parts[2 * i].clone(),
parts[2 * i + 1].clone(),
);
map.insert(parts[2 * i].clone(), parts[2 * i + 1].clone());
}
Ok(map)
}
Expand Down

0 comments on commit d7f50e5

Please sign in to comment.