Skip to content

Commit 8a85b6f

Browse files
prontpront
committed
fix(vdev): replace script wrapper macro with shared argument utility
1 parent 5fd6c57 commit 8a85b6f

5 files changed

Lines changed: 189 additions & 69 deletions

File tree

‎vdev/src/commands/check/mod.rs‎

Lines changed: 42 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,26 +11,49 @@ mod markdown;
1111
mod rust;
1212
mod scripts;
1313

14-
crate::cli_subcommands! {
15-
"Check parts of the Vector code base..."
16-
changelog_fragments,
17-
generated_docs,
18-
component_features,
19-
component_examples,
20-
deny,
21-
docs,
22-
events,
23-
examples,
24-
fmt,
25-
licenses,
26-
markdown,
27-
rust,
28-
scripts,
14+
use crate::utils::command::ScriptArgs;
15+
16+
/// Check parts of the Vector code base...
17+
#[derive(clap::Args, Debug)]
18+
pub(super) struct Cli {
19+
#[command(subcommand)]
20+
command: Commands,
2921
}
3022

31-
// These should eventually be migrated to Rust code
23+
#[derive(clap::Subcommand, Debug)]
24+
enum Commands {
25+
ChangelogFragments(changelog_fragments::Cli),
26+
GeneratedDocs(generated_docs::Cli),
27+
ComponentFeatures(component_features::Cli),
28+
ComponentExamples(component_examples::Cli),
29+
Deny(deny::Cli),
30+
/// Check that all /docs files are valid
31+
Docs(ScriptArgs),
32+
Events(events::Cli),
33+
Examples(examples::Cli),
34+
Fmt(fmt::Cli),
35+
Licenses(licenses::Cli),
36+
Markdown(markdown::Cli),
37+
Rust(rust::Cli),
38+
Scripts(scripts::Cli),
39+
}
3240

33-
crate::script_wrapper! {
34-
docs = "Check that all /docs files are valid"
35-
=> "check-docs.sh"
41+
impl Cli {
42+
pub fn exec(self) -> anyhow::Result<()> {
43+
match self.command {
44+
Commands::ChangelogFragments(cli) => cli.exec(),
45+
Commands::GeneratedDocs(cli) => cli.exec(),
46+
Commands::ComponentFeatures(cli) => cli.exec(),
47+
Commands::ComponentExamples(cli) => cli.exec(),
48+
Commands::Deny(cli) => cli.exec(),
49+
Commands::Docs(args) => args.exec("check-docs.sh"),
50+
Commands::Events(cli) => cli.exec(),
51+
Commands::Examples(cli) => cli.exec(),
52+
Commands::Fmt(cli) => cli.exec(),
53+
Commands::Licenses(cli) => cli.exec(),
54+
Commands::Markdown(cli) => cli.exec(),
55+
Commands::Rust(cli) => cli.exec(),
56+
Commands::Scripts(cli) => cli.exec(),
57+
}
58+
}
3659
}

‎vdev/src/commands/mod.rs‎

Lines changed: 69 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -113,25 +113,76 @@ cli_commands! {
113113
version,
114114
}
115115

116-
/// This macro creates a wrapper for an existing script.
117-
#[macro_export]
118-
macro_rules! script_wrapper {
119-
( $mod:ident = $doc:literal => $script:literal ) => {
120-
pastey::paste! {
121-
mod $mod {
122-
#[doc = $doc]
123-
#[derive(clap::Args, Debug)]
124-
#[command()]
125-
pub(super) struct Cli {
126-
args: Vec<String>,
127-
}
116+
#[cfg(test)]
117+
mod tests {
118+
use clap::{CommandFactory as _, error::ErrorKind};
128119

129-
impl Cli {
130-
pub(super) fn exec(self) -> anyhow::Result<()> {
131-
$crate::app::exec(concat!("scripts/", $script), self.args, true)
132-
}
133-
}
120+
use super::Cli;
121+
122+
const SCRIPT_COMMANDS: &[(&str, &str)] = &[
123+
("check", "docs"),
124+
("package", "archive"),
125+
("package", "deb"),
126+
("package", "msi"),
127+
("package", "rpm"),
128+
("release", "docker"),
129+
("release", "s3"),
130+
];
131+
132+
#[test]
133+
fn script_commands_forward_arguments() {
134+
let cases: &[(&[&str], &[&str])] = &[
135+
(&[], &[]),
136+
(&["0.58.0"], &["0.58.0"]),
137+
(
138+
&["--chart-version", "0.46.0"],
139+
&["--chart-version", "0.46.0"],
140+
),
141+
(&["--chart-version=0.46.0"], &["--chart-version=0.46.0"]),
142+
(
143+
&["-x", "path with spaces", "-1"],
144+
&["-x", "path with spaces", "-1"],
145+
),
146+
(&["value", "--help", "-v"], &["value", "--help", "-v"]),
147+
(&["--", "--help"], &["--help"]),
148+
(
149+
&["--", "--chart-version", "0.46.0"],
150+
&["--chart-version", "0.46.0"],
151+
),
152+
];
153+
154+
for &(group, command) in SCRIPT_COMMANDS {
155+
for &(args, expected) in cases {
156+
let matches = Cli::command()
157+
.try_get_matches_from(
158+
["vdev", group, command]
159+
.into_iter()
160+
.chain(args.iter().copied()),
161+
)
162+
.unwrap_or_else(|error| panic!("{group} {command} {args:?}: {error}"));
163+
let script = matches
164+
.subcommand_matches(group)
165+
.unwrap()
166+
.subcommand_matches(command)
167+
.unwrap();
168+
let forwarded: Vec<_> = script
169+
.get_many::<String>("args")
170+
.into_iter()
171+
.flatten()
172+
.map(String::as_str)
173+
.collect();
174+
assert_eq!(forwarded, expected, "{group} {command} {args:?}");
134175
}
135176
}
136-
};
177+
}
178+
179+
#[test]
180+
fn script_commands_keep_vdev_help() {
181+
for &(group, command) in SCRIPT_COMMANDS {
182+
let error = Cli::command()
183+
.try_get_matches_from(["vdev", group, command, "--help"])
184+
.unwrap_err();
185+
assert_eq!(error.kind(), ErrorKind::DisplayHelp, "{group} {command}");
186+
}
187+
}
137188
}

‎vdev/src/commands/package.rs‎

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,31 @@
1-
crate::cli_subcommands! {
2-
"Package Vector in various formats..."
3-
archive, deb, msi, rpm,
4-
}
1+
use crate::utils::command::ScriptArgs;
52

6-
crate::script_wrapper! {
7-
archive = "Create a .tar.gz package for the specified $TARGET"
8-
=> "package-archive.sh"
9-
}
10-
crate::script_wrapper! {
11-
deb = "Create a .deb package to be distributed in the APT package manager"
12-
=> "package-deb.sh"
3+
/// Package Vector in various formats...
4+
#[derive(clap::Args, Debug)]
5+
pub(super) struct Cli {
6+
#[command(subcommand)]
7+
command: Commands,
138
}
14-
crate::script_wrapper! {
15-
msi = "Create a .msi package for Windows"
16-
=> "package-msi.sh"
9+
10+
#[derive(clap::Subcommand, Debug)]
11+
enum Commands {
12+
/// Create a .tar.gz package for the specified $TARGET
13+
Archive(ScriptArgs),
14+
/// Create a .deb package to be distributed in the APT package manager
15+
Deb(ScriptArgs),
16+
/// Create a .msi package for Windows
17+
Msi(ScriptArgs),
18+
/// Create a .rpm package to be distributed in the YUM package manager
19+
Rpm(ScriptArgs),
1720
}
18-
crate::script_wrapper! {
19-
rpm = "Create a .rpm package to be distributed in the YUM package manager"
20-
=> "package-rpm.sh"
21+
22+
impl Cli {
23+
pub fn exec(self) -> anyhow::Result<()> {
24+
match self.command {
25+
Commands::Archive(args) => args.exec("package-archive.sh"),
26+
Commands::Deb(args) => args.exec("package-deb.sh"),
27+
Commands::Msi(args) => args.exec("package-msi.sh"),
28+
Commands::Rpm(args) => args.exec("package-rpm.sh"),
29+
}
30+
}
2131
}

‎vdev/src/commands/release/mod.rs‎

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ mod workflow;
77
use anyhow::{Result, ensure};
88
use semver::Version;
99

10+
use crate::utils::command::ScriptArgs;
11+
1012
fn ensure_stable(version: &Version, label: &str) -> Result<()> {
1113
ensure!(
1214
version.pre.is_empty() && version.build.is_empty(),
@@ -23,22 +25,36 @@ fn preparation_branch(version: &Version) -> String {
2325
)
2426
}
2527

26-
crate::cli_subcommands! {
27-
"Manage the release process..."
28-
channel,
29-
docker,
30-
generate_cue,
31-
github,
32-
prepare,
33-
workflow,
34-
s3,
28+
/// Manage the release process...
29+
#[derive(clap::Args, Debug)]
30+
pub(super) struct Cli {
31+
#[command(subcommand)]
32+
command: Commands,
3533
}
3634

37-
crate::script_wrapper! {
38-
docker = "Build the Vector docker images and optionally push it to the registry"
39-
=> "build-docker.sh"
35+
#[derive(clap::Subcommand, Debug)]
36+
enum Commands {
37+
Channel(channel::Cli),
38+
/// Build the Vector docker images and optionally push it to the registry
39+
Docker(ScriptArgs),
40+
GenerateCue(generate_cue::Cli),
41+
Github(github::Cli),
42+
Prepare(prepare::Cli),
43+
Workflow(workflow::Cli),
44+
/// Uploads archives and packages to AWS S3
45+
S3(ScriptArgs),
4046
}
41-
crate::script_wrapper! {
42-
s3 = "Uploads archives and packages to AWS S3"
43-
=> "release-s3.sh"
47+
48+
impl Cli {
49+
pub fn exec(self) -> Result<()> {
50+
match self.command {
51+
Commands::Channel(cli) => cli.exec(),
52+
Commands::Docker(args) => args.exec("build-docker.sh"),
53+
Commands::GenerateCue(cli) => cli.exec(),
54+
Commands::Github(cli) => cli.exec(),
55+
Commands::Prepare(cli) => cli.exec(),
56+
Commands::Workflow(cli) => cli.exec(),
57+
Commands::S3(args) => args.exec("release-s3.sh"),
58+
}
59+
}
4460
}

‎vdev/src/utils/command.rs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,26 @@ use std::{
55
process::{self, Command},
66
};
77

8+
use crate::app::CommandExt as _;
9+
10+
/// Arguments forwarded to a repository script.
11+
#[derive(clap::Args, Debug)]
12+
pub struct ScriptArgs {
13+
/// Arguments passed to the script (use `-- --help` for the script's help).
14+
#[arg(allow_hyphen_values = true, trailing_var_arg = true)]
15+
args: Vec<String>,
16+
}
17+
18+
impl ScriptArgs {
19+
/// Run a script from the repository's scripts directory with the forwarded arguments.
20+
pub fn exec(self, script: &str) -> anyhow::Result<()> {
21+
Command::script(script)
22+
.args(self.args)
23+
.in_repo()
24+
.check_run()
25+
}
26+
}
27+
828
/// Trait for chaining command arguments
929
pub trait ChainArgs {
1030
fn chain_args<I: Into<OsString>>(&self, args: impl IntoIterator<Item = I>) -> Vec<OsString>;

0 commit comments

Comments
 (0)