[pve-devel] [PATCH installer 0/5] chroot, assistant: replace clap with pico-args

[pve-devel] [PATCH installer 0/5] chroot, assistant: replace clap with pico-args

[Christoph Heiss]

This series first and foremost replaces clap, which is used by
proxmox-chroot and proxmox-auto-install-assistant for parsing CLI
arguments.

clap likes to completely overhaul its API with ~each major release,
which causes quite some churn. It is also quite heavyweight, both in
compile time and binary size impact (see patches #2 & #3 for details on
that). 

We already use pico-args in other tools such as pmg-log-tracker and
vma-to-pbs. Thomas also favored removing clap in the past, most recently
in [0].

Patches #4 through #5 are two other small cleanups & optimizations, esp.
#4 dropping another big dependency (regex) at least for one tool.

[0] https://lore.proxmox.com/pve-devel/f5a03ff5-7190-4d19-8fa9-3a93bffc1eb6@proxmox.com/

Christoph Heiss (5):
  common: introduce simple cli subcommand parser based on pico-args
  chroot: replace clap with pico-args for command argument parsing
  assistant: replace clap with pico-args for command argument parsing
  chroot: replace btrfs parsing regex with lsblk json parsing
  tui: drop unused dependencies from manifest

 Cargo.toml                                 |   1 +
 debian/control                             |   2 +-
 proxmox-auto-install-assistant/Cargo.toml  |   6 +-
 proxmox-auto-install-assistant/src/main.rs | 536 +++++++++++++++------
 proxmox-auto-installer/Cargo.toml          |   1 -
 proxmox-auto-installer/src/answer.rs       |   6 +-
 proxmox-auto-installer/src/utils.rs        |   7 +-
 proxmox-chroot/Cargo.toml                  |   6 +-
 proxmox-chroot/src/main.rs                 | 207 +++++---
 proxmox-installer-common/Cargo.toml        |   4 +
 proxmox-installer-common/src/cli.rs        |  62 +++
 proxmox-installer-common/src/lib.rs        |   3 +
 proxmox-tui-installer/Cargo.toml           |   2 -
 13 files changed, 612 insertions(+), 231 deletions(-)
 create mode 100644 proxmox-installer-common/src/cli.rs

-- 
2.49.0



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH installer 1/5] common: introduce simple cli subcommand parser based on pico-args

[Christoph Heiss]

This is a very slight abstraction on top of pico-args, to reduce
boilerplate code a bit for CLI subcommand handling.

No functional changes.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 Cargo.toml                          |  1 +
 debian/control                      |  1 +
 proxmox-installer-common/Cargo.toml |  4 ++
 proxmox-installer-common/src/cli.rs | 62 +++++++++++++++++++++++++++++
 proxmox-installer-common/src/lib.rs |  3 ++
 5 files changed, 71 insertions(+)
 create mode 100644 proxmox-installer-common/src/cli.rs

diff --git a/Cargo.toml b/Cargo.toml
index bfec8f7..4165b96 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -17,6 +17,7 @@ edition = "2024"
 [workspace.dependencies]
 anyhow = "1.0"
 log = "0.4.20"
+pico-args = "0.5"
 regex = "1.7"
 serde = "1.0"
 serde_json = "1.0"
diff --git a/debian/control b/debian/control
index dec07ed..51c45a4 100644
--- a/debian/control
+++ b/debian/control
@@ -15,6 +15,7 @@ Build-Depends: cargo:native,
                librust-glob-0.3-dev,
                librust-hex-0.4-dev,
                librust-native-tls-dev,
+               librust-pico-args-0.5-dev,
                librust-pretty-assertions-1.4-dev,
                librust-regex-1+default-dev (>= 1.7~~),
                librust-rustls-0.21+dangerous-configuration-dev,
diff --git a/proxmox-installer-common/Cargo.toml b/proxmox-installer-common/Cargo.toml
index 4bdb2b0..8a6fb5c 100644
--- a/proxmox-installer-common/Cargo.toml
+++ b/proxmox-installer-common/Cargo.toml
@@ -22,6 +22,9 @@ rustls-native-certs = { version = "0.6", optional = true }
 sha2 = { version = "0.10", optional = true }
 ureq = { version = "2.6", features = [ "native-certs", "native-tls" ], optional = true }
 
+# `cli` feature
+pico-args = { workspace = true, optional = true }
+
 [features]
 http = [
     "dep:hex",
@@ -31,6 +34,7 @@ http = [
     "dep:sha2",
     "dep:ureq"
 ]
+cli = [ "dep:pico-args" ]
 
 [dev-dependencies]
 pretty_assertions = "1.4"
diff --git a/proxmox-installer-common/src/cli.rs b/proxmox-installer-common/src/cli.rs
new file mode 100644
index 0000000..1539001
--- /dev/null
+++ b/proxmox-installer-common/src/cli.rs
@@ -0,0 +1,62 @@
+//! Provides a simple command line parsing interface, with special support for
+//! (one-level deep) subcommands.
+
+use std::process;
+
+use anyhow::Result;
+
+pub use pico_args::Arguments;
+
+pub trait Subcommand {
+    /// Parses the arguments for this command from an [`pico_args::Arguments`].
+    fn parse(args: &mut Arguments) -> Result<Self>
+    where
+        Self: Sized;
+
+    /// Print command usage to stderr.
+    fn print_usage()
+    where
+        Self: Sized;
+
+    /// Runs the commands action.
+    fn run(&self) -> Result<()>;
+}
+
+pub struct AppInfo<'a> {
+    pub global_help: &'a str,
+    pub on_command: fn(Option<&str>, &mut Arguments) -> Result<()>,
+}
+
+pub fn run(info: AppInfo) -> process::ExitCode {
+    if let Err(err) = parse_args(&info) {
+        eprintln!("Error: {err:#}\n\n{}", info.global_help);
+        process::ExitCode::FAILURE
+    } else {
+        process::ExitCode::SUCCESS
+    }
+}
+
+fn parse_args(info: &AppInfo) -> Result<()> {
+    let mut args = pico_args::Arguments::from_env();
+    let subcommand = args.subcommand()?;
+
+    if subcommand.is_none() && args.contains(["-h", "--help"]) {
+        eprintln!("{}", info.global_help);
+        Ok(())
+    } else if args.contains(["-V", "--version"]) {
+        eprintln!("{} v{}", env!("CARGO_PKG_NAME"), env!("CARGO_PKG_VERSION"));
+        Ok(())
+    } else {
+        (info.on_command)(subcommand.as_deref(), &mut args)
+    }
+}
+
+pub fn handle_command<T: Subcommand>(args: &mut pico_args::Arguments) -> Result<()> {
+    if args.contains(["-h", "--help"]) {
+        T::print_usage();
+    } else if let Err(err) = T::parse(args).and_then(|cmd| cmd.run()) {
+        eprint!("Error: {err:#}");
+    }
+
+    Ok(())
+}
diff --git a/proxmox-installer-common/src/lib.rs b/proxmox-installer-common/src/lib.rs
index 3dc3bfb..ea907a0 100644
--- a/proxmox-installer-common/src/lib.rs
+++ b/proxmox-installer-common/src/lib.rs
@@ -7,6 +7,9 @@ pub mod utils;
 #[cfg(feature = "http")]
 pub mod http;
 
