Skip to content

Commit

Permalink
The status code of every subprocess is checked
Browse files Browse the repository at this point in the history
Every command will now print it's stderr to the terminal if it fails.
Previously the status code was not checked. _Creating_ the process was
checked (with expect or `?`) but the status code of the process on
completion was not.
  • Loading branch information
simonrw authored and David-OConnor committed Aug 27, 2020
1 parent 860408d commit dc91c9a
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 42 deletions.
15 changes: 9 additions & 6 deletions src/build.rs
Expand Up @@ -173,10 +173,11 @@ pub fn build(
// Twine has too many dependencies to install when the environment, like we do with `wheel`, and
// for now, it's easier to install using pip
// todo: Install using own tools instead of pip; this is the last dependence on pip.
Command::new(paths.bin.join("python"))
let output = Command::new(paths.bin.join("python"))
.args(&["-m", "pip", "install", "twine"])
.status()
.output()
.expect("Problem installing Twine");
util::check_command_output(&output, "failed to install twine");

// let twine_url = "https://files.pythonhosted.org/packages/c4/43/b9c56d378f5d0b9bee7be564b5c5fb65c65e5da6e82a97b6f50c2769249a/twine-2.0.0-py3-none-any.whl";
// install::download_and_install_package(
Expand All @@ -197,10 +198,11 @@ pub fn build(
println!("🛠️️ Building the package...");
// todo: Run build script first, right?
if let Some(build_file) = &cfg.build {
Command::new(paths.bin.join("python"))
let output = Command::new(paths.bin.join("python"))
.arg(&build_file)
.status()
.output()
.unwrap_or_else(|_| panic!("Problem building using {}", build_file));
util::check_command_output(&output, "failed to run build script");
}

// Command::new(paths.bin.join("python"))
Expand Down Expand Up @@ -228,10 +230,11 @@ pub(crate) fn publish(bin_path: &PathBuf, cfg: &crate::Config) {
};

println!("Uploading to {}", repo_url);
Command::new(bin_path.join("twine"))
let output = Command::new(bin_path.join("twine"))
.args(&["upload", "--repository-url", &repo_url, "dist/*"])
.status()
.output()
.expect("Problem publishing");
util::check_command_output(&output, "publishing");
}

#[cfg(test)]
Expand Down
19 changes: 12 additions & 7 deletions src/commands.rs
Expand Up @@ -83,21 +83,23 @@ pub fn find_py_version(alias: &str) -> Option<crate::Version> {
/// Additionally, create the __pypackages__ directory if not already created.
pub fn create_venv(py_alias: &str, lib_path: &Path, name: &str) -> Result<(), Box<dyn Error>> {
// While creating the lib path, we're creating the __pypackages__ structure.
Command::new(py_alias)
let output = Command::new(py_alias)
.args(&["-m", "venv", name])
.current_dir(lib_path.join("../"))
.output()?;
util::check_command_output(&output, "creating virtual environment");

Ok(())
}

// todo: DRY for using a path instead of str. use impl Into<PathBuf> ?
pub fn create_venv2(py_alias: &Path, lib_path: &Path, name: &str) -> Result<(), Box<dyn Error>> {
// While creating the lib path, we're creating the __pypackages__ structure.
Command::new(py_alias)
let output = Command::new(py_alias)
.args(&["-m", "venv", name])
.current_dir(lib_path.join("../"))
.output()?;
util::check_command_output(&output, "creating virtual environment");

Ok(())
}
Expand All @@ -108,7 +110,8 @@ pub fn run_python(
args: &[String],
) -> Result<(), Box<dyn Error>> {
util::set_pythonpath(lib_paths);
Command::new(bin_path.join("python")).args(args).status()?;
let output = Command::new(bin_path.join("python")).args(args).output()?;
util::check_command_output(&output, "running python");
Ok(())
}

Expand All @@ -119,18 +122,20 @@ pub fn download_git_repo(repo: &str, dest_path: &Path) -> Result<(), Box<dyn Err
util::abort("Can't find Git on the PATH. Is it installed?");
}

Command::new("git")
let output = Command::new("git")
.current_dir(dest_path)
.args(&["clone", repo])
.status()?;
.output()?;
util::check_command_output(&output, "cloning repo");
Ok(())
}

