diff mbox series

[1/2] cargo.bbclass: show PACKAGECONFIG_CONFARGS in bbnote

Message ID 20250320165517.129442-1-martin.jansa@gmail.com
State Accepted, archived
Commit 38d953b2ffd4e0cee9e77f97988e44be105023c6
Headers show
Series [1/2] cargo.bbclass: show PACKAGECONFIG_CONFARGS in bbnote | expand

Commit Message

Martin Jansa March 20, 2025, 4:55 p.m. UTC 
From: Martin Jansa <martin.jansa@gmail.com>

* PACKAGECONFIG_CONFARGS was added in:
  https://git.openembedded.org/openembedded-core/commit/?id=16745b20452de60ae2474433cc1a2fb1ed9f6a64
  but it wasn't added in bbnote above which might lead to confusing errors like I got now:

  NOTE: cargo build -v --frozen --target aarch64-webos-linux-gnu --release --manifest-path=.../git//Cargo.toml
  error: unexpected argument '--cfg' found

  Usage: cargo build --verbose... --frozen --target [<TRIPLE>] --release --manifest-path <PATH>

  and was wondering where --cfg came from.

* it was from recipe where we already use:
  RUSTFLAGS:append = " ${PACKAGECONFIG_CONFARGS}"
  it will be difficult to use PACKAGECONFIG for RUSTFLAGS and prevent
  them to be used here for cargo as well, what about the recipes which
  need them to explicitly append them to CARGO_BUILD_FLAGS ?

Signed-off-by: Martin Jansa <martin.jansa@gmail.com>
---
 meta/classes-recipe/cargo.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Martin Jansa March 20, 2025, 4:59 p.m. UTC | #1 
This 2nd one was supposed to be sent only as RFC, but I forgot to
update subject sorry.

And this version also has a typo "= ?=", will send v2 if we want to do
something like this (reverting the
https://git.openembedded.org/openembedded-core/commit/?id=16745b20452de60ae2474433cc1a2fb1ed9f6a64
is imho better as it doesn't respect the original author from
https://patchwork.yoctoproject.org/project/oe-core/patch/20220606132653.60232-4-brgl@bgdev.pl/
and with CARGO_BUILD_FLAGS_PACKAGECONFIG it's imho more confusing than
useful as the default)

