Re: [PATCH proxmox 1/3] system-report: add crate for shared report generation

[Erik Fastermann <e.fastermann@proxmox.com>]

Thank you for the review. Some notes/questions below. I will send the v2 
ASAP.

[snip]

>> diff --git a/proxmox-system-report/debian/copyright b/proxmox-system- 
>> report/debian/copyright
>> new file mode 100644
>> index 00000000..77952eba
>> --- /dev/null
>> +++ b/proxmox-system-report/debian/copyright
>> @@ -0,0 +1,18 @@
>> +Format: https://www.debian.org/doc/packaging-manuals/copyright- 
>> format/1.0/
>> +
>> +Files:
>> + *
>> +Copyright: 2019 - 2026 Proxmox Server Solutions GmbH 
>> <support@proxmox.com>
> 
> question: not sure about the copyright timespan here, but since the 
> factored out code lived in proxmox-backup and that has 2019-2026 this is 
> fine I guess?

I checked and for the recently added `proxmox-disks` only 'Copyright: 
2026' is used and that crate was also factored out from PBS, so I will 
do the same.

[snip]

>> +    vec![
>> +        ("proxmox-boot-tool", vec!["status"]),
>> +        ("df", vec!["-h", "-T"]),
>> +        (
>> +            "lsblk",
>> +            vec![
>> +                "--ascii",
>> +                "-M",
>> +                "-o",
>> +                "+HOTPLUG,ROTA,PHY-SEC,FSTYPE,MODEL,TRAN",
>> +            ],
>> +        ),
>> +        ("bash", vec!["-c", "ls -l /dev/disk/by-*/"]),
>> +        ("zpool", vec!["status"]),
>> +        ("zfs", vec!["list"]),
>> +        ("zarcstat", vec![]),
>> +        ("dmidecode", vec!["-t", "bios"]),
>> +        ("lscpu", vec![]),
>> +        ("lspci", vec!["-nnk"]),
>> +        ("ip", vec!["-details", "-statistics", "a"]),
>> +        ("ip", vec!["-4", "route", "show"]),
>> +        ("ip", vec!["-6", "route", "show"]),
> 
> comment: these commands depend on external executables. So there should 
> be a dependency in debian/control as well.

E.g. `proxmox-disks` calls `zpool` and `sgdisk`, which are also not in 
the `Depends` section. In fact all crates I checked in this repository 
only have other Rust related `Depends`. It looks like either PBS or PDM 
depend on them directly or they are pulled in by the installer.

The `Depends` section of PDM or PBS might need some work in general. 
E.g. `iproute2` is missing from PDM `Depends`, but is given for PBS. I 
saw there was a patch which added some to PBS [0], but that was not 
applied to PDM. It would probably be good to check what dependencies 
should be added. Should this be part of this series? What do you think?

[snip]


[0]: 
https://lore.proxmox.com/all/d7bd1b5d-dc09-4eac-9de5-4382160da88f@proxmox.com