lerouxrgd / datachannel-rs

Rust wrappers for libdatachannel
Mozilla Public License 2.0
135 stars 26 forks source link

Build script seems to be building libdatachannel twice with the `static` feature enabled #29

Closed JRF63 closed 2 years ago

JRF63 commented 2 years ago

If I'm not misreading it, this one would still run after the first config.build();. Is that intentional?

I was working (not yet ready for a PR) on making the static feature work on Windows when I noticed I have both datachannel.lib and datachannel-static.lib inside OUT_DIR/lib/Debug.

lerouxrgd commented 2 years ago

Yes because the first one has config.build_target("datachannel-static");

JRF63 commented 2 years ago

Yes, but exactly one of them is being linked. Why build the shared library too? I had the tests working with just the one for datachannel-static.

If I'm not clear, I think with the static feature it is building both datachannel and datachannel-static but is only linking with datachannel-static.

lerouxrgd commented 2 years ago

If I remember correctly config.build_target("datachannel-static") will build only datachannel-static, so you still need to build all other static libs (usrsctplib, etc) in order to link them.

It's possible that libdatachannel's CMake files have changed since I made this build.rs though, I'll try to write a CI that checks all that.

JRF63 commented 2 years ago

It does only build that. The code path is just hitting both:

#[cfg(feature = 'static')]
{
    // static
}

// shared

instead of

if cfg!(feature = 'static') {
    // static
} else {
    // shared
}

or

#[cfg(feature = 'static')]
{
    // static
}

#[cfg(not(feature = 'static'))]
{
    // shared
}

I'm closing this for now, thanks for your time. I just saw it while doing a fix for handling CMake if it auto-chooses Visual Studio or XCode.