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.

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

Implement --errors for ghcid.txt #5

Merged
merged 2 commits into from
Aug 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion src/clap/camino.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ impl TypedValueParser for Utf8PathBufValueParser {
clap::Error::raw(
clap::error::ErrorKind::InvalidUtf8,
format!("Path isn't UTF-8: {path_buf:?}"),
).with_cmd(cmd)
)
.with_cmd(cmd)
})
})
}
Expand Down
2 changes: 0 additions & 2 deletions src/clap/humantime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ use std::time::Duration;
use clap::builder::StringValueParser;
use clap::builder::TypedValueParser;
use clap::builder::ValueParserFactory;
use clap::error::ContextKind;
use clap::error::ContextValue;
Comment on lines -8 to -9
Copy link
Member Author

Choose a reason for hiding this comment

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

Unused imports from before CI.

use humantime::DurationError;
use miette::LabeledSpan;
use miette::MietteDiagnostic;
Expand Down
2 changes: 1 addition & 1 deletion src/clap/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ mod error_message;
mod humantime;
mod rust_backtrace;

pub use rust_backtrace::RustBacktrace;
pub use self::humantime::DurationValueParser;
pub use error_message::value_validation_error;
pub use rust_backtrace::RustBacktrace;
7 changes: 5 additions & 2 deletions src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ pub struct Opts {
#[arg(long)]
pub test: Option<String>,

/// A file to write compilation errors to. This is analogous to `ghcid.txt`.
#[arg(long)]
pub errors: Option<Utf8PathBuf>,

/// Options to modify file watching.
#[command(flatten)]
pub watch: WatchOpts,
Expand Down Expand Up @@ -80,8 +84,7 @@ pub struct LoggingOpts {
#[arg(long, default_value = "ghcid_ng=info")]
pub tracing_filter: String,

/// How to display backtraces in error messages. '0' for no backtraces, '1' for standard
/// backtraces, and 'full' to display source snippets.
Comment on lines -83 to -84
Copy link
Member Author

Choose a reason for hiding this comment

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

Now that this argument is a ValueEnum, the possible values and their descriptions are displayed nicely in the --help output:

--backtrace <BACKTRACE>
    How to display backtraces in error messages

    [env: RUST_BACKTRACE=]
    [default: 0]

    Possible values:
    - 0:    Hide backtraces in errors
    - 1:    Display backtraces in errors
    - full: Display backtraces with all stack frames in errors

/// How to display backtraces in error messages.
#[arg(long, env = "RUST_BACKTRACE", default_value = "0")]
pub backtrace: RustBacktrace,
}
Expand Down
44 changes: 28 additions & 16 deletions src/ghci/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use miette::WrapErr;
use tokio::io::AsyncBufReadExt;
use tokio::io::BufReader;
use tokio::process::Child;
use tokio::process::ChildStderr;
use tokio::process::Command;
use tokio::sync::mpsc;
use tokio::sync::oneshot;
Expand All @@ -29,6 +28,9 @@ use stdin::StdinEvent;
mod stdout;
use stdout::GhciStdout;

mod stderr;
use stderr::GhciStderr;

mod show_modules;
use show_modules::ModuleSet;

Expand Down Expand Up @@ -67,6 +69,8 @@ pub struct Ghci {
sync_count: AtomicUsize,
/// The currently-loaded modules in this `ghci` session.
modules: ModuleSet,
/// Path to write errors to, if any. Like `ghcid.txt`.
error_path: Option<Utf8PathBuf>,
}

impl Debug for Ghci {
Expand All @@ -81,7 +85,10 @@ impl Ghci {
/// This starts a number of asynchronous tasks to manage the `ghci` session's input and output
/// streams.
#[instrument(skip_all, level = "debug", name = "ghci")]
pub async fn new(command_arc: Arc<Mutex<Command>>) -> miette::Result<Arc<Mutex<Self>>> {
pub async fn new(
command_arc: Arc<Mutex<Command>>,
error_path: Option<Utf8PathBuf>,
) -> miette::Result<Arc<Mutex<Self>>> {
let mut child = {
let mut command = command_arc.lock().await;

Expand All @@ -98,11 +105,12 @@ impl Ghci {

let stdin = child.stdin.take().unwrap();
let stdout = child.stdout.take().unwrap();
let stderr = BufReader::new(child.stderr.take().unwrap());
let stderr = child.stderr.take().unwrap();

// TODO: Is this a good capacity? Maybe it should just be 1.
let (stdin_sender, stdin_receiver) = mpsc::channel(8);
let (stdout_sender, stdout_receiver) = mpsc::channel(8);
let (stderr_sender, stderr_receiver) = mpsc::channel(8);

// So we want to put references to the `Ghci` struct we return in our tasks, but we don't
// have that struct yet. So we create some trivial tasks to construct a valid `Ghci`, and
Expand All @@ -120,6 +128,7 @@ impl Ghci {
stdin_channel: stdin_sender.clone(),
sync_count: AtomicUsize::new(0),
modules: Default::default(),
error_path: error_path.clone(),
}));

let (init_sender, init_receiver) = oneshot::channel::<()>();
Expand All @@ -130,12 +139,22 @@ impl Ghci {
ghci: Arc::downgrade(&ret),
reader: IncrementalReader::new(stdout).with_writer(tokio::io::stdout()),
stdin_sender: stdin_sender.clone(),
stderr_sender,
receiver: stdout_receiver,
buffer: vec![0; LINE_BUFFER_CAPACITY],
}
.run(init_sender),
);
let stderr = task::spawn(stderr_task(stderr));
let stderr = task::spawn(
GhciStderr {
ghci: Arc::downgrade(&ret),
reader: BufReader::new(stderr).lines(),
receiver: stderr_receiver,
buffer: String::with_capacity(LINE_BUFFER_CAPACITY),
error_path,
}
.run(),
);
let stdin = task::spawn(
GhciStdin {
ghci: Arc::downgrade(&ret),
Expand All @@ -157,7 +176,10 @@ impl Ghci {
// Wait for the stdout job to start up.
init_receiver.await.into_diagnostic()?;

let (initialize_event, init_receiver) = StdinEvent::initialize();
let (initialize_event, init_receiver) = {
let (sender, receiver) = oneshot::channel();
(StdinEvent::Initialize(sender), receiver)
};

// Perform start-of-session initialization.
stdin_sender
Expand Down Expand Up @@ -234,7 +256,7 @@ impl Ghci {
let mut guard = this.lock().await;
guard.stop().await?;
let command = guard.command.clone();
return Self::new(command).await;
return Self::new(command, guard.error_path.clone()).await;
}

if !add.is_empty() {
Expand Down Expand Up @@ -328,16 +350,6 @@ impl Ghci {
}
}

#[instrument(skip_all, level = "debug")]
async fn stderr_task(stderr: BufReader<ChildStderr>) -> miette::Result<()> {
let mut lines = stderr.lines();
while let Some(line) = lines.next_line().await.into_diagnostic()? {
tracing::info!("[ghci stderr] {line}");
}

Ok(())
}

fn format_bulleted_list(items: &[impl Display]) -> String {
if items.is_empty() {
String::new()
Expand Down
88 changes: 88 additions & 0 deletions src/ghci/stderr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
use std::sync::Weak;

use camino::Utf8PathBuf;
use miette::IntoDiagnostic;
use tokio::fs::File;
use tokio::io::AsyncWriteExt;
use tokio::io::BufReader;
use tokio::io::BufWriter;
use tokio::io::Lines;
use tokio::process::ChildStderr;
use tokio::sync::mpsc;
use tokio::sync::oneshot;
use tokio::sync::Mutex;
use tracing::instrument;

use super::Ghci;

/// An event sent to a `ghci` session's stderr channel.
#[derive(Debug)]
pub enum StderrEvent {
/// Write to the `error_path` (`ghcid.txt`) file, if any.
Write(oneshot::Sender<()>),
}

pub struct GhciStderr {
pub ghci: Weak<Mutex<Ghci>>,
pub reader: Lines<BufReader<ChildStderr>>,
pub receiver: mpsc::Receiver<StderrEvent>,
pub buffer: String,
pub error_path: Option<Utf8PathBuf>,
}

impl GhciStderr {
#[instrument(skip_all, name = "stderr", level = "debug")]
pub async fn run(mut self) -> miette::Result<()> {
loop {
// TODO: Could this cause problems where we get an event and a final stderr line is only
// processed after we write the error log?
tokio::select! {
Ok(Some(line)) = self.reader.next_line() => {
self.ingest_line(line).await;
}
Some(event) = self.receiver.recv() => {
match event {
StderrEvent::Write(sender) => {
let res = self.write().await;
let _ = sender.send(());
res?;
},
}
}
}
}
}

#[instrument(skip(self), level = "debug")]
async fn ingest_line(&mut self, line: String) {
self.buffer.push_str(&line);
self.buffer.push('\n');
eprintln!("{line}");
}

#[instrument(skip_all, level = "debug")]
async fn write(&mut self) -> miette::Result<()> {
if self.buffer.is_empty() {
// Nothing to do, don't wipe the error log file.
tracing::debug!("Buffer empty, not writing");
return Ok(());
}

if let Some(path) = &self.error_path {
tracing::debug!(?path, bytes = self.buffer.len(), "Writing error log");
let file = File::create(path).await.into_diagnostic()?;
let mut writer = BufWriter::new(file);
writer
.write_all(self.buffer.as_bytes())
.await
.into_diagnostic()?;
// This is load-bearing! If we don't properly flush/shutdown the handle, nothing gets
// written!
writer.shutdown().await.into_diagnostic()?;
}

self.buffer.clear();

Ok(())
}
}
8 changes: 0 additions & 8 deletions src/ghci/stdin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,6 @@ pub enum StdinEvent {
ShowModules(oneshot::Sender<ModuleSet>),
}

impl StdinEvent {
/// Construct an initialize event and the corresponding receiver.
pub fn initialize() -> (Self, oneshot::Receiver<()>) {
let (sender, receiver) = oneshot::channel();
(Self::Initialize(sender), receiver)
}
}

pub struct GhciStdin {
pub ghci: Weak<Mutex<Ghci>>,
pub stdin: ChildStdin,
Expand Down
14 changes: 13 additions & 1 deletion src/ghci/stdout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use crate::incremental_reader::WriteBehavior;
use crate::sync_sentinel::SyncSentinel;

use super::show_modules::ModuleSet;
use super::stderr::StderrEvent;
use super::stdin::StdinEvent;
use super::Ghci;

Expand All @@ -33,6 +34,7 @@ pub struct GhciStdout {
pub ghci: Weak<Mutex<Ghci>>,
pub reader: IncrementalReader<ChildStdout, Stdout>,
pub stdin_sender: mpsc::Sender<StdinEvent>,
pub stderr_sender: mpsc::Sender<StderrEvent>,
pub receiver: mpsc::Receiver<StdoutEvent>,
pub buffer: Vec<u8>,
}
Expand Down Expand Up @@ -102,7 +104,8 @@ impl GhciStdout {
)
.await?;
tracing::debug!(?lines, "Got data from ghci");
let _ = sender.send(());
// Tell the stderr stream to write the error log and then finish.
let _ = self.stderr_sender.send(StderrEvent::Write(sender)).await;
Ok(())
}

Expand All @@ -124,6 +127,15 @@ impl GhciStdout {
.read_until(prompt_patterns, WriteBehavior::Hide, &mut self.buffer)
.await?;
tracing::debug!(?lines, "Got data from ghci");

// Tell the stderr stream to write the error log and then finish.
let (err_sender, err_receiver) = oneshot::channel();
let _ = self
.stderr_sender
.send(StderrEvent::Write(err_sender))
.await;
let _ = err_receiver.await;

sentinel.finish();
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ async fn main() -> miette::Result<()> {
.wrap_err("Failed to split `--command` value into arguments")?,
));

let ghci = Ghci::new(ghci_command).await?;
let ghci = Ghci::new(ghci_command, opts.errors.clone()).await?;
let watcher = Watcher::new(
ghci,
&opts.watch.paths,
Expand Down
7 changes: 7 additions & 0 deletions src/watcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ use crate::ghci::Ghci;
/// A [`watchexec`] watcher which waits for file changes and sends reload events to the contained
/// `ghci` session.
pub struct Watcher {
/// The inner `Watchexec` struct.
///
/// This field isn't read, but it has to be here or the watcher stops working. Dropping this
/// drops the watcher tasks too.
#[allow(dead_code)]
inner: Arc<Watchexec>,
/// A handle to wait on the file watcher task.
pub handle: JoinHandle<Result<(), watchexec::error::CriticalError>>,
}
Expand Down Expand Up @@ -58,6 +64,7 @@ impl Watcher {
let watcher_handle = watcher.main();

Ok(Self {
inner: watcher,
handle: watcher_handle,
})
}
Expand Down
Loading