+#[cfg(feature = "cli")]
+pub mod cli;
+
 pub const RUNTIME_DIR: &str = "/run/proxmox-installer";
 
 /// Default placeholder value for the administrator email address.
-- 
2.49.0



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH installer 2/5] chroot: replace clap with pico-args for command argument parsing

[Christoph Heiss]

Drops clap for pico-args, as the former is pretty heavy-weight and
completely overhauls its API with ~every major release.

As for the resulting binary, there is a ~18% decrease in binary size,
from 2.9M to 2.4MiB (after stripping).

   text    data     bss     dec     hex filename
2743595  274576     424 3018595  2e0f63 proxmox-chroot-clap
2208631  261224     408 2470263  25b177 proxmox-chroot-pico

Further, the dependency tree goes from 40 to 29 dependencies, as well as
the package compile time on my machine (on hot caches), roughly measured
with

`cargo clean && time cargo build --package proxmox-chroot --release`

from ~20s to ~16s, so a nice little change w.r.t. to that aspect too.

No functional changes intended.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-chroot/Cargo.toml  |   6 +-
 proxmox-chroot/src/main.rs | 159 +++++++++++++++++++++++++++----------
 2 files changed, 120 insertions(+), 45 deletions(-)

diff --git a/proxmox-chroot/Cargo.toml b/proxmox-chroot/Cargo.toml
index 7a1390a..c043938 100644
--- a/proxmox-chroot/Cargo.toml
+++ b/proxmox-chroot/Cargo.toml
@@ -9,8 +9,6 @@ homepage = "https://www.proxmox.com"
 
 [dependencies]
 anyhow.workspace = true
-proxmox-installer-common.workspace = true
-serde_json.workspace = true
 regex.workspace = true
-
-clap = { version = "4.0", features = ["derive"] }
+proxmox-installer-common = { workspace = true, features = [ "cli" ] }
+serde_json.workspace = true
diff --git a/proxmox-chroot/src/main.rs b/proxmox-chroot/src/main.rs
index ef339ed..037f079 100644
--- a/proxmox-chroot/src/main.rs
+++ b/proxmox-chroot/src/main.rs
@@ -1,13 +1,20 @@
+//! Tool to help prepare a successfully installed target environment for
+//! chroot'ing.
+//!
+//! Can also be used in the rescue environment for mounting an existing system.
+
+#![forbid(unsafe_code)]
+
 use std::{
-    fs, io,
+    env, fs, io,
     path::{self, Path, PathBuf},
-    process::Command,
+    process::{self, Command},
+    str::FromStr,
 };
 
 use anyhow::{Result, bail};
-use clap::{Args, Parser, Subcommand, ValueEnum};
 use proxmox_installer_common::{
-    RUNTIME_DIR,
+    RUNTIME_DIR, cli,
     options::FsType,
     setup::{InstallConfig, SetupInfo},
 };
@@ -18,46 +25,87 @@ static BINDMOUNTS: [&str; 4] = ["dev", "proc", "run", "sys"];
 const TARGET_DIR: &str = "/target";
 const ZPOOL_NAME: &str = "rpool";
 
