Skip to content

Commit fe240ee

Browse files
gabriele-0201pepyakin
authored andcommitted
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 981a617 commit fe240ee

File tree

9 files changed

+539
-487
lines changed

9 files changed

+539
-487
lines changed

Cargo.lock

Lines changed: 200 additions & 234 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
@@ -41,7 +41,7 @@ futures = { version = "0.3.29" }
4141
jsonrpsee = { version = "0.20.3" }
4242
tracing = { version = "0.1.40" }
4343
tracing-subscriber = { version = "0.3.18" }
44-
tokio = { version = "1.34.0" }
44+
tokio = { version = "1.36.0" }
4545
async-trait = { version = "0.1.74" }
4646
fex = { version = "0.4.3" }
4747
hex-literal = { version = "0.4.1" }
@@ -184,5 +184,6 @@ pallet-ikura-blobs = { path = "ikura/chain/pallets/blobs", default-features = fa
184184
pallet-ikura-length-fee-adjustment = { path = "ikura/chain/pallets/length-fee-adjustment", default-features = false }
185185

186186
# xtask
187-
duct = { version = "0.13.7" }
187+
xshell = { version = "0.2.5" }
188+
nix = { version = "0.27.1" }
188189
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 & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,37 @@
1-
use crate::{cli::BuildParams, logging::create_with_logs};
2-
use duct::cmd;
1+
use crate::{
2+
cli::BuildParams,
3+
logging::{create_log_file, WithLogs},
4+
};
5+
use xshell::cmd;
36

47
// TODO: https://github.com/thrumdev/blobs/issues/225
58

6-
pub fn build(project_path: &std::path::Path, params: BuildParams) -> anyhow::Result<()> {
9+
pub async fn build(project_path: &std::path::Path, params: BuildParams) -> anyhow::Result<()> {
710
if params.skip {
811
return Ok(());
912
}
1013

14+
let sh = xshell::Shell::new()?;
15+
1116
tracing::info!("Building logs redirected {}", params.log_path);
12-
let with_logs = create_with_logs(project_path, params.log_path);
17+
let log_path = create_log_file(project_path, &params.log_path);
1318

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

18-
with_logs(
19-
"Building ikura-node",
20-
cmd!(&cargo, "build", "-p", "ikura-node", "--release"),
21-
)
22-
.run()?;
23-
24-
with_logs(
25-
"Building ikura-shim",
26-
cmd!(&cargo, "build", "-p", "ikura-shim", "--release"),
27-
)
28-
.run()?;
29-
30-
let sov_demo_rollup_path = project_path.join("demo/sovereign/demo-rollup/");
31-
#[rustfmt::skip]
32-
with_logs(
33-
"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()?;
23+
cmd!(sh, "{cargo} build -p ikura-node --release")
24+
.run_with_logs("Building ikura-node", &log_path)
25+
.await??;
26+
27+
cmd!(sh, "{cargo} build -p ikura-node --release")
28+
.run_with_logs("Building ikura-shim", &log_path)
29+
.await??;
30+
31+
sh.change_dir(project_path.join("demo/sovereign/demo-rollup/"));
32+
cmd!(sh, "{cargo} build --release")
33+
.run_with_logs("Building sovereign demo-rollup", &log_path)
34+
.await??;
4235

4336
Ok(())
4437
}

xtask/src/logging.rs

Lines changed: 133 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,65 +1,153 @@
1-
use std::path::Path;
2-
use std::{io::Write, path::PathBuf};
3-
use tracing::{info, warn};
1+
use std::{
2+
fs::{create_dir_all, File},
3+
io::Write,
4+
path::{Path, PathBuf},
5+
process::{Command as StdCommand, Stdio},
6+
};
7+
use tokio::{
8+
process::{Child, Command as TokioCommand},
9+
task::JoinHandle,
10+
};
11+
use tracing::warn;
412

513
// If log_path is relative it will be made absolute relative to the project_path
614
//
715
// 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> {
16+
pub fn create_log_file(project_path: &Path, log_path: &String) -> Option<PathBuf> {
917
let mut log_path: PathBuf = Path::new(&log_path).to_path_buf();
1018

1119
if log_path.is_relative() {
1220
log_path = project_path.join(log_path);
1321
}
1422

1523
if let Some(prefix) = log_path.parent() {
16-
std::fs::create_dir_all(prefix)?;
24+
create_dir_all(prefix)
25+
.map_err(|e| warn!("Impossible to redirect logs, using stdout instead. Error: {e}"))
26+
.ok()?;
1727
}
18-
std::fs::File::create(&log_path)?;
19-
Ok(log_path)
28+
29+
File::create(&log_path)
30+
.map_err(|e| warn!("Impossible to redirect logs, using stdout instead. Error: {e}"))
31+
.ok()?;
32+
33+
Some(log_path)
2034
}
2135

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
36+
// These methods will accept a command description and a log path.
2537
//
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-
};
38+
// The description will be logged at the info log level. The command will be modified and converted to
39+
// a tokio::process::Command, redirecting stdout and stderr.
40+
//
41+
// If the log is None, stdout and stderr will be redirected to the caller's stdout.
42+
// If it contains a path, an attempt will be made to use it to redirect the command's
43+
// stdout and stderr. If, for any reason, the log file cannot be opened or created,
44+
// redirection will default back to the caller's stdout.
45+
pub trait WithLogs {
46+
fn with_logs(self, description: &str, log_path: &Option<PathBuf>) -> TokioCommand;
47+
fn spawn_with_logs(
48+
self,
49+
description: &str,
50+
log_path: &Option<PathBuf>,
51+
) -> anyhow::Result<Child>;
52+
fn run_with_logs(
53+
self,
54+
description: &str,
55+
log_path: &Option<PathBuf>,
56+
) -> JoinHandle<anyhow::Result<()>>;
57+
}
58+
59+
impl WithLogs for StdCommand {
60+
fn with_logs(self, description: &str, log_path: &Option<PathBuf>) -> TokioCommand {
61+
tracing::info!("{description}");
62+
63+
let (stdout, stderr) = log_path
64+
.as_ref()
65+
.and_then(|log_path| {
66+
match std::fs::File::options()
67+
.append(true)
68+
.create(true)
69+
.open(log_path.clone())
70+
{
71+
// If log file exists then use it
72+
Ok(mut log_out_file) => {
73+
let Ok(log_err_file) = log_out_file.try_clone() else {
74+
return Some((Stdio::inherit(), Stdio::inherit()));
75+
};
76+
77+
let _ = log_out_file
78+
.write(format!("{}\n", description).as_bytes())
79+
.map_err(|e| {
80+
warn!("Error writing into {}, error: {e}", log_path.display())
81+
});
82+
let _ = log_out_file.flush().map_err(|e| {
83+
warn!("Error writing into {}, error: {e}", log_path.display())
84+
});
85+
Some((Stdio::from(log_out_file), Stdio::from(log_err_file)))
86+
}
87+
// If log file does not exist then use inherited stdout and stderr
88+
Err(_) => Some((Stdio::inherit(), Stdio::inherit())),
89+
}
90+
})
91+
// If log file is not specified use inherited stdout and stderr
92+
.unwrap_or((Stdio::inherit(), Stdio::inherit()));
93+
94+
let mut command = TokioCommand::from(self);
95+
command.stderr(stderr).stdout(stdout);
96+
command
97+
}
98+
99+
fn spawn_with_logs(
100+
self,
101+
description: &str,
102+
log_path: &Option<PathBuf>,
103+
) -> anyhow::Result<tokio::process::Child> {
104+
self.with_logs(description, log_path)
105+
.kill_on_drop(true)
106+
.spawn()
107+
.map_err(|e| e.into())
108+
}
37109

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-
};
110+
fn run_with_logs(
111+
self,
112+
description: &str,
113+
log_path: &Option<PathBuf>,
114+
) -> tokio::task::JoinHandle<anyhow::Result<()>> {
115+
let description = String::from(description);
116+
let log_path = log_path.clone();
117+
tokio::task::spawn(async move {
118+
let exit_status: anyhow::Result<std::process::ExitStatus> = self
119+
.spawn_with_logs(&description, &log_path)?
120+
.wait()
121+
.await
122+
.map_err(|e| e.into());
123+
match exit_status?.code() {
124+
Some(code) if code != 0 => Err(anyhow::anyhow!(
125+
"{description}, exit with status code: {code}",
126+
)),
127+
_ => Ok(()),
128+
}
129+
})
130+
}
131+
}
45132

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();
133+
impl<'a> WithLogs for xshell::Cmd<'a> {
134+
fn with_logs(self, description: &str, log_path: &Option<PathBuf>) -> TokioCommand {
135+
StdCommand::from(self).with_logs(description, log_path)
136+
}
52137

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-
};
138+
fn spawn_with_logs(
139+
self,
140+
description: &str,
141+
log_path: &Option<PathBuf>,
142+
) -> anyhow::Result<tokio::process::Child> {
143+
StdCommand::from(self).spawn_with_logs(description, log_path)
144+
}
63145

64-
Box::new(with_logs)
146+
fn run_with_logs(
147+
self,
148+
description: &str,
149+
log_path: &Option<PathBuf>,
150+
) -> tokio::task::JoinHandle<anyhow::Result<()>> {
151+
StdCommand::from(self).run_with_logs(description, log_path)
152+
}
65153
}

0 commit comments

Comments
 (0)