/// Initialize a new git repo.
pub fn git_init(dir: &Path) -> Result<(), Box<dyn Error>> {
Command::new("git")
let output = Command::new("git")
.current_dir(dir)
.args(&["init", "--quiet"])
.status()?;
.output()?;
util::check_command_output(&output, "initializing git repository");
Ok(())
}
86 changes: 57 additions & 29 deletions src/install.rs
Expand Up @@ -351,38 +351,65 @@ pub fn download_and_install_package(
replace_distutils(&extracted_parent.join("setup.py"));

#[cfg(target_os = "windows")]
Command::new(paths.bin.join("python"))
.current_dir(&extracted_parent)
.args(&["setup.py", "bdist_wheel"])
.output()
.expect(&format!(
"Problem running setup.py bdist_wheel in folder: {:?}. Py path: {:?}",
&extracted_parent,
paths.bin.join("python")
));

{
let output = Command::new(paths.bin.join("python"))
.current_dir(&extracted_parent)
.args(&["setup.py", "bdist_wheel"])
.output()
.expect(&format!(
"Problem running setup.py bdist_wheel in folder: {:?}. Py path: {:?}",
&extracted_parent,
paths.bin.join("python")
));
util::check_command_output_with(&output, |s| {
panic!(
"running setup.py bdist_wheel in folder {:?}. Py path: {:?}: {}",
s
)
});
}
// The Linux and Mac builds appear to be unable to build wheels due to
// missing the ctypes library; revert to system python.
#[cfg(target_os = "linux")]
Command::new("python3")
.current_dir(&extracted_parent)
.args(&["setup.py", "bdist_wheel"])
.output()
.expect(&format!(
"Problem running setup.py bdist_wheel in folder: {:?}. Py path: {:?}",
&extracted_parent,
paths.bin.join("python")
));
{
let output = Command::new("python3")
.current_dir(&extracted_parent)
.args(&["setup.py", "bdist_wheel"])
.output()
.expect(&format!(
"Problem running setup.py bdist_wheel in folder: {:?}. Py path: {:?}",
&extracted_parent,
paths.bin.join("python")
));
util::check_command_output_with(&output, |s| {
panic!(
"running setup.py bdist_wheel in folder {:?}. Py path: {:?}: {}",
&extracted_parent,
paths.bin.join("python"),
s
);
});
}
#[cfg(target_os = "macos")]
Command::new("python3")
.current_dir(&extracted_parent)
.args(&["setup.py", "bdist_wheel"])
.output()
.expect(&format!(
"Problem running setup.py bdist_wheel in folder: {:?}. Py path: {:?}",
&extracted_parent,
paths.bin.join("python")
));
{
let output = Command::new("python3")
.current_dir(&extracted_parent)
.args(&["setup.py", "bdist_wheel"])
.output()
.expect(&format!(
"Problem running setup.py bdist_wheel in folder: {:?}. Py path: {:?}",
&extracted_parent,
paths.bin.join("python")
));
util::check_command_output_with(&output, |s| {
panic!(
"running setup.py bdist_wheel in folder {:?}. Py path: {:?}: {}",
&extracted_parent,
paths.bin.join("python"),
s
);
});
}

let dist_path = &extracted_parent.join("dist");
if !dist_path.exists() {
Expand Down Expand Up @@ -621,13 +648,14 @@ pub fn download_and_install_git(
//}

// Build a wheel from the repo
Command::new(paths.bin.join("python"))
let output = Command::new(paths.bin.join("python"))
// We assume that the module code is in the repo's immediate subfolder that has
// the package's name.
.current_dir(&git_path.join(&folder_name))
.args(&["setup.py", "bdist_wheel"])
.output()
.expect("Problem running setup.py bdist_wheel");
util::check_command_output(&output, "running setup.py bdist_wheel");

let archive_path = util::find_first_file(&git_path.join(folder_name).join("dist"));
let filename = archive_path
Expand Down
18 changes: 18 additions & 0 deletions src/util.rs
Expand Up @@ -888,3 +888,21 @@ pub fn find_dont_uninstall(reqs: &[Req], dev_reqs: &[Req]) -> Vec<String> {

result
}

// Internal function to handle error reporting for commands.
//
// Panics on subprocess failure printing error message
pub(crate) fn check_command_output(output: &process::Output, msg: &str) {
check_command_output_with(output, |s| panic!("{}: {}", msg, s));
}

// Internal function to handle error reporting for commands.
//
// Panics on subprocess failure printing error message
pub(crate) fn check_command_output_with(output: &process::Output, f: impl Fn(&str)) {
if !output.status.success() {
let stderr =
std::str::from_utf8(&output.stderr).expect("building string from command output");
f(&stderr)
}
}

0 comments on commit dc91c9a

Please sign in to comment.