Re: [gentoo-dev] [PATCH] cargo.eclass: Add cargo_target_dir helper function

[Lucio Sauer <watermanpaint@posteo.net>]

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

On Fri, Jun 14, 2024 at 02:54:47PM +0100, James Le Cuirot wrote:
> On Thu, 2024-06-13 at 21:32 +0000, Lucio Sauer wrote:
> > On Thu, Jun 13, 2024 at 04:03:44PM +0100, James Le Cuirot wrote:
> > > Several Cargo-based ebuilds cannot use cargo_src_install for various
> > > reasons and manually install binaries from within the target directory
> > > instead. It is common to see `target/$(usex debug debug release)`, but
> > > this lacks the target ABI when cross-compiling, so provide a helper
> > > function.
> > > 
> > > There are some multilib Cargo-based ebuilds that always set the target
> > > ABI, even when not cross-compiling. It would be simpler to do this in
> > > general, so once ebuilds have been updated to use this new helper, I
> > > might change the eclass again accordingly.
> > > 
> > > Signed-off-by: James Le Cuirot <chewi@gentoo.org>
> > > ---
> > >  eclass/cargo.eclass | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > > 
> > > diff --git a/eclass/cargo.eclass b/eclass/cargo.eclass
> > > index 40d98211ce7f9..2036fb635d569 100644
> > > --- a/eclass/cargo.eclass
> > > +++ b/eclass/cargo.eclass
> > > @@ -319,6 +319,16 @@ _cargo_gen_git_config() {
> > >  	fi
> > >  }
> > >  
> > > +# @FUNCTION: cargo_target_dir
> > > +# @DESCRIPTION:
> > > +# Return the directory within target that contains the build, e.g.
> > > +# target/aarch64-unknown-linux-gnu/release.
> > > +cargo_target_dir() {
> > > +	local abi
> > > +	tc-is-cross-compiler && abi=$(rust_abi)
> > > +	echo "${CARGO_TARGET_DIR:-target}${abi+/}${abi}/$(usex debug debug release)"
> > > +}
> > > +
> > How about giving this function an optional parameter that gets passed to
> > rust_abi so that you can get the build directory for targets other than
> > CHOST?
> > 
> > What initially motivated this suggestion is a quirk in app-misc/anki's
> > build system together with your recent change to cargo.eclass 375da3684,
> > which enables cross-compilation by setting environment variables.
> > Here, I first build the 'build system runner' for CBUILD, which then
> > compiles the actual package for CHOST. I think I can avoid explicitly
> > setting --target $(rust ${CBUILD}) for the runner for now, but it would
> > be handy to easily get the build directory for CBUILD in the future!
> 
> I did consider that, but for the sake of simplicity, I think I'd prefer just
> overriding CHOST when you need to do this. There are already similar examples
> of this.
> 
> I had a look at app-misc/anki. I think you do need to give --target to the
> runner right now if that's building something, otherwise how would it know
> what the target is?
Cargo builds for the "host architecture" (CBUILD, right?) if no target
is supplied.  Since I currently compile the runner before ever calling
cargo_src_compile, CARGO_BUILD_TARGET="$(rust_abi) has not been exported
yet and Cargo falls back to compiling for CBUILD. Because this is not
future-proof and my current approach is lacking, I'll switch to
cargo_src_compile with the upcoming PR.

> Perhaps the runner respects CARGO_BUILD_TARGET?
Yes, it does.

> If so, I
> was thinking of making a cargo_env wrapper helper that would set that and
> other variables in the context of the given command. This would be used in
> cargo_src_compile, as well as cargo_src_install and cargo_src_test in case any
> rebuilding occurs. It could also be used for other custom scenarios like this
> one. What do you think?
Sounds like a good idea. Ebuild writers will be able to specify the
platform that cargo_env is supposed to set environment variables for,
right?

> 
> diff --git a/app-misc/anki/anki-24.04.1.ebuild b/app-misc/anki/anki-24.04.1.ebuild
> index 54dfbf94f31b2..2764659ccbf51 100644
> --- a/app-misc/anki/anki-24.04.1.ebuild
> +++ b/app-misc/anki/anki-24.04.1.ebuild
> @@ -871,9 +871,9 @@ src_compile() {
>         #    * generate the ninja file and run ninja afterwards
>         #    * create the Python wheel files in "${S}"/out/wheels
>  
> -       cargo build --release --package runner || die
> +       tc-env_build cargo_src_compile --package runner
Thank you for bringing this to my attention. I forgot that I need to set
cross vars for cc-rs for the runner to compile in a cross-compilation
scenario.

-- 
Lucio Sauer
>         if use gui; then
> -               out/rust/release/runner build -- $(get_NINJAOPTS) wheels  || die
> +               cargo_env "$(CHOST=${CBUILD} cargo_target_dir)/runner" build -- $(get_NINJAOPTS) wheels || die
>         else
>                 cargo_src_compile --package anki-sync-server
>         fi
> @@ -901,7 +901,7 @@ src_test() {
>                 "${S}"/build/ninja_gen/src/cargo.rs || die
>  
>         for runner in pytest rust_test jest; do
> -               out/rust/release/runner build -- $(get_NINJAOPTS) check:$runner  || \
> +               cargo_env "$(CHOST=${CBUILD} cargo_target_dir)/runner" build -- $(get_NINJAOPTS) check:$runner || \
>                 die "check:$runner failed!"
>         done
>  }

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 618 bytes --]