[Buildroot] [PATCH 1/1] package/pkg-cargo: ensure host/target rustflags are properly split

[James Hilliard]

On Sun, Jul 24, 2022 at 3:23 PM Arnout Vandecappelle <arnout at mind.be> wrote:
>
>
>
> On 24/07/2022 23:06, James Hilliard wrote:
> > On Sun, Jul 24, 2022 at 2:59 PM Arnout Vandecappelle <arnout at mind.be> wrote:
> >>
> >>
> >>
> >> On 24/07/2022 04:01, James Hilliard wrote:
> >>> Set HOST_LDFLAGS RUSTFLAGS via the host-config feature, see:
> >>> https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#host-config
> >>>
> >>> We have to enable this nightly feature first using:
> >>> CARGO_UNSTABLE_HOST_CONFIG="true"
> >>>
> >>> Separately set target RUSTFLAGS for the target triple specific env
> >>> variable.
> >>>
> >>> Signed-off-by: James Hilliard <james.hilliard1 at gmail.com>
> >>> ---
> >>>    package/pkg-cargo.mk | 11 ++++++++---
> >>>    1 file changed, 8 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/package/pkg-cargo.mk b/package/pkg-cargo.mk
> >>> index f7e3f39503..439d443b0d 100644
> >>> --- a/package/pkg-cargo.mk
> >>> +++ b/package/pkg-cargo.mk
> >>> @@ -32,7 +32,10 @@ PKG_COMMON_CARGO_ENV = \
> >>>    # using nighly features on stable releases, i.e features that are not
> >>>    # yet considered stable.
> >>>    #
> >>> -# CARGO_UNSTABLE_TARGET_APPLIES_TO_HOST="true" "enables the nightly
> >>> +# CARGO_UNSTABLE_HOST_CONFIG="true" enables the host specific
> >>> +# configuration feature
> >>> +#
> >>> +# CARGO_UNSTABLE_TARGET_APPLIES_TO_HOST="true" enables the nightly
> >>>    # configuration option target-applies-to-host value to be set
> >>>    #
> >>>    # CARGO_TARGET_APPLIES_TO_HOST="false" is actually setting the value
> >>> @@ -41,9 +44,11 @@ PKG_COMMON_CARGO_ENV = \
> >>>    PKG_CARGO_ENV = \
> >>>        $(PKG_COMMON_CARGO_ENV) \
> >>>        __CARGO_TEST_CHANNEL_OVERRIDE_DO_NOT_USE_THIS="nightly" \
> >>> +     CARGO_UNSTABLE_HOST_CONFIG="true" \
> >>>        CARGO_UNSTABLE_TARGET_APPLIES_TO_HOST="true" \
> >>>        CARGO_TARGET_APPLIES_TO_HOST="false" \
> >>>        CARGO_BUILD_TARGET="$(RUSTC_TARGET_NAME)" \
> >>> +     CARGO_HOST_RUSTFLAGS="$(addprefix -C link-args=,$(HOST_LDFLAGS))" \
> >>>        CARGO_TARGET_$(call UPPERCASE,$(RUSTC_TARGET_NAME))_LINKER=$(notdir $(TARGET_CROSS))gcc
> >>>
> >>>    #
> >>> @@ -51,7 +56,8 @@ PKG_CARGO_ENV = \
> >>>    # and should be removed when fixed upstream
> >>>    #
> >>>    ifeq ($(NORMALIZED_ARCH),arm)
> >>> -     PKG_CARGO_ENV += RUSTFLAGS="-Clink-arg=-Wl,--allow-multiple-definition"
> >>> +     PKG_CARGO_ENV += \
> >>> +             CARGO_TARGET_$(call UPPERCASE,$(RUSTC_TARGET_NAME))_RUSTFLAGS="-Clink-arg=-Wl,--allow-multiple-definition"
> >>
> >>    I don't understand why this is needed. We don't set RUSTFLAGS anywhere else,
> >> so it should be fine? Or is it because RUSTFLAGS is also used for native build
> >> even if CARGO_HOST_RUSTFLAGS is set?
> >
> > So cargo target builds tend to actually build artifacts for both the
> > host and the
> > target architecture, since cargo build-scripts are themselves written
> > in rust, thus
> > passing plain RUSTFLAGS has inconsistent behavior it seems in regards to
> > which artifact builds the RUSTFLAGS get applied to(seems to have changed
> > across cargo versions as well). Splitting them explicitly should be
> > more correct.
>
>   Ah, yes, that makes sense. However, this splitting wouldn't work at all if
> host target name == target target name (e.g. x86_64-unknown-linux-gnu) - in
> particular, we DO NOT want to pass HOST_LDFLAGS for linking stuff for the
> target. HOST_LDFLAGS contains the rpath pointing into HOST_DIR/lib.

Using CARGO_HOST_RUSTFLAGS should not pass the flags to the target build
even when host target name == target target name.

We also are setting CARGO_TARGET_APPLIES_TO_HOST="false" which should
split the flags even for cases where host target name == target target name.

>
> >
> >>
> >>    (Anyway, this hunk can probably be removed, the bug that triggered it is fixed.)
> >>
> >>>    endif
> >>>
> >>>    HOST_PKG_CARGO_ENV = \
> >>> @@ -128,7 +134,6 @@ else # ifeq ($(4),target)
> >>>    define $(2)_BUILD_CMDS
> >>>        cd $$($$(PKG)_SRCDIR) && \
> >>>        $$(HOST_MAKE_ENV) \
> >>> -             RUSTFLAGS="$$(addprefix -C link-args=,$$(HOST_LDFLAGS))" \
> >>
> >>    Why can this be removed? You added CARGO_HOST_RUSTFLAGS to the target build,
> >> but we still want HOST_LDFLAGS in the host package build, no?
> >
> > I think this works ok since it's not a cross build like the target
> > build. Target builds
> > generate both native/and target artifacts unlike host builds.
>
>   I don't think it will work (when linking against C code), because I don't see
> where else the rpath into HOST_DIR will be passed. At the moment we have only
> one package in that situation I think: host-sentry-cli.

Oh, I was moving this somewhere else but must have accidentally
dropped it instead,
will respin.

>
>
>   Regards,
>   Arnout
>
>
> >
> >>
> >>    Regards,
> >>    Arnout
> >>
> >>>                $$(HOST_CONFIGURE_OPTS) \
> >>>                $$(HOST_PKG_CARGO_ENV) \
> >>>                $$($(2)_CARGO_ENV) \