| Message ID | 20220724214743.2324998-1-james.hilliard1@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2,1/1] package/pkg-cargo: ensure host/target rustflags are properly split | expand |
On 24/07/2022 23:47, 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 It's not clear to me *why* this should be changed. What issue is fixed by it? > 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. This part is also not entirely clear to me. Isn't that the same thing when cross-compiling (like it's the case in autotools or meson)? Or are non-specific RUSTFLAGS also applied to host builds when cross-compiling? > Move RUSTFLAGS for host builds to HOST_PKG_CARGO_ENV so that they > are set for host cargo builds which use python infrastructure. This makes sense. I therefore applied just this bit, the rest of the patch should get a bit more clarification. > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > --- > Changes v1 -> v2: > - add RUSTFLAGS to HOST_PKG_CARGO_ENV > --- > package/pkg-cargo.mk | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/package/pkg-cargo.mk b/package/pkg-cargo.mk > index f7e3f39503..1c105762f8 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 This is also not really clear. I think it is needed for the CARGO_HOST_RUSTFLAGS and CARGO_TARGET_xxx_RUSTFLAGS variables to take effect, is that correct? > +# > +# 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,11 +56,13 @@ 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" Kind of tangential to this change: would it make sense to have a $(PKG)_RUSTFLAGS? Regards, Arnout > endif > > HOST_PKG_CARGO_ENV = \ > - $(PKG_COMMON_CARGO_ENV) > + $(PKG_COMMON_CARGO_ENV) \ > + RUSTFLAGS="$(addprefix -C link-args=,$(HOST_LDFLAGS))" > > ################################################################################ > # inner-cargo-package -- defines how the configuration, compilation and > @@ -128,7 +135,6 @@ else # ifeq ($(4),target) > define $(2)_BUILD_CMDS > cd $$($$(PKG)_SRCDIR) && \ > $$(HOST_MAKE_ENV) \ > - RUSTFLAGS="$$(addprefix -C link-args=,$$(HOST_LDFLAGS))" \ > $$(HOST_CONFIGURE_OPTS) \ > $$(HOST_PKG_CARGO_ENV) \ > $$($(2)_CARGO_ENV) \ > @@ -169,7 +175,6 @@ ifndef $(2)_INSTALL_CMDS > define $(2)_INSTALL_CMDS > cd $$($$(PKG)_SRCDIR) && \ > $$(HOST_MAKE_ENV) \ > - RUSTFLAGS="$$(addprefix -C link-args=,$$(HOST_LDFLAGS))" \ > $$(HOST_CONFIGURE_OPTS) \ > $$(HOST_PKG_CARGO_ENV) \ > $$($(2)_CARGO_ENV) \
On Sun, Apr 9, 2023 at 1:38 PM Arnout Vandecappelle <arnout@mind.be> wrote: > > > > On 24/07/2022 23:47, 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 > > It's not clear to me *why* this should be changed. What issue is fixed by it? There's potentially an issue where target rustflags get passed to the host build script artifact build I think. > > > 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. > > This part is also not entirely clear to me. Isn't that the same thing when > cross-compiling (like it's the case in autotools or meson)? Or are non-specific > RUSTFLAGS also applied to host builds when cross-compiling? There's consistency issues in how rustflags get applied with build scripts, this approach should ensure rustflags are explicitly set for the correct host/target build artifacts. Some more details: https://github.com/rust-lang/cargo/pull/10395 > > > Move RUSTFLAGS for host builds to HOST_PKG_CARGO_ENV so that they > > are set for host cargo builds which use python infrastructure. > > This makes sense. I therefore applied just this bit, the rest of the patch > should get a bit more clarification. > > > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > > --- > > Changes v1 -> v2: > > - add RUSTFLAGS to HOST_PKG_CARGO_ENV > > --- > > package/pkg-cargo.mk | 15 ++++++++++----- > > 1 file changed, 10 insertions(+), 5 deletions(-) > > > > diff --git a/package/pkg-cargo.mk b/package/pkg-cargo.mk > > index f7e3f39503..1c105762f8 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 > > This is also not really clear. I think it is needed for the > CARGO_HOST_RUSTFLAGS and CARGO_TARGET_xxx_RUSTFLAGS variables to take effect, is > that correct? Needed to ungate anything under CARGO_HOST_xxx like CARGO_HOST_RUSTFLAGS AFAIU. > > > +# > > +# 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,11 +56,13 @@ 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" > > Kind of tangential to this change: would it make sense to have a $(PKG)_RUSTFLAGS? I'm not sure, I'm not aware of any specific issues needing package specific rustflags. > > > Regards, > Arnout > > > endif > > > > HOST_PKG_CARGO_ENV = \ > > - $(PKG_COMMON_CARGO_ENV) > > + $(PKG_COMMON_CARGO_ENV) \ > > + RUSTFLAGS="$(addprefix -C link-args=,$(HOST_LDFLAGS))" > > > > ################################################################################ > > # inner-cargo-package -- defines how the configuration, compilation and > > @@ -128,7 +135,6 @@ else # ifeq ($(4),target) > > define $(2)_BUILD_CMDS > > cd $$($$(PKG)_SRCDIR) && \ > > $$(HOST_MAKE_ENV) \ > > - RUSTFLAGS="$$(addprefix -C link-args=,$$(HOST_LDFLAGS))" \ > > $$(HOST_CONFIGURE_OPTS) \ > > $$(HOST_PKG_CARGO_ENV) \ > > $$($(2)_CARGO_ENV) \ > > @@ -169,7 +175,6 @@ ifndef $(2)_INSTALL_CMDS > > define $(2)_INSTALL_CMDS > > cd $$($$(PKG)_SRCDIR) && \ > > $$(HOST_MAKE_ENV) \ > > - RUSTFLAGS="$$(addprefix -C link-args=,$$(HOST_LDFLAGS))" \ > > $$(HOST_CONFIGURE_OPTS) \ > > $$(HOST_PKG_CARGO_ENV) \ > > $$($(2)_CARGO_ENV) \
Hi James, On 10/04/2023 10:43, James Hilliard wrote: > On Sun, Apr 9, 2023 at 1:38 PM Arnout Vandecappelle <arnout@mind.be> wrote: >> >> >> >> On 24/07/2022 23:47, 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 >> >> It's not clear to me *why* this should be changed. What issue is fixed by it? > > There's potentially an issue where target rustflags get passed to the > host build script artifact build I think. After an hour of additional investigation I think I understand the issue you're trying to solve. So I'm about to apply this, but I rewrote the commit message completely and I'd like you to review it before I push. package/pkg-cargo: ensure host/target rustflags are properly split In Cargo, it is quite typical for "build scripts" to be written in Rust and therefore they need to be compiled as part of the overall build. In cross-compilation, that means a mixed host and target build. Unfortunately, by default Cargo makes no distinction between the RUSTFLAGS used for the host and the target. There is, however, an unstable feature to make this distinction [1][2]. We already have CARGO_TARGET_APPLIES_TO_HOST="false". This makes sure that any configuration that we make for the target doesn't automatically apply to the host as well. However, this only applies for per-target configuration, for example the setting of "cc" in the config.toml generated by package/rust/rust.mk. Flags that are passed with RUSTFLAGS still apply to both host and target. Therefore, we need to use the CARGO_TARGET_<tuple>_RUSTFLAGS environment variable instead of plain RUSTFLAGS. This, however, doesn't allow us to specify flags that apply only to the host. We could use CARGO_TARGET_<hosttuple>_RUSTFLAGS for that, but that doesn't work in case the host and target tuple are the same. For this, we need another unstable feature, enabled with CARGO_UNSTABLE_HOST_CONFIG="true". With this enabled, we can specify flags that apply only for the host build using CARGO_HOST_RUSTFLAGS. Currently, we don't have any such flags, but we really should: we should pass the proper link flags to point to $(HOST_DIR)/lib. Therefore, add CARGO_HOST_RUSTFLAGS doing exactly that. [1] https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#host-config [2] https://github.com/rust-lang/cargo/pull/10395 It really should have been two separate commits: one that simply uses CARGO_TARGET_<tuple>_RUSTFLAGS for the one we already have, and another one that introduces CARGO_HOST_RUSTFLAGS and the unstable flag to enable it. But OK, not worth splitting it up now. Regards, Arnout > >> >>> 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. >> >> This part is also not entirely clear to me. Isn't that the same thing when >> cross-compiling (like it's the case in autotools or meson)? Or are non-specific >> RUSTFLAGS also applied to host builds when cross-compiling? > > There's consistency issues in how rustflags get applied with build scripts, this > approach should ensure rustflags are explicitly set for the correct host/target > build artifacts. > > Some more details: > https://github.com/rust-lang/cargo/pull/10395 > >> >>> Move RUSTFLAGS for host builds to HOST_PKG_CARGO_ENV so that they >>> are set for host cargo builds which use python infrastructure. >> >> This makes sense. I therefore applied just this bit, the rest of the patch >> should get a bit more clarification. >> >>> >>> Signed-off-by: James Hilliard <james.hilliard1@gmail.com> >>> --- >>> Changes v1 -> v2: >>> - add RUSTFLAGS to HOST_PKG_CARGO_ENV >>> --- >>> package/pkg-cargo.mk | 15 ++++++++++----- >>> 1 file changed, 10 insertions(+), 5 deletions(-) >>> >>> diff --git a/package/pkg-cargo.mk b/package/pkg-cargo.mk >>> index f7e3f39503..1c105762f8 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 >> >> This is also not really clear. I think it is needed for the >> CARGO_HOST_RUSTFLAGS and CARGO_TARGET_xxx_RUSTFLAGS variables to take effect, is >> that correct? > > Needed to ungate anything under CARGO_HOST_xxx like CARGO_HOST_RUSTFLAGS AFAIU. > >> >>> +# >>> +# 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,11 +56,13 @@ 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" >> >> Kind of tangential to this change: would it make sense to have a $(PKG)_RUSTFLAGS? > > I'm not sure, I'm not aware of any specific issues needing package specific > rustflags. > >> >> >> Regards, >> Arnout >> >>> endif >>> >>> HOST_PKG_CARGO_ENV = \ >>> - $(PKG_COMMON_CARGO_ENV) >>> + $(PKG_COMMON_CARGO_ENV) \ >>> + RUSTFLAGS="$(addprefix -C link-args=,$(HOST_LDFLAGS))" >>> >>> ################################################################################ >>> # inner-cargo-package -- defines how the configuration, compilation and >>> @@ -128,7 +135,6 @@ else # ifeq ($(4),target) >>> define $(2)_BUILD_CMDS >>> cd $$($$(PKG)_SRCDIR) && \ >>> $$(HOST_MAKE_ENV) \ >>> - RUSTFLAGS="$$(addprefix -C link-args=,$$(HOST_LDFLAGS))" \ >>> $$(HOST_CONFIGURE_OPTS) \ >>> $$(HOST_PKG_CARGO_ENV) \ >>> $$($(2)_CARGO_ENV) \ >>> @@ -169,7 +175,6 @@ ifndef $(2)_INSTALL_CMDS >>> define $(2)_INSTALL_CMDS >>> cd $$($$(PKG)_SRCDIR) && \ >>> $$(HOST_MAKE_ENV) \ >>> - RUSTFLAGS="$$(addprefix -C link-args=,$$(HOST_LDFLAGS))" \ >>> $$(HOST_CONFIGURE_OPTS) \ >>> $$(HOST_PKG_CARGO_ENV) \ >>> $$($(2)_CARGO_ENV) \
diff --git a/package/pkg-cargo.mk b/package/pkg-cargo.mk index f7e3f39503..1c105762f8 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,11 +56,13 @@ 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" endif HOST_PKG_CARGO_ENV = \ - $(PKG_COMMON_CARGO_ENV) + $(PKG_COMMON_CARGO_ENV) \ + RUSTFLAGS="$(addprefix -C link-args=,$(HOST_LDFLAGS))" ################################################################################ # inner-cargo-package -- defines how the configuration, compilation and @@ -128,7 +135,6 @@ else # ifeq ($(4),target) define $(2)_BUILD_CMDS cd $$($$(PKG)_SRCDIR) && \ $$(HOST_MAKE_ENV) \ - RUSTFLAGS="$$(addprefix -C link-args=,$$(HOST_LDFLAGS))" \ $$(HOST_CONFIGURE_OPTS) \ $$(HOST_PKG_CARGO_ENV) \ $$($(2)_CARGO_ENV) \ @@ -169,7 +175,6 @@ ifndef $(2)_INSTALL_CMDS define $(2)_INSTALL_CMDS cd $$($$(PKG)_SRCDIR) && \ $$(HOST_MAKE_ENV) \ - RUSTFLAGS="$$(addprefix -C link-args=,$$(HOST_LDFLAGS))" \ $$(HOST_CONFIGURE_OPTS) \ $$(HOST_PKG_CARGO_ENV) \ $$($(2)_CARGO_ENV) \
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. Move RUSTFLAGS for host builds to HOST_PKG_CARGO_ENV so that they are set for host cargo builds which use python infrastructure. Signed-off-by: James Hilliard <james.hilliard1@gmail.com> --- Changes v1 -> v2: - add RUSTFLAGS to HOST_PKG_CARGO_ENV --- package/pkg-cargo.mk | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)