[gentoo-dev] [PATCH 1/3] cargo.eclass: Add cargo_env helper and use it in compile, test, install

[gentoo-dev] [PATCH 1/3] cargo.eclass: Add cargo_env helper and use it in compile, test, install

[James Le Cuirot]

Rust packages have a tendency to rebuild parts during test and install.
It is not clear whether this can be addressed. We were therefore relying
on some environment variables set during the compile phase for
cross-compiling to work in the later phases. This is not ideal,
especially if you need to build for multiple targets.

These environment variables can also be useful in other contexts, such
as the build runner in app-misc/anki.

This change moves the setting of these variables into a separate helper
that is now used in all these phases and can be used by ebuilds too. The
variables are now kept local to each invocation of this helper,
preventing leakage.

Signed-off-by: James Le Cuirot <chewi@gentoo.org>
---
 eclass/cargo.eclass | 37 +++++++++++++++++++++++--------------
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/eclass/cargo.eclass b/eclass/cargo.eclass
index 7db34efb4e174..b6d5fe21f0a7b 100644
--- a/eclass/cargo.eclass
+++ b/eclass/cargo.eclass
@@ -523,26 +523,23 @@ cargo_src_configure() {
 	[[ ${ECARGO_ARGS[@]} ]] && einfo "Configured with: ${ECARGO_ARGS[@]}"
 }
 
-# @FUNCTION: cargo_src_compile
+# @FUNCTION: cargo_env
+# @USAGE: Command with its arguments
 # @DESCRIPTION:
