#168 macros: pass CFLAGS/CXXFLAGS to the processes started by cargo
Merged 2 years ago by zbyszek. Opened 2 years ago by alebastr.
fedora-rust/ alebastr/rust2rpm cflags  into  main

file modified
+5
@@ -22,6 +22,11 @@ 

  rustdoc = "%{__rustdoc}"\

  rustflags = %{__global_rustflags_toml}\

  \

+ [env]\

+ CFLAGS = "%{build_cflags}"\

+ CXXFLAGS = "%{build_cxxflags}"\

+ LDFLAGS = "%{build_ldflags}"\

+ \

  [install]\

  root = "%{buildroot}%{_prefix}"\

  \

This is mainly targeted to the crates using rust-cc, where we want to apply system-wide compiler flags when invoking the compiler. cc supports CFLAGS and CXXFLAGS, but we never set those.
Defining that per project is not possible, as we have no way to inherit any build context from a dependency package.

Possibly also affects: bindgen, cxx, any other crate that invokes the C/C++ compiler and passes the CFLAGSS


You can't see the compiler command cc invokes, so verifying the effect of the change on an arbitrary rust pkg is actually hard. At least I found that %cargo_build -- -vv is able to pass through the output of build.rs, so I did verify that it fixes the flags for rhbz#2028895/rhbz#2028900.

I also confirmed that the variable exported before calling %cargo_build takes precedence over the [env] in a cargo config, so we are still able to override the values.

I did not test if that breaks anything. I guess creating a copr project and sending everything that depends on the aforementioned crates for rebuild would be sufficient; let me know if that is necessary.

Looks like this should work. (Unless the flags contain a quote mark, so let's just hope this doesn't happen ;)).

@decathorpe, @ignatenkobrain, opinions?

There actually is some quote macro in RPM for this purpose :)

I think this should be fine, but one should double check how I'd this really does what it is supposed to do.

Also it would be good to try to rebuild stuff and see what breaks. But that's more like nice to have, don't see a reason to block PR on this.

I'd like to at least make sure we don't break building "normal" Rust crates, crates that link to system shared libraries with 'cc', and crates that build C code (like ring)

@ignatenkobrain, if you mean %{quote:...} - it does something else. There's a new %{shescape:...} macro in the development branch of rpm, but it's not even in rawhide yet.
Anyways, there's so many places where we do various forms of CFLAGS="%{build_cflags}", that a quote mark there would break the whole distro :)

I did check a few crates, but will do more builds in copr later. (At this point I'm half convinced that we should always run cargo build with -vv, despite the increased noise level. Sadly, there's no config option for that).

Specifically for ring, compilation succeeds and the commands are below:
Before:

[ring 0.16.19] running "cc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-m64" "-I" "include" "-Wall" "-Wextra" "-std=c1x" "-Wbad-function-cast" "-Wnested-externs" "-Wstrict-prototypes" "-pedantic" "-pedantic-errors" "-Wall" "-Wextra" "-Wcast-align" "-Wcast-qual" "-Wconversion" "-Wenum-compare" "-Wfloat-equal" "-Wformat=2" "-Winline" "-Winvalid-pch" "-Wmissing-field-initializers" "-Wmissing-include-dirs" "-Wredundant-decls" "-Wshadow" "-Wsign-compare" "-Wsign-conversion" "-Wundef" "-Wuninitialized" "-Wwrite-strings" "-fno-strict-aliasing" "-fvisibility=hidden" "-fstack-protector" "-g3" "-DNDEBUG" "-c" "-o/builddir/build/BUILD/ring-0.16.19/target/release/build/ring-04010eb2b5b329d0/out/mem.o" "crypto/mem.c"

After:

[ring 0.16.19] running "cc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-m64" "-O2" "-flto=auto" "-ffat-lto-objects" "-fexceptions" "-g" "-grecord-gcc-switches" "-pipe" "-Wall" "-Werror=format-security" "-Wp,-D_FORTIFY_SOURCE=2" "-Wp,-D_GLIBCXX_ASSERTIONS" "-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1" "-fstack-protector-strong" "-specs=/usr/lib/rpm/redhat/redhat-annobin-cc1" "-m64" "-mtune=generic" "-fasynchronous-unwind-tables" "-fstack-clash-protection" "-fcf-protection" "-I" "include" "-std=c1x" "-Wbad-function-cast" "-Wnested-externs" "-Wstrict-prototypes" "-pedantic" "-pedantic-errors" "-Wall" "-Wextra" "-Wcast-align" "-Wcast-qual" "-Wconversion" "-Wenum-compare" "-Wfloat-equal" "-Wformat=2" "-Winline" "-Winvalid-pch" "-Wmissing-field-initializers" "-Wmissing-include-dirs" "-Wredundant-decls" "-Wshadow" "-Wsign-compare" "-Wsign-conversion" "-Wundef" "-Wuninitialized" "-Wwrite-strings" "-fno-strict-aliasing" "-fvisibility=hidden" "-fstack-protector" "-g3" "-DNDEBUG" "-c" "-o/builddir/build/BUILD/ring-0.16.19/target/release/build/ring-04010eb2b5b329d0/out/mem.o" "crypto/mem.c"

This change seems to be doing the "right thing" :tm: and doesn't break any of the tested packages, so :thumbsup: from me. It's probably "bad" that we didn't compile (Bo)ring(SSL) with the correct flags to begin with ... :see_no_evil:

Pull-Request has been merged by zbyszek

2 years ago
Metadata