On Thu, Mar 20, 2025 at 5:55 PM Martin Jansa via
lists.openembedded.org <martin.jansa=gmail.com@lists.openembedded.org>
wrote:
>
> From: Martin Jansa <martin.jansa@gmail.com>
>
> * some recipes might be using PACKAGECONFIG_CONFARGS as:
>   RUSTFLAGS:append = " ${PACKAGECONFIG_CONFARGS}"
>
>   and then it's difficult to prevent the same args being used
>   here for cargo call as well add intermedate variable
>   CARGO_BUILD_FLAGS_PACKAGECONFIG
>   which could be set to empty in the recipe to avoid that.
>
> * alternatively we can just revert:
>   https://git.openembedded.org/openembedded-core/commit/?id=16745b20452de60ae2474433cc1a2fb1ed9f6a64
>   and let each recipe decide how it wants to use PACKAGECONFIG_CONFARGS
>
> Signed-off-by: Martin Jansa <martin.jansa@gmail.com>
> ---
>  meta/classes-recipe/cargo.bbclass | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/meta/classes-recipe/cargo.bbclass b/meta/classes-recipe/cargo.bbclass
> index 2dd28e95d3..63e550f5d0 100644
> --- a/meta/classes-recipe/cargo.bbclass
> +++ b/meta/classes-recipe/cargo.bbclass
> @@ -43,12 +43,14 @@ CARGO_BUILD_FLAGS = "-v --frozen --target ${RUST_HOST_SYS} ${BUILD_MODE} --manif
>  # change if CARGO_BUILD_FLAGS changes.
>  BUILD_DIR = "${@['release', 'debug'][d.getVar('DEBUG_BUILD') == '1']}"
>  CARGO_TARGET_SUBDIR = "${RUST_HOST_SYS}/${BUILD_DIR}"
> +CARGO_BUILD_FLAGS_PACKAGECONFIG = ?= "${PACKAGECONFIG_CONFARGS}"
> +CARGO_BUILD_FLAGS:append = " ${CARGO_BUILD_FLAGS_PACKAGECONFIG}"
>  oe_cargo_build () {
>         export RUSTFLAGS="${RUSTFLAGS}"
>         bbnote "Using rust targets from ${RUST_TARGET_PATH}"
>         bbnote "cargo = $(which ${CARGO})"
> -       bbnote "${CARGO} build ${CARGO_BUILD_FLAGS} ${PACKAGECONFIG_CONFARGS} $@"
> -       "${CARGO}" build ${CARGO_BUILD_FLAGS} ${PACKAGECONFIG_CONFARGS} "$@"
> +       bbnote "${CARGO} build ${CARGO_BUILD_FLAGS} $@"
> +       "${CARGO}" build ${CARGO_BUILD_FLAGS} "$@"
>  }
>
>  do_compile[progress] = "outof:\s+(\d+)/(\d+)"
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#213418): https://lists.openembedded.org/g/openembedded-core/message/213418
> Mute This Topic: https://lists.openembedded.org/mt/111812382/3617156
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [martin.jansa@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Richard Purdie March 20, 2025, 5:06 p.m. UTC | #2 
On Thu, 2025-03-20 at 17:59 +0100, Martin Jansa via lists.openembedded.org wrote:
> This 2nd one was supposed to be sent only as RFC, but I forgot to
> update subject sorry.
> 
> And this version also has a typo "= ?=", will send v2 if we want to do
> something like this (reverting the
> https://git.openembedded.org/openembedded-core/commit/?id=16745b20452de60ae2474433cc1a2fb1ed9f6a64
> is imho better as it doesn't respect the original author from
> https://patchwork.yoctoproject.org/project/oe-core/patch/20220606132653.60232-4-brgl@bgdev.pl/
> and with CARGO_BUILD_FLAGS_PACKAGECONFIG it's imho more confusing than
> useful as the default)

I have a strong preference for not carrying things like this. If there
are existing recipes that break, we should just fix them and move on
rather than letting the past dictate the future IMO.

I'm trying really hard to simplify things where we can.

Cheers,

Richard
Richard Purdie March 20, 2025, 5:16 p.m. UTC | #3 
On Thu, 2025-03-20 at 17:06 +0000, Richard Purdie via
lists.openembedded.org wrote:
> On Thu, 2025-03-20 at 17:59 +0100, Martin Jansa via
> lists.openembedded.org wrote:
> > This 2nd one was supposed to be sent only as RFC, but I forgot to
> > update subject sorry.
> > 
> > And this version also has a typo "= ?=", will send v2 if we want to
> > do
> > something like this (reverting the
> > https://git.openembedded.org/openembedded-core/commit/?id=16745b20452de60ae2474433cc1a2fb1ed9f6a64
> > is imho better as it doesn't respect the original author from
> > https://patchwork.yoctoproject.org/project/oe-core/patch/20220606132653.60232-4-brgl@bgdev.pl/
> > and with CARGO_BUILD_FLAGS_PACKAGECONFIG it's imho more confusing
> > than
> > useful as the default)
> 
> I have a strong preference for not carrying things like this. If
> there
> are existing recipes that break, we should just fix them and move on
> rather than letting the past dictate the future IMO.
> 
> I'm trying really hard to simplify things where we can.

Martin pointed out I was misreading things.

If there are recipes using packageconfig in RUSTFLAGS then this will
break that. We probably do need to revert as I really don't like the
alternative variable mess. It does mean recipes will specifically need
to add the flags to the carbo build parameters but that is the lesser
of several evils IMO.

Cheers,

Richard
Martin Jansa March 20, 2025, 5:36 p.m. UTC | #4 
On Thu, Mar 20, 2025 at 6:16 PM Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
>
> On Thu, 2025-03-20 at 17:06 +0000, Richard Purdie via
> lists.openembedded.org wrote:
> > On Thu, 2025-03-20 at 17:59 +0100, Martin Jansa via
> > lists.openembedded.org wrote:
> > > This 2nd one was supposed to be sent only as RFC, but I forgot to
> > > update subject sorry.
> > >
> > > And this version also has a typo "= ?=", will send v2 if we want to
> > > do
> > > something like this (reverting the
> > > https://git.openembedded.org/openembedded-core/commit/?id=16745b20452de60ae2474433cc1a2fb1ed9f6a64
> > > is imho better as it doesn't respect the original author from
> > > https://patchwork.yoctoproject.org/project/oe-core/patch/20220606132653.60232-4-brgl@bgdev.pl/
> > > and with CARGO_BUILD_FLAGS_PACKAGECONFIG it's imho more confusing
> > > than
> > > useful as the default)
> >
> > I have a strong preference for not carrying things like this. If
> > there
> > are existing recipes that break, we should just fix them and move on
> > rather than letting the past dictate the future IMO.
> >
> > I'm trying really hard to simplify things where we can.
>
> Martin pointed out I was misreading things.
>
> If there are recipes using packageconfig in RUSTFLAGS then this will
> break that. We probably do need to revert as I really don't like the
> alternative variable mess. It does mean recipes will specifically need
> to add the flags to the carbo build parameters but that is the lesser
> of several evils IMO.

Agreed about the revert being lesser evil.

I know very little about rust/cargo, so it's also possible that we're
using rust/cargo wrong and there is a better way to achieve
conditional compilation with flags passed to cargo not by RUSTFLAGS.

I've checked the recipe where we're using:
RUSTFLAGS:append = " ${PACKAGECONFIG_CONFARGS}"
and it's public as:
https://github.com/webosose/meta-webosose/blob/master/meta-webos/recipes-multimedia/gstreamer/gstreamer1.0-plugins-webosrs.bb

With the RUSTFLAGS then being used as
PACKAGECONFIG[unifieddecodebin] = '--cfg feature=\"unifieddecodebin\"'
in the recipe and in code e.g.
https://github.com/webosose/gst-plugins-webos/blob/%40rust/gst-rs/unifiedbin/src/lib.rs
as:
#[cfg(feature = "unifieddecodebin")]
...

Will send revert unless Jean-Pierre or Bartosz have better idea how to
deal with this.

Cheers,
Jean-Pierre Geslin March 20, 2025, 5:51 p.m. UTC | #5 
Hello all,

First good catch Martin!
Regarding cargo usage it's better to use feature flag (--feature) as RUSTFLAGS is passed at each compiler invocation (you risk to have a conflict with another feature in other crate dependency compilation and enable it without wanting that).

I'm really sorry if I credited the author incorrectly, I'd made the same fix before looking to see if it had already been proposed.

I think both approaches are respectable, but the revert will keep Rust recipes apart forever, which may not be the most unified IMHO.
Jean-Pierre Geslin March 20, 2025, 6:11 p.m. UTC | #6 
In my recipe I use it as:

CARGO_BUILD_FLAGS:append = " ${PACKAGECONFIG_CONFARGS}"
The recipe is public here:
https://github.com/Jarsop/meta-zenoh/blob/master/meta-zenoh/recipes-connectivity/zenoh/zenoh_git.bb

Best regards

Le jeu. 20 mars 2025 à 18:36, Martin Jansa <martin.jansa@gmail.com> a
écrit :

> On Thu, Mar 20, 2025 at 6:16 PM Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> >
> > On Thu, 2025-03-20 at 17:06 +0000, Richard Purdie via
> > lists.openembedded.org wrote:
> > > On Thu, 2025-03-20 at 17:59 +0100, Martin Jansa via
> > > lists.openembedded.org wrote:
> > > > This 2nd one was supposed to be sent only as RFC, but I forgot to
> > > > update subject sorry.
> > > >
> > > > And this version also has a typo "= ?=", will send v2 if we want to
> > > > do
> > > > something like this (reverting the
> > > >
> https://git.openembedded.org/openembedded-core/commit/?id=16745b20452de60ae2474433cc1a2fb1ed9f6a64
> > > > is imho better as it doesn't respect the original author from
> > > >
> https://patchwork.yoctoproject.org/project/oe-core/patch/20220606132653.60232-4-brgl@bgdev.pl/
> > > > and with CARGO_BUILD_FLAGS_PACKAGECONFIG it's imho more confusing
> > > > than
> > > > useful as the default)
> > >
> > > I have a strong preference for not carrying things like this. If
> > > there
> > > are existing recipes that break, we should just fix them and move on
> > > rather than letting the past dictate the future IMO.
> > >
> > > I'm trying really hard to simplify things where we can.
> >
> > Martin pointed out I was misreading things.
> >
> > If there are recipes using packageconfig in RUSTFLAGS then this will
> > break that. We probably do need to revert as I really don't like the
> > alternative variable mess. It does mean recipes will specifically need
> > to add the flags to the carbo build parameters but that is the lesser
> > of several evils IMO.
>
> Agreed about the revert being lesser evil.
>
> I know very little about rust/cargo, so it's also possible that we're
> using rust/cargo wrong and there is a better way to achieve
> conditional compilation with flags passed to cargo not by RUSTFLAGS.
>
> I've checked the recipe where we're using:
> RUSTFLAGS:append = " ${PACKAGECONFIG_CONFARGS}"
> and it's public as:
>
> https://github.com/webosose/meta-webosose/blob/master/meta-webos/recipes-multimedia/gstreamer/gstreamer1.0-plugins-webosrs.bb
>
> With the RUSTFLAGS then being used as
> PACKAGECONFIG[unifieddecodebin] = '--cfg feature=\"unifieddecodebin\"'
> in the recipe and in code e.g.
>
> https://github.com/webosose/gst-plugins-webos/blob/%40rust/gst-rs/unifiedbin/src/lib.rs
> as:
> #[cfg(feature = "unifieddecodebin")]
> ...
>
> Will send revert unless Jean-Pierre or Bartosz have better idea how to
> deal with this.
>
> Cheers,
>
Alexander Kanavin March 24, 2025, 6:55 p.m. UTC | #7 
I basically agree with Jean-Pierre. Optional features should not be
set via RUSTFLAGS at all, rather Cargo.toml in the source tree should
be extended to include them as described here:

https://doc.rust-lang.org/cargo/reference/features.html

So gst-plugins-webos should be fixed, then the recipe for it, then I'd
say we should reconsider this patch. If a recipe inherits cargo, then
it is expected to pass configuration settings via cargo, otherwise it
needs to use its own custom class.

Alex

On Thu, 20 Mar 2025 at 19:11, Jean-Pierre Geslin via
lists.openembedded.org <jarsoper=gmail.com@lists.openembedded.org>
wrote:
>
> In my recipe I use it as:
>
> CARGO_BUILD_FLAGS:append = " ${PACKAGECONFIG_CONFARGS}"
> The recipe is public here: https://github.com/Jarsop/meta-zenoh/blob/master/meta-zenoh/recipes-connectivity/zenoh/zenoh_git.bb
>
> Best regards
>
> Le jeu. 20 mars 2025 à 18:36, Martin Jansa <martin.jansa@gmail.com> a écrit :
>>
>> On Thu, Mar 20, 2025 at 6:16 PM Richard Purdie
>> <richard.purdie@linuxfoundation.org> wrote:
>> >
>> > On Thu, 2025-03-20 at 17:06 +0000, Richard Purdie via
>> > lists.openembedded.org wrote:
>> > > On Thu, 2025-03-20 at 17:59 +0100, Martin Jansa via
>> > > lists.openembedded.org wrote:
>> > > > This 2nd one was supposed to be sent only as RFC, but I forgot to
>> > > > update subject sorry.
>> > > >
>> > > > And this version also has a typo "= ?=", will send v2 if we want to
>> > > > do
>> > > > something like this (reverting the
>> > > > https://git.openembedded.org/openembedded-core/commit/?id=16745b20452de60ae2474433cc1a2fb1ed9f6a64
>> > > > is imho better as it doesn't respect the original author from
>> > > > https://patchwork.yoctoproject.org/project/oe-core/patch/20220606132653.60232-4-brgl@bgdev.pl/
>> > > > and with CARGO_BUILD_FLAGS_PACKAGECONFIG it's imho more confusing
>> > > > than
>> > > > useful as the default)
>> > >
>> > > I have a strong preference for not carrying things like this. If
>> > > there
>> > > are existing recipes that break, we should just fix them and move on
>> > > rather than letting the past dictate the future IMO.
>> > >
>> > > I'm trying really hard to simplify things where we can.
>> >
>> > Martin pointed out I was misreading things.
>> >
>> > If there are recipes using packageconfig in RUSTFLAGS then this will
>> > break that. We probably do need to revert as I really don't like the
>> > alternative variable mess. It does mean recipes will specifically need
>> > to add the flags to the carbo build parameters but that is the lesser
>> > of several evils IMO.
>>
>> Agreed about the revert being lesser evil.
>>
>> I know very little about rust/cargo, so it's also possible that we're
>> using rust/cargo wrong and there is a better way to achieve
>> conditional compilation with flags passed to cargo not by RUSTFLAGS.
>>
>> I've checked the recipe where we're using:
>> RUSTFLAGS:append = " ${PACKAGECONFIG_CONFARGS}"
>> and it's public as:
>> https://github.com/webosose/meta-webosose/blob/master/meta-webos/recipes-multimedia/gstreamer/gstreamer1.0-plugins-webosrs.bb
>>
>> With the RUSTFLAGS then being used as
>> PACKAGECONFIG[unifieddecodebin] = '--cfg feature=\"unifieddecodebin\"'
>> in the recipe and in code e.g.
>> https://github.com/webosose/gst-plugins-webos/blob/%40rust/gst-rs/unifiedbin/src/lib.rs
>> as:
>> #[cfg(feature = "unifieddecodebin")]
>> ...
>>
>> Will send revert unless Jean-Pierre or Bartosz have better idea how to
>> deal with this.
>>
>> Cheers,
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#213425): https://lists.openembedded.org/g/openembedded-core/message/213425
> Mute This Topic: https://lists.openembedded.org/mt/111812482/1686489
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kanavin@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Martin Jansa March 24, 2025, 7:30 p.m. UTC | #8 
I had to patch Cargo.toml in gst-plugins-webos to define all possible
options in [features] to avoid "error: the package
'gst-plugin-unifiedbin' does not contain these features:
unifieddecodebin, .." and then I was able to change our PACKAGECONFIG
to pass them with --features instead of --cfg feature and use them
with cargo as the previous patch did. I didn't verify it in runtime
yet, but it passed the build.

If that's the recommended option, then I'm fine with that and only 1/2
should be merged and not this 2/2 nor the revert RP sent earlier
today.

Regards,

On Mon, Mar 24, 2025 at 7:55 PM Alexander Kanavin
<alex.kanavin@gmail.com> wrote:
>
> I basically agree with Jean-Pierre. Optional features should not be
> set via RUSTFLAGS at all, rather Cargo.toml in the source tree should
> be extended to include them as described here:
>
> https://doc.rust-lang.org/cargo/reference/features.html
>
> So gst-plugins-webos should be fixed, then the recipe for it, then I'd
> say we should reconsider this patch. If a recipe inherits cargo, then
> it is expected to pass configuration settings via cargo, otherwise it
> needs to use its own custom class.
>
> Alex
>
> On Thu, 20 Mar 2025 at 19:11, Jean-Pierre Geslin via
> lists.openembedded.org <jarsoper=gmail.com@lists.openembedded.org>
> wrote:
> >
> > In my recipe I use it as:
> >
> > CARGO_BUILD_FLAGS:append = " ${PACKAGECONFIG_CONFARGS}"
> > The recipe is public here: https://github.com/Jarsop/meta-zenoh/blob/master/meta-zenoh/recipes-connectivity/zenoh/zenoh_git.bb
> >
> > Best regards
> >
> > Le jeu. 20 mars 2025 à 18:36, Martin Jansa <martin.jansa@gmail.com> a écrit :
> >>
> >> On Thu, Mar 20, 2025 at 6:16 PM Richard Purdie
> >> <richard.purdie@linuxfoundation.org> wrote:
> >> >
> >> > On Thu, 2025-03-20 at 17:06 +0000, Richard Purdie via
> >> > lists.openembedded.org wrote:
> >> > > On Thu, 2025-03-20 at 17:59 +0100, Martin Jansa via
> >> > > lists.openembedded.org wrote:
> >> > > > This 2nd one was supposed to be sent only as RFC, but I forgot to
> >> > > > update subject sorry.
> >> > > >
> >> > > > And this version also has a typo "= ?=", will send v2 if we want to
> >> > > > do
> >> > > > something like this (reverting the
> >> > > > https://git.openembedded.org/openembedded-core/commit/?id=16745b20452de60ae2474433cc1a2fb1ed9f6a64
> >> > > > is imho better as it doesn't respect the original author from
> >> > > > https://patchwork.yoctoproject.org/project/oe-core/patch/20220606132653.60232-4-brgl@bgdev.pl/
> >> > > > and with CARGO_BUILD_FLAGS_PACKAGECONFIG it's imho more confusing
> >> > > > than
> >> > > > useful as the default)
> >> > >
> >> > > I have a strong preference for not carrying things like this. If
> >> > > there
> >> > > are existing recipes that break, we should just fix them and move on
> >> > > rather than letting the past dictate the future IMO.
> >> > >
> >> > > I'm trying really hard to simplify things where we can.
> >> >
> >> > Martin pointed out I was misreading things.
> >> >
> >> > If there are recipes using packageconfig in RUSTFLAGS then this will
> >> > break that. We probably do need to revert as I really don't like the
> >> > alternative variable mess. It does mean recipes will specifically need
> >> > to add the flags to the carbo build parameters but that is the lesser
> >> > of several evils IMO.
> >>
> >> Agreed about the revert being lesser evil.
> >>
> >> I know very little about rust/cargo, so it's also possible that we're
> >> using rust/cargo wrong and there is a better way to achieve
> >> conditional compilation with flags passed to cargo not by RUSTFLAGS.
> >>
> >> I've checked the recipe where we're using:
> >> RUSTFLAGS:append = " ${PACKAGECONFIG_CONFARGS}"
> >> and it's public as:
> >> https://github.com/webosose/meta-webosose/blob/master/meta-webos/recipes-multimedia/gstreamer/gstreamer1.0-plugins-webosrs.bb
> >>
> >> With the RUSTFLAGS then being used as
> >> PACKAGECONFIG[unifieddecodebin] = '--cfg feature=\"unifieddecodebin\"'
> >> in the recipe and in code e.g.
> >> https://github.com/webosose/gst-plugins-webos/blob/%40rust/gst-rs/unifiedbin/src/lib.rs
> >> as:
> >> #[cfg(feature = "unifieddecodebin")]
> >> ...
> >>
> >> Will send revert unless Jean-Pierre or Bartosz have better idea how to
> >> deal with this.
> >>
> >> Cheers,
> >
> >
> > -=-=-=-=-=-=-=-=-=-=-=-
> > Links: You receive all messages sent to this group.
> > View/Reply Online (#213425): https://lists.openembedded.org/g/openembedded-core/message/213425
> > Mute This Topic: https://lists.openembedded.org/mt/111812482/1686489
> > Group Owner: openembedded-core+owner@lists.openembedded.org
> > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kanavin@gmail.com]
> > -=-=-=-=-=-=-=-=-=-=-=-
> >
Richard Purdie March 24, 2025, 10:29 p.m. UTC | #9 
On Mon, 2025-03-24 at 20:30 +0100, Martin Jansa wrote:
> I had to patch Cargo.toml in gst-plugins-webos to define all possible
> options in [features] to avoid "error: the package
> 'gst-plugin-unifiedbin' does not contain these features:
> unifieddecodebin, .." and then I was able to change our PACKAGECONFIG
> to pass them with --features instead of --cfg feature and use them
> with cargo as the previous patch did. I didn't verify it in runtime
> yet, but it passed the build.
> 
> If that's the recommended option, then I'm fine with that and only
> 1/2
> should be merged and not this 2/2 nor the revert RP sent earlier
> today.

If we all agree then I can drop the revert and add 1/2 back and merge
that...

Cheers,

Richard
akuster808 March 31, 2025, 4:18 p.m. UTC | #10 
Hello,

In meta-security,  suricata is  mixed C and Rust  build  so I use 
PACKAGECONFIG for the autoconf part and now the RUST portion is 
complaining about configuration options it does not understand.

Rust error:
error: unexpected argument '--with-libcap_ng-includes' found

reverting the commits fixes my issue.

BR,
Armin

On 3/24/25 3:30 PM, Martin Jansa via lists.openembedded.org wrote:
> I had to patch Cargo.toml in gst-plugins-webos to define all possible
> options in [features] to avoid "error: the packagekkkk
> 'gst-plugin-unifiedbin' does not contain these features:
> unifieddecodebin, .." and then I was able to change our PACKAGECONFIG
> to pass them with --features instead of --cfg feature and use them
> with cargo as the previous patch did. I didn't verify it in runtime
> yet, but it passed the build.
>
> If that's the recommended option, then I'm fine with that and only 1/2
> should be merged and not this 2/2 nor the revert RP sent earlier
> today.
>
> Regards,
>
> On Mon, Mar 24, 2025 at 7:55 PM Alexander Kanavin
> <alex.kanavin@gmail.com> wrote:
>> I basically agree with Jean-Pierre. Optional features should not be
>> set via RUSTFLAGS at all, rather Cargo.toml in the source tree should
>> be extended to include them as described here:
>>
>> https://doc.rust-lang.org/cargo/reference/features.html
>>
>> So gst-plugins-webos should be fixed, then the recipe for it, then I'd
>> say we should reconsider this patch. If a recipe inherits cargo, then
>> it is expected to pass configuration settings via cargo, otherwise it
>> needs to use its own custom class.
>>
>> Alex
>>
>> On Thu, 20 Mar 2025 at 19:11, Jean-Pierre Geslin via
>> lists.openembedded.org <jarsoper=gmail.com@lists.openembedded.org>
>> wrote:
>>> In my recipe I use it as:
>>>
>>> CARGO_BUILD_FLAGS:append = " ${PACKAGECONFIG_CONFARGS}"
>>> The recipe is public here: https://github.com/Jarsop/meta-zenoh/blob/master/meta-zenoh/recipes-connectivity/zenoh/zenoh_git.bb
>>>
>>> Best regards
>>>
>>> Le jeu. 20 mars 2025 à 18:36, Martin Jansa <martin.jansa@gmail.com> a écrit :
>>>> On Thu, Mar 20, 2025 at 6:16 PM Richard Purdie
>>>> <richard.purdie@linuxfoundation.org> wrote:
>>>>> On Thu, 2025-03-20 at 17:06 +0000, Richard Purdie via
>>>>> lists.openembedded.org wrote:
>>>>>> On Thu, 2025-03-20 at 17:59 +0100, Martin Jansa via
>>>>>> lists.openembedded.org wrote:
>>>>>>> This 2nd one was supposed to be sent only as RFC, but I forgot to
>>>>>>> update subject sorry.
>>>>>>>
>>>>>>> And this version also has a typo "= ?=", will send v2 if we want to
>>>>>>> do
>>>>>>> something like this (reverting the
>>>>>>> https://git.openembedded.org/openembedded-core/commit/?id=16745b20452de60ae2474433cc1a2fb1ed9f6a64
>>>>>>> is imho better as it doesn't respect the original author from
>>>>>>> https://patchwork.yoctoproject.org/project/oe-core/patch/20220606132653.60232-4-brgl@bgdev.pl/
>>>>>>> and with CARGO_BUILD_FLAGS_PACKAGECONFIG it's imho more confusing
>>>>>>> than
>>>>>>> useful as the default)
>>>>>> I have a strong preference for not carrying things like this. If
>>>>>> there
>>>>>> are existing recipes that break, we should just fix them and move on
>>>>>> rather than letting the past dictate the future IMO.
>>>>>>
>>>>>> I'm trying really hard to simplify things where we can.
>>>>> Martin pointed out I was misreading things.
>>>>>
>>>>> If there are recipes using packageconfig in RUSTFLAGS then this will
>>>>> break that. We probably do need to revert as I really don't like the
>>>>> alternative variable mess. It does mean recipes will specifically need
>>>>> to add the flags to the carbo build parameters but that is the lesser
>>>>> of several evils IMO.
>>>> Agreed about the revert being lesser evil.
>>>>
>>>> I know very little about rust/cargo, so it's also possible that we're
>>>> using rust/cargo wrong and there is a better way to achieve
>>>> conditional compilation with flags passed to cargo not by RUSTFLAGS.
>>>>
>>>> I've checked the recipe where we're using:
>>>> RUSTFLAGS:append = " ${PACKAGECONFIG_CONFARGS}"
>>>> and it's public as:
>>>> https://github.com/webosose/meta-webosose/blob/master/meta-webos/recipes-multimedia/gstreamer/gstreamer1.0-plugins-webosrs.bb
>>>>
>>>> With the RUSTFLAGS then being used as
>>>> PACKAGECONFIG[unifieddecodebin] = '--cfg feature=\"unifieddecodebin\"'
>>>> in the recipe and in code e.g.
>>>> https://github.com/webosose/gst-plugins-webos/blob/%40rust/gst-rs/unifiedbin/src/lib.rs
>>>> as:
>>>> #[cfg(feature = "unifieddecodebin")]
>>>> ...
>>>>
>>>> Will send revert unless Jean-Pierre or Bartosz have better idea how to
>>>> deal with this.
>>>>
>>>> Cheers,
>>>
>>>
>>>
>>>
>>> -=-=-=-=-=-=-=-=-=-=-=-
>>> Links: You receive all messages sent to this group.
>>> View/Reply Online (#213571): https://lists.openembedded.org/g/openembedded-core/message/213571
>>> Mute This Topic: https://lists.openembedded.org/mt/111812482/3616698
>>> Group Owner: openembedded-core+owner@lists.openembedded.org
>>> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [akuster808@gmail.com]
>>> -=-=-=-=-=-=-=-=-=-=-=-
>>>
Alexander Kanavin March 31, 2025, 4:28 p.m. UTC | #11 
On Mon, 31 Mar 2025 at 18:18, akuster808 <akuster808@gmail.com> wrote:

> In meta-security,  suricata is  mixed C and Rust  build  so I use
> PACKAGECONFIG for the autoconf part and now the RUST portion is
> complaining about configuration options it does not understand.
>
> Rust error:
> error: unexpected argument '--with-libcap_ng-includes' found
>
> reverting the commits fixes my issue.

The recipe inherits both autotools and cargo, and then overwrites
configure(), compile() and install() with its own versions that
manually run the rust bits:

https://git.yoctoproject.org/meta-security/tree/recipes-ids/suricata/suricata_7.0.0.bb?h=master

It needs to be fixed to use cargo_common class, similar to librsvg.
Inheriting cargo class means cargo (and not something else) is used as
the top build system.

Alex
Anton Antonov May 16, 2025, 3:07 p.m. UTC | #12 
Hello,

This change also breaks recipes where cargo is used as the top build system but PACKAGECONFIG_CONFARGS should not be included into cargo parameters. See, for example:
https://git.yoctoproject.org/meta-security/tree/meta-parsec/recipes-parsec/parsec-service/parsec-service_1.4.1.bb

What is the reason for this change? If a recipe needs to use PACKAGECONFIG for cargo parameters then the recipe can update CARGO_BUILD_FLAGS accordingly.
With this change I don't have an option to not include  PACKAGECONFIG_CONFARGS into parameters. Can it be reverted?

Anton
Alexander Kanavin May 16, 2025, 3:55 p.m. UTC | #13 
On Fri, 16 May 2025 at 17:07, Anton Antonov via lists.openembedded.org
<anton.antonov=arm.com@lists.openembedded.org> wrote:
>    This change also breaks recipes where cargo is used as the top build system but PACKAGECONFIG_CONFARGS should not be included into cargo parameters. See, for example:
> https://git.yoctoproject.org/meta-security/tree/meta-parsec/recipes-parsec/parsec-service/parsec-service_1.4.1.bb
>
>   What is the reason for this change? If a recipe needs to use PACKAGECONFIG for cargo parameters then the recipe can update CARGO_BUILD_FLAGS accordingly.
> With this change I don't have an option to not include  PACKAGECONFIG_CONFARGS into parameters. Can it be reverted?

The reason is that the standard usage of PACKAGECONFIG definitions is
to define and directly supply arguments to the top level build tool.
This is how autotools, cmake, meson and other classes work, and this
simply makes cargo class consistent them. If you need to supply these
arguments some other way, then you can follow for example mesa.inc and
define empty PACKAGECONFIGS:

PACKAGECONFIG[amd] = ""
PACKAGECONFIG[svga] = ""
PACKAGECONFIG[virgl] = ""

And then use them to form a custom variable, which you can use as you see fit:

GALLIUMDRIVERS:append = "${@bb.utils.contains('PACKAGECONFIG',
'gallium-llvm', '${GALLIUMDRIVERS_LLVM}', '', d)}"
GALLIUMDRIVERS:append = "${@bb.utils.contains('PACKAGECONFIG', 'amd',
',r600', '', d)}"
GALLIUMDRIVERS:append = "${@bb.utils.contains('PACKAGECONFIG',
'virgl', ',virgl', '', d)}"