-# Build the package using cargo build.
-cargo_src_compile() {
-	debug-print-function ${FUNCNAME} "$@"
-
-	[[ ${_CARGO_GEN_CONFIG_HAS_RUN} ]] || \
-		die "FATAL: please call cargo_gen_config before using ${FUNCNAME}"
-
+# Run the given command under an environment needed for performing tasks with
+# Cargo such as building.
+cargo_env() {
 	filter-lto
 	tc-export AR CC CXX PKG_CONFIG
 
 	if tc-is-cross-compiler; then
-		export CARGO_BUILD_TARGET=$(rust_abi)
+		declare -x CARGO_BUILD_TARGET=$(rust_abi)
 		local TRIPLE=${CARGO_BUILD_TARGET//-/_}
-		export CARGO_TARGET_"${TRIPLE^^}"_LINKER=$(tc-getCC)
+		declare -x CARGO_TARGET_"${TRIPLE^^}"_LINKER=$(tc-getCC)
 
 		# Set vars for cc-rs crate.
 		tc-export_build_env
-		export \
+		declare -x \
 			HOST_AR=$(tc-getBUILD_AR)
 			HOST_CC=$(tc-getBUILD_CC)
 			HOST_CXX=$(tc-getBUILD_CXX)
@@ -550,9 +547,21 @@ cargo_src_compile() {
 			HOST_CXXFLAGS=${BUILD_CXXFLAGS}
 	fi
 
+	"${@}"
+}
+
+# @FUNCTION: cargo_src_compile
+# @DESCRIPTION:
+# Build the package using cargo build.
+cargo_src_compile() {
+	debug-print-function ${FUNCNAME} "$@"
+
+	[[ ${_CARGO_GEN_CONFIG_HAS_RUN} ]] || \
+		die "FATAL: please call cargo_gen_config before using ${FUNCNAME}"
+
 	set -- cargo build $(usex debug "" --release) ${ECARGO_ARGS[@]} "$@"
 	einfo "${@}"
-	"${@}" || die "cargo build failed"
+	cargo_env "${@}" || die "cargo build failed"
 }
 
 # @FUNCTION: cargo_src_install
@@ -573,7 +582,7 @@ cargo_src_install() {
 		$(usex debug --debug "") \
 		${ECARGO_ARGS[@]} "$@"
 	einfo "${@}"
-	"${@}" || die "cargo install failed"
+	cargo_env "${@}" || die "cargo install failed"
 
 	rm -f "${ED}/usr/.crates.toml" || die
 	rm -f "${ED}/usr/.crates2.json" || die
@@ -590,7 +599,7 @@ cargo_src_test() {
 
 	set -- cargo test $(usex debug "" --release) ${ECARGO_ARGS[@]} "$@"
 	einfo "${@}"
-	"${@}" || die "cargo test failed"
+	cargo_env "${@}" || die "cargo test failed"
 }
 
 fi
-- 
2.45.2

[gentoo-dev] [PATCH 2/3] cargo.eclass: Handle LDFLAGS and RUSTFLAGS better

[James Le Cuirot]

LDFLAGS are not currently honoured by Cargo builds at all. It would be
particularly advantageous to honour -fuse-ld because alternative linkers
like mold are known to be significantly faster at handling Rust.

As things stand, the eclass sets the linker to CC when cross-compiling,
but it does so erroneously due to a shell quoting issue. If CC includes
arguments, an error occurs when setting the CARGO_TARGET_*_LINKER
variable. Even with the right quoting, Cargo still fails because this
variable is not allowed to include arguments. They have to be specified
via RUSTFLAGS instead.

We would also like to configure the build host linker properly when
cross-compiling, but strangely there is no equivalent linker variable
for the build host. It can only be set via RUSTFLAGS. For consistency,
we now use RUSTFLAGS for the target host linker as well.

Some ebuilds already set RUSTFLAGS, so some consideration was given to
how to handle these. When set, Cargo prioritises RUSTFLAGS over
CARGO_BUILD_RUSTFLAGS and CARGO_TARGET_*_RUSTFLAGS, so we need it unset
to allow different flags for the build and target hosts. We can still
include its contents in the latter variables for convenience though.

It should not be necessary for ebuilds to figure out which Rust ABI is
applicable in order to set flags only for the target host, so the helper
reads from a simple CARGO_TARGET_RUSTFLAGS variable without the triple
for convenience.

Unfortunately, I have not yet encountered a package that makes use of
CARGO_BUILD_RUSTFLAGS while cross-compiling, but as far as I can tell,
it should work.

Signed-off-by: James Le Cuirot <chewi@gentoo.org>
---
 eclass/cargo.eclass | 47 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 34 insertions(+), 13 deletions(-)

diff --git a/eclass/cargo.eclass b/eclass/cargo.eclass
index b6d5fe21f0a7b..65eaee6f84e4b 100644
--- a/eclass/cargo.eclass
+++ b/eclass/cargo.eclass
@@ -527,27 +527,48 @@ cargo_src_configure() {
 # @USAGE: Command with its arguments
 # @DESCRIPTION:
 # Run the given command under an environment needed for performing tasks with
-# Cargo such as building.
+# Cargo such as building. RUSTFLAGS is used for both the build and target host.
+# CARGO_BUILD_RUSTFLAGS and CARGO_TARGET_RUSTFLAGS are used for just the build
+# host and target host respectively. Ensure these are set consistently between
+# Cargo invocations, otherwise rebuilds will occur.
 cargo_env() {
 	filter-lto
 	tc-export AR CC CXX PKG_CONFIG
 
+	# Set vars for cc-rs crate.
+	tc-export_build_env
+	declare -x \
+		HOST_AR=$(tc-getBUILD_AR)
+		HOST_CC=$(tc-getBUILD_CC)
+		HOST_CXX=$(tc-getBUILD_CXX)
+		HOST_CFLAGS=${BUILD_CFLAGS}
+		HOST_CXXFLAGS=${BUILD_CXXFLAGS}
+
+	# The default linker is "cc" so override by setting linker to CC in the
+	# RUSTFLAGS. The given linker cannot include any arguments, so split these
+	# into link-args along with LDFLAGS. Also include external RUSTFLAGS.
+	local LD_A=( ${HOST_CC} ${BUILD_LDFLAGS} )
+	declare -x CARGO_BUILD_RUSTFLAGS="-C linker=${LD_A[0]}"
+	[[ ${#LD_A[@]} -gt 1 ]] && declare CARGO_BUILD_RUSTFLAGS+="$(printf -- ' -C link-arg=%s' "${LD_A[@]:1}")"
+	declare CARGO_BUILD_RUSTFLAGS+=" ${RUSTFLAGS} ${CARGO_BUILD_RUSTFLAGS}"
+
 	if tc-is-cross-compiler; then
 		declare -x CARGO_BUILD_TARGET=$(rust_abi)
-		local TRIPLE=${CARGO_BUILD_TARGET//-/_}
-		declare -x CARGO_TARGET_"${TRIPLE^^}"_LINKER=$(tc-getCC)
-
-		# Set vars for cc-rs crate.
-		tc-export_build_env
-		declare -x \
-			HOST_AR=$(tc-getBUILD_AR)
-			HOST_CC=$(tc-getBUILD_CC)
-			HOST_CXX=$(tc-getBUILD_CXX)
-			HOST_CFLAGS=${BUILD_CFLAGS}
-			HOST_CXXFLAGS=${BUILD_CXXFLAGS}
+		local TRIPLE=${CARGO_BUILD_TARGET//-/_} LD_A=( $(tc-getCC) ${LDFLAGS} )
+		declare -x CARGO_TARGET_"${TRIPLE^^}"_RUSTFLAGS="-C linker=${LD_A[0]}"
+		[[ ${#LD_A[@]} -gt 1 ]] && declare CARGO_TARGET_"${TRIPLE^^}"_RUSTFLAGS+="$(printf -- ' -C link-arg=%s' "${LD_A[@]:1}")"
+		declare CARGO_TARGET_"${TRIPLE^^}"_RUSTFLAGS+=" ${RUSTFLAGS} ${CARGO_TARGET_RUSTFLAGS}"
+	else
+		CARGO_BUILD_RUSTFLAGS+=" ${CARGO_TARGET_RUSTFLAGS}"
 	fi
 
-	"${@}"
+	(
+		# Bare RUSTFLAGS will override the above, even if empty, so unset it
+		# locally. Do this in a subshell so that it remains set afterwards.
+		unset RUSTFLAGS
+
+		"${@}"
+	)
 }
 
 # @FUNCTION: cargo_src_compile
-- 
2.45.2

[gentoo-dev] [PATCH 3/3] cargo.eclass: Explicitly tell rustc not to strip binaries

[James Le Cuirot]

Most projects don't strip binaries in release mode by default, but there
are exceptions like app-misc/broot.

Signed-off-by: James Le Cuirot <chewi@gentoo.org>
---
 eclass/cargo.eclass | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/eclass/cargo.eclass b/eclass/cargo.eclass
index 65eaee6f84e4b..7ae40608aeb5a 100644
--- a/eclass/cargo.eclass
+++ b/eclass/cargo.eclass
@@ -550,14 +550,14 @@ cargo_env() {
 	local LD_A=( ${HOST_CC} ${BUILD_LDFLAGS} )
 	declare -x CARGO_BUILD_RUSTFLAGS="-C linker=${LD_A[0]}"
 	[[ ${#LD_A[@]} -gt 1 ]] && declare CARGO_BUILD_RUSTFLAGS+="$(printf -- ' -C link-arg=%s' "${LD_A[@]:1}")"
-	declare CARGO_BUILD_RUSTFLAGS+=" ${RUSTFLAGS} ${CARGO_BUILD_RUSTFLAGS}"
+	declare CARGO_BUILD_RUSTFLAGS+=" -C strip=none ${RUSTFLAGS} ${CARGO_BUILD_RUSTFLAGS}"
 
 	if tc-is-cross-compiler; then
 		declare -x CARGO_BUILD_TARGET=$(rust_abi)
 		local TRIPLE=${CARGO_BUILD_TARGET//-/_} LD_A=( $(tc-getCC) ${LDFLAGS} )
 		declare -x CARGO_TARGET_"${TRIPLE^^}"_RUSTFLAGS="-C linker=${LD_A[0]}"
 		[[ ${#LD_A[@]} -gt 1 ]] && declare CARGO_TARGET_"${TRIPLE^^}"_RUSTFLAGS+="$(printf -- ' -C link-arg=%s' "${LD_A[@]:1}")"
-		declare CARGO_TARGET_"${TRIPLE^^}"_RUSTFLAGS+=" ${RUSTFLAGS} ${CARGO_TARGET_RUSTFLAGS}"
+		declare CARGO_TARGET_"${TRIPLE^^}"_RUSTFLAGS+=" -C strip=none ${RUSTFLAGS} ${CARGO_TARGET_RUSTFLAGS}"
 	else
 		CARGO_BUILD_RUSTFLAGS+=" ${CARGO_TARGET_RUSTFLAGS}"
 	fi
-- 
2.45.2

Re: [gentoo-dev] [PATCH 1/3] cargo.eclass: Add cargo_env helper and use it in compile, test, install

[Eli Schwartz]

[-- Attachment #1.1: Type: text/plain, Size: 970 bytes --]

On 7/24/24 6:07 PM, James Le Cuirot wrote:
> Rust packages have a tendency to rebuild parts during test and install.
> It is not clear whether this can be addressed. We were therefore relying
> on some environment variables set during the compile phase for
> cross-compiling to work in the later phases. This is not ideal,
> especially if you need to build for multiple targets.


Oof, yeah, as I mentioned on Monday in -dev, I have a package that
actually produces meaningfully different programs if you rebuild it
during test.

dev-util/ruff will, when built with FEATURES=test, not show color when
you run it. Don't know why, feel a bit too freaked out about rebuilding
for tests *at all* to even think about fixing this in a "src_test
compatible way".

So I wonder if maybe we can somehow switch to e.g. doing the test phase
in a different directory such that it doesn't modify the artifacts we
actually want to install.


-- 
Eli Schwartz


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [gentoo-dev] [PATCH 1/3] cargo.eclass: Add cargo_env helper and use it in compile, test, install

[James Le Cuirot]

[-- Attachment #1: Type: text/plain, Size: 1310 bytes --]

On Wed, 2024-07-24 at 18:14 -0400, Eli Schwartz wrote:
> On 7/24/24 6:07 PM, James Le Cuirot wrote:
> > Rust packages have a tendency to rebuild parts during test and install.
> > It is not clear whether this can be addressed. We were therefore relying
> > on some environment variables set during the compile phase for
> > cross-compiling to work in the later phases. This is not ideal,
> > especially if you need to build for multiple targets.
> 
> 
> Oof, yeah, as I mentioned on Monday in -dev, I have a package that
> actually produces meaningfully different programs if you rebuild it
> during test.
> 
> dev-util/ruff will, when built with FEATURES=test, not show color when
> you run it. Don't know why, feel a bit too freaked out about rebuilding
> for tests *at all* to even think about fixing this in a "src_test
> compatible way".
> 
> So I wonder if maybe we can somehow switch to e.g. doing the test phase
> in a different directory such that it doesn't modify the artifacts we
> actually want to install.

It can happen due to files or environment variables changing. It sounds like
the tests inadvertently cause that in this case. You could copy the
$(cargo_target_dir) to $(cargo_target_dir)-test and then set CARGO_TARGET_DIR
to the latter before running the tests.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 858 bytes --]