From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 1A8211FF17A for ; Tue, 28 Oct 2025 14:28:57 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 4A6181B7DF; Tue, 28 Oct 2025 14:29:29 +0100 (CET) Date: Tue, 28 Oct 2025 14:29:24 +0100 From: Wolfgang Bumiller To: Hannes Laimer Message-ID: References: <20251024144250.145205-1-h.laimer@proxmox.com> <20251024144250.145205-2-h.laimer@proxmox.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20251024144250.145205-2-h.laimer@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1761658152472 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.074 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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [schema2rust.pm] Subject: Re: [pdm-devel] [PATCH proxmox v2 1/3] pve-api-types: schema2rust: generate arrays for types with format `-list` X-BeenThere: pdm-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Datacenter Manager development discussion Cc: pdm-devel@lists.proxmox.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pdm-devel-bounces@lists.proxmox.com Sender: "pdm-devel" I followed this up with a cleanup, see comments below. On Fri, Oct 24, 2025 at 04:42:47PM +0200, Hannes Laimer wrote: > Since [1] the PVE API does take in actual arrays for parameters with a > format that ends in `-list`. With this we generate `Vec` for > those, return types are not affected, so they will still just generate a > `String`. > > [1] pve-common 69d9edcc ("section config: implement array support") > > Signed-off-by: Hannes Laimer > --- > since v2, thanks @Wolfgang: > - check if ref(schema->format) > - drop not needed defind-ness check > - use `0` as init value for `__list_format_as_array` for consistency > (insterad of undef) > - small perl improvments > > pve-api-types/generator-lib/Schema2Rust.pm | 41 +++++++++++++++++++++- > 1 file changed, 40 insertions(+), 1 deletion(-) > > diff --git a/pve-api-types/generator-lib/Schema2Rust.pm b/pve-api-types/generator-lib/Schema2Rust.pm > index c5ac4aad..322f6a01 100644 > --- a/pve-api-types/generator-lib/Schema2Rust.pm > +++ b/pve-api-types/generator-lib/Schema2Rust.pm > @@ -39,6 +39,7 @@ my $dedup_enum = {}; > my $dedup_array_types = {}; > > our $__err_path = ''; > +our $__list_format_as_array = 0; > > my sub to_doc_comment : prototype($); > my sub strip_doc_comment : prototype($); > @@ -509,6 +510,8 @@ my sub print_method_without_body : prototype($$$$$) { > print {$out} " .maybe_bool_arg(\"$name\", p_$rust_name)\n"; > } elsif ($arg->{is_string_list}) { > print {$out} " .maybe_list_arg(\"$name\", p_$rust_name)\n"; > + } elsif ($arg->{is_option_vec}) { > + print {$out} " .maybe_list_arg(\"$name\", &p_$rust_name)\n"; Replaced this to also deal with non-optional args. > } elsif ($arg->{optional}) { > print {$out} " .maybe_arg(\"$name\", &p_$rust_name)\n"; > } else { > @@ -534,7 +537,7 @@ my sub print_method_without_body : prototype($$$$$) { > > if ($arg->{type} eq 'Option') { > print {$out} " .maybe_bool_arg(\"$name\", $rust_name)\n"; > - } elsif ($arg->{is_string_list}) { > + } elsif ($arg->{is_string_list} || $arg->{is_option_vec}) { Replaced this to also deal with non-optional args. > print {$out} " .maybe_list_arg(\"$name\", &$rust_name)\n"; > } elsif ($arg->{optional}) { > print {$out} " .maybe_arg(\"$name\", &$rust_name)\n"; > @@ -809,6 +812,27 @@ my sub type_of : prototype($) { > my ($schema) = @_; > > my $kind = $schema->{type}; > + > + # If type is 'string' but format ends with `-list`, treat as array > + # but only for inputs to endpoints. > + # > + # We want a typed Vec, and since [1] the PVE API does accept > + # actual arrays for parameters with a format ending in `-list`. > + # This is only for inputs though, so we can only have Vec's for > + # inputs, returns are still string lists. > + # > + # [1] pve-common 69d9edcc ("section config: implement array support") > + if ( > + $kind eq 'string' > + && defined($schema->{format}) > + && !ref($schema->{format}) > + && $schema->{format} =~ /-list$/ > + ) { > + if ($__list_format_as_array) { > + return 'array'; > + } > + } I moved this into handle_def. `type_of` should return the original type, not apply magic, as this is the *initial* thing we check in handle_def, and we otherwise have no way of knowing that this was not *actually* an array. > + > return $kind if $kind; > > if (exists $schema->{properties}) { > @@ -1132,6 +1156,14 @@ my sub array_type : prototype($$$) { > optional => undef, > description => '', > }; > + if (!$schema->{items} && !ref($schema->{format}) && $schema->{format} =~ /-list$/) { > + my $format_name = $schema->{format} =~ s/-list$//r; > + $schema->{items} = { > + description => "List item of type $format_name.", > + format => $format_name, > + type => 'string', > + }; > + } > > my $items = delete $schema->{items} or die "missing 'items' in array schema\n"; > my $description = $items->{description}; > @@ -1242,6 +1274,7 @@ my sub make_struct_field : prototype($$$$) { > #my $schema = $$inout_schema; > # (in case the type was already an option type, don't duplicate the `Option<>`) > if ($optional && !$def->{optional}) { > + $def->{is_option_vec} = ($def->{type} =~ /^Vec<.*>$/) ? 1 : 0; ^ I removed this completely. It is not necessary, if the `$def` contains the info that this is a "string list turned into an array". The context here does not imply anything about why the type is currently a `Vec` in rust, and `is_option_vec` is not telling enough to mean "this is actually a string-list we want to concatenate with NUL bytes when used as a parameter". Also... why only deal with *optional* ones - we currently don't have mandatory ones, but they do exist in the API. > $def->{type} = "Option<$def->{type}>"; > $def->{api}->{optional} = $def->{optional} = true; > push $def->{attrs}->@*, "#[serde(default, skip_serializing_if = \"Option::is_none\")]"; > @@ -1578,6 +1611,8 @@ my sub url_parameters : prototype($) { > my sub method_parameters : prototype($$$$$) { > my ($def, $api_url, $param_name, $api_method, $rust_method_name) = @_; > > + local $__list_format_as_array = 1; > + > my $url_params = url_parameters($api_url); > > my $parameters = $api_method->{parameters} // {}; > @@ -1611,6 +1646,7 @@ my sub method_parameters : prototype($$$$$) { > } > > if ($def->{optional}) { > + $def->{is_option_vec} = ($def->{type} =~ /^Vec<.*>$/) ? 1 : 0; Same as above. > $def->{type} = "Option<$def->{type}>"; > } > > @@ -1638,6 +1674,7 @@ my sub method_parameters : prototype($$$$$) { > > handle_def($def, \$schema, namify_type($rust_method_name, $param)); > if ($def->{optional}) { > + $def->{is_option_vec} = ($def->{type} =~ /^Vec<.*>$/) ? 1 : 0; Same as above. > $def->{type} = "Option<$def->{type}>"; > } > push @$input, $def; > @@ -1676,6 +1713,8 @@ my sub method_parameters : prototype($$$$$) { > my sub method_return_type : prototype($$$$) { > my ($def, $method, $return_name, $extra) = @_; > > + local $__list_format_as_array = 0; > + > if (defined(my $returns = $extra->{'output-type'})) { > $def->{output_type} = $returns; > return; > -- > 2.47.3 _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel