Add test to use decoder and disable LTO flags when these are detected

[DoumanAsh]

No description provided.

[UMCEKO]

actually, i tried this locally and i don't think it works — cmake.cflag("-fno-lto") puts the flag before the inherited CFLAGS, so the resulting cmake invocation looks like:

-DCMAKE_C_FLAGS= -fno-lto -ffunction-sections -fdata-sections -fPIC -m64 -w ... -flto=auto

gcc takes the last -flto* flag, so -flto=auto wins and LTO stays on. quick check:

$ gcc -c -fno-lto -O2 -flto=auto -o t1.o t.c && readelf -SW t1.o | grep -c '\.gnu\.lto'
15
$ gcc -c -O2 -flto=auto -fno-lto -o t2.o t.c && readelf -SW t2.o | grep -c '\.gnu\.lto'
0

so the .gnu.lto_* sections (and the __gnu_lto_slim marker) come back, and rust-lld would still fail.

same issue applies to my own PR — i didn't catch it because i validated by appending -fno-lto to the CFLAGS env myself instead of running through opusic-sys's build.rs. my mistake.

a working fix would need to either filter -flto* out of the inherited CFLAGS env before invoking cmake, or pass -fno-lto via something that lands after env CFLAGS in CMAKE_C_FLAGS. happy to push a follow-up if you'd like — most direct shape is something like:

if let Ok(cflags) = std::env::var("CFLAGS") {
    if cflags.contains("-flto") {
        let filtered: String = cflags
            .split_whitespace()
            .filter(|f| !f.starts_with("-flto"))
            .collect::<Vec<_>>()
            .join(" ");
        cmake.env("CFLAGS", filtered);
    }
}

will verify properly this time before saying it works.

[DoumanAsh]

Ah I see... I would expect Cmake::cflag to append it to the end, but it is good to know that this function is buggy
Then manually setting CFLAGS is the best option

[DoumanAsh]

Looking at cmake crate, they just set CMAKE_C_FLAGS
https://github.com/rust-lang/cmake-rs/blob/main/src/lib.rs#L753

So I think if we do the same it will also fail, so we probably need to override CFLAGS env itself

[UMCEKO]

verified end-to-end this time using the original failing reproducer (kopuz on Arch with makepkg CFLAGS="... -flto=auto"):

1. PR #22 as-is — backported the exact logic into the registry's opusic-sys 0.5.8 build.rs, did a clean rebuild. Build still fails:

rust-lld: error: undefined symbol: opus_decoder_destroy
rust-lld: error: undefined symbol: opus_decode
rust-lld: error: undefined symbol: opus_decoder_create

The cmake invocation that gets emitted:

-DCMAKE_C_FLAGS= -fno-lto -ffunction-sections -fdata-sections -fPIC -m64 -w \
  -march=native -O3 -pipe -fno-plt -fexceptions ... -flto=auto

-fno-lto from cmake.cflag() lands before the inherited CFLAGS, so gcc's last-wins puts it back to -flto=auto. The CMAKE_INTERPROCEDURAL_OPTIMIZATION=off define doesn't help here either — it stops cmake from adding its own LTO flag, but doesn't strip one already in CFLAGS.

2. cmake.env("CFLAGS", filtered) — also doesn't help. cmake-rs reads CFLAGS at config-time and bakes it into -DCMAKE_C_FLAGS=...; setting an env var on the cmake child process is too late.

3. Filter -flto* out of the env before cmake-rs reads it — works. Patched build.rs:

for var in ["CFLAGS", "CXXFLAGS"] {
    if let Ok(value) = std::env::var(var) {
        if value.contains("-flto") {
            let filtered: String = value
                .split_whitespace()
                .filter(|f| !f.starts_with("-flto"))
                .collect::<Vec<_>>()
                .join(" ");
            unsafe { std::env::set_var(var, filtered); }
        }
    }
}
let mut cmake = cmake::Config::new(CURRENT_DIR);

After the fix:

• cmake invocation has no -flto in CMAKE_C_FLAGS
• readelf -SW opus_decoder.c.o | grep -c '\.gnu\.lto' → 0 (no slim LTO sections)
• kopuz binary links cleanly, contains opus_decoder_create at a real address

set_var is unsafe in 2024 edition (fine in 2018/2021); inside a build script it's single-threaded and safe in practice. The starts_with("-flto") filter covers -flto, -flto=auto, -flto=thin, -flto-partition=..., etc.

happy to push this as a follow-up commit on top of #22 if you'd like — apologies for the noise on the first round, should've actually reproduced before commenting.

[DoumanAsh]

2. cmake.env("CFLAGS", filtered) — also doesn't help. cmake-rs reads CFLAGS at config-time and bakes it into -DCMAKE_C_FLAGS=...; setting an env var on the cmake child process is too late.

This looks like bug in cmake crate
I looked at it source code, they only pass env to build command, not configure...
I will make PR there later

[DoumanAsh]

@UMCEKO I adopted your approach, thanks for testing. Please confirm everything works on your system with clean build and I will publish new version
f0d2946

[DoumanAsh]

@UMCEKO Sorry, Actually I just noticed one thing...
https://github.com/rust-lang/cmake-rs/blob/main/src/lib.rs#L353

We can override configure options via separate option 😓
Do you want to try it instead of using set_var?

Here is updated solution:

//Disable LTO if someone tries to force it (e.g. Arch makepkg)
//This is necessary because cmake crate doesn't pass env variables at configure step
fn fix_build_env(cmake: &mut cmake::Config) {
    for (var, cmake_var) in [("CFLAGS", "CMAKE_C_FLAGS"), ("CXXFLAGS", "CMAKE_CXX_FLAGS")] {
        if let Ok(value) = std::env::var(var) {
            if value.contains("-flto") {
                println!("cargo:warning=env::{var} contains LTO option. Overriding it...");
                let filtered: String = value
                    .split_whitespace()
                    .filter(|f| !f.starts_with("-flto"))
                    .collect::<Vec<_>>()
                    .join(" ");
                cmake
                     .configure_arg(format!("-D{cmake_var}={filtered}"))
                     .env(var, filtered);
            }
        }
    }
}

[UMCEKO]

tested the configure_arg version on a clean build with the same makepkg CFLAGS reproducer — works. Compiled 666/666, kopuz binary links and contains opus_decoder_create at a real address (0xe41090). configure_arg lands after cmake-rs's own -DCMAKE_C_FLAGS=... in the cmake invocation, so it overrides cleanly. nice — much better than set_var. ready to merge from my side.

[DoumanAsh]

I published new version 0.7.3 with this PR