Skip to content
This repository has been archived by the owner on Oct 15, 2022. It is now read-only.

Commit

Permalink
src/nix: make CallOpts::argstr take an OsString
Browse files Browse the repository at this point in the history
nix is content with arbitrary bytestrings, which is nice if you have
e.g. a path that contains non-UTF-8 characters.

So we don’t require a rust `str`, rather an OsStr(ing).

It can still be used with normal strings, because of the `AsRef`
instances.
  • Loading branch information
Profpatsch committed Nov 15, 2019
1 parent 5cb2ded commit 55f5259
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 18 deletions.
21 changes: 13 additions & 8 deletions src/nix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ use crossbeam_channel as chan;
use serde_json;
use std::collections::HashMap;
use std::ffi::{OsStr, OsString};
use std::io::BufReader;
use std::path::{Path, PathBuf};
use std::process::{ChildStderr, ChildStdout, Command, ExitStatus, Stdio};
use std::thread;
Expand All @@ -48,7 +47,7 @@ use vec1::Vec1;
pub struct CallOpts<'a> {
input: Input<'a>,
attribute: Option<String>,
argstrs: HashMap<String, String>,
argstrs: HashMap<OsString, OsString>,
stderr_line_tx: Option<chan::Sender<OsString>>,
}

Expand Down Expand Up @@ -184,8 +183,13 @@ impl<'a> CallOpts<'a> {
/// output.unwrap(), "Hello, Jill!"
/// );
/// ```
pub fn argstr(&mut self, name: &str, value: &str) -> &mut Self {
self.argstrs.insert(name.to_string(), value.to_string());
pub fn argstr<P, Q>(&mut self, name: P, value: Q) -> &mut Self
where
P: AsRef<OsStr>,
Q: AsRef<OsStr>,
{
self.argstrs
.insert(name.as_ref().to_owned(), value.as_ref().to_owned());
self
}

Expand Down Expand Up @@ -392,7 +396,7 @@ impl<'a> CallOpts<'a> {
let stderr_handle: ChildStderr = nix_proc.stderr.take().expect("failed to take stderr");
let stderr_tx = self.stderr_line_tx.clone();
let stderr_thread = thread::spawn(move || {
let reader = osstrlines::Lines::from(BufReader::new(stderr_handle));
let reader = osstrlines::Lines::from(std::io::BufReader::new(stderr_handle));
if let Some(tx) = stderr_tx {
for line in reader {
tx.send(line.unwrap()).expect("Receiver for nix.rs hung up");
Expand All @@ -404,7 +408,8 @@ impl<'a> CallOpts<'a> {

// 2. spawn a stdout handling thread (?)
let stdout_handle: ChildStdout = nix_proc.stdout.take().expect("failed to take stdout");
let stdout_thread = thread::spawn(move || stdout_fn(BufReader::new(stdout_handle)));
let stdout_thread =
thread::spawn(move || stdout_fn(std::io::BufReader::new(stdout_handle)));

// 3. wait on the process
let nix_proc_result = nix_proc.wait().expect("nix wasn't running");
Expand Down Expand Up @@ -434,8 +439,8 @@ impl<'a> CallOpts<'a> {

for (name, value) in self.argstrs.iter() {
ret.push(OsStr::new("--argstr"));
ret.push(OsStr::new(name));
ret.push(OsStr::new(value));
ret.push(&name);
ret.push(&value);
}

match self.input {
Expand Down
15 changes: 5 additions & 10 deletions src/ops/upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,14 @@ pub fn main(upgrade_target: cli::UpgradeTo, cas: &ContentAddressable) -> OpResul
let expr = {
let src = match UpgradeSource::from_cli_argument(upgrade_target) {
Ok(src) => Ok(src),
Err(UpgradeSourceError::LocalPathNotFound(p)) => Err(ExitError::errmsg(format!(
Err(UpgradeSourceError::LocalPathNotFound(p)) => Err(ExitError::user_error(format!(
"Cannot upgrade to local repository {}: path not found",
p.display()
))),
Err(UpgradeSourceError::CantCanonicalizeLocalPath(err)) => Err(ExitError::errmsg(
Err(UpgradeSourceError::CantCanonicalizeLocalPath(err)) => Err(ExitError::temporary(
format!("Problem accessing local repository:\n{:?}", err),
)),
Err(UpgradeSourceError::ReleaseNixDoesntExist(p)) => Err(ExitError::errmsg(format!(
Err(UpgradeSourceError::ReleaseNixDoesntExist(p)) => Err(ExitError::user_error(format!(
"{} does not exist, are you sure this is a lorri repository?",
p.display()
))),
Expand All @@ -105,16 +105,11 @@ pub fn main(upgrade_target: cli::UpgradeTo, cas: &ContentAddressable) -> OpResul
match src {
UpgradeSource::Branch(b) => {
expr.argstr("type", "branch");
expr.argstr("branch", &b);
expr.argstr("branch", b);
}
UpgradeSource::Local(p) => {
expr.argstr("type", "local");
expr.argstr(
"path",
p.to_str()
// TODO: this is unnecessary, argstr() should take an OsStr()
.expect("Requested Lorri source directory not UTF-8 clean"),
);
expr.argstr("path", p);
}
}
// ugly hack to prevent expr from being mutable outside,
Expand Down

0 comments on commit 55f5259

Please sign in to comment.