Alex
Anton Antonov May 16, 2025, 4:39 p.m. UTC | #14 
On Fri, May 16, 2025 at 08:55 AM, Alexander Kanavin wrote:

> 
> If you need to supply these
> arguments some other way, then you can follow for example mesa.inc and
> define empty PACKAGECONFIGS:
> 
> PACKAGECONFIG[amd] = ""
> PACKAGECONFIG[svga] = ""
> PACKAGECONFIG[virgl] = ""
> 
> And then use them to form a custom variable, which you can use as you see
> fit:
> 
> GALLIUMDRIVERS:append = "${@bb.utils.contains('PACKAGECONFIG',
> 'gallium-llvm', '${GALLIUMDRIVERS_LLVM}', '', d)}"
> GALLIUMDRIVERS:append = "${@bb.utils.contains('PACKAGECONFIG', 'amd',
> ',r600', '', d)}"

I think using PACKAGECONFIG to define cargo features list is a normal use-case in rust based recipes. With this change is not easily achievable because ${PACKAGECONFIG_CONFARGS} is unquoted in cargo.bbclass and we can't use it with CARGO_BUILD_FLAGS += " --features" if there are more than one feature.
So, if the change is not going to be reverted then we realistically have two options - either complicate the recipes with a lot of conditions like you suggest or just add a comment that oe_cargo_build() is broken in cargo.bbclass and define our own do_compile()

