From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <g.goller@proxmox.com>
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 33757BA6D9
for <pbs-devel@lists.proxmox.com>; Thu, 14 Dec 2023 14:44:46 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 0D7AF178A3
for <pbs-devel@lists.proxmox.com>; Thu, 14 Dec 2023 14:44:16 +0100 (CET)
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
for <pbs-devel@lists.proxmox.com>; Thu, 14 Dec 2023 14:44:15 +0100 (CET)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 1BE604761B
for <pbs-devel@lists.proxmox.com>; Thu, 14 Dec 2023 14:44:15 +0100 (CET)
Message-ID: <8315ac99-6081-4492-a209-a9d8f0a532b0@proxmox.com>
Date: Thu, 14 Dec 2023 14:44:14 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To: Thomas Lamprecht <t.lamprecht@proxmox.com>,
Proxmox Backup Server development discussion <pbs-devel@lists.proxmox.com>
References: <20231211085902.20747-1-g.goller@proxmox.com>
<20231211085902.20747-3-g.goller@proxmox.com>
<7319fa0e-46e9-4c9b-bcdf-6bc43876d61e@proxmox.com>
Content-Language: en-US
From: Gabriel Goller <g.goller@proxmox.com>
In-Reply-To: <7319fa0e-46e9-4c9b-bcdf-6bc43876d61e@proxmox.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
X-SPAM-LEVEL: Spam detection results: 0
AWL -0.145 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
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 v2 proxmox-backup 2/2] status: use Option on
avail/used datastore attrs
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
<pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Thu, 14 Dec 2023 13:44:46 -0000
On 12/14/23 08:55, Thomas Lamprecht wrote:
> Am 11/12/2023 um 09:59 schrieb Gabriel Goller:
>> [..]
> Talked a bit with Dietmar off-list and checked this out a bit closer, having
> to check each property separately seems like slightly annoying extra work we
> might be able to avoid while not loosing anything.
>
> I.e., what about keeping this as is and instead add a new usage property that
> is an option of a struct with all three values as u64?
>
> We get this info from file-system level, so if we cannot get all at once it'd
> be rather wrong anyway (from both our access control system and how we gather
> those data from the kernel), so I think this is an all or nothing.
>
> We could then can mark the other three i64 properties as deprecated and remove
> them with the next major version, thus having a clean compatibility cut and a
> easier to use API.
I like this idea.
> Not directly related, but while at it we could also improve how such things are
> marked as "to be removed at version X).
> In PVE we use FIXME comments with a (roughly) common style, but that is naturally
> error-prone, but here we have a full compilation step, so we could add a warning
> that is only emitted if the crate version is >= next-major version (and possibly
> some "future-deprecation feature to more easily check for them manually), and thus
> find all of those occurrences easily.
Hmm I looked into this a little bit and wrote a quick macro inspired by this
crate [0]. It's basically a `proc_macro_attribute` that takes a specific
semver condition
and gets the current crate version using the `CARGO_PKG_VERSION`
environment variable.
Then if the version matches, it adds a `#[deprecated]` attribute or
panics (which produces
a compiler error). For example:
#[proxmox_api_macro::deprecate_from(remove = ">= 4.x.x", note = "remove
total and use total_new")]
pub struct DataStoreStatusListItem {
pub total: i64,
pub total_new: Option<u64>,
}
This won't change anything currently, but will deprecate the struct with
the specified
message when the version hits `>=4.x.x`.
This would be perfect, but it has two problems:
1) This is a `proc_macro_attribute`, so it can only be set on the
struct/function and not on a
specific field. I don't think this is possible to implement using a
derive macro (although
please correct me if I'm wrong) and using a function-style macro
would be kinda ugly, cause we would
have to write (If this is even possible):
deprecate_from!(pub total: i64, remove=">=4.x.x")
Although I don't think this is necessarily a deal breaker, cause we
can use this to simply trigger
the deprecation notice + nearly always we have to change multiple
fields in the struct to remove a field.
2) The bigger problem is that we (AFAIK) can't get the workspace
package version inside the macro. `CARGO_PKG_VERSION`
only returns the version of the crate where the invocation is in.
For example for the example above, the version is
`0.1.0`, the version of `pbs-api-types`.
Let me know what you think!
[0]: https://crates.io/crates/deprecate_until