Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(eas): allow path/to/output & add clean-up #115

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix(eas): allow path/to/output & add clean-up #115

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4ff7fb8
fix(eas): allow `path/to/output` & add clean-up
ZeroEkkusu Jan 27, 2023
8682c11
chore: format
ZeroEkkusu Jan 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 35 additions & 4 deletions etk-asm/src/bin/eas.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ use etk_cli::io::HexWrite;

use etk_asm::ingest::{Error, Ingest};

use std::fs::File;
use std::fs::{create_dir_all, File};
use std::io::prelude::*;
use std::path::PathBuf;
use std::path::{Path, PathBuf};

use clap::StructOpt;

Expand All @@ -19,6 +19,11 @@ struct Opt {
}

fn create(path: PathBuf) -> File {
match create_dir_all(path.parent().unwrap()) {
Ok(_) => (),
Err(why) => panic!("couldn't create parent directories: {}", why),
}

match File::create(&path) {
Err(why) => panic!("couldn't create `{}`: {}", path.display(), why),
Ok(file) => file,
Expand All @@ -38,15 +43,41 @@ fn main() {
fn run() -> Result<(), Error> {
let opt: Opt = clap::Parser::parse();

let mut out_file_exists = false;
let mut out_file_content = vec![];
if let Some(o) = &opt.out {
if Path::new(o).exists() {
out_file_exists = true;
match std::fs::read(o) {
Ok(content) => out_file_content = content,
Err(e) => panic!("couldn't backup existing file: {}", e),
}
}
}

let mut out: Box<dyn Write> = match opt.out {
Some(o) => Box::new(create(o)),
Some(ref o) => Box::new(create(o.to_path_buf())),
None => Box::new(std::io::stdout()),
};

let hex_out = HexWrite::new(&mut out);

let mut ingest = Ingest::new(hex_out);
ingest.ingest_file(opt.input)?;

if let Err(e) = ingest.ingest_file(opt.input) {
if out_file_exists {
match std::fs::write(&opt.out.unwrap(), &out_file_content) {
Ok(_) => (),
Err(e) => panic!("couldn't restore existing file.\n{}", e),
};
Comment on lines +69 to +72
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is to only modify the output if the compilation succeeds, but leave the file untouched otherwise?

This particular approach will fail to restore the file if, say, there's a panic.

What would you think of using something like tempfile to make a temporary file in the same directory as the intended output, then moving the file into the intended output?

So to write to a file a/b/test.foo, you'd do:

  1. Create a/b/test.foo.tmp using tempfile.
  2. ingest_file(...).
  3. std::fs::rename from a/b/test.foo.tmp to a/b/test.foo.

Copy link
Author

@ZeroEkkusu ZeroEkkusu Jan 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, @SamWilsn. Yes, that was the idea - don’t create a file or overwrite the existing one with the same name if eas fails.

tempfile sounds like a good option.

Sorry I won’t be able to make the changes - I won’t have a computer with me for a while.

I’d recommend the following approach:

  1. Create a temporary file in the current directory
  2. Ingest
  3. If successful, create directories on the out path that don’t exist, and move + rename the file

This way, neither new directories nor an output file will be created if the compilation fails, which is cleaner.

Let me know what you think.

} else if let Some(o) = &opt.out {
match std::fs::remove_file(o) {
Ok(_) => (),
Err(e) => panic!("couldn't clean up.\n{}", e),
}
}
return Err(e);
}

out.write_all(b"\n").unwrap();

Expand Down