-/// Helper tool to prepare everything to `chroot` into an installation
-#[derive(Parser, Debug)]
-#[command(author, version, about, long_about = None)]
-struct Cli {
-    #[command(subcommand)]
-    command: Commands,
-}
-
-#[derive(Subcommand, Debug)]
-enum Commands {
-    Prepare(CommandPrepare),
-    Cleanup(CommandCleanup),
-}
-
-/// Mount the root file system and bind mounts in preparation to chroot into the installation
-#[derive(Args, Debug)]
-struct CommandPrepare {
+/// Arguments for the `prepare` command.
+struct CommandPrepareArgs {
     /// Filesystem used for the installation. Will try to automatically detect it after a
     /// successful installation.
-    #[arg(short, long, value_enum)]
     filesystem: Option<Filesystems>,
 
-    /// Numerical ID of `rpool` ZFS pool to import. Needed if multiple pools of name `rpool` are present.
-    #[arg(long)]
+    /// Numerical ID of the `rpool` ZFS pool to import. Needed if multiple pools of name `rpool`
+    /// are present.
     rpool_id: Option<u64>,
 
     /// UUID of the BTRFS file system to mount. Needed if multiple BTRFS file systems are present.
-    #[arg(long)]
     btrfs_uuid: Option<String>,
 }
 
-/// Unmount everything. Use once done with chroot.
-#[derive(Args, Debug)]
-struct CommandCleanup {
+impl cli::Subcommand for CommandPrepareArgs {
+    fn parse(args: &mut cli::Arguments) -> Result<Self> {
+        Ok(Self {
+            filesystem: args.opt_value_from_str(["-f", "--filesystem"])?,
+            rpool_id: args.opt_value_from_fn("--rpool-id", str::parse)?,
+            btrfs_uuid: args.opt_value_from_str("--btrfs-uuid")?,
+        })
+    }
+
+    fn print_usage() {
+        eprintln!(
+            r#"Mount the root file system and bind mounts in preparation to chroot into the installation.
+
+USAGE:
+  {} prepare <OPTIONS>
+
+OPTIONS:
+  -f, --filesystem <FILESYSTEM>  Filesystem used for the installation. Will try to automatically detect it after a successful installation. [possible values: zfs, ext4, xfs, btrfs]
+      --rpool-id <RPOOL_ID>      Numerical ID of `rpool` ZFS pool to import. Needed if multiple pools of name `rpool` are present.
+      --btrfs-uuid <BTRFS_UUID>  UUID of the BTRFS file system to mount. Needed if multiple BTRFS file systems are present.
+  -h, --help                       Print this help
+  -V, --version                    Print version
+"#,
+            env!("CARGO_PKG_NAME")
+        );
+    }
+
+    fn run(&self) -> Result<()> {
+        prepare(self)
+    }
+}
+
+/// Arguments for the `cleanup` command.
+struct CommandCleanupArgs {
     /// Filesystem used for the installation. Will try to automatically detect it by default.
-    #[arg(short, long, value_enum)]
     filesystem: Option<Filesystems>,
 }
 
-#[derive(Copy, Clone, Debug, ValueEnum)]
+impl cli::Subcommand for CommandCleanupArgs {
+    fn parse(args: &mut cli::Arguments) -> Result<Self> {
+        Ok(Self {
+            filesystem: args.opt_value_from_str(["-f", "--filesystem"])?,
+        })
+    }
+
+    fn print_usage() {
+        eprintln!(
+            r#"Unmount everything. Use once done with the chroot.
+
+USAGE:
+  {} cleanup <OPTIONS>
+
+OPTIONS:
+  -f, --filesystem <FILESYSTEM>    Filesystem used for the installation. Will try to automatically detect it after a successful installation. [possible values: zfs, ext4, xfs, btrfs]
+  -h, --help                       Print this help
+  -V, --version                    Print version
+"#,
+            env!("CARGO_PKG_NAME")
+        );
+    }
+
+    fn run(&self) -> Result<()> {
+        cleanup(self)
+    }
+}
+
+#[derive(Copy, Clone, Debug)]
 enum Filesystems {
     Zfs,
     Ext4,
@@ -76,19 +124,48 @@ impl From<FsType> for Filesystems {
     }
 }
 
-fn main() {
-    let args = Cli::parse();
-    let res = match &args.command {
-        Commands::Prepare(args) => prepare(args),
-        Commands::Cleanup(args) => cleanup(args),
-    };
-    if let Err(err) = res {
-        eprintln!("{err:#}");
-        std::process::exit(1);
+impl FromStr for Filesystems {
+    type Err = anyhow::Error;
+
+    fn from_str(s: &str) -> Result<Self> {
+        match s {
+            "ext4" => Ok(Filesystems::Ext4),
+            "xfs" => Ok(Filesystems::Xfs),
+            _ if s.starts_with("zfs") => Ok(Filesystems::Zfs),
+            _ if s.starts_with("btrfs") => Ok(Filesystems::Btrfs),
+            _ => bail!("unknown filesystem"),
+        }
     }
 }
 
-fn prepare(args: &CommandPrepare) -> Result<()> {
+fn main() -> process::ExitCode {
+    cli::run(cli::AppInfo {
+        global_help: &format!(
+            r#"Helper tool to set up a `chroot` into an existing installation.
+
+USAGE:
+  {} <COMMAND> <OPTIONS>
+
+COMMANDS:
+  prepare            Mount the root file system and bind mounts in preparation to chroot into the installation.
+  cleanup            Unmount everything. Use once done with the chroot.
+
+GLOBAL OPTIONS:
+  -h, --help         Print help
+  -V, --version      Print version information
+"#,
+            env!("CARGO_PKG_NAME")
+        ),
+        on_command: |s, args| match s {
+            Some("prepare") => cli::handle_command::<CommandPrepareArgs>(args),
+            Some("cleanup") => cli::handle_command::<CommandCleanupArgs>(args),
+            Some(s) => bail!("unknown subcommand '{s}'"),
+            None => bail!("subcommand required"),
+        },
+    })
+}
+
+fn prepare(args: &CommandPrepareArgs) -> Result<()> {
     let fs = get_fs(args.filesystem)?;
 
     fs::create_dir_all(TARGET_DIR)?;
@@ -108,7 +185,7 @@ fn prepare(args: &CommandPrepare) -> Result<()> {
     Ok(())
 }
 
-fn cleanup(args: &CommandCleanup) -> Result<()> {
+fn cleanup(args: &CommandCleanupArgs) -> Result<()> {
     let fs = get_fs(args.filesystem)?;
 
     if let Err(e) = bind_umount() {
-- 
2.49.0



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH installer 3/5] assistant: replace clap with pico-args for command argument parsing

[Christoph Heiss]

Drops clap for pico-args, as the former is pretty heavy-weight and
completely overhauls its API with ~every major release.

While it results in a bit more boilerplate code, it makes (especially
long-term) maintenance simpler.

As for the resulting binary, there is a ~13.5% decrease in binary size,
from 4M to 3.4MiB (after stripping).

   text    data     bss     dec     hex filename
3793413  299000     616 4093029  3e7465 proxmox-auto-install-assistant-clap
3245990  286136     472 3532598  35e736 proxmox-auto-install-assistant-pico

Further, the dependency tree goes from 114 to 103 dependencies, as well
as the package compile time on my machine (on hot caches), roughly
measured with

`cargo clean && time cargo build --package proxmox-chroot --release`

from ~31 to ~28s.

A big chunk of the added lines are simply the help menus for the
different subcommands of the tool, the actual code changes are rather
small.

No functional changes intended.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 debian/control                             |   1 -
 proxmox-auto-install-assistant/Cargo.toml  |   6 +-
 proxmox-auto-install-assistant/src/main.rs | 536 +++++++++++++++------
 proxmox-auto-installer/Cargo.toml          |   1 -
 proxmox-auto-installer/src/answer.rs       |   6 +-
 proxmox-auto-installer/src/utils.rs        |   7 +-
 6 files changed, 387 insertions(+), 170 deletions(-)

diff --git a/debian/control b/debian/control
index 51c45a4..a2005a2 100644
--- a/debian/control
+++ b/debian/control
@@ -10,7 +10,6 @@ Build-Depends: cargo:native,
                libpve-common-perl,
                librsvg2-bin,
                librust-anyhow-1-dev,
-               librust-clap-4+derive-dev,
                librust-cursive-0.21+crossterm-backend-dev,
                librust-glob-0.3-dev,
                librust-hex-0.4-dev,
diff --git a/proxmox-auto-install-assistant/Cargo.toml b/proxmox-auto-install-assistant/Cargo.toml
index 43a968f..9b4a9c4 100644
--- a/proxmox-auto-install-assistant/Cargo.toml
+++ b/proxmox-auto-install-assistant/Cargo.toml
@@ -12,13 +12,9 @@ homepage = "https://www.proxmox.com"
 
 [dependencies]
 anyhow.workspace = true
-log.workspace = true
-proxmox-installer-common.workspace = true
 proxmox-auto-installer.workspace = true
-serde = { workspace = true, features = ["derive"] }
+proxmox-installer-common = { workspace = true, features = [ "cli" ] }
 serde_json.workspace = true
 toml.workspace = true
-regex.workspace = true
 
-clap = { version = "4.0", features = ["derive"] }
 glob = "0.3"
diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs
index b64623b..6ba6617 100644
--- a/proxmox-auto-install-assistant/src/main.rs
+++ b/proxmox-auto-install-assistant/src/main.rs
@@ -1,25 +1,31 @@
-use anyhow::{Result, bail, format_err};
-use clap::{Args, Parser, Subcommand, ValueEnum};
+//! This tool can be used to prepare a Proxmox installation ISO for automated installations.
+//! Additional uses are to validate the format of an answer file or to test match filters and print
+//! information on the properties to match against for the current hardware.
+
+#![forbid(unsafe_code)]
+
+use anyhow::{Context, Result, bail, format_err};
 use glob::Pattern;
-use serde::Serialize;
 use std::{
     collections::BTreeMap,
     fmt, fs,
     io::{self, Read},
     path::{Path, PathBuf},
-    process::{Command, Stdio},
+    process::{self, Command, Stdio},
+    str::FromStr,
 };
 
 use proxmox_auto_installer::{
     answer::{Answer, FilterMatch},
     sysinfo::SysInfo,
     utils::{
-        AutoInstSettings, FetchAnswerFrom, HttpOptions, get_matched_udev_indexes, get_nic_list,
-        get_single_udev_index, verify_email_and_root_password_settings, verify_first_boot_settings,
+        AutoInstSettings, FetchAnswerFrom, HttpOptions, default_partition_label,
+        get_matched_udev_indexes, get_nic_list, get_single_udev_index,
+        verify_email_and_root_password_settings, verify_first_boot_settings,
         verify_locale_settings,
     },
 };
-use proxmox_installer_common::{FIRST_BOOT_EXEC_MAX_SIZE, FIRST_BOOT_EXEC_NAME};
+use proxmox_installer_common::{FIRST_BOOT_EXEC_MAX_SIZE, FIRST_BOOT_EXEC_NAME, cli};
 
 static PROXMOX_ISO_FLAG: &str = "/auto-installer-capable";
 
@@ -27,225 +33,439 @@ static PROXMOX_ISO_FLAG: &str = "/auto-installer-capable";
 /// [LocaleInfo](`proxmox_installer_common::setup::LocaleInfo`) struct.
 const LOCALE_INFO: &str = include_str!("../../locale-info.json");
 
-/// This tool can be used to prepare a Proxmox installation ISO for automated installations.
-/// Additional uses are to validate the format of an answer file or to test match filters and
-/// print information on the properties to match against for the current hardware.
-#[derive(Parser, Debug)]
-#[command(author, version, about, long_about = None)]
-struct Cli {
-    #[command(subcommand)]
-    command: Commands,
+/// Arguments for the `device-info` command.
+struct CommandDeviceInfoArgs {
+    /// Device type for which information should be shown.
+    device_type: AllDeviceTypes,
 }
 
-#[derive(Subcommand, Debug)]
-enum Commands {
-    PrepareIso(CommandPrepareISO),
-    ValidateAnswer(CommandValidateAnswer),
-    DeviceMatch(CommandDeviceMatch),
-    DeviceInfo(CommandDeviceInfo),
-    SystemInfo(CommandSystemInfo),
+impl cli::Subcommand for CommandDeviceInfoArgs {
+    fn parse(args: &mut cli::Arguments) -> Result<Self> {
+        Ok(Self {
+            device_type: args
+                .opt_value_from_str(["-t", "--type"])?
+                .unwrap_or(AllDeviceTypes::All),
+        })
+    }
+
+    fn print_usage() {
+        eprintln!(
+            r#"Show device information that can be used for filters.
+
+USAGE:
+  {} device-info [OPTIONS]
+
+OPTIONS:
+  -t, --type <type>  For which device type information should be shown [default: all] [possible values: all, network, disk]
+  -h, --help         Print this help
+  -V, --version      Print version
+    "#,
+            env!("CARGO_PKG_NAME")
+        );
+    }
+
+    fn run(&self) -> Result<()> {
+        info(self)
+    }
 }
 
-/// Show device information that can be used for filters
-#[derive(Args, Debug)]
-struct CommandDeviceInfo {
-    /// For which device type information should be shown
-    #[arg(name="type", short, long, value_enum, default_value_t=AllDeviceTypes::All)]
-    device: AllDeviceTypes,
-}
-
-/// Test which devices the given filter matches against
-///
-/// Filters support the following syntax:
-/// - `?`               Match a single character
-/// - `*`               Match any number of characters
-/// - `[a]`, `[0-9]`  Specific character or range of characters
-/// - `[!a]`          Negate a specific character of range
-///
-/// To avoid globbing characters being interpreted by the shell, use single quotes.
-/// Multiple filters can be defined.
-///
-/// Examples:
-/// Match disks against the serial number and device name, both must match:
-///
-/// ```sh
-/// proxmox-auto-install-assistant match --filter-match all disk 'ID_SERIAL_SHORT=*2222*' 'DEVNAME=*nvme*'
-/// ```
-#[derive(Args, Debug)]
-#[command(verbatim_doc_comment)]
-struct CommandDeviceMatch {
-    /// Device type to match the filter against
-    r#type: Devicetype,
+/// Arguments for the `device-match` command.
+struct CommandDeviceMatchArgs {
+    /// Device type to match the filter against.
+    device_type: DeviceType,
 
     /// Filter in the format KEY=VALUE where the key is the UDEV key and VALUE the filter string.
     /// Multiple filters are possible, separated by a space.
     filter: Vec<String>,
 
     /// Defines if any filter or all filters must match.
-    #[arg(long, value_enum, default_value_t=FilterMatch::Any)]
     filter_match: FilterMatch,
 }
 
-/// Validate if an answer file is formatted correctly.
-#[derive(Args, Debug)]
-struct CommandValidateAnswer {
-    /// Path to the answer file
+impl cli::Subcommand for CommandDeviceMatchArgs {
+    fn parse(args: &mut cli::Arguments) -> Result<Self> {
+        let filter_match = args
+            .opt_value_from_str("--filter-match")?
+            .unwrap_or(FilterMatch::Any);
+
+        let device_type = args.free_from_str().context("parsing device type")?;
+        let mut filter = vec![];
+        while let Some(s) = args.opt_free_from_str()? {
+            filter.push(s);
+        }
+
+        Ok(Self {
+            device_type,
+            filter,
+            filter_match,
+        })
+    }
+
+    fn print_usage() {
+        eprintln!(
+            r#"Test which devices the given filter matches against.
+
+Filters support the following syntax:
+- `?`               Match a single character
+- `*`               Match any number of characters
+- `[a]`, `[0-9]`  Specific character or range of characters
+- `[!a]`          Negate a specific character of range
+
+To avoid globbing characters being interpreted by the shell, use single quotes.
+Multiple filters can be defined.
+
+Examples:
+Match disks against the serial number and device name, both must match:
+
+$ proxmox-auto-install-assistant match --filter-match all disk 'ID_SERIAL_SHORT=*2222*' 'DEVNAME=*nvme*'
+
+USAGE:
+  {} device-match [OPTIONS] <TYPE> [FILTER]...
+
+ARGUMENTS:
+  <TYPE>
+          Device type to match the filter against
+
+          [possible values: network, disk]
+
+  [FILTER]...
+          Filter in the format KEY=VALUE where the key is the UDEV key and VALUE the filter string. Multiple filters are possible, separated by a space.
+
+OPTIONS:
+      --filter-match <FILTER_MATCH>
+          Defines if any filter or all filters must match [default: any] [possible values: any, all]
+
+  -h, --help         Print this help
+  -V, --version      Print version
+    "#,
+            env!("CARGO_PKG_NAME")
+        );
+    }
+
+    fn run(&self) -> Result<()> {
+        match_filter(self)
+    }
+}
+
+/// Arguments for the `validate-answer` command.
+struct CommandValidateAnswerArgs {
+    /// Path to the answer file.
     path: PathBuf,
-    #[arg(short, long, default_value_t = false)]
+    /// Whether to also show the full answer as parsed.
     debug: bool,
 }
 
-/// Prepare an ISO for automated installation.
-///
-/// The behavior of how to fetch an answer file must be set with the '--fetch-from' parameter. The
-/// answer file can be:{n}
-/// * integrated into the ISO itself ('iso'){n}
-/// * present on a partition / file-system, matched by its label ('partition'){n}
-/// * requested via an HTTP Post request ('http').
-///
-/// The URL for the HTTP mode can be defined for the ISO with the '--url' argument. If not present,
-/// it will try to get a URL from a DHCP option (250, TXT) or by querying a DNS TXT record for the
-/// domain 'proxmox-auto-installer.{search domain}'.
-///
-/// The TLS certificate fingerprint can either be defined via the '--cert-fingerprint' argument or
-/// alternatively via the custom DHCP option (251, TXT) or in a DNS TXT record located at
-/// 'proxmox-auto-installer-cert-fingerprint.{search domain}'.
-///
-/// The latter options to provide the TLS fingerprint will only be used if the same method was used
-/// to retrieve the URL. For example, the DNS TXT record for the fingerprint will only be used, if
-/// no one was configured with the '--cert-fingerprint' parameter and if the URL was retrieved via
-/// the DNS TXT record.
-///
-/// If the 'partition' mode is used, the '--partition-label' parameter can be used to set the
-/// partition label the auto-installer should search for. This defaults to 'proxmox-ais'.
-#[derive(Args, Debug)]
-struct CommandPrepareISO {
-    /// Path to the source ISO to prepare
+impl cli::Subcommand for CommandValidateAnswerArgs {
+    fn parse(args: &mut cli::Arguments) -> Result<Self> {
+        Ok(Self {
+            debug: args.contains(["-d", "--debug"]),
+            // Needs to be last
+            path: args.free_from_str()?,
+        })
+    }
+
+    fn print_usage() {
+        eprintln!(
+            r#"Validate if an answer file is formatted correctly.
+
+USAGE:
+  {} validate-answer [OPTIONS] <PATH>
+
+ARGUMENTS:
+  <PATH>  Path to the answer file.
+
+OPTIONS:
+  -d, --debug        Also show the full answer as parsed.
+  -h, --help         Print this help
+  -V, --version      Print version
+    "#,
+            env!("CARGO_PKG_NAME")
+        );
+    }
+
+    fn run(&self) -> Result<()> {
+        validate_answer(self)
+    }
+}
+
+/// Arguments for the `prepare-iso` command.
+struct CommandPrepareISOArgs {
+    /// Path to the source ISO to prepare.
     input: PathBuf,
 
     /// Path to store the final ISO to, defaults to an auto-generated file name depending on mode
     /// and the same directory as the source file is located in.
-    #[arg(long)]
     output: Option<PathBuf>,
 
     /// Where the automatic installer should fetch the answer file from.
-    #[arg(long, value_enum)]
     fetch_from: FetchAnswerFrom,
 
     /// Include the specified answer file in the ISO. Requires the '--fetch-from'  parameter
     /// to be set to 'iso'.
-    #[arg(long)]
     answer_file: Option<PathBuf>,
 
-    /// Specify URL for fetching the answer file via HTTP
-    #[arg(long)]
+    /// Specify URL for fetching the answer file via HTTP.
     url: Option<String>,
 
     /// Pin the ISO to the specified SHA256 TLS certificate fingerprint.
-    #[arg(long)]
     cert_fingerprint: Option<String>,
 
     /// Staging directory to use for preparing the new ISO file. Defaults to the directory of the
     /// input ISO file.
-    #[arg(long)]
     tmp: Option<String>,
 
     /// Can be used in combination with `--fetch-from partition` to set the partition label
     /// the auto-installer will search for.
     // FAT can only handle 11 characters (per specification at least, drivers might allow more),
     // so shorten "Automated Installer Source" to "AIS" to be safe.
-    #[arg(long, default_value_t = { "proxmox-ais".to_owned() } )]
     partition_label: String,
 
     /// Executable file to include, which should be run on the first system boot after the
     /// installation. Can be used for further bootstrapping the new system.
     ///
     /// Must be appropriately enabled in the answer file.
-    #[arg(long)]
     on_first_boot: Option<PathBuf>,
 }
 
-/// Show the system information that can be used to identify a host.
-///
-/// The shown information is sent as POST HTTP request when fetching the answer file for the
-/// automatic installation through HTTP, You can, for example, use this to return a dynamically
-/// assembled answer file.
-#[derive(Args, Debug)]
-struct CommandSystemInfo {}
+impl cli::Subcommand for CommandPrepareISOArgs {
+    fn parse(args: &mut cli::Arguments) -> Result<Self> {
+        Ok(Self {
+            output: args.opt_value_from_str("--output")?,
+            fetch_from: args.value_from_str("--fetch-from")?,
+            answer_file: args.opt_value_from_str("--answer-file")?,
+            url: args.opt_value_from_str("--url")?,
+            cert_fingerprint: args.opt_value_from_str("--cert-fingerprint")?,
+            tmp: args.opt_value_from_str("--tmp")?,
+            partition_label: args
+                .opt_value_from_str("--partition-label")?
+                .unwrap_or_else(default_partition_label),
+            on_first_boot: args.opt_value_from_str("--on-first-boot")?,
+            // Needs to be last
+            input: args.free_from_str()?,
+        })
+    }
 
-#[derive(Args, Debug)]
-struct GlobalOpts {
-    /// Output format
-    #[arg(long, short, value_enum)]
-    format: OutputFormat,
+    fn print_usage() {
+        eprintln!(
+            r#"Prepare an ISO for automated installation.
+
+The behavior of how to fetch an answer file must be set with the '--fetch-from' parameter.
+The answer file can be:
+ * integrated into the ISO itself ('iso')
+ * present on a partition / file-system, matched by its label ('partition')
+ * requested via an HTTP Post request ('http').
+
+The URL for the HTTP mode can be defined for the ISO with the '--url' argument. If not present, it
+will try to get a URL from a DHCP option (250, TXT) or by querying a DNS TXT record for the domain
+'proxmox-auto-installer.{{search domain}}'.
+
+The TLS certificate fingerprint can either be defined via the '--cert-fingerprint' argument or
+alternatively via the custom DHCP option (251, TXT) or in a DNS TXT record located at
+'proxmox-auto-installer-cert-fingerprint.{{search domain}}'.
+
+The latter options to provide the TLS fingerprint will only be used if the same method was used to
+retrieve the URL. For example, the DNS TXT record for the fingerprint will only be used, if no one
+was configured with the '--cert-fingerprint' parameter and if the URL was retrieved via the DNS TXT
+record.
+
+If the 'partition' mode is used, the '--partition-label' parameter can be used to set the partition
+label the auto-installer should search for. This defaults to 'proxmox-ais'.
+
+USAGE:
+  {} prepare-iso [OPTIONS] --fetch-from <FETCH_FROM> <INPUT>
+
+ARGUMENTS:
+  <INPUT>
+          Path to the source ISO to prepare
+
+OPTIONS:
+      --output <OUTPUT>
+          Path to store the final ISO to, defaults to an auto-generated file name depending on mode
+          and the same directory as the source file is located in.
+
+      --fetch-from <FETCH_FROM>
+          Where the automatic installer should fetch the answer file from.
+
+          [possible values: iso, http, partition]
+
+      --answer-file <ANSWER_FILE>
+          Include the specified answer file in the ISO. Requires the '--fetch-from' parameter to
+          be set to 'iso'.
+
+      --url <URL>
+          Specify URL for fetching the answer file via HTTP.
+
+      --cert-fingerprint <CERT_FINGERPRINT>
+          Pin the ISO to the specified SHA256 TLS certificate fingerprint.
+
+      --tmp <TMP>
+          Staging directory to use for preparing the new ISO file. Defaults to the directory of the
+          input ISO file.
+
+      --partition-label <PARTITION_LABEL>
+          Can be used in combination with `--fetch-from partition` to set the partition label the
+          auto-installer will search for.
+
+          [default: proxmox-ais]
+
+      --on-first-boot <ON_FIRST_BOOT>
+          Executable file to include, which should be run on the first system boot after the
+          installation. Can be used for further bootstrapping the new system.
+
+          Must be appropriately enabled in the answer file.
+
+  -h, --help         Print this help
+  -V, --version      Print version
+    "#,
+            env!("CARGO_PKG_NAME")
+        );
+    }
+
+    fn run(&self) -> Result<()> {
+        prepare_iso(self)
+    }
 }
 
-#[derive(Clone, Debug, ValueEnum, PartialEq)]
+/// Arguments for the `system-info` command.
+struct CommandSystemInfoArgs;
+
+impl cli::Subcommand for CommandSystemInfoArgs {
+    fn parse(_: &mut cli::Arguments) -> Result<Self> {
+        Ok(Self)
+    }
+
+    fn print_usage() {
+        eprintln!(
+            r#"Show the system information that can be used to identify a host.
+
+The shown information is sent as POST HTTP request when fetching the answer file for the
+automatic installation through HTTP, You can, for example, use this to return a dynamically
+assembled answer file.
+
+USAGE:
+  {} system-info [OPTIONS]
+
+OPTIONS:
+  -h, --help         Print this help
+  -V, --version      Print version
+    "#,
+            env!("CARGO_PKG_NAME")
+        );
+    }
+
+    fn run(&self) -> Result<()> {
+        show_system_info(self)
+    }
+}
+
+#[derive(PartialEq)]
 enum AllDeviceTypes {
     All,
     Network,
     Disk,
 }
 
-#[derive(Clone, Debug, ValueEnum)]
-enum Devicetype {
+impl FromStr for AllDeviceTypes {
+    type Err = anyhow::Error;
+
+    fn from_str(s: &str) -> Result<Self> {
+        match s.to_lowercase().as_ref() {
+            "all" => Ok(AllDeviceTypes::All),
+            "network" => Ok(AllDeviceTypes::Network),
+            "disk" => Ok(AllDeviceTypes::Disk),
+            _ => bail!("unknown device type '{s}'"),
+        }
+    }
+}
+
+enum DeviceType {
     Network,
     Disk,
 }
 
-#[derive(Clone, Debug, ValueEnum)]
-enum OutputFormat {
-    Pretty,
-    Json,
-}
+impl FromStr for DeviceType {
+    type Err = anyhow::Error;
 
-#[derive(Serialize)]
-struct Devs {
-    disks: Option<BTreeMap<String, BTreeMap<String, String>>>,
-    nics: Option<BTreeMap<String, BTreeMap<String, String>>>,
-}
-
-fn main() {
-    let args = Cli::parse();
-    let res = match &args.command {
-        Commands::PrepareIso(args) => prepare_iso(args),
-        Commands::ValidateAnswer(args) => validate_answer(args),
-        Commands::DeviceInfo(args) => info(args),
-        Commands::DeviceMatch(args) => match_filter(args),
-        Commands::SystemInfo(args) => show_system_info(args),
-    };
-    if let Err(err) = res {
-        eprintln!("Error: {err:?}");
-        std::process::exit(1);
+    fn from_str(s: &str) -> Result<Self> {
+        match s.to_lowercase().as_ref() {
+            "network" => Ok(DeviceType::Network),
+            "disk" => Ok(DeviceType::Disk),
+            _ => bail!("unknown device type '{s}'"),
+        }
     }
 }
 
-fn info(args: &CommandDeviceInfo) -> Result<()> {
-    let mut devs = Devs {
-        disks: None,
-        nics: None,
-    };
+fn main() -> process::ExitCode {
+    cli::run(cli::AppInfo {
+        global_help: &format!(
+            r#"This tool can be used to prepare a Proxmox installation ISO for automated installations.
+Additional uses are to validate the format of an answer file or to test match filters and
+print information on the properties to match against for the current hardware
 
-    if args.device == AllDeviceTypes::Network || args.device == AllDeviceTypes::All {
+USAGE:
+  {} <COMMAND>
+
+COMMANDS:
+  prepare-iso      Prepare an ISO for automated installation
+  validate-answer  Validate if an answer file is formatted correctly
+  device-match     Test which devices the given filter matches against
+  device-info      Show device information that can be used for filters
+  system-info      Show the system information that can be used to identify a host
+
+GLOBAL OPTIONS:
+  -h, --help       Print help
+  -V, --version    Print version
+"#,
+            env!("CARGO_PKG_NAME")
+        ),
+        on_command: |s, args| match s {
+            Some("prepare-iso") => cli::handle_command::<CommandPrepareISOArgs>(args),
+            Some("validate-answer") => cli::handle_command::<CommandValidateAnswerArgs>(args),
+            Some("device-match") => cli::handle_command::<CommandDeviceMatchArgs>(args),
+            Some("device-info") => cli::handle_command::<CommandDeviceInfoArgs>(args),
+            Some("system-info") => cli::handle_command::<CommandSystemInfoArgs>(args),
+            Some(s) => bail!("unknown subcommand '{s}'"),
+            None => bail!("subcommand required"),
+        },
+    })
+}
+
+fn info(args: &CommandDeviceInfoArgs) -> Result<()> {
+    let nics = if matches!(
+        args.device_type,
+        AllDeviceTypes::All | AllDeviceTypes::Network
+    ) {
         match get_nics() {
-            Ok(res) => devs.nics = Some(res),
+            Ok(res) => Some(res),
             Err(err) => bail!("Error getting NIC data: {err}"),
         }
-    }
-    if args.device == AllDeviceTypes::Disk || args.device == AllDeviceTypes::All {
+    } else {
+        None
+    };
+
+    let disks = if matches!(args.device_type, AllDeviceTypes::All | AllDeviceTypes::Disk) {
         match get_disks() {
-            Ok(res) => devs.disks = Some(res),
+            Ok(res) => Some(res),
             Err(err) => bail!("Error getting disk data: {err}"),
         }
-    }
-    println!("{}", serde_json::to_string_pretty(&devs).unwrap());
+    } else {
+        None
+    };
+
+    serde_json::to_writer_pretty(
+        std::io::stdout(),
+        &serde_json::json!({
+            "disks": disks,
+            "nics": nics,
+        }),
+    )?;
     Ok(())
 }
 
-fn match_filter(args: &CommandDeviceMatch) -> Result<()> {
-    let devs: BTreeMap<String, BTreeMap<String, String>> = match args.r#type {
-        Devicetype::Disk => get_disks().unwrap(),
-        Devicetype::Network => get_nics().unwrap(),
+fn match_filter(args: &CommandDeviceMatchArgs) -> Result<()> {
+    let devs: BTreeMap<String, BTreeMap<String, String>> = match args.device_type {
+        DeviceType::Disk => get_disks().unwrap(),
+        DeviceType::Network => get_nics().unwrap(),
     };
     // parse filters
 
@@ -266,21 +486,21 @@ fn match_filter(args: &CommandDeviceMatch) -> Result<()> {
     }
 
     // align return values
-    let result = match args.r#type {
-        Devicetype::Disk => {
+    let result = match args.device_type {
+        DeviceType::Disk => {
             get_matched_udev_indexes(&filters, &devs, args.filter_match == FilterMatch::All)
         }
-        Devicetype::Network => get_single_udev_index(&filters, &devs).map(|r| vec![r]),
+        DeviceType::Network => get_single_udev_index(&filters, &devs).map(|r| vec![r]),
     };
 
     match result {
-        Ok(result) => println!("{}", serde_json::to_string_pretty(&result).unwrap()),
+        Ok(result) => serde_json::to_writer_pretty(std::io::stdout(), &result)?,
         Err(err) => bail!("Error matching filters: {err}"),
     }
     Ok(())
 }
 
-fn validate_answer(args: &CommandValidateAnswer) -> Result<()> {
+fn validate_answer(args: &CommandValidateAnswerArgs) -> Result<()> {
     let answer = parse_answer(&args.path)?;
     if args.debug {
         println!("Parsed data from answer file:\n{:#?}", answer);
@@ -288,7 +508,7 @@ fn validate_answer(args: &CommandValidateAnswer) -> Result<()> {
     Ok(())
 }
 
-fn show_system_info(_args: &CommandSystemInfo) -> Result<()> {
+fn show_system_info(_args: &CommandSystemInfoArgs) -> Result<()> {
     match SysInfo::as_json_pretty() {
         Ok(res) => println!("{res}"),
         Err(err) => eprintln!("Error fetching system info: {err}"),
@@ -296,7 +516,7 @@ fn show_system_info(_args: &CommandSystemInfo) -> Result<()> {
     Ok(())
 }
 
-fn prepare_iso(args: &CommandPrepareISO) -> Result<()> {
+fn prepare_iso(args: &CommandPrepareISOArgs) -> Result<()> {
     check_prepare_requirements(args)?;
     let uuid = get_iso_uuid(&args.input)?;
 
@@ -393,7 +613,7 @@ fn prepare_iso(args: &CommandPrepareISO) -> Result<()> {
     Ok(())
 }
 
-fn final_iso_location(args: &CommandPrepareISO) -> PathBuf {
+fn final_iso_location(args: &CommandPrepareISOArgs) -> PathBuf {
     if let Some(specified) = args.output.clone() {
         return specified;
     }
@@ -606,11 +826,11 @@ fn parse_answer(path: impl AsRef<Path> + fmt::Debug) -> Result<Answer> {
     }
 }
 
-fn check_prepare_requirements(args: &CommandPrepareISO) -> Result<()> {
+fn check_prepare_requirements(args: &CommandPrepareISOArgs) -> Result<()> {
     match Path::try_exists(&args.input) {
         Ok(true) => (),
-        Ok(false) => bail!("Source file does not exist."),
-        Err(_) => bail!("Source file does not exist."),
+        Ok(false) => bail!("Source file {:?} does not exist.", args.input),
+        Err(err) => bail!("Failed to stat source file {:?}: {err:#}", args.input),
     }
 
     match Command::new("xorriso")
diff --git a/proxmox-auto-installer/Cargo.toml b/proxmox-auto-installer/Cargo.toml
index 221d5d2..8a5283e 100644
--- a/proxmox-auto-installer/Cargo.toml
+++ b/proxmox-auto-installer/Cargo.toml
@@ -19,7 +19,6 @@ serde_json.workspace = true
 serde_plain.workspace = true
 toml.workspace = true
 
-clap = { version = "4.0", features = ["derive"] }
 glob = "0.3"
 
 [dev-dependencies]
diff --git a/proxmox-auto-installer/src/answer.rs b/proxmox-auto-installer/src/answer.rs
index 34065e8..eb1d829 100644
--- a/proxmox-auto-installer/src/answer.rs
+++ b/proxmox-auto-installer/src/answer.rs
@@ -1,5 +1,4 @@
 use anyhow::{Result, format_err};
-use clap::ValueEnum;
 use proxmox_installer_common::{
     options::{
         BtrfsCompressOption, BtrfsRaidLevel, FsType, ZfsChecksumOption, ZfsCompressOption,
@@ -364,13 +363,16 @@ pub enum DiskSelection {
     Selection(Vec<String>),
     Filter(BTreeMap<String, String>),
 }
-#[derive(Clone, Deserialize, Debug, PartialEq, ValueEnum)]
+
+#[derive(Clone, Deserialize, Debug, PartialEq)]
 #[serde(rename_all = "lowercase", deny_unknown_fields)]
 pub enum FilterMatch {
     Any,
     All,
 }
 
+serde_plain::derive_fromstr_from_deserialize!(FilterMatch);
+
 #[derive(Clone, Deserialize, Serialize, Debug, PartialEq)]
 #[serde(rename_all = "lowercase", deny_unknown_fields)]
 pub enum Filesystem {
diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
index 365e01a..e049748 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -1,5 +1,4 @@
 use anyhow::{Context, Result, bail};
-use clap::ValueEnum;
 use glob::Pattern;
 use log::info;
 use std::{collections::BTreeMap, process::Command};
@@ -95,7 +94,7 @@ pub fn get_single_udev_index(
     Ok(dev_index.unwrap())
 }
 
-#[derive(Deserialize, Serialize, Debug, Clone, ValueEnum, PartialEq)]
+#[derive(Deserialize, Serialize, Debug, Clone, PartialEq)]
 #[serde(rename_all = "lowercase", deny_unknown_fields)]
 pub enum FetchAnswerFrom {
     Iso,
@@ -103,6 +102,8 @@ pub enum FetchAnswerFrom {
     Partition,
 }
 
+serde_plain::derive_fromstr_from_deserialize!(FetchAnswerFrom);
+
 #[derive(Deserialize, Serialize, Clone, Default, PartialEq, Debug)]
 pub struct HttpOptions {
     #[serde(default, skip_serializing_if = "Option::is_none")]
@@ -121,7 +122,7 @@ pub struct AutoInstSettings {
     pub http: HttpOptions,
 }
 
-fn default_partition_label() -> String {
+pub fn default_partition_label() -> String {
     "proxmox-ais".to_owned()
 }
 
-- 
2.49.0



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH installer 4/5] chroot: replace btrfs parsing regex with lsblk json parsing

[Christoph Heiss]

Makes the whole routine a lot more robust by parsing the JSON output of
`lsblk` instead of the human-readable output from `btrfs filesystem
show`.

The labels of Btrfs filesystems are pretty much arbitrary labels, e.g.
the following is a valid filesystem and thus output from `btrfs
filesystem show`:

Label: 'uuid: 70361d98-7492-45d4-a0e4-bf8fee9203a3'  uuid: ab158685-c0b7-4540-a739-72c43a773282
        Total devices 1 FS bytes used 144.00KiB
        devid    1 size 8.00GiB used 536.00MiB path /dev/sda

.. which would break the current parsing code.

Also drops the usage of a regex, thus being able to eliminate the whole
regex machinery from the final binary. This reduces (stripped) binary
size significantly, from 2.4 MiB to ~727 KiB, a ~70%(!) decrease.

   text	   data	    bss	    dec	    hex	filename
2213016	 261208	    408	2474632	 25c288	proxmox-chroot-old
 702481	  27776	    360	 730617	  b25f9	proxmox-chroot-new

No functional changes.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-chroot/Cargo.toml  |  2 +-
 proxmox-chroot/src/main.rs | 48 +++++++++++++++++++++++++++-----------
 2 files changed, 35 insertions(+), 15 deletions(-)

diff --git a/proxmox-chroot/Cargo.toml b/proxmox-chroot/Cargo.toml
index c043938..a6a705d 100644
--- a/proxmox-chroot/Cargo.toml
+++ b/proxmox-chroot/Cargo.toml
@@ -9,6 +9,6 @@ homepage = "https://www.proxmox.com"
 
 [dependencies]
 anyhow.workspace = true
-regex.workspace = true
 proxmox-installer-common = { workspace = true, features = [ "cli" ] }
+serde = { workspace = true, features = [ "derive" ] }
 serde_json.workspace = true
diff --git a/proxmox-chroot/src/main.rs b/proxmox-chroot/src/main.rs
index 037f079..2cff630 100644
--- a/proxmox-chroot/src/main.rs
+++ b/proxmox-chroot/src/main.rs
@@ -18,7 +18,7 @@ use proxmox_installer_common::{
     options::FsType,
     setup::{InstallConfig, SetupInfo},
 };
-use regex::Regex;
+use serde::Deserialize;
 
 const ANSWER_MP: &str = "answer";
 static BINDMOUNTS: [&str; 4] = ["dev", "proc", "run", "sys"];
@@ -355,29 +355,49 @@ fn mount_btrfs(btrfs_uuid: Option<String>) -> Result<()> {
     Ok(())
 }
 
+/// Searches for all Btrfs filesystems present on the system.
+///
+/// # Returns
+///
+/// The UUID of that filesystem, if a single Btrfs filesystem is found.
+/// If multiple are found, returns a (human-readable) error with all possible
+/// filesystem UUIDs.
 fn get_btrfs_uuid() -> Result<String> {
-    let output = Command::new("btrfs")
-        .arg("filesystem")
-        .arg("show")
+    let output = Command::new("lsblk")
+        .args(["--json", "--fs", "--list", "--output", "fstype,uuid"])
         .output()?;
+
     if !output.status.success() {
         bail!(
             "Error checking for BTRFS file systems: {}",
             String::from_utf8(output.stderr)?
         );
     }
-    let out = String::from_utf8(output.stdout)?;
-    let mut uuids = Vec::new();
 
-    let re_uuid =
-        Regex::new(r"uuid: ([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})$")?;
-    for line in out.lines() {
-        if let Some(cap) = re_uuid.captures(line) {
-            if let Some(uuid) = cap.get(1) {
-                uuids.push(uuid.as_str());
-            }
-        }
+    #[derive(Deserialize)]
+    struct Entry {
+        fstype: Option<String>,
+        uuid: Option<String>,
     }
+
+    #[derive(Deserialize)]
+    struct LsblkOutput {
+        blockdevices: Vec<Entry>,
+    }
+
+    let out: LsblkOutput = serde_json::from_slice(&output.stdout)?;
+    let uuids = out
+        .blockdevices
+        .iter()
+        .filter_map(|entry| match entry {
+            Entry {
+                fstype: Some(fstype),
+                uuid: Some(uuid),
+            } if fstype == "btrfs" => Some(uuid),
+            _ => None,
+        })
+        .collect::<Vec<&String>>();
+
     match uuids.len() {
         0 => bail!("Could not find any BTRFS UUID"),
         i if i > 1 => {
-- 
2.49.0



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH installer 5/5] tui: drop unused dependencies from manifest

[Christoph Heiss]

There are not used anywhere in the tui crate, so drop them.

No functional changes.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-tui-installer/Cargo.toml | 2 --
 1 file changed, 2 deletions(-)

diff --git a/proxmox-tui-installer/Cargo.toml b/proxmox-tui-installer/Cargo.toml
index 403d1ef..139c85c 100644
--- a/proxmox-tui-installer/Cargo.toml
+++ b/proxmox-tui-installer/Cargo.toml
@@ -9,8 +9,6 @@ homepage = "https://www.proxmox.com"
 
 [dependencies]
 proxmox-installer-common.workspace = true
-serde = { workspace = true, features = ["derive"] }
 serde_json.workspace = true
-regex.workspace = true
 
 cursive = { version = "0.21", default-features = false, features = ["crossterm-backend"] }
-- 
2.49.0



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel