From: "Elias Huhsovitz" <e.huhsovitz@proxmox.com>
To: "Max R. Carrara" <m.carrara@proxmox.com>, <pve-devel@lists.proxmox.com>
Subject: Re: [PATCH proxmox-perl-rs v2] make: improve library path detection for tests
Date: Wed, 05 Aug 2026 12:24:03 +0200 [thread overview]
Message-ID: <DKGXQED7NVUY.CCX82ZAM7Q8B@proxmox.com> (raw)
In-Reply-To: <20260714134110.335651-1-m.carrara@proxmox.com>
Nice patch. The less hacky solutions, the better!
2 small nits inline.
On Tue Jul 14, 2026 at 3:41 PM CEST, Max R. Carrara wrote:
> Export LD_LIBRARY_PATH containing the absolute path to the current
> Rust target's directory in the top-level Makefile.
>
> This allows the bootstrap machinery in Proxmox::Lib::PVE to find the
> compiled .so file when testing and building.
>
> Since it's not needed anymore, remove the `sed` workaround in the
> `all` target.
>
> Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
> ---
> Changes v1 --> v2:
> * Use 'improve' instead of 'fix' in the commit message because 'fix'
> implies that something was broken, which it wasn't
> * Use LD_LIBRARY_PATH instead of PERL5LIB, as the latter didn't
> actually work -- was a false positive on my side, as the .so file
> installed on my workstation was used
>
> Also, tested this more thoroughly this time around by adding `panic!()`
> statements in parts of the resource scheduling source code so that I
> could really see that the freshly compiled library is being used.
> Checked this for both `make check` and `make deb`, so the built .so file
> should always get picked up.
>
> pve-rs/Makefile | 7 +++----
> pve-rs/test/Makefile | 2 +-
> 2 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/pve-rs/Makefile b/pve-rs/Makefile
> index bb1cd2d..c7f2aa4 100644
> --- a/pve-rs/Makefile
> +++ b/pve-rs/Makefile
> @@ -48,6 +48,9 @@ else
> TARGET_DIR=debug
> endif
>
> +# Used to tell tests where the compiled .so file resides
> +export LD_LIBRARY_PATH=$(shell pwd)/target/$(TARGET_DIR)
nit: I would use `$(CURDIR)` instead of `$(shell pwd)`, because it
unaffected by the shell environment.
nit: Why not use `:=` instead of `=`? The values of CURDIR and TARGET
are known and should not change over the course of the Makefile
(unless i missed something).
e.g, I would change it to:
export LD_LIBRARY_PATH:=$(CURDIR)/target/$(TARGET_DIR)
> # better way to automate this without a massively annoying cargo-metatdata + jq
> # chain that spits out errors?
> PROXMOX_DOC_CRATES := \
> @@ -75,10 +78,6 @@ PROXMOX_DOC_URL_FLAGS = $(foreach crate,$(PROXMOX_DOC_CRATES),$(call make_rustdo
>
> all: PVE
> cargo build $(CARGO_BUILD_ARGS)
> - mkdir -p test/Proxmox/Lib
> - sed -r -e \
> - 's@^sub libfile.*$$@sub libfile { "$(shell pwd)/target/$(TARGET_DIR)/libpve_rs.so"; }@' \
> - Proxmox/Lib/PVE.pm >test/Proxmox/Lib/PVE.pm
>
> Proxmox: Proxmox/Lib/PVE.pm
> PVE: $(PERLMOD_PACKAGE_FILES)
> diff --git a/pve-rs/test/Makefile b/pve-rs/test/Makefile
> index 29dc955..2399b86 100644
> --- a/pve-rs/test/Makefile
> +++ b/pve-rs/test/Makefile
> @@ -1,5 +1,5 @@
> .PHONY: test
> -test: Proxmox/Lib/PVE.pm
> +test:
> @echo "-- running pve-rs tests --"
> perl -I. -I.. -I../.. ./resource_scheduling.pl
>
> --
> 2.47.3
prev parent reply other threads:[~2026-08-05 10:24 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-14 13:41 [PATCH proxmox-perl-rs v2] make: improve library path detection for tests Max R. Carrara
2026-08-05 10:24 ` Elias Huhsovitz [this message]
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=DKGXQED7NVUY.CCX82ZAM7Q8B@proxmox.com \
--to=e.huhsovitz@proxmox.com \
--cc=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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.