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

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

[James Le Cuirot]

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)"
+}
+
 # @FUNCTION: cargo_src_unpack
 # @DESCRIPTION:
 # Unpacks the package and the cargo registry.
-- 
2.45.1

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

[Lucio Sauer]

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

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!

-- 
Lucio Sauer
>  # @FUNCTION: cargo_src_unpack
>  # @DESCRIPTION:
>  # Unpacks the package and the cargo registry.
> -- 
> 2.45.1
> 
> 

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

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

[James Le Cuirot]

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

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

[James Le Cuirot]

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..7463e1a0a600a 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}/$(usex debug debug release)"
+}
+
 # @FUNCTION: cargo_src_unpack
 # @DESCRIPTION:
 # Unpacks the package and the cargo registry.
--
2.45.1

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

[Florian Schmaus]

[-- Attachment #1.1.1: Type: text/plain, Size: 917 bytes --]

On 13/06/2024 17.03, 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.

Glad that someone is working on better cross-compilation support for 
rust. :)

That said, many rust ebuilds generate shell completions by invoking the 
just compiled binary in src_install(). This does not work out of the box 
when cross-compiling.

Is there anything we can or should do about it (besides binfmt_misc)? 
For example, asking the according projects to implement a mechanism to 
extract the completions files from the binary without running it, or 
have the build system generate the completion files as result of the build?

- Flow




[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 17797 bytes --]

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

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

[Lucio Sauer]

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

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

[Ionen Wolkens]

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

On Sat, Jun 15, 2024 at 08:14:34PM +0200, Florian Schmaus wrote:
> On 13/06/2024 17.03, 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.
> 
> Glad that someone is working on better cross-compilation support for 
> rust. :)
> 
> That said, many rust ebuilds generate shell completions by invoking the 
> just compiled binary in src_install(). This does not work out of the box 
> when cross-compiling.
> 
> Is there anything we can or should do about it (besides binfmt_misc)? 
> For example, asking the according projects to implement a mechanism to 
> extract the completions files from the binary without running it, or 
> have the build system generate the completion files as result of the build?

All I can think of without changing entirely how they're generated is:
1. (worst) either make a 2nd build using CBUILD (yeah no), or BDEPEND
   on same-version self with a USE only when cross compiling
2. (lazy) skip completions w/ ewarn if tc-is-cross-compiler, just so
   it at least won't break cross over completion files that the user
   may not even care much about
3. generate them yourself with each releases and ship in a tarball,
   works out for users but extra burden for maintainer over little
4. ask upstream to provide proper release tarballs if not already and
   include pre-generated completions/docs/etc... so it's otherwise
   only a problem with arbitrary git checkouts
(3+4 may possibly be more complicated if cargo features / USE change
 cli options and thus completion files)

For one of my ebuilds I went with better-than-nothing #2.
-- 
ionen

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

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

[James Le Cuirot]

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

On Sat, 2024-06-15 at 17:56 -0400, Ionen Wolkens wrote:
> On Sat, Jun 15, 2024 at 08:14:34PM +0200, Florian Schmaus wrote:
> > On 13/06/2024 17.03, 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.
> > 
> > Glad that someone is working on better cross-compilation support for 
> > rust. :)
> > 
> > That said, many rust ebuilds generate shell completions by invoking the 
> > just compiled binary in src_install(). This does not work out of the box 
> > when cross-compiling.
> > 
> > Is there anything we can or should do about it (besides binfmt_misc)? 
> > For example, asking the according projects to implement a mechanism to 
> > extract the completions files from the binary without running it, or 
> > have the build system generate the completion files as result of the build?
> 
> All I can think of without changing entirely how they're generated is:
> 1. (worst) either make a 2nd build using CBUILD (yeah no), or BDEPEND
>    on same-version self with a USE only when cross compiling
> 2. (lazy) skip completions w/ ewarn if tc-is-cross-compiler, just so
>    it at least won't break cross over completion files that the user
>    may not even care much about
> 3. generate them yourself with each releases and ship in a tarball,
>    works out for users but extra burden for maintainer over little
> 4. ask upstream to provide proper release tarballs if not already and
>    include pre-generated completions/docs/etc... so it's otherwise
>    only a problem with arbitrary git checkouts
> (3+4 may possibly be more complicated if cargo features / USE change
>  cli options and thus completion files)
> 
> For one of my ebuilds I went with better-than-nothing #2.

I've been doing this work for Flatcar, which doesn't use any of the packages
affected by this, but it does still concern me a little. Unfortunately, none
of the options are great.

I've seen #2 in one or two cases, and thanks for doing that.

For #1, the BDEPEND approach isn't possible with current Portage/PMS. The
experimental EAPI 5-hdepend, which led to BDEPEND in EAPI 7, had a USE flag
called targetroot (terrible name!) for this purpose. It wasn't based on
cross-compiling, because Portage/PMS don't actually have any concept of that.
It applied when ROOT!=/. We decided not to keep it though. Chances are that /
won't already have an identical or at least compatible version already
installed. Doing another temporary build was therefore deemed preferable. On
the plus side, that's quite easy with Cargo. On the minus side, almost all
Rust builds take ages. It hardly seems worth it for shell completions.

I really don't see #4 happening, given that most end-user applications are
normally installed using cargo install.

Regarding binfmt_misc, you could explicitly invoke via qemu-user instead,
making it less dependent on a particular setup. I have hacked together some
helpers to do stuff like this because we really need it for Perl. In Perl's
case, it would be mandatory when cross-compiling because there's really no
other way. I wouldn't want it to be mandatory in this case though.

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