From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 29AB971A9E for ; Fri, 13 May 2022 09:18:17 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 1E7382E91D for ; Fri, 13 May 2022 09:18:17 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 25F5D2E8D8 for ; Fri, 13 May 2022 09:18:13 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id EA1F3434DD for ; Fri, 13 May 2022 09:18:12 +0200 (CEST) Message-ID: Date: Fri, 13 May 2022 09:18:11 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.0 Content-Language: en-US To: Thomas Lamprecht , Proxmox Backup Server development discussion References: <20220505135252.466838-1-s.sterz@proxmox.com> <20220505135252.466838-5-s.sterz@proxmox.com> <59dbe363-e716-d241-1270-c7ecca384514@proxmox.com> From: Stefan Sterz In-Reply-To: <59dbe363-e716-d241-1270-c7ecca384514@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 1.417 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -2.888 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: Re: [pbs-devel] [PATCH proxmox-backup v1 1/2] fix #4001: datastore/catalog: add number of files to directory entry X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 May 2022 07:18:17 -0000 On 5/12/22 17:27, Thomas Lamprecht wrote: > Am 5/5/22 um 15:52 schrieb Stefan Sterz: >> when listing the content of a catalog, add the number of files >> contained in the directory as its size. also removes redundant code, >> the size of a file is already set when creating the archive entry. >> > > did you mean to write "the mtime of a file ..." as, we obviously need to > set the size differently to get the number of children, but you dropped > setting the mtime, which is OK as ArchiveEntry::new does that already, > or did I overlooked something? > sorry for the confusingly worded comment. the following is a bit more verbose than it might need to be in order to be a bit clearer ^^' : the original code would assign the `size` _and_ `mtime` again from the `DirEntryAttribute` if it was of type `DirEntryAttribute::File`. however, that is redundant as _both_ are already set by `ArchiveEntry::new()` and `ArchiveEntry::new_with_size()` from the same `DirEntryAttribute`. as you pointed out. therefore, i removed those conditional assignments and added another one for `DirEntryAttribute::Directory` because here `ArchiveEntry::new()` wouldn't set any size. hopefully this makes more sense. >> Signed-off-by: Stefan Sterz >> --- >> this requires patch two from the widget toolkit part of this patch to >> be applied already. otherwise the formatting and sorting of the folder >> "size" will be wrong. > > ok so no hard depends. > no, it will build just fine and no errors will be logged in the browser console either, but the amount of items will be displayed as the size in bytes and sorting will work accordingly, which would be confusing in most cases id assume. >> >> pbs-datastore/src/catalog.rs | 10 ++++++---- >> 1 file changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/pbs-datastore/src/catalog.rs b/pbs-datastore/src/catalog.rs >> index c07b71a6..6cb8aeb4 100644 >> --- a/pbs-datastore/src/catalog.rs >> +++ b/pbs-datastore/src/catalog.rs >> @@ -706,10 +706,11 @@ impl CatalogReader { >> components.push(b'/'); >> components.extend(&direntry.name); >> let mut entry = ArchiveEntry::new(&components, Some(&direntry.attr)); >> - if let DirEntryAttribute::File { size, mtime } = direntry.attr { >> - entry.size = size.into(); >> - entry.mtime = mtime.into(); >> + >> + if let DirEntryAttribute::Directory { start: _ } = direntry.attr { >> + entry.size = Some(u64::try_from(self.read_dir(&direntry)?.len())?); >> } >> + >> res.push(entry); >> } >> >> @@ -911,7 +912,8 @@ pub struct ArchiveEntry { >> pub entry_type: String, >> /// Is this entry a leaf node, or does it have children (i.e. a directory)? >> pub leaf: bool, >> - /// The file size, if entry_type is 'f' (file) >> + /// The file size, if entry_type is 'f' (file) or the amount of files in a >> + /// directory if entry_type is 'd' (directory) >> #[serde(skip_serializing_if = "Option::is_none")] >> pub size: Option, >> /// The file "last modified" time stamp, if entry_type is 'f' (file) >