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

[James Le Cuirot <chewi@gentoo.org>]

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

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? Perhaps the runner respects CARGO_BUILD_TARGET? 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?

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
        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: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 858 bytes --]