Anton
Alexander Kanavin May 16, 2025, 5:19 p.m. UTC | #15 
Can you put —features directly into packageconfig definitions? Does cargo
accept multiple —features flags?

Alex

On Fri 16. May 2025 at 18.39, Anton Antonov via lists.openembedded.org
<anton.antonov=arm.com@lists.openembedded.org> wrote:

> On Fri, May 16, 2025 at 08:55 AM, Alexander Kanavin wrote:
>
> If you need to supply these
> arguments some other way, then you can follow for example mesa.inc and
> define empty PACKAGECONFIGS:
>
> PACKAGECONFIG[amd] = ""
> PACKAGECONFIG[svga] = ""
> PACKAGECONFIG[virgl] = ""
>
> And then use them to form a custom variable, which you can use as you see
> fit:
>
> GALLIUMDRIVERS:append = "${@bb.utils.contains('PACKAGECONFIG',
> 'gallium-llvm', '${GALLIUMDRIVERS_LLVM}', '', d)}"
> GALLIUMDRIVERS:append = "${@bb.utils.contains('PACKAGECONFIG', 'amd',
> ',r600', '', d)}"
>
>   I think using PACKAGECONFIG to define cargo features list is a normal
> use-case in rust based recipes. With this change is not easily achievable
> because ${PACKAGECONFIG_CONFARGS} is unquoted in cargo.bbclass and we can't
> use it with CARGO_BUILD_FLAGS += " --features" if there are more than one
> feature.
>   So, if the change is not going to be reverted then we realistically have
> two options - either complicate the recipes with a lot of conditions like
> you suggest or just add a comment that oe_cargo_build() is broken in
> cargo.bbclass and define our own do_compile()
>
> Anton
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#216759):
> https://lists.openembedded.org/g/openembedded-core/message/216759
> Mute This Topic: https://lists.openembedded.org/mt/111812482/1686489
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> alex.kanavin@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
Marko, Peter May 16, 2025, 5:43 p.m. UTC | #16 
Yes, you can have multiple --features statements in cargo command line.
We use multiple packageconfigs in our cargo recipes successfully which then looks something like:
PACKAGECONFIG ?= “feature1 feature2”
PACKAGECONFIG[feature1] = "--features feature1,"
PACKAGECONFIG[feature2] = "--features feature2,"
PACKAGECONFIG[feature3] = "--features feature3,"

