Skip to content

Commit 440a1e0

Browse files
committed
xtask: migrating from duct to xshell
xshell does not handle async, so it also requires the addition of Tokio. process group IDs are used to kill all xtask subprocesses
1 parent 8e8ed0e commit 440a1e0

File tree

9 files changed

+508
-467
lines changed

9 files changed

+508
-467
lines changed

Cargo.lock

Lines changed: 196 additions & 230 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ futures = { version = "0.3.29" }
4242
jsonrpsee = { version = "0.20.3" }
4343
tracing = { version = "0.1.40" }
4444
tracing-subscriber = { version = "0.3.18" }
45-
tokio = { version = "1.34.0" }
45+
tokio = { version = "1.36.0" }
4646
async-trait = { version = "0.1.74" }
4747
fex = { version = "0.4.3" }
4848
hex-literal = { version = "0.4.1" }
@@ -181,5 +181,6 @@ pallet-ikura-blobs = { path = "ikura/chain/pallets/blobs", default-features = fa
181181
pallet-ikura-length-fee-adjustment = { path = "ikura/chain/pallets/length-fee-adjustment", default-features = false }
182182

183183
# xtask
184-
duct = { version = "0.13.7" }
184+
xshell = { version = "0.2.5" }
185+
nix = { version = "0.27.1" }
185186
ctrlc = { version = "3.4.2" }

xtask/Cargo.toml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,11 @@ license.workspace = true
1010
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
1111

1212
[dependencies]
13-
duct = { workspace = true }
13+
xshell = { workspace = true }
14+
nix = { workspace = true, features = ["signal", "process"] }
15+
tokio = { workspace = true, features = ["rt", "macros", "rt-multi-thread", "time", "process", "sync", "signal"] }
1416
clap = { workspace = true, features = ["derive"] }
1517
anyhow = { workspace = true }
1618
tracing = { workspace = true }
1719
tracing-subscriber = { workspace = true, features = ["env-filter"] }
18-
ctrlc = { workspace = true }
20+
serde_json = { workspace = true }

xtask/src/build.rs

Lines changed: 21 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,42 @@
1-
use crate::{cli::BuildParams, logging::create_with_logs};
2-
use duct::cmd;
1+
use crate::{cli::BuildParams, logging::create_log_file, run_with_logs};
32

43
// TODO: https://github.com/thrumdev/blobs/issues/225
54

6-
pub fn build(project_path: &std::path::Path, params: BuildParams) -> anyhow::Result<()> {
5+
pub async fn build(project_path: &std::path::Path, params: BuildParams) -> anyhow::Result<()> {
76
if params.skip {
87
return Ok(());
98
}
109

10+
let sh = xshell::Shell::new()?;
11+
1112
tracing::info!("Building logs redirected {}", params.log_path);
12-
let with_logs = create_with_logs(project_path, params.log_path);
13+
let log_path = create_log_file(project_path, &params.log_path);
1314

1415
// `it is advisable to use CARGO environmental variable to get the right cargo`
1516
// quoted by xtask readme
1617
let cargo = std::env::var("CARGO").unwrap_or_else(|_| "cargo".to_string());
1718

18-
with_logs(
19+
run_with_logs!(
1920
"Building ikura-node",
20-
cmd!(&cargo, "build", "-p", "ikura-node", "--release"),
21-
)
22-
.run()?;
21+
sh,
22+
"{cargo} build -p ikura-node --release",
23+
log_path
24+
)?;
2325

24-
with_logs(
26+
run_with_logs!(
2527
"Building ikura-shim",
26-
cmd!(&cargo, "build", "-p", "ikura-shim", "--release"),
27-
)
28-
.run()?;
28+
sh,
29+
"{cargo} build -p ikura-node --release",
30+
log_path
31+
)?;
2932

30-
let sov_demo_rollup_path = project_path.join("demo/sovereign/demo-rollup/");
31-
#[rustfmt::skip]
32-
with_logs(
33+
sh.change_dir(project_path.join("demo/sovereign/demo-rollup/"));
34+
run_with_logs!(
3335
"Building sovereign demo-rollup",
34-
cmd!(
35-
"sh", "-c",
36-
format!(
37-
"cd {} && {cargo} build --release",
38-
sov_demo_rollup_path.to_string_lossy()
39-
)
40-
),
41-
).run()?;
36+
sh,
37+
"{cargo} build --release",
38+
log_path
39+
)?;
4240

4341
Ok(())
4442
}

xtask/src/logging.rs

Lines changed: 81 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,65 +1,100 @@
1-
use std::path::Path;
2-
use std::{io::Write, path::PathBuf};
3-
use tracing::{info, warn};
1+
use std::path::{Path, PathBuf};
2+
use tracing::warn;
43

54
// If log_path is relative it will be made absolute relative to the project_path
65
//
76
// The absolute path of where the log file is created is returned
8-
fn create_log_file(project_path: &Path, log_path: &String) -> std::io::Result<PathBuf> {
7+
pub fn create_log_file(project_path: &Path, log_path: &String) -> Option<String> {
98
let mut log_path: PathBuf = Path::new(&log_path).to_path_buf();
109

1110
if log_path.is_relative() {
1211
log_path = project_path.join(log_path);
1312
}
1413

1514
if let Some(prefix) = log_path.parent() {
16-
std::fs::create_dir_all(prefix)?;
15+
std::fs::create_dir_all(prefix)
16+
.map_err(|e| warn!("Impossible redirect logs, using stdout instead. Error: {e}"))
17+
.ok()?;
1718
}
18-
std::fs::File::create(&log_path)?;
19-
Ok(log_path)
19+
20+
std::fs::File::create(&log_path)
21+
.map_err(|e| warn!("Impossible redirect logs, using stdout instead. Error: {e}"))
22+
.ok()?;
23+
Some(log_path.to_string_lossy().to_string())
2024
}
2125

22-
// If the log file cannot be created due to any reasons,
23-
// such as lack of permission to create files or new folders in the path,
24-
// things will be printed to stdout instead of being redirected to the logs file
26+
// This macro will accept a description of the command, an xshell::Shell, a command
27+
// (a string on which format will be called), the log_path, and optionally the process group
28+
// used to create the command.
2529
//
26-
// The returned closure will accept a description of the command and the command itself as a duct::Expression.
27-
// The description will be printed to both stdout and the log file, if possible, while
28-
// to the expression will be added the redirection of the logs, if possible.
29-
pub fn create_with_logs(
30-
project_path: &Path,
31-
log_path: String,
32-
) -> Box<dyn Fn(&str, duct::Expression) -> duct::Expression> {
33-
let without_logs = |description: &str, cmd: duct::Expression| -> duct::Expression {
34-
info!("{description}");
35-
cmd
36-
};
37-
38-
let log_path = match create_log_file(project_path, &log_path) {
39-
Ok(log_path) => log_path,
40-
Err(e) => {
41-
warn!("Impossible redirect logs, using stdout instead. Error: {e}");
42-
return Box::new(without_logs);
43-
}
44-
};
30+
// The description will be logged with the info log level, and the command will be created
31+
// using the provided shell and converted to a tokio::process::Command to redirect stdout and stderr.
32+
//
33+
// If log is None, then things will be redirected to stdout.
34+
// Instead, if it contains a path, it will be used to redirect the command's output.
35+
//
36+
// The provided path needs to be a valid path, `create_log_file` should be used before use this macro
37+
#[macro_export]
38+
macro_rules! cmd_with_logs {
39+
($description:expr, $sh:expr, $cmd:literal, $log:expr $(,$gpid:literal)?) => {{
40+
use std::process::Stdio;
41+
tracing::info!("{}", $description);
42+
let (stdout, stderr) = match $log {
43+
None => (Stdio::inherit(), Stdio::inherit()),
44+
Some(ref log_path) => {
45+
// redirecting stdout and stderr into log_path
46+
let mut log_out_file = std::fs::File::options()
47+
.append(true)
48+
.create(true)
49+
.open(&log_path)
50+
.expect("Log file does not exist");
51+
let log_err_file = log_out_file.try_clone()?;
4552

46-
let with_logs = move |description: &str, cmd: duct::Expression| -> duct::Expression {
47-
// The file has just been created
48-
let mut log_file = std::fs::File::options()
49-
.append(true)
50-
.open(&log_path)
51-
.unwrap();
53+
use std::io::Write;
54+
let _ = log_out_file
55+
.write(format!("{}\n", $description).as_bytes())
56+
.map_err(|e| tracing::warn!("Error writing into {log_path}, error: {e}"));
57+
let _ = log_out_file
58+
.flush()
59+
.map_err(|e| tracing::warn!("Error writing into {log_path}, error: {e}"));
60+
(Stdio::from(log_out_file), Stdio::from(log_err_file))
61+
}
62+
};
63+
#[allow(unused_mut)]
64+
let mut std_cmd = std::process::Command::from(xshell::cmd!($sh, $cmd));
65+
$(
66+
use std::os::unix::process::CommandExt;
67+
std_cmd.process_group($gpid);
68+
)?
69+
tokio::process::Command::from(std_cmd)
70+
.stderr(stderr)
71+
.stdout(stdout)
72+
}};
73+
}
5274

53-
info!("{description}");
54-
let log_path = log_path.to_string_lossy();
55-
let _ = log_file
56-
.write(format!("{}\n", description).as_bytes())
57-
.map_err(|e| warn!("Error writing into {log_path}, error: {e}",));
58-
let _ = log_file
59-
.flush()
60-
.map_err(|e| warn!("Error writing into {log_path}, error: {e}",));
61-
cmd.stderr_to_stdout().stdout_file(log_file)
62-
};
75+
#[macro_export]
76+
macro_rules! spawn_with_logs {
77+
($description:expr, $sh:expr, $cmd:literal, $log:expr) => {{
78+
crate::cmd_with_logs!($description, $sh, $cmd, $log)
79+
.kill_on_drop(true)
80+
.spawn()
81+
}};
82+
}
6383

64-
Box::new(with_logs)
84+
#[macro_export]
85+
macro_rules! run_with_logs {
86+
($description:expr, $sh:expr, $cmd:literal, $log:expr) => {{
87+
let exit_status: anyhow::Result<std::process::ExitStatus> =
88+
crate::spawn_with_logs!($description, $sh, $cmd, $log)?
89+
.wait()
90+
.await
91+
.map_err(|e| e.into());
92+
match exit_status?.code() {
93+
Some(code) if code != 0 => Err(anyhow::anyhow!(
94+
"{}, exit with status code: {code}",
95+
$description
96+
)),
97+
_ => Ok(()),
98+
}
99+
}};
65100
}

0 commit comments

Comments
 (0)