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

[gentoo-dev] [PATCH 1/5 v3] 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/5 v3] 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, 33 insertions(+), 14 deletions(-)

diff --git a/eclass/cargo.eclass b/eclass/cargo.eclass
index b6d5fe21f0a7b..596598ca8585a 100644
--- a/eclass/cargo.eclass
+++ b/eclass/cargo.eclass
@@ -527,27 +527,46 @@ 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
+	local -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} )
+	local -x CARGO_BUILD_RUSTFLAGS="${RUSTFLAGS} ${CARGO_BUILD_RUSTFLAGS} -C linker=${LD_A[0]}"
+	[[ ${#LD_A[@]} -gt 1 ]] && local CARGO_BUILD_RUSTFLAGS+="$(printf -- ' -C link-arg=%s' "${LD_A[@]:1}")"
+
 	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 -x CARGO_BUILD_TARGET=$(rust_abi)
+		local TRIPLE=${CARGO_BUILD_TARGET//-/_} LD_A=( $(tc-getCC) ${LDFLAGS} )
+		local -x CARGO_TARGET_"${TRIPLE^^}"_RUSTFLAGS="${RUSTFLAGS} ${CARGO_TARGET_RUSTFLAGS} -C linker=${LD_A[0]}"
+		[[ ${#LD_A[@]} -gt 1 ]] && local CARGO_TARGET_"${TRIPLE^^}"_RUSTFLAGS+="$(printf -- ' -C link-arg=%s' "${LD_A[@]:1}")"
+	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/5 v3] 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 596598ca8585a..44d3f7ee31f59 100644
--- a/eclass/cargo.eclass
+++ b/eclass/cargo.eclass
@@ -548,13 +548,13 @@ cargo_env() {
 	# 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} )
-	local -x CARGO_BUILD_RUSTFLAGS="${RUSTFLAGS} ${CARGO_BUILD_RUSTFLAGS} -C linker=${LD_A[0]}"
+	local -x CARGO_BUILD_RUSTFLAGS="-C strip=none ${RUSTFLAGS} ${CARGO_BUILD_RUSTFLAGS} -C linker=${LD_A[0]}"
 	[[ ${#LD_A[@]} -gt 1 ]] && local CARGO_BUILD_RUSTFLAGS+="$(printf -- ' -C link-arg=%s' "${LD_A[@]:1}")"

 	if tc-is-cross-compiler; then
 		local -x CARGO_BUILD_TARGET=$(rust_abi)
 		local TRIPLE=${CARGO_BUILD_TARGET//-/_} LD_A=( $(tc-getCC) ${LDFLAGS} )
-		local -x CARGO_TARGET_"${TRIPLE^^}"_RUSTFLAGS="${RUSTFLAGS} ${CARGO_TARGET_RUSTFLAGS} -C linker=${LD_A[0]}"
+		local -x CARGO_TARGET_"${TRIPLE^^}"_RUSTFLAGS="-C strip=none ${RUSTFLAGS} ${CARGO_TARGET_RUSTFLAGS} -C linker=${LD_A[0]}"
 		[[ ${#LD_A[@]} -gt 1 ]] && local CARGO_TARGET_"${TRIPLE^^}"_RUSTFLAGS+="$(printf -- ' -C link-arg=%s' "${LD_A[@]:1}")"
 	else
 		CARGO_BUILD_RUSTFLAGS+=" ${CARGO_TARGET_RUSTFLAGS}"
--
2.45.2

[gentoo-dev] [PATCH 4/5 v3] cargo.eclass: Shadow flag variables so that LTO filtering remains local

[James Le Cuirot]

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

diff --git a/eclass/cargo.eclass b/eclass/cargo.eclass
index 44d3f7ee31f59..9f0bffee0e048 100644
--- a/eclass/cargo.eclass
+++ b/eclass/cargo.eclass
@@ -532,7 +532,16 @@ cargo_src_configure() {
 # host and target host respectively. Ensure these are set consistently between
 # Cargo invocations, otherwise rebuilds will occur.
 cargo_env() {
+	# Shadow flag variables so that filtering below remains local.
+	local flag
+	for flag in $(all-flag-vars); do
+		local -x "${flag}=${!flag}"
+	done
+
+	# Rust extensions are incompatible with C/C++ LTO compiler see e.g.
+	# https://bugs.gentoo.org/910220
 	filter-lto
+
 	tc-export AR CC CXX PKG_CONFIG

 	# Set vars for cc-rs crate.
--
2.45.2

[gentoo-dev] [PATCH 5/5 v3] distutils-r1.eclass: Use cargo_env when appropriate for flag handling

[James Le Cuirot]

cargo_env handles linker flags and enables cross-compiling. It also
handles LTO filtering, so we can remove that from this eclass.

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

diff --git a/eclass/distutils-r1.eclass b/eclass/distutils-r1.eclass
index fa8edb5cdfb77..743242b2acee8 100644
--- a/eclass/distutils-r1.eclass
+++ b/eclass/distutils-r1.eclass
@@ -1251,7 +1251,9 @@ distutils_pep517_install() {
 		die "mydistutilsargs are banned in PEP517 mode (use DISTUTILS_ARGS)"
 	fi

-	local config_settings=
+	local cmd=() config_settings=
+	has cargo ${INHERITED} && cmd+=( cargo_env )
+
 	case ${DISTUTILS_USE_PEP517} in
 		maturin)
 			# `maturin pep517 build-wheel --help` for options
@@ -1390,7 +1392,7 @@ distutils_pep517_install() {

 	local build_backend=$(_distutils-r1_get_backend)
 	einfo "  Building the wheel for ${PWD#${WORKDIR}/} via ${build_backend}"
-	local cmd=(
+	cmd+=(
 		"${EPYTHON}" -m gpep517 build-wheel
 			--prefix="${EPREFIX}/usr"
 			--backend "${build_backend}"
@@ -1792,16 +1794,6 @@ distutils-r1_run_phase() {
 		# bug fixes from Cython (this works only when setup.py is using
 		# cythonize() but it's better than nothing)
 		local -x CYTHON_FORCE_REGEN=1
-
-		# Rust extensions are incompatible with C/C++ LTO compiler
-		# see e.g. https://bugs.gentoo.org/910220
-		if has cargo ${INHERITED}; then
-			local x
-			for x in $(all-flag-vars); do
-				local -x "${x}=${!x}"
-			done
-			filter-lto
-		fi
 	fi

 	# silence warnings when pydevd is loaded on Python 3.11+
--
2.45.2

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

[Sam James]

James Le Cuirot <chewi@gentoo.org> writes:

> 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.

The series looks OK but please let Ionen cast an eye over it as well as
mgorny, espec. for distutils-r1.

(Also, remember to CC eclass maints!)

>
> 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

Re: [gentoo-dev] [PATCH 4/5 v3] cargo.eclass: Shadow flag variables so that LTO filtering remains local

[Sam James]

James Le Cuirot <chewi@gentoo.org> writes:

> Signed-off-by: James Le Cuirot <chewi@gentoo.org>
> ---
>  eclass/cargo.eclass | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/eclass/cargo.eclass b/eclass/cargo.eclass
> index 44d3f7ee31f59..9f0bffee0e048 100644
> --- a/eclass/cargo.eclass
> +++ b/eclass/cargo.eclass
> @@ -532,7 +532,16 @@ cargo_src_configure() {
>  # host and target host respectively. Ensure these are set consistently between
>  # Cargo invocations, otherwise rebuilds will occur.
>  cargo_env() {
> +	# Shadow flag variables so that filtering below remains local.
> +	local flag
> +	for flag in $(all-flag-vars); do
> +		local -x "${flag}=${!flag}"
> +	done
> +
> +	# Rust extensions are incompatible with C/C++ LTO compiler see e.g.
> +	# https://bugs.gentoo.org/910220
>  	filter-lto
> +

Maybe mention in the commit message that this is what we do for
distutils-r1 too.