Peter

From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Alexander Kanavin via lists.openembedded.org
Sent: Friday, May 16, 2025 19:19
To: anton.antonov@arm.com
Cc: openembedded-core@lists.openembedded.org
Subject: Re: [OE-core] [PATCH 2/2] cargo.bbclass: allow to avoid appending PACKAGECONFIG_CONFARGS

Can you put —features directly into packageconfig definitions? Does cargo accept multiple —features flags?

Alex

On Fri 16. May 2025 at 18.39, Anton Antonov via lists.openembedded.org<http://lists.openembedded.org> <anton.antonov=arm.com@lists.openembedded.org<mailto:arm.com@lists.openembedded.org>> wrote:
On Fri, May 16, 2025 at 08:55 AM, Alexander Kanavin wrote:
If you need to supply these
arguments some other way, then you can follow for example mesa.inc and
define empty PACKAGECONFIGS:

PACKAGECONFIG[amd] = ""
PACKAGECONFIG[svga] = ""
PACKAGECONFIG[virgl] = ""

And then use them to form a custom variable, which you can use as you see fit:

GALLIUMDRIVERS:append = "${@bb.utils.contains('PACKAGECONFIG',
'gallium-llvm', '${GALLIUMDRIVERS_LLVM}', '', d)}<mailto:$%7b@bb.utils.contains('PACKAGECONFIG',%0b'gallium-llvm',%20'$%7bGALLIUMDRIVERS_LLVM%7d',%20'',%20d)%7d>"
GALLIUMDRIVERS:append = "${@bb.utils.contains('PACKAGECONFIG', 'amd',
',r600', '', d)}<mailto:$%7b@bb.utils.contains('PACKAGECONFIG',%20'amd',%0b',r600',%20'',%20d)%7d>"
  I think using PACKAGECONFIG to define cargo features list is a normal use-case in rust based recipes. With this change is not easily achievable because ${PACKAGECONFIG_CONFARGS} is unquoted in cargo.bbclass and we can't use it with CARGO_BUILD_FLAGS += " --features" if there are more than one feature.
  So, if the change is not going to be reverted then we realistically have two options - either complicate the recipes with a lot of conditions like you suggest or just add a comment that oe_cargo_build() is broken in cargo.bbclass and define our own do_compile()
