diff --git a/Cargo.lock b/Cargo.lock index 8be6fd972..63c3b8922 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1632,6 +1632,7 @@ name = "tectonic" version = "0.0.0-dev.0" dependencies = [ "app_dirs2", + "atty", "byte-unit", "cc", "cfg-if", diff --git a/Cargo.toml b/Cargo.toml index ce859b17c..53cf34e68 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,6 +43,7 @@ vcpkg = "0.2.10" [dependencies] app_dirs = { version = "2", package = "app_dirs2" } +atty = "0.2" byte-unit = "^4.0" cfg-if = "0.1" structopt = "0.3" diff --git a/dist/azure-coverage.yml b/dist/azure-coverage.yml index 0f216fbfe..43e19dfe6 100644 --- a/dist/azure-coverage.yml +++ b/dist/azure-coverage.yml @@ -29,6 +29,7 @@ steps: displayName: Set up code coverage - bash: | + set -xeuo pipefail # As of Rust 1.44, test executables land in target/debug/deps/ instead of # target/debug/, which messes up current cargo-kcov (0.5.2) because it tries # to search for those executables. Work around with `cp`. One of the @@ -38,21 +39,22 @@ steps: # a search based on the hardlink count. Hacky and fragile but this should # get us going. Hopefully kcov will get fixed where this will become # unneccessary anyway. - rm target/debug/deps/tectonic-???????????????? cargo test --no-run find target/debug/deps/tectonic-???????????????? -links 2 -print -delete cp -vp target/debug/deps/*-???????????????? target/debug/ displayName: cargo test --no-run - bash: | + set -xeuo pipefail git add . cranko release-workflow commit git show HEAD displayName: Make release commit -- bash: RUNNING_COVERAGE=1 cargo kcov --no-clean-rebuild +- bash: cargo kcov --no-clean-rebuild displayName: cargo kcov - bash: | + set -xeuo pipefail bash <(curl -s https://codecov.io/bash) displayName: Report coverage results diff --git a/src/bin/tectonic.rs b/src/bin/tectonic.rs index dc229533e..fc301b143 100644 --- a/src/bin/tectonic.rs +++ b/src/bin/tectonic.rs @@ -13,10 +13,10 @@ use std::time; use tectonic::config::PersistentConfig; use tectonic::driver::{OutputFormat, PassSetting, ProcessingSessionBuilder}; use tectonic::errors::{ErrorKind, Result}; +use tectonic::status::plain::PlainStatusBackend; use tectonic::status::termcolor::TermcolorStatusBackend; use tectonic::status::{ChatterLevel, StatusBackend}; - -use tectonic::{errmsg, tt_error, tt_error_styled, tt_note}; +use tectonic::{errmsg, tt_error, tt_note}; #[derive(Debug, StructOpt)] #[structopt(name = "Tectonic", about = "Process a (La)TeX document")] @@ -37,11 +37,14 @@ struct CliOptions { /// How much chatter to print when running #[structopt(long = "chatter", short, name = "level", default_value = "default", possible_values(&["default", "minimal"]))] chatter_level: String, + /// Enable/disable colorful log output. + #[structopt(long = "color", name = "when", default_value = "auto", possible_values(&["always", "auto", "never"]))] + cli_color: String, /// Use only resource files cached locally #[structopt(short = "C", long)] only_cached: bool, /// The kind of output to generate - #[structopt(long, name = "format", default_value = "pdf", possible_values(&["pdf", "html", "xdv", "aux", "format"]))] + #[structopt(long, name = "format", default_value = "pdf", possible_values(&["pdf", "html", "xdv", "aux", "fmt"]))] outfmt: String, /// Write Makefile-format rules expressing the dependencies of this run to #[structopt(long, name = "dest_path")] @@ -71,11 +74,7 @@ struct CliOptions { #[structopt(name = "outdir", short, long, parse(from_os_str))] outdir: Option, } -fn inner( - args: CliOptions, - config: PersistentConfig, - status: &mut TermcolorStatusBackend, -) -> Result<()> { +fn inner(args: CliOptions, config: PersistentConfig, status: &mut dyn StatusBackend) -> Result<()> { let mut sess_builder = ProcessingSessionBuilder::default(); let format_path = args.format; sess_builder @@ -187,10 +186,8 @@ fn inner( "something bad happened inside {}; its output follows:\n", engine ); - tt_error_styled!(status, "==============================================================================="); - status.dump_to_stderr(&output); - tt_error_styled!(status, "==============================================================================="); - tt_error_styled!(status, ""); + + status.dump_error_logs(&output); } } } @@ -235,8 +232,19 @@ fn main() { // something I'd be relatively OK with since it'd only affect the progam // UI, not the processing results). - let mut status = - TermcolorStatusBackend::new(ChatterLevel::from_str(&args.chatter_level).unwrap()); + let chatter_level = ChatterLevel::from_str(&args.chatter_level).unwrap(); + let use_cli_color = match &*args.cli_color { + "always" => true, + "auto" => atty::is(atty::Stream::Stdout), + "never" => false, + _ => unreachable!(), + }; + + let mut status = if use_cli_color { + Box::new(TermcolorStatusBackend::new(chatter_level)) as Box + } else { + Box::new(PlainStatusBackend::new(chatter_level)) as Box + }; // For now ... @@ -249,8 +257,8 @@ fn main() { // function ... all so that we can print out the word "error:" in red. // This code parallels various bits of the `error_chain` crate. - if let Err(ref e) = inner(args, config, &mut status) { - status.bare_error(e); + if let Err(ref e) = inner(args, config, &mut *status) { + tt_error!(status, ""; e); process::exit(1) } } diff --git a/src/driver.rs b/src/driver.rs index 4427d6554..0f53bf3ec 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -633,7 +633,7 @@ impl ProcessingSession { /// Assess whether we need to rerun an engine. This is the case if there /// was a file that the engine read and then rewrote, and the rewritten /// version is different than the version that it read in. - fn is_rerun_needed(&self, status: &mut S) -> Option { + fn is_rerun_needed(&self, status: &mut dyn StatusBackend) -> Option { // TODO: we should probably wire up diagnostics since I expect this // stuff could get finicky and we're going to want to be able to // figure out why rerun detection is breaking. @@ -664,7 +664,7 @@ impl ProcessingSession { } #[allow(dead_code)] - fn _dump_access_info(&self, status: &mut S) { + fn _dump_access_info(&self, status: &mut dyn StatusBackend) { for (name, info) in &self.events.0 { if info.access_pattern != AccessPattern::Read { let r = match info.read_digest { @@ -697,7 +697,7 @@ impl ProcessingSession { /// - run BibTeX, if it seems to be required /// - repeat the last two steps as often as needed /// - write the output files to disk, including a Makefile if it was requested. - pub fn run(&mut self, status: &mut S) -> Result<()> { + pub fn run(&mut self, status: &mut dyn StatusBackend) -> Result<()> { // Do we need to generate the format file? let generate_format = if self.output_format == OutputFormat::Format { @@ -817,10 +817,10 @@ impl ProcessingSession { Ok(()) } - fn write_files( + fn write_files( &mut self, mut mf_dest_maybe: Option<&mut File>, - status: &mut S, + status: &mut dyn StatusBackend, only_logs: bool, ) -> Result { let root = match self.output_path { @@ -909,11 +909,7 @@ impl ProcessingSession { /// The "default" pass really runs a bunch of sub-passes. It is a "Do What /// I Mean" operation. - fn default_pass( - &mut self, - bibtex_first: bool, - status: &mut S, - ) -> Result { + fn default_pass(&mut self, bibtex_first: bool, status: &mut dyn StatusBackend) -> Result { // If `bibtex_first` is true, we start by running bibtex, and run // proceed with the standard rerun logic. Otherwise, we run TeX, // auto-detect whether we need to run bibtex, possibly run it, and @@ -1011,7 +1007,7 @@ impl ProcessingSession { } /// Use the TeX engine to generate a format file. - fn make_format_pass(&mut self, status: &mut S) -> Result { + fn make_format_pass(&mut self, status: &mut dyn StatusBackend) -> Result { if self.io.bundle.is_none() { return Err( ErrorKind::Msg("cannot create formats without using a bundle".to_owned()).into(), @@ -1089,10 +1085,10 @@ impl ProcessingSession { } /// Run one pass of the TeX engine. - fn tex_pass( + fn tex_pass( &mut self, rerun_explanation: Option<&str>, - status: &mut S, + status: &mut dyn StatusBackend, ) -> Result> { let result = { let mut stack = self.io.as_stack(); @@ -1131,7 +1127,7 @@ impl ProcessingSession { Ok(warnings) } - fn bibtex_pass(&mut self, status: &mut S) -> Result { + fn bibtex_pass(&mut self, status: &mut dyn StatusBackend) -> Result { let result = { let mut stack = self.io.as_stack(); let mut engine = BibtexEngine::new(); @@ -1167,7 +1163,7 @@ impl ProcessingSession { Ok(0) } - fn xdvipdfmx_pass(&mut self, status: &mut S) -> Result { + fn xdvipdfmx_pass(&mut self, status: &mut dyn StatusBackend) -> Result { { let mut stack = self.io.as_stack(); let mut engine = XdvipdfmxEngine::new().with_date(self.build_date); @@ -1185,7 +1181,7 @@ impl ProcessingSession { Ok(0) } - fn spx2html_pass(&mut self, status: &mut S) -> Result { + fn spx2html_pass(&mut self, status: &mut dyn StatusBackend) -> Result { { let mut stack = self.io.as_stack(); let mut engine = Spx2HtmlEngine::new(); diff --git a/src/status/mod.rs b/src/status/mod.rs index 0a7454a42..663dc9f02 100644 --- a/src/status/mod.rs +++ b/src/status/mod.rs @@ -4,6 +4,7 @@ //! A framework for showing status messages to the user. +pub mod plain; pub mod termcolor; use std::cmp; @@ -77,6 +78,10 @@ pub trait StatusBackend { None, ) } + + /// This is used to print TeX engine logs after it encountered errors. This prints the log, + /// surrounded by lines of equal signs. + fn dump_error_logs(&mut self, output: &[u8]); } /// Report a formatted informational message to the user. @@ -133,4 +138,5 @@ impl NoopStatusBackend { impl StatusBackend for NoopStatusBackend { fn report(&mut self, _kind: MessageKind, _args: Arguments, _err: Option<&Error>) {} + fn dump_error_logs(&mut self, _output: &[u8]) {} } diff --git a/src/status/plain.rs b/src/status/plain.rs new file mode 100644 index 000000000..af5090b88 --- /dev/null +++ b/src/status/plain.rs @@ -0,0 +1,67 @@ +use std::fmt::Arguments; + +use super::{ChatterLevel, MessageKind, StatusBackend}; +use crate::errors::Error; +use std::io::{self, Write}; + +pub struct PlainStatusBackend { + chatter: ChatterLevel, +} + +impl PlainStatusBackend { + pub fn new(chatter: ChatterLevel) -> Self { + PlainStatusBackend { chatter } + } +} + +impl StatusBackend for PlainStatusBackend { + fn report(&mut self, kind: MessageKind, args: Arguments, err: Option<&Error>) { + if kind == MessageKind::Note && self.chatter <= ChatterLevel::Minimal { + return; + } + + let prefix = match kind { + MessageKind::Note => "note:", + MessageKind::Warning => "warning:", + MessageKind::Error => "error:", + }; + if kind == MessageKind::Note { + println!("{} {}", prefix, args); + } else { + eprintln!("{} {}", prefix, args); + } + if let Some(e) = err { + for item in e.iter() { + eprintln!("caused by: {}", item); + } + if let Some(backtrace) = e.backtrace() { + eprintln!("debugging: backtrace follows:"); + eprintln!("{:?}", backtrace); + } + } + } + + fn note_highlighted(&mut self, before: &str, highlighted: &str, after: &str) { + if self.chatter > ChatterLevel::Minimal { + self.report( + MessageKind::Note, + format_args!("{}{}{}", before, highlighted, after), + None, + ); + } + } + + fn dump_error_logs(&mut self, output: &[u8]) { + eprintln!( + "===============================================================================" + ); + + io::stderr() + .write_all(output) + .expect("write to stderr failed"); + + eprintln!( + "===============================================================================" + ); + } +} diff --git a/src/status/termcolor.rs b/src/status/termcolor.rs index 36a54e2ce..7099a1c1b 100644 --- a/src/status/termcolor.rs +++ b/src/status/termcolor.rs @@ -137,12 +137,6 @@ impl TermcolorStatusBackend { }); } } - - pub fn dump_to_stderr(&mut self, output: &[u8]) { - self.stderr - .write_all(output) - .expect("write to stderr failed"); - } } /// Show formatted text to the user, styled as an error message. @@ -185,4 +179,20 @@ impl StatusBackend for TermcolorStatusBackend { writeln!(self.stdout, "{}", after).expect("write to stdout failed"); } } + + fn dump_error_logs(&mut self, output: &[u8]) { + tt_error_styled!( + self, + "===============================================================================" + ); + + self.stderr + .write_all(output) + .expect("write to stderr failed"); + + tt_error_styled!( + self, + "===============================================================================" + ); + } } diff --git a/tests/executable.rs b/tests/executable.rs index ffd33540f..c6c925f98 100644 --- a/tests/executable.rs +++ b/tests/executable.rs @@ -102,8 +102,14 @@ fn setup_and_copy_files(files: &[&str]) -> TempDir { .prefix("tectonic_executable_test") .tempdir() .unwrap(); - let executable_test_dir = - PathBuf::from(env::var("CARGO_MANIFEST_DIR").unwrap()).join("tests/executable"); + + // `cargo kcov` (0.5.2) does not set this variable: + let executable_test_dir = if let Some(v) = env::var_os("CARGO_MANIFEST_DIR") { + PathBuf::from(v) + } else { + PathBuf::new() + } + .join("tests/executable"); for file in files { // Create parent directories, if the file is not at the root of `tests/executable/` @@ -165,60 +171,36 @@ fn check_file(tempdir: &TempDir, rest: &str) { #[test] fn bad_chatter_1() { - if env::var("RUNNING_COVERAGE").is_ok() { - return; - } - let output = run_tectonic(&PathBuf::from("."), &["-", "--chatter=reticent"]); error_or_panic(output); } #[test] fn bad_input_path_1() { - if env::var("RUNNING_COVERAGE").is_ok() { - return; - } - let output = run_tectonic(&PathBuf::from("."), &["/"]); error_or_panic(output); } #[test] fn bad_input_path_2() { - if env::var("RUNNING_COVERAGE").is_ok() { - return; - } - let output = run_tectonic(&PathBuf::from("."), &["somedir/.."]); error_or_panic(output); } #[test] fn bad_outfmt_1() { - if env::var("RUNNING_COVERAGE").is_ok() { - return; - } - let output = run_tectonic(&PathBuf::from("."), &["-", "--outfmt=dd"]); error_or_panic(output); } #[test] fn help_flag() { - if env::var("RUNNING_COVERAGE").is_ok() { - return; - } - let output = run_tectonic(&PathBuf::from("."), &["-h"]); success_or_panic(output); } #[test] // GitHub #31 fn relative_include() { - if env::var("RUNNING_COVERAGE").is_ok() { - return; - } - let fmt_arg = get_plain_format_arg(); let tempdir = setup_and_copy_files(&[ "subdirectory/relative_include.tex", @@ -235,10 +217,6 @@ fn relative_include() { #[test] fn stdin_content() { - if env::var("RUNNING_COVERAGE").is_ok() { - return; - } - // No input files here, but output files are created. let fmt_arg = get_plain_format_arg(); let tempdir = setup_and_copy_files(&[]); @@ -253,10 +231,6 @@ fn stdin_content() { // Regression #36 #[test] fn test_space() { - if env::var("RUNNING_COVERAGE").is_ok() { - return; - } - let fmt_arg = get_plain_format_arg(); let tempdir = setup_and_copy_files(&["test space.tex"]); @@ -266,10 +240,6 @@ fn test_space() { #[test] fn test_outdir() { - if env::var("RUNNING_COVERAGE").is_ok() { - return; - } - let fmt_arg = get_plain_format_arg(); let tempdir = setup_and_copy_files(&["subdirectory/content/1.tex"]); @@ -290,10 +260,6 @@ fn test_outdir() { // panic unwinding broken: https://github.com/rust-embedded/cross/issues/343 #[cfg(not(all(target_arch = "arm", target_env = "musl")))] fn test_bad_outdir() { - if env::var("RUNNING_COVERAGE").is_ok() { - panic!() - } - let fmt_arg = get_plain_format_arg(); let tempdir = setup_and_copy_files(&["subdirectory/content/1.tex"]); @@ -313,10 +279,6 @@ fn test_bad_outdir() { // panic unwinding broken: https://github.com/rust-embedded/cross/issues/343 #[cfg(not(all(target_arch = "arm", target_env = "musl")))] fn test_outdir_is_file() { - if env::var("RUNNING_COVERAGE").is_ok() { - panic!() - } - let fmt_arg = get_plain_format_arg(); let tempdir = setup_and_copy_files(&["test space.tex", "subdirectory/content/1.tex"]); @@ -333,10 +295,6 @@ fn test_outdir_is_file() { #[test] fn test_keep_logs_on_error() { - if env::var("RUNNING_COVERAGE").is_ok() { - return; - } - // No input files here, but output files are created. let fmt_arg = get_plain_format_arg(); let tempdir = setup_and_copy_files(&[]); @@ -355,3 +313,29 @@ fn test_keep_logs_on_error() { assert!(log.contains(r"job aborted, no legal \end found")); } + +#[test] +fn test_no_color() { + // No input files here, but output files are created. + let fmt_arg = get_plain_format_arg(); + + let tempdir = setup_and_copy_files(&[]); + let output_nocolor = run_tectonic_with_stdin( + tempdir.path(), + &[&fmt_arg, "-", "--color=never"], + "no end to this file", + ); + + // Output is not a terminal, so these two should be the same + let tempdir = setup_and_copy_files(&[]); + let output_autocolor = run_tectonic_with_stdin( + tempdir.path(), + &[&fmt_arg, "-", "--color=auto"], + "no end to this file", + ); + + assert_eq!(output_nocolor, output_autocolor); + + error_or_panic(output_nocolor); + error_or_panic(output_autocolor); +}