389ds / 389-ds-base

The enterprise-class Open Source LDAP server for Linux
https://www.port389.org/
Other
210 stars 88 forks source link

389-ds-base does not honor standard Fedora compiler flags for Rust #6306

Open vashirov opened 3 weeks ago

vashirov commented 3 weeks ago

Issue Description Cloned from https://bugzilla.redhat.com/show_bug.cgi?id=2167213

Currently, the Rust module in 389-ds-base is built without default Fedora compiler flags for Rust code (i.e. "-Copt-level=3 -Cdebuginfo=2 -Ccodegen-units=1 -Clink-arg=-Wl,-z,relro -Clink-arg=-Wl,-z,now --cap-lints=warn") - essentially, the code is not fully optimized, and does not contain debuginfo or frame pointers.

RUSTFLAGS are the standard environment variable for setting compiler flags for rustc (similar to CFLAGS / CXXFLAGS / LDFLAGS), but they aren't set by default (and not yet included in %set_build_flags, but I've reported an RFE about this).

It appears that the 389-ds-base build system currently hard-codes these compiler flags: --release --verbose -- -C debuginfo=2

This results in Rust code compiled as part of 389-ds-base not being optimized at the same level as other Rust code in Fedora (i.e. -Copt-level=3 is default in Fedora, but --release mode only implies -Copt-level=2), and it also doesn't include the other mandatory compiler flags, which result in better quality of generated code (and on rawhide, includes frame pointers on x86_64 and aarch64).

Firstyear commented 3 weeks ago

https://doc.rust-lang.org/cargo/reference/profiles.html#opt-level

"It is recommended to experiment with different levels to find the right balance for your project. There may be surprising results, such as level 3 being slower than 2, or the "s" and "z" levels not being necessarily smaller. "

It's not wise to apply blanket settings from the distro, they may not always be correct.

decathorpe commented 3 weeks ago

It's not wise to apply blanket settings from the distro, they may not always be correct.

At least in Fedora, default compiler flags MUST be honored unless there is a documented reason not to do so.