Anton
Anton Antonov May 16, 2025, 5:47 p.m. UTC | #17 
On Fri, May 16, 2025 at 10:19 AM, Alexander Kanavin wrote:

> 
> Can you put —features directly into packageconfig definitions? Does cargo
> accept multiple —features flags?

Oh, yes it works. Thanks. The only small problem is I can't use comma-separated features in  PACKAGECONFIG because it itself is comma-separated. So I have to add --features before each feature in the same line. It doesn't look bad if I use -F instead of --features though:

PACKAGECONFIG ??= "PKCS11 MBED-CRYPTO"
have_TPM = "${@bb.utils.contains('DISTRO_FEATURES', 'tpm2', 'TPM', '', d)}"
PACKAGECONFIG:append = " ${@bb.utils.contains('BBFILE_COLLECTIONS', 'tpm-layer', '${have_TPM}', '', d)}"

PACKAGECONFIG[ALL] = "-F all-providers -F cryptoki/generate-bindings -F tss-esapi/generate-bindings,,tpm2-tss libts,tpm2-tss libtss2-tcti-device libts"
PACKAGECONFIG[TPM] = "-F tpm-provider -F tss-esapi/generate-bindings,,tpm2-tss,tpm2-tss libtss2-tcti-device"
PACKAGECONFIG[PKCS11] = "-F pkcs11-provider -F cryptoki/generate-bindings,"
PACKAGECONFIG[MBED-CRYPTO] = "-F mbed-crypto-provider,"
PACKAGECONFIG[CRYPTOAUTHLIB] = "-F cryptoauthlib-provider,"
PACKAGECONFIG[TS] = "-F trusted-service-provider,,libts,libts"

Anton
diff mbox series

Patch

diff --git a/meta/classes-recipe/cargo.bbclass b/meta/classes-recipe/cargo.bbclass
index 461d100dd9..2dd28e95d3 100644
--- a/meta/classes-recipe/cargo.bbclass
+++ b/meta/classes-recipe/cargo.bbclass
@@ -47,7 +47,7 @@  oe_cargo_build () {
 	export RUSTFLAGS="${RUSTFLAGS}"
 	bbnote "Using rust targets from ${RUST_TARGET_PATH}"
 	bbnote "cargo = $(which ${CARGO})"
-	bbnote "${CARGO} build ${CARGO_BUILD_FLAGS} $@"
+	bbnote "${CARGO} build ${CARGO_BUILD_FLAGS} ${PACKAGECONFIG_CONFARGS} $@"
 	"${CARGO}" build ${CARGO_BUILD_FLAGS} ${PACKAGECONFIG_CONFARGS} "$@"
 }