public inbox for pdm-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Lukas Wagner" <l.wagner@proxmox.com>
To: "Erik Fastermann" <e.fastermann@proxmox.com>,
	<pbs-devel@lists.proxmox.com>, <pdm-devel@lists.proxmox.com>
Subject: Re: [PATCH proxmox v3 1/5] system-report: add crate for shared report generation
Date: Thu, 20 Aug 2026 15:32:05 +0200	[thread overview]
Message-ID: <DKTT4J9PI9NC.VUQPGPZ6OZHA@proxmox.com> (raw)
In-Reply-To: <20260811114332.283776-2-e.fastermann@proxmox.com>

Hi Erik,

thanks a lot for tackling this!

On Tue Aug 11, 2026 at 1:43 PM CEST, Erik Fastermann wrote:
> Factor the shared report generation from PBS and PDM into a new
> proxmox-system-report crate, so both generate their reports from a
> single implementation.
>
> The crate defines the common section order (FILES, COMMANDS,
> FUNCTIONS) and the set of general commands run on every product;
> products pass their own additional files, commands and functions on
> top. It produces no report on its own, so the user-visible changes
> land in the respective server packages.
>
> Suggested-by: Lukas Wagner <l.wagner@proxmox.com>
> Suggested-by: Christian Ebner <c.ebner@proxmox.com>
> Reviewed-by: Christian Ebner <c.ebner@proxmox.com>
> Tested-by: Christian Ebner <c.ebner@proxmox.com>
> Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
> ---
>  Cargo.toml                                 |   2 +
>  proxmox-system-report/Cargo.toml           |  16 ++
>  proxmox-system-report/debian/control       |  36 ++++
>  proxmox-system-report/debian/copyright     |  18 ++
>  proxmox-system-report/debian/debcargo.toml |   7 +
>  proxmox-system-report/src/lib.rs           | 239 +++++++++++++++++++++
>  6 files changed, 318 insertions(+)
>  create mode 100644 proxmox-system-report/Cargo.toml
>  create mode 100644 proxmox-system-report/debian/control
>  create mode 100644 proxmox-system-report/debian/copyright
>  create mode 100644 proxmox-system-report/debian/debcargo.toml

This is missing a debian/changelog file, without it 
`make proxmox-sytem-report-deb` does not work.

[...]


> +/// Generate a system report as a Markdown-like document.
> +///
> +/// The report has three top-level sections, always emitted in this order:
> +/// `FILES`, `COMMANDS` and `FUNCTIONS`. The project-specific entries passed in
> +/// are merged with a set of built-in, general-purpose entries common to all
> +/// products.
> +///
> +/// Missing files, unreadable directories and commands that fail to spawn are
> +/// reported inline instead of aborting the report.
> +pub fn generate_report(
> +    project_files: Vec<FileGroup>,
> +    project_commands: Vec<StaticArgsCommandSpec>,
> +    project_function_calls: Vec<FunctionMapping>,
> +) -> String {
> +    // We deliberately output the shared files before the project specific files
> +    // to preserve the legacy report ordering.
> +    let file_contents = common_files()
> +        .iter()
> +        .chain(&project_files)
> +        .map(|group| {
> +            let (group, files) = group;
> +            let group_content = files
> +                .iter()
> +                .map(|file_name| {
> +                    let path = Path::new(file_name);
> +                    if path.is_dir() {
> +                        get_directory_content(path)
> +                    } else {
> +                        get_file_content(file_name)
> +                    }
> +                })
> +                .collect::<Vec<String>>()
> +                .join("\n\n");
> +
> +            format!("### {group}\n\n{group_content}")
> +        })
> +        .collect::<Vec<String>>()
> +        .join("\n\n");
> +
> +    let static_command_outputs = project_commands
> +        .into_iter()
> +        .chain(common_commands())
> +        .map(|(command, args)| get_command_output(command, &args));
> +
> +    let dynamic_command_outputs = common_dynamic_commands()
> +        .into_iter()
> +        .map(|(command, args)| {
> +            let args: Vec<_> = args.iter().map(String::as_str).collect();
> +            get_command_output(command, &args)
> +        });
> +
> +    let command_outputs = static_command_outputs
> +        .chain(dynamic_command_outputs)
> +        .collect::<Vec<String>>()
> +        .join("\n\n");
> +
> +    let function_outputs = project_function_calls
> +        .iter()
> +        .chain(&common_function_calls())
> +        .map(|(desc, function)| {
> +            let output = function();
> +            format!("#### {desc}\n{}\n", output.trim_end())
> +        })
> +        .collect::<Vec<String>>()
> +        .join("\n\n");
> +
> +    format!(
> +        "## FILES\n\n{file_contents}\n## COMMANDS\n\n{command_outputs}\n## FUNCTIONS\n\n{function_outputs}\n"
> +    )
> +}


Just as a side note, I think this is actually a case where a builder
pattern could be very elegant, e.g. something like

let report = SystemReport::builder()
    .with_common_files()
    .with_file("/etc/hosts")
    .with_common_commands()
    .with_command("proxmox-datacenter-manager", &["subscription", "status"])
    ...
    .build();


I just wanted to mention it, from my side the current approach is also
fine, don't feel obligated to change it.




  reply	other threads:[~2026-08-20 13:32 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-11 11:43 [PATCH proxmox{,-backup,-datacenter-manager} v3 0/5] factor system report into shared crate Erik Fastermann
2026-08-11 11:43 ` [PATCH proxmox v3 1/5] system-report: add crate for shared report generation Erik Fastermann
2026-08-20 13:32   ` Lukas Wagner [this message]
2026-08-20 13:36     ` Christian Ebner
2026-08-20 13:46       ` Lukas Wagner
2026-08-11 11:43 ` [PATCH proxmox-backup v3 2/5] d/control: depend on tools used by the system report Erik Fastermann
2026-08-11 11:43 ` [PATCH proxmox-backup v3 3/5] report: use shared proxmox-system-report crate Erik Fastermann
2026-08-11 11:43 ` [PATCH proxmox-datacenter-manager v3 4/5] d/control: depend on tools used by the system report Erik Fastermann
2026-08-20 12:53   ` applied: " Lukas Wagner
2026-08-11 11:43 ` [PATCH proxmox-datacenter-manager v3 5/5] report: use shared proxmox-system-report crate Erik Fastermann
2026-08-12  9:59 ` [PATCH proxmox{,-backup,-datacenter-manager} v3 0/5] factor system report into shared crate Nicolas Frey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DKTT4J9PI9NC.VUQPGPZ6OZHA@proxmox.com \
    --to=l.wagner@proxmox.com \
    --cc=e.fastermann@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    --cc=pdm-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal