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

[Christian Ebner <c.ebner@proxmox.com>]

On 7/29/26 1:55 PM, Erik Fastermann wrote:
> 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.

Right, since this is a lib only and does not produce any executable by 
itself, it is not correct to pull these in. Disregard my comment with 
respect to that.

> 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?

Yes, adding a patch to assure such dependencies is especially important 
if PBS or PDM are installed on top of a vanilla Debian and not via the 
installer.