Skip to content
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
16 changes: 8 additions & 8 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
steps:
- uses: actions/checkout@v4
- name: Setup Rust
uses: leynos/shared-actions/.github/actions/setup-rust@v1.1.0
uses: leynos/shared-actions/.github/actions/setup-rust@c6559452842af6a83b83429129dccaf910e34562
- name: Show Ninja version
run: ninja --version
- name: Format
Expand All @@ -26,17 +26,17 @@ jobs:
run: make lint
- name: Test
run: make test
- name: Install cargo-tarpaulin
run: cargo install cargo-tarpaulin
- name: Run coverage
run: cargo tarpaulin --out lcov
- name: Test and Measure Coverage
uses: leynos/shared-actions/.github/actions/generate-coverage@c6559452842af6a83b83429129dccaf910e34562
with:
output-path: lcov.info
format: lcov
- name: Upload coverage data to CodeScene
env:
CS_ACCESS_TOKEN: ${{ secrets.CS_ACCESS_TOKEN }}
if: ${{ env.CS_ACCESS_TOKEN != '' }}
uses: leynos/shared-actions/.github/actions/upload-codescene-coverage@v1.2.1
if: ${{ env.CS_ACCESS_TOKEN }}
uses: leynos/shared-actions/.github/actions/upload-codescene-coverage@c6559452842af6a83b83429129dccaf910e34562
with:
format: lcov
access-token: ${{ env.CS_ACCESS_TOKEN }}
installer-checksum: ${{ vars.CODESCENE_CLI_SHA256 }}

