public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Max Carrara" <m.carrara@proxmox.com>
To: "Proxmox VE development discussion" <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH v2 pve-common 00/12] Introduce and Package PVE::Path & PVE::Filesystem
Date: Thu, 02 Jan 2025 16:54:00 +0100	[thread overview]
Message-ID: <D6RPJ276WHKU.1WEFTKRBGG5PN@proxmox.com> (raw)
In-Reply-To: <47ba1e8e-c5d1-4873-91c4-9927b3b72e8c@proxmox.com>

On Thu Jan 2, 2025 at 2:53 PM CET, Fiona Ebner wrote:
> Am 02.01.25 um 14:46 schrieb Fiona Ebner:
> > Am 20.12.24 um 19:51 schrieb Max Carrara:
> >> Introduce and Package PVE::Path & PVE::Filesystem - v2
> >> ======================================================
> > 
> > Just an idea, but I'd like to have a discussion about it: Instead of
> > using Perl for such new general helper modules, would it be nicer to use
> > Rust+perlmod?
> > 
> > If our long-term goal is to migrate the Proxmox VE Perl code to Rust,
> > then we need to switch these modules over at some point in any case (or
> > drop them after switching over all users of the modules). Are there good
> > reasons not to start out with Rust+perlmod already?
> > 

Depends on what you mean with nicer: I was reluctant to use perlmod
here for a couple reasons:

1. We appear to have everything in the pve-rs crate right now
(libpve-rs-perl), so I had assumed that if I wanted to use perlmod here,
then I'd have to put my implementations into that crate as well.

This in turn would mean that for a simple path op library I'd need to
pull in pve-rs as dependency, which also contains a bunch of different
things that aren't concerned with path op stuff.

Perhaps I'm misunderstanding the purpose of the pve-rs crate, but I
decided against using perlmod here solely because I didn't want to add
any additional dependencies to this library unless otherwise necessary.

Right now as of this series, no additional dependencies besides some
Perl core modules are needed; the library can exist on its own.

2. I'm uncertain whether we actually want to have multiple repositories
or packages using perlmod (instead of having just pve-rs).

If we can use perlmod for individual modules, as in, add perlmod *alone*
as a dependency for packages like this one, then implement features and
add dependencies selectively, I'd be open to it.

Perhaps as an example, what I'd ideally prefer is something like
Python's cryptography is using PyO3 -- there's a Rust part and then
there's a Python part that's using the things implemented in Rust; only
whatever's necessary is pulled in [1].

3. Related to 1. and 2., there isn't any clear indication / guide / rule
of thumb / etc. on how perlmod ought to be used and in which contexts it
should be used.

4. Should we decide to use perlmod here eventually, individual functions
can still be implemented in Rust separately. Right now, there wasn't
really a need to use Rust, because PVE::Path works at most with strings
and a couple arrays here and there; there are no complex data structures
that need to be made typesafe.

> > You state that you (also) took inspiration from Rust's `std::path` so
> > could we just use that itself, wrapping via perlmod? Or would the
> > wrapping be too ugly here or lead to performance issues?

5. I'm not sure about the performance overhead, but it would certainly
be somewhat ugly, because all which PVE::Path essentially does consists
of string and array operations. If we used perlmod here hypothetically,
all that we'd be doing is give the Rust side a string or an array,
convert that to a PathBuf / Path or an iterable, perform the requested
operation and give the result back to Perl. It just seems a little
unnecessary to me.

6. I reckon that the places in which those two little libraries here
will be used will most likely be replaced by a pure Rust implementation
as a whole -- IMO there's no need to use perlmod for every single
smaller library if the Perl code using them gets replaced by Rust.

In other words, IMO a top-down approach such as replacing higher-level
subroutines or entire API calls would probably yield better results
rather than a bottom-up approach. (I believe there's a pattern for this
-- strangler pattern? I'd have to look it up tbh)

>
> Or depending on whether it's nicer, also wrapping helpers from
> proxmox-sys and friends where we already have similar functionality in
> our Rust code.

7. While I'm a big fan of re-using existing code, I don't think it
applies here -- I think it's fine to keep *certain* things separate and
decoupled from one another until we actually find that there's a lot of
common functionality between two or more things (speaking generally
here). For PVE::Path and PVE::Filesystem in particular, we can always
bridge over to Rust via perlmod for individual functions if needed (4.)
nevertheless, if that even ends up being necessary (6.).

With all that being said, I hope I could convey my reasoning here and
shine some light on my design decisions -- please let me know what you
think! And thanks for having a look :)

[1]: https://github.com/pyca/cryptography/tree/7fd5f95354e33d9ca90ba854e9cbda958968043a/src


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


  reply	other threads:[~2025-01-02 15:54 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-20 18:51 Max Carrara
2024-12-20 18:51 ` [pve-devel] [PATCH v2 pve-common 01/12] introduce PVE::Path Max Carrara
2025-01-08 14:05   ` Wolfgang Bumiller
2025-01-09  9:56     ` Max Carrara
2025-01-09 11:06       ` Wolfgang Bumiller
2025-01-09 12:56         ` Max Carrara
2024-12-20 18:51 ` [pve-devel] [PATCH v2 pve-common 02/12] test: add directory for tests of PVE::Path module Max Carrara
2024-12-20 18:51 ` [pve-devel] [PATCH v2 pve-common 03/12] test: add tests for path_is_absolute and path_is_relative of PVE::Path Max Carrara
2024-12-20 18:51 ` [pve-devel] [PATCH v2 pve-common 04/12] test: add tests for path_components " Max Carrara
2024-12-20 18:52 ` [pve-devel] [PATCH v2 pve-common 05/12] test: add tests for path_join " Max Carrara
2024-12-20 18:52 ` [pve-devel] [PATCH v2 pve-common 06/12] test: add tests for path_push " Max Carrara
2024-12-20 18:52 ` [pve-devel] [PATCH v2 pve-common 07/12] test: add tests for path_parent " Max Carrara
2024-12-20 18:52 ` [pve-devel] [PATCH v2 pve-common 08/12] test: add tests for path_starts_with, path_ends_with, path_equals Max Carrara
2024-12-20 18:52 ` [pve-devel] [PATCH v2 pve-common 09/12] test: add tests for file path ops functions of PVE::Path Max Carrara
2024-12-20 18:52 ` [pve-devel] [PATCH v2 pve-common 10/12] test: add tests for path_normalize " Max Carrara
2024-12-20 18:52 ` [pve-devel] [PATCH v2 pve-common 11/12] introduce PVE::Filesystem Max Carrara
2024-12-20 18:52 ` [pve-devel] [PATCH v2 pve-common 12/12] debian: introduce package libproxmox-fs-path-utils-perl Max Carrara
2025-01-02 13:46 ` [pve-devel] [PATCH v2 pve-common 00/12] Introduce and Package PVE::Path & PVE::Filesystem Fiona Ebner
2025-01-02 13:53   ` Fiona Ebner
2025-01-02 15:54     ` Max Carrara [this message]
2025-01-03  9:49       ` Fiona Ebner
2025-01-03 10:41         ` Thomas Lamprecht
2025-01-03 12:37           ` Fiona Ebner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=D6RPJ276WHKU.1WEFTKRBGG5PN@proxmox.com \
    --to=m.carrara@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal