fix: pager via libc::dup2, harden pre-push hook stdin handling
* New `output::pager` module spawns `$FJ_PAGER` / `$PAGER` / `less -FRX` when stdout is a TTY and dup2's our stdout onto its stdin. The `PagerGuard` restores the original stdout and waits on the child on drop so all output flushes before exit. * Wired into the top-level dispatch: list/view/diff/api/search/status output is now paged automatically. Short output passes through via `less -F`. Global `--no-pager` flag and `FJ_NO_PAGER` env opt out. * libc 0.2 added as a small dep (needed for dup/dup2/close). * Pre-push hook now drains and closes stdin at the top, then runs every step with `</dev/null`. Previously a test or build could in principle inherit git push's stdin (the list of refs being pushed) and block if it ever tried to read it. Adds CARGO_TERM_PROGRESS_WHEN=never so the progress bar doesn't muddle non-TTY runs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
7e8b8cc860
commit
fbf1354367
1
Cargo.lock
generated
1
Cargo.lock
generated
|
|
@ -447,6 +447,7 @@ dependencies = [
|
||||||
"indicatif",
|
"indicatif",
|
||||||
"is-terminal",
|
"is-terminal",
|
||||||
"keyring",
|
"keyring",
|
||||||
|
"libc",
|
||||||
"owo-colors",
|
"owo-colors",
|
||||||
"reqwest",
|
"reqwest",
|
||||||
"serde",
|
"serde",
|
||||||
|
|
|
||||||
|
|
@ -36,6 +36,7 @@ dialoguer = { version = "0.11", default-features = false, features = ["password"
|
||||||
is-terminal = "0.4"
|
is-terminal = "0.4"
|
||||||
textwrap = "0.16"
|
textwrap = "0.16"
|
||||||
futures-util = "0.3"
|
futures-util = "0.3"
|
||||||
|
libc = "0.2"
|
||||||
|
|
||||||
[profile.release]
|
[profile.release]
|
||||||
lto = "thin"
|
lto = "thin"
|
||||||
|
|
|
||||||
|
|
@ -2,6 +2,10 @@
|
||||||
# fj pre-push hook — local CI gate.
|
# fj pre-push hook — local CI gate.
|
||||||
# Runs fmt-check, clippy, tests, and a release build. With FJ_E2E=1 also
|
# Runs fmt-check, clippy, tests, and a release build. With FJ_E2E=1 also
|
||||||
# runs the live-API smoke suite against the currently signed-in host.
|
# runs the live-API smoke suite against the currently signed-in host.
|
||||||
|
#
|
||||||
|
# This script is invoked by git push with the list of refs being pushed on
|
||||||
|
# stdin. We close stdin ourselves and run every step with stdin redirected
|
||||||
|
# from /dev/null so cargo / tests can't accidentally block waiting for input.
|
||||||
|
|
||||||
set -euo pipefail
|
set -euo pipefail
|
||||||
|
|
||||||
|
|
@ -13,25 +17,40 @@ if [[ "${FJ_SKIP_PREPUSH:-0}" = "1" ]]; then
|
||||||
exit 0
|
exit 0
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
# Drain whatever git fed us on stdin so we don't deadlock if a child inherits
|
||||||
|
# our stdin and blocks. Then close it for the rest of the hook.
|
||||||
|
cat >/dev/null || true
|
||||||
|
exec 0</dev/null
|
||||||
|
|
||||||
|
# Compact output even when stderr is a TTY (the hook usually isn't run
|
||||||
|
# interactively).
|
||||||
|
export CARGO_TERM_COLOR=auto
|
||||||
|
export CARGO_TERM_PROGRESS_WHEN=never
|
||||||
|
|
||||||
step() {
|
step() {
|
||||||
printf '\n\033[1;34m== %s ==\033[0m\n' "$*"
|
printf '\n\033[1;34m== %s ==\033[0m\n' "$*" >&2
|
||||||
|
}
|
||||||
|
|
||||||
|
run() {
|
||||||
|
# Run with stdin closed and stdout/stderr passed through.
|
||||||
|
"$@" </dev/null
|
||||||
}
|
}
|
||||||
|
|
||||||
step "cargo fmt --check"
|
step "cargo fmt --check"
|
||||||
cargo fmt --all -- --check
|
run cargo fmt --all -- --check
|
||||||
|
|
||||||
step "cargo clippy (deny warnings)"
|
step "cargo clippy (deny warnings)"
|
||||||
cargo clippy --all-targets --all-features -- -D warnings
|
run cargo clippy --all-targets --all-features -- -D warnings
|
||||||
|
|
||||||
step "cargo test"
|
step "cargo test"
|
||||||
cargo test --all --locked
|
run cargo test --all --locked --no-fail-fast
|
||||||
|
|
||||||
step "cargo build --release"
|
step "cargo build --release"
|
||||||
cargo build --release --locked
|
run cargo build --release --locked
|
||||||
|
|
||||||
if [[ "${FJ_E2E:-0}" = "1" ]]; then
|
if [[ "${FJ_E2E:-0}" = "1" ]]; then
|
||||||
step "E2E smoke (live API)"
|
step "E2E smoke (live API)"
|
||||||
./scripts/e2e-smoke.sh
|
run ./scripts/e2e-smoke.sh
|
||||||
fi
|
fi
|
||||||
|
|
||||||
printf '\n\033[1;32m✓ pre-push checks passed\033[0m\n'
|
printf '\n\033[1;32m✓ pre-push checks passed\033[0m\n' >&2
|
||||||
|
|
|
||||||
|
|
@ -44,6 +44,10 @@ pub struct Cli {
|
||||||
#[arg(long, global = true, env = "FJ_DEBUG")]
|
#[arg(long, global = true, env = "FJ_DEBUG")]
|
||||||
pub debug: bool,
|
pub debug: bool,
|
||||||
|
|
||||||
|
/// Skip piping long output through `$FJ_PAGER` / `$PAGER` (default: less).
|
||||||
|
#[arg(long, global = true)]
|
||||||
|
pub no_pager: bool,
|
||||||
|
|
||||||
#[command(subcommand)]
|
#[command(subcommand)]
|
||||||
pub command: Command,
|
pub command: Command,
|
||||||
}
|
}
|
||||||
|
|
@ -116,6 +120,27 @@ pub struct CompletionArgs {
|
||||||
|
|
||||||
pub async fn run(cli: Cli) -> Result<()> {
|
pub async fn run(cli: Cli) -> Result<()> {
|
||||||
crate::client::set_debug(cli.debug);
|
crate::client::set_debug(cli.debug);
|
||||||
|
if cli.no_pager {
|
||||||
|
// SAFETY: setting an env var is safe; child processes inherit it.
|
||||||
|
unsafe { std::env::set_var("FJ_NO_PAGER", "1") };
|
||||||
|
}
|
||||||
|
// Page commands whose output can run long: any list, view, diff, or
|
||||||
|
// commits. Each call site re-enters this helper if it wants pager support.
|
||||||
|
// We start it conditionally here too so most `--help` invocations stay
|
||||||
|
// unpaged.
|
||||||
|
let _pager = match &cli.command {
|
||||||
|
Command::Repo(_)
|
||||||
|
| Command::Issue(_)
|
||||||
|
| Command::Pr(_)
|
||||||
|
| Command::Release(_)
|
||||||
|
| Command::Search(_)
|
||||||
|
| Command::Status(_)
|
||||||
|
| Command::Label(_)
|
||||||
|
| Command::Run(_)
|
||||||
|
| Command::Api(_) => crate::output::pager::maybe_start(),
|
||||||
|
_ => None,
|
||||||
|
};
|
||||||
|
|
||||||
match cli.command {
|
match cli.command {
|
||||||
Command::Auth(cmd) => auth::run(cmd).await,
|
Command::Auth(cmd) => auth::run(cmd).await,
|
||||||
Command::Repo(cmd) => repo::run(cmd, cli.host.as_deref()).await,
|
Command::Repo(cmd) => repo::run(cmd, cli.host.as_deref()).await,
|
||||||
|
|
|
||||||
|
|
@ -1,3 +1,5 @@
|
||||||
|
pub mod pager;
|
||||||
|
|
||||||
use std::io::IsTerminal;
|
use std::io::IsTerminal;
|
||||||
|
|
||||||
use anyhow::Result;
|
use anyhow::Result;
|
||||||
|
|
|
||||||
110
src/output/pager.rs
Normal file
110
src/output/pager.rs
Normal file
|
|
@ -0,0 +1,110 @@
|
||||||
|
//! Pager redirection. When stdout is a TTY (and the user hasn't opted out),
|
||||||
|
//! `maybe_start()` spawns `$FJ_PAGER` / `$PAGER` / `less -FRX` and dup2's our
|
||||||
|
//! stdout to its stdin. The returned guard waits on the child on drop, so all
|
||||||
|
//! output gets flushed before the process exits.
|
||||||
|
//!
|
||||||
|
//! The dup2 strategy means the rest of fj keeps using plain `println!` /
|
||||||
|
//! `print!` without knowing it's piped. `less -FRX` exits immediately if the
|
||||||
|
//! content fits one screen, so short output is unaffected.
|
||||||
|
|
||||||
|
use std::io::IsTerminal;
|
||||||
|
use std::os::fd::IntoRawFd;
|
||||||
|
use std::os::unix::io::AsRawFd;
|
||||||
|
use std::process::{Child, Command, Stdio};
|
||||||
|
|
||||||
|
pub struct PagerGuard {
|
||||||
|
/// Saved copy of the original stdout fd so we can restore it before
|
||||||
|
/// waiting on the child (otherwise the child would block reading from a
|
||||||
|
/// stdin pipe we still hold open).
|
||||||
|
saved_stdout: Option<i32>,
|
||||||
|
child: Option<Child>,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl Drop for PagerGuard {
|
||||||
|
fn drop(&mut self) {
|
||||||
|
// Make sure anything buffered in stdout reaches the pager before we
|
||||||
|
// close our write end.
|
||||||
|
use std::io::Write;
|
||||||
|
let _ = std::io::stdout().flush();
|
||||||
|
|
||||||
|
if let Some(saved) = self.saved_stdout.take() {
|
||||||
|
unsafe {
|
||||||
|
libc::dup2(saved, libc::STDOUT_FILENO);
|
||||||
|
libc::close(saved);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if let Some(mut c) = self.child.take() {
|
||||||
|
let _ = c.wait();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// If conditions are right, redirect this process's stdout to a pager. Returns
|
||||||
|
/// `None` if paging is disabled, stdout isn't a TTY, or spawning the pager
|
||||||
|
/// failed.
|
||||||
|
///
|
||||||
|
/// The returned guard must outlive all `println!`/`print!` calls produced by
|
||||||
|
/// the current command.
|
||||||
|
pub fn maybe_start() -> Option<PagerGuard> {
|
||||||
|
if std::env::var_os("FJ_NO_PAGER").is_some() {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
if !std::io::stdout().is_terminal() {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
let pager_cmd = std::env::var("FJ_PAGER")
|
||||||
|
.ok()
|
||||||
|
.or_else(|| std::env::var("PAGER").ok())
|
||||||
|
.unwrap_or_else(|| "less -FRX".into());
|
||||||
|
let pager_cmd = pager_cmd.trim().to_string();
|
||||||
|
if pager_cmd.is_empty() || pager_cmd == "cat" {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
let mut parts = pager_cmd.split_whitespace();
|
||||||
|
let program = parts.next()?;
|
||||||
|
let args: Vec<&str> = parts.collect();
|
||||||
|
|
||||||
|
// Save the current stdout so the guard can restore it.
|
||||||
|
let saved = unsafe { libc::dup(libc::STDOUT_FILENO) };
|
||||||
|
if saved < 0 {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
|
||||||
|
let mut child = match Command::new(program)
|
||||||
|
.args(&args)
|
||||||
|
.stdin(Stdio::piped())
|
||||||
|
.stdout(Stdio::inherit())
|
||||||
|
.stderr(Stdio::inherit())
|
||||||
|
.spawn()
|
||||||
|
{
|
||||||
|
Ok(c) => c,
|
||||||
|
Err(_) => {
|
||||||
|
unsafe { libc::close(saved) };
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
// Take the child's stdin pipe and dup it onto our stdout.
|
||||||
|
let Some(stdin) = child.stdin.take() else {
|
||||||
|
unsafe { libc::close(saved) };
|
||||||
|
let _ = child.kill();
|
||||||
|
let _ = child.wait();
|
||||||
|
return None;
|
||||||
|
};
|
||||||
|
let pipe_fd = stdin.as_raw_fd();
|
||||||
|
let result = unsafe { libc::dup2(pipe_fd, libc::STDOUT_FILENO) };
|
||||||
|
if result < 0 {
|
||||||
|
unsafe { libc::close(saved) };
|
||||||
|
let _ = child.kill();
|
||||||
|
let _ = child.wait();
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
// We dup'd the pipe fd onto stdout. Drop the ChildStdin handle without
|
||||||
|
// closing its underlying fd (it's now stdout's fd, owned by the process).
|
||||||
|
let _ = stdin.into_raw_fd();
|
||||||
|
|
||||||
|
Some(PagerGuard {
|
||||||
|
saved_stdout: Some(saved),
|
||||||
|
child: Some(child),
|
||||||
|
})
|
||||||
|
}
|
||||||
Loading…
Reference in a new issue