Skip to content

Commit

Permalink
Use cargo's pretty printing for diagnostics
Browse files Browse the repository at this point in the history
Also generally improves diagnostic messages and docs, and cleans
up (slightly) how we handle the --args option

Includes bump to 0.1.2
  • Loading branch information
cmyr committed Mar 17, 2019
1 parent 0b407a6 commit ee51894
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 27 deletions.
3 changes: 2 additions & 1 deletion Cargo.lock

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

3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "cargo-instruments"
version = "0.1.1"
version = "0.1.2"
authors = ["Colin Rofls <[email protected]>"]
license = "MIT"
description = "Generate Xcode Instruments trace files for binary targets."
Expand All @@ -16,3 +16,4 @@ chrono = "0.4.6"
cargo = "0.34.0"
failure = "0.1.5"
structopt = { version = "0.2", default-features = false }
termcolor = "1.0"
38 changes: 36 additions & 2 deletions src/app.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
//! The main application logic.

use std::path::PathBuf;
use std::process::Command;

use cargo::core::Workspace;
use cargo::ops::CompileOptions;
use cargo::util::config::Config;
use failure::{format_err, Error};
use termcolor::Color;

use crate::instruments;
use crate::opt::{CargoOpts, Opts, Target};
Expand All @@ -26,9 +28,32 @@ pub(crate) fn run(args: Opts) -> Result<(), Error> {
let workspace = Workspace::new(&manifest_path, &cfg)?;
let workspace_root = manifest_path.parent().unwrap().to_owned();

let exec_path = build_target(&args, &workspace)?;
let exec_path = match build_target(&args, &workspace) {
Ok(path) => path,
Err(e) => {
workspace.config().shell().status_with_color("Failed", &e, Color::Red)?;
return Ok(());
}
};

let relpath = exec_path.strip_prefix(&workspace_root).unwrap_or(exec_path.as_path());
workspace.config().shell().status("Profiling", relpath.to_string_lossy())?;

instruments::run(&args, exec_path, workspace_root)
let trace_path = match instruments::run(&args, exec_path, &workspace_root) {
Ok(path) => path,
Err(e) => {
workspace.config().shell().status_with_color("Failed", &e, Color::Red)?;
return Ok(());
}
};

let reltrace = trace_path.strip_prefix(&workspace_root).unwrap_or(trace_path.as_path());
workspace.config().shell().status("Wrote Trace", reltrace.to_string_lossy())?;
if args.open {
workspace.config().shell().status("Opening", reltrace.to_string_lossy())?;
open_file(&trace_path)?;
}
Ok(())
}

/// Attempts to build the specified target. On success, returns the path to
Expand Down Expand Up @@ -102,3 +127,12 @@ fn validate_target(target: &Target, workspace: &Workspace) -> Result<(), Error>
Ok(())
}
}

fn open_file(file: &PathBuf) -> Result<(), Error> {
let status = Command::new("open").arg(file).status()?;

if !status.success() {
return Err(format_err!("open failed"));
}
Ok(())
}
31 changes: 12 additions & 19 deletions src/instruments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,11 @@ pub(crate) fn list() -> Result<String, Error> {
Ok(output)
}

pub(crate) fn run(args: &Opts, exec_path: PathBuf, workspace_root: PathBuf) -> Result<(), Error> {
pub(crate) fn run(
args: &Opts,
exec_path: PathBuf,
workspace_root: &PathBuf,
) -> Result<PathBuf, Error> {
let outfile = get_out_file(args, &exec_path, &workspace_root)?;
let template = resolve_template(&args);

Expand All @@ -80,17 +84,15 @@ pub(crate) fn run(args: &Opts, exec_path: PathBuf, workspace_root: PathBuf) -> R
command.args(args.target_args.as_slice());
}

let status = command.status()?;

if !status.success() {
return Err(format_err!("instruments failed"));
}
let output = command.output()?;

if args.open {
open_file(&outfile)?;
if !output.status.success() {
let stderr =
String::from_utf8(output.stderr).unwrap_or(String::from("failed to capture stderr"));
Err(format_err!("instruments errored: {}", stderr))
} else {
Ok(outfile)
}

Ok(())
}

fn get_out_file(
Expand Down Expand Up @@ -125,15 +127,6 @@ fn get_target_dir(workspace_root: &PathBuf) -> Result<PathBuf, Error> {
Ok(target_dir)
}

fn open_file(file: &PathBuf) -> Result<(), Error> {
let status = Command::new("open").arg(file).status()?;

if !status.success() {
return Err(format_err!("open failed"));
}
Ok(())
}

fn now_timestamp() -> impl std::fmt::Display {
use chrono::prelude::*;
let now = Local::now();
Expand Down
18 changes: 14 additions & 4 deletions src/opt.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
//! CLI argument handling

use std::ffi::OsString;
use std::fmt;
use std::path::PathBuf;
use structopt::StructOpt;
Expand Down Expand Up @@ -49,9 +48,10 @@ pub(crate) struct Opts {
#[structopt(long)]
pub(crate) open: bool,

/// Arguments passed to the target binary
#[structopt(short = "a", long = "args", parse(from_os_str))]
pub(crate) target_args: Vec<OsString>,
/// Arguments passed to the target binary. If you're having trouble,
/// try wrapping args in double and single quotes, like --args '"-h --fun plz"'.
#[structopt(short = "a", long = "args", value_name = "ARGS")]
pub(crate) target_args: Vec<String>,
}

/// The target, parsed from args.
Expand Down Expand Up @@ -128,4 +128,14 @@ mod tests {
let opts = Opts::from_iter(&["instruments"]);
assert_eq!(opts.limit, None);
}


#[test]
fn var_args() {
// this isn't ideal, but works for now
let opts = Opts::from_iter(&["instruments", "alloc", "--limit", "808", "--args", "hi -h --bin"]);
assert_eq!(opts.template, "alloc");
assert_eq!(opts.limit, Some(808));
assert_eq!(opts.target_args, vec!["hi -h --bin"]);
}
}

0 comments on commit ee51894

Please sign in to comment.