1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
target/
**/*.rs.bk
.crush
1 change: 1 addition & 0 deletions CRUSH.md
6 changes: 6 additions & 0 deletions docs/netsuke-design.md
Original file line number Diff line number Diff line change
Expand Up @@ -1155,6 +1155,12 @@ The command construction will follow this pattern:
streamed to the user's console, potentially with additional formatting or
status updates from Netsuke itself.

In the initial implementation a small helper wraps `Command::new` to forward
the `-j` and `-C` flags and any explicit build targets. Standard output and
error are piped and written back to Netsuke's own streams so users see Ninja's
messages in order. A non-zero exit status or failure to spawn the process is
reported as an `io::Error` for the CLI to surface.

### 6.2 The Criticality of Shell Escaping

A primary security responsibility for Netsuke is the prevention of command
Expand Down
4 changes: 2 additions & 2 deletions docs/roadmap.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ compilation pipeline from parsing to execution.
- [x] Write logic to generate Ninja rule statements from ir::Action structs
and build statements from ir::BuildEdge structs. *(done)*

- [ ] Implement the process management logic in `main.rs` to invoke the ninja
executable as a subprocess using `std::process::Command`.
- [x] Implement the process management logic in `main.rs` to invoke the ninja
executable as a subprocess using `std::process::Command`. *(done)*

- **Success Criterion:**

Expand Down
4 changes: 2 additions & 2 deletions src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
//! use netsuke::ast::NetsukeManifest;
//! use netsuke::ast::StringOrList;
//!
//! let yaml = r#"netsuke_version: \"1.0.0\"\ntargets:\n - name: hello\n recipe:\n kind: command\n command: \"echo hi\""#;
//! let yaml = "netsuke_version: \"1.0.0\"\ntargets:\n - name: hello\n recipe:\n kind: command\n command: \"echo hi\"";
//! let manifest: NetsukeManifest = serde_yml::from_str(yaml).expect("parse");
//! if let StringOrList::String(name) = &manifest.targets[0].name {
//! assert_eq!(name, "hello");
Expand Down Expand Up @@ -50,7 +50,7 @@ use std::collections::HashMap;
/// ```rust
/// use netsuke::ast::NetsukeManifest;
/// # fn main() -> Result<(), Box<dyn std::error::Error>> {
/// let yaml = "netsuke_version: 1.0.0\ntargets:\n - name: hello\n recipe:\n kind: command\n command: echo hi";
/// let yaml = "netsuke_version: \"1.0.0\"\ntargets:\n - name: hello\n recipe:\n kind: command\n command: echo hi";
/// let manifest: NetsukeManifest = serde_yml::from_str(yaml)?;
/// assert_eq!(manifest.targets.len(), 1);
/// # Ok(()) }
Expand Down
20 changes: 10 additions & 10 deletions src/ir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,11 +322,11 @@ fn find_cycle(targets: &HashMap<PathBuf, BuildEdge>) -> Option<Vec<PathBuf>> {

stack.push(node.clone());

if let Some(edge) = targets.get(node) {
let deps_result = visit_dependencies(targets, &edge.inputs, stack, states);
if let Some(cycle) = deps_result {
return Some(cycle);
}
if let Some(cycle) = targets
.get(node)
.and_then(|edge| visit_dependencies(targets, &edge.inputs, stack, states))
{
return Some(cycle);
}

stack.pop();
Expand All @@ -341,11 +341,10 @@ fn find_cycle(targets: &HashMap<PathBuf, BuildEdge>) -> Option<Vec<PathBuf>> {
states: &mut HashMap<PathBuf, VisitState>,
) -> Option<Vec<PathBuf>> {
for dep in deps {
if targets.contains_key(dep) {
let visit_result = visit(targets, dep, stack, states);
if let Some(cycle) = visit_result {
return Some(cycle);
}
if targets.contains_key(dep)
&& let Some(cycle) = visit(targets, dep, stack, states)
{
return Some(cycle);
}
}
None
Expand All @@ -355,6 +354,7 @@ fn find_cycle(targets: &HashMap<PathBuf, BuildEdge>) -> Option<Vec<PathBuf>> {
let mut stack = Vec::new();

for node in targets.keys() {
// Skip nodes we've already processed to avoid redundant traversal.
if states.contains_key(node) {
continue;
}
Expand Down
15 changes: 13 additions & 2 deletions src/main.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,17 @@
//! Application entry point.
//!
//! Parses command-line arguments and delegates execution to [`runner::run`].

use netsuke::{cli::Cli, runner};
use std::process::ExitCode;

fn main() {
fn main() -> ExitCode {
let cli = Cli::parse_with_default();
runner::run(cli);
match runner::run(&cli) {
Ok(()) => ExitCode::SUCCESS,
Err(err) => {
eprintln!("{err}");
ExitCode::FAILURE
}
}
}
87 changes: 80 additions & 7 deletions src/runner.rs
Original file line number Diff line number Diff line change
@@ -1,23 +1,96 @@
//! CLI execution and command dispatch logic.
//!
//! This module keeps [`main`] minimal by providing a single entry point that
//! handles command execution. It currently prints which command was invoked.
//! handles command execution. It now delegates build requests to the Ninja
//! subprocess, streaming its output back to the user.

use crate::cli::{Cli, Commands};
use std::io::{self, BufRead, BufReader, Write};
use std::path::Path;
use std::process::{Command, Stdio};
use std::thread;

/// Execute the parsed [`Cli`] commands.
pub fn run(cli: Cli) {
match cli.command.unwrap_or(Commands::Build {
///
/// # Errors
///
/// Returns an [`io::Error`] if the Ninja process fails to spawn or exits with a
/// non-zero status code.
pub fn run(cli: &Cli) -> io::Result<()> {
let command = cli.command.clone().unwrap_or(Commands::Build {
targets: Vec::new(),
}) {
Commands::Build { targets } => {
println!("Building targets: {targets:?}");
}
});
match command {
Commands::Build { targets } => run_ninja(Path::new("ninja"), cli, &targets),
Commands::Clean => {
println!("Clean requested");
Ok(())
}
Commands::Graph => {
println!("Graph requested");
Ok(())
}
}
}

/// Invoke the Ninja executable with the provided CLI settings.
///
/// The function forwards the job count and working directory to Ninja and
/// streams its standard output and error back to the user.
///
/// # Errors
///
/// Returns an [`io::Error`] if the Ninja process fails to spawn or reports a
/// non-zero exit status.
///
/// # Panics
///
/// Panics if the child's output streams cannot be captured.
pub fn run_ninja(program: &Path, cli: &Cli, targets: &[String]) -> io::Result<()> {
let mut cmd = Command::new(program);
if let Some(dir) = &cli.directory {
cmd.current_dir(dir).arg("-C").arg(dir);
}
if let Some(jobs) = cli.jobs {
cmd.arg("-j").arg(jobs.to_string());
}
cmd.args(targets);
cmd.stdout(Stdio::piped());
cmd.stderr(Stdio::piped());

let mut child = cmd.spawn()?;
let stdout = child.stdout.take().expect("child stdout");
let stderr = child.stderr.take().expect("child stderr");

let out_handle = thread::spawn(move || {
let reader = BufReader::new(stdout);
let mut handle = io::stdout();
for line in reader.lines().map_while(Result::ok) {
let _ = writeln!(handle, "{line}");
}
});
let err_handle = thread::spawn(move || {
let reader = BufReader::new(stderr);
let mut handle = io::stderr();
for line in reader.lines().map_while(Result::ok) {
let _ = writeln!(handle, "{line}");
}
});

let status = child.wait()?;
let _ = out_handle.join();
let _ = err_handle.join();

if status.success() {
Ok(())
} else {
#[expect(
clippy::io_other_error,
reason = "use explicit error kind for compatibility with older Rust"
)]
Err(io::Error::new(
io::ErrorKind::Other,
format!("ninja exited with {status}"),
))
}
}
25 changes: 25 additions & 0 deletions tests/cucumber.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,41 @@
//! Cucumber test runner and world state.

use cucumber::World;

/// Shared state for Cucumber scenarios.
#[derive(Debug, Default, World)]
pub struct CliWorld {
pub cli: Option<netsuke::cli::Cli>,
pub cli_error: Option<String>,
pub manifest: Option<netsuke::ast::NetsukeManifest>,
pub manifest_error: Option<String>,
pub build_graph: Option<netsuke::ir::BuildGraph>,
/// Generated Ninja file content.
pub ninja: Option<String>,
/// Status of the last process execution (true for success, false for
/// failure).
pub run_status: Option<bool>,
/// Error message from the last failed process execution.
pub run_error: Option<String>,
/// Temporary directory handle for test isolation.
pub temp: Option<tempfile::TempDir>,
/// Original `PATH` value restored after each scenario.
pub original_path: Option<std::ffi::OsString>,
}

impl Drop for CliWorld {
fn drop(&mut self) {
if let Some(path) = self.original_path.take() {
// SAFETY: nightly marks `set_var` as unsafe; restore path for isolation.
unsafe {
std::env::set_var("PATH", path);
}
}
}
}

mod steps;
mod support;

#[tokio::main]
async fn main() {
Expand Down
19 changes: 19 additions & 0 deletions tests/features/ninja_process.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
Feature: Ninja process execution

Scenario: Ninja succeeds
Given a fake ninja executable that exits with 0
And the CLI is parsed with ""
When the ninja process is run
Then the command should succeed

Scenario: Ninja fails
Given a fake ninja executable that exits with 1
And the CLI is parsed with ""
When the ninja process is run
Then the command should fail with error "ninja exited with exit status: 1"

Scenario: Ninja missing
Given no ninja executable is available
And the CLI is parsed with ""
When the ninja process is run
Then the command should fail with error "No such file or directory"
38 changes: 38 additions & 0 deletions tests/runner_tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
//! Unit tests for Ninja process invocation.

use netsuke::cli::{Cli, Commands};
use netsuke::runner;
use rstest::rstest;
use std::path::{Path, PathBuf};

/// Creates a default CLI configuration for testing Ninja invocation.
fn test_cli() -> Cli {
Cli {
file: PathBuf::from("Netsukefile"),
directory: None,
jobs: None,
command: Some(Commands::Build {
targets: Vec::new(),
}),
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

mod support;

#[rstest]
#[case(0, true)]
#[case(1, false)]
fn run_ninja_status(#[case] code: i32, #[case] succeeds: bool) {
let (_dir, path) = support::fake_ninja(code);
let cli = test_cli();
let result = runner::run_ninja(&path, &cli, &[]);
assert_eq!(result.is_ok(), succeeds);
}

#[rstest]
fn run_ninja_not_found() {
let cli = test_cli();
let err =
runner::run_ninja(Path::new("does-not-exist"), &cli, &[]).expect_err("process should fail");
assert_eq!(err.kind(), std::io::ErrorKind::NotFound);
}
4 changes: 3 additions & 1 deletion tests/steps/cli_steps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

use crate::CliWorld;
use clap::Parser;
use cucumber::{then, when};
use cucumber::{given, then, when};
use netsuke::cli::{Cli, Commands};
use std::path::PathBuf;

Expand Down Expand Up @@ -42,6 +42,7 @@ fn extract_build(world: &CliWorld) -> Option<&Vec<String>> {
clippy::needless_pass_by_value,
reason = "Cucumber requires owned String arguments"
)]
#[given(expr = "the CLI is parsed with {string}")]
#[when(expr = "the CLI is parsed with {string}")]
fn parse_cli(world: &mut CliWorld, args: String) {
apply_cli(world, &args);
Expand All @@ -51,6 +52,7 @@ fn parse_cli(world: &mut CliWorld, args: String) {
clippy::needless_pass_by_value,
reason = "Cucumber requires owned String arguments"
)]
#[given(expr = "the CLI is parsed with invalid arguments {string}")]
#[when(expr = "the CLI is parsed with invalid arguments {string}")]
fn parse_cli_invalid(world: &mut CliWorld, args: String) {
apply_cli(world, &args);
Expand Down
1 change: 1 addition & 0 deletions tests/steps/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ mod cli_steps;
mod ir_steps;
mod manifest_steps;
mod ninja_steps;
mod process_steps;
Loading
Loading