rust/treefile: Include filename in more error msgs

This uses the `Context` feature of the failure crate to make error
messages more useful when we fail to open a file. The difference with
`map_err` is that one can still obtain the underlying error from the
context if need be. Though surprisingly, the normal `Display` for a
`Context` doesn't include the original error, so we essentially have to
do a prefix here (see [1]).

Before:

```
error: Failed to load YAML treefile: No such file or directory (os error 2)
```

After:

```
error: Failed to load YAML treefile: Can't open file "treecompose-post.sh": No such file or directory (os error 2)
```

[1] https://github.com/rust-lang-nursery/failure/issues/182

Closes: #1735
Approved by: cgwalters
This commit is contained in:
Jonathan Lebon 2019-01-21 09:07:26 -05:00 committed by Atomic Bot
parent 1594140a33
commit 25d0213d15

View File

@ -30,6 +30,7 @@ use std::path::Path;
use std::{collections, fs, io};
use utils;
use failure::Fallible;
use failure::ResultExt;
const ARCH_X86_64: &'static str = "x86_64";
const INCLUDE_MAXDEPTH: u32 = 50;
@ -139,6 +140,12 @@ fn treefile_parse_stream<R: io::Read>(
Ok(treefile)
}
/// Open file and provide context containing filename on failures.
fn open_file<P: AsRef<Path>>(filename: P) -> Fallible<fs::File> {
return Ok(fs::File::open(filename.as_ref()).with_context(
|e| format!("Can't open file {:?}: {}", filename.as_ref().display(), e))?);
}
// If a passwd/group file is provided explicitly, load it as a fd
fn load_passwd_file<P: AsRef<Path>>(
basedir: P,
@ -147,7 +154,7 @@ fn load_passwd_file<P: AsRef<Path>>(
if let &Some(ref v) = v {
let basedir = basedir.as_ref();
if let Some(ref path) = v.filename {
return Ok(Some(fs::File::open(basedir.join(path))?));
return Ok(Some(open_file(basedir.join(path))?));
}
}
return Ok(None);
@ -160,12 +167,7 @@ fn treefile_parse<P: AsRef<Path>>(
arch: Option<&str>,
) -> Fallible<ConfigAndExternals> {
let filename = filename.as_ref();
let mut f = io::BufReader::new(fs::File::open(filename).map_err(|e| {
io::Error::new(
e.kind(),
format!("Opening {:?}: {}", filename, e.to_string()),
)
})?);
let mut f = io::BufReader::new(open_file(filename)?);
let basename = filename
.file_name()
.map(|s| s.to_string_lossy())
@ -182,14 +184,14 @@ fn treefile_parse<P: AsRef<Path>>(
)
})?;
let postprocess_script = if let Some(ref postprocess) = tf.postprocess_script.as_ref() {
Some(fs::File::open(filename.with_file_name(postprocess))?)
Some(open_file(filename.with_file_name(postprocess))?)
} else {
None
};
let mut add_files: collections::HashMap<String, fs::File> = collections::HashMap::new();
if let Some(ref add_file_names) = tf.add_files.as_ref() {
for (name, _) in add_file_names.iter() {
add_files.insert(name.clone(), fs::File::open(filename.with_file_name(name))?);
add_files.insert(name.clone(), open_file(filename.with_file_name(name))?);
}
}
let parent = filename.parent().unwrap();
@ -813,6 +815,16 @@ packages:
let rojig = tf.rojig.as_ref().unwrap();
assert!(rojig.name == "exampleos");
}
#[test]
fn test_open_file_nonexistent() {
let path = "/usr/share/empty/manifest.yaml";
match treefile_parse(path, None) {
Err(ref e) => assert!(e.to_string().starts_with(
format!("Can't open file {:?}:", path).as_str())),
Ok(_) => panic!("Expected nonexistent treefile error for {}", path),
}
}
}
mod ffi {