<div dir="ltr"><div>Hi Fabian,</div><div><br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="msg7646337196901425628"><div><div>On Sat, Apr 15, 2023, at 2:32 AM, Zixing Liu wrote:<br></div><blockquote type="cite" id="m_7646337196901425628qt"><div dir="ltr"><div>Hi Ximin and Fabian,<br></div><div><br></div><div><div>Recently I sent an email about the Rust toolchain LTO issue to the Debian Rust Maintainer mailing list. Due to the high-volume nature of that list, my email communication probably did not reach you. So here is a copy of the email in case the previous one landed inside your spam folder:<br></div></div></div></blockquote><div><br></div><div>Hi, I did see the previous one but had no time to answer yet. We do actually have a separate, lower-volume discussion list at <a href="mailto:debian-rust@lists.debian.org" target="_blank">debian-rust@lists.debian.org</a> (although the response ratio there is also not optimal, to be honest ;)).<br></div></div></div></blockquote><div><br></div><div>Okay, I will send future email communications to that mailing list instead. <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="msg7646337196901425628"><div><div></div><div><br></div><blockquote type="cite" id="m_7646337196901425628qt"><div dir="ltr"><div><div><div>I am Zixing Liu from Canonical. Recently it came to my attention that `dh-cargo` and the `cargo` wrapper do not honour the `DEB_BUILD_OPTIONS=optimize=-/+lto` flag.<br></div><div>Instead, whatever the project upstream defines in the `Cargo.toml` file (the `lto` parameter) takes precedence.<br></div><div><br></div><div>To better control the building parameters and avoid issues like <a href="https://github.com/rust-lang/rust/pull/89041" target="_blank">https://github.com/rust-lang/rust/pull/89041</a>, we need to be able to switch on or off LTO regardless of what the upstream project declares.<br></div><div><br></div><div>I propose adding a segment in the `dh-cargo` wrapper where we can pass the argument `--config "profile.release.lto = true/false"` to the `cargo` accordingly.<br></div><div><br></div><div>However, one thing I need some help with is the Rust toolchain's LTO option is a tri-state one, where it has `false,` `"thin,"` and `"fat"` (or "true") three possible values. In Debian, since the `DEB_BUILD_OPTIONS` is designed around GCC's conventions, it does not quite fit in our situation: `fat` may cause issues with debug information collection (see <a href="https://github.com/rust-lang/rust/pull/89041" target="_blank">https://github.com/rust-lang/rust/pull/89041</a> for more information, this is an LLVM bug that has been around for quite a while now), and `thin` may generate inferior machine code during the LTO phase when comparing to `fat.` So we can't use a 100% bullet-proof default value for the `+lto` situation.<br></div><div><br></div><div>What's your opinion on this? I can handle the implementation if we reach a conclusion.<br></div></div></div></div></blockquote><div><br></div><div>I don't have any strong opinions either way. <br></div><div><br></div><div>One possible approach would be to do the following:<br></div><div><br></div><div>nothing set => keep whatever is in Cargo.toml<br></div><div>+lto set, LTO enabled in Cargo.toml => keep Cargo.toml<br></div><div>+lto set, LTO not enabled in Cargo.toml => override Cargo.toml with thin for now, since that causes less problems (?)<br></div><div>-lto set => disable LTO, regardless of Cargo.toml<br></div><div><br></div><div>the second and third option could be customized by patching Cargo.toml, or, if really needed, we could allow forcing the set value via an ENV variable as well.</div></div></div></blockquote><div> </div><div>Actually, we could pass the `--config  "profile.release.lto = <value>` parameter to the cargo to achieve the same effect without touching the Cargo.toml file.</div><div>Passing `-Clto` through `RUSTFLAGS` has its own problems where if the project contains procedural macros or non-final-binary artifacts, the compiler will abort the compilation and complain that `-Clto` does not apply for Rust rlib libraries and proc_macro crates.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="msg7646337196901425628"><div><div>an alternate approach would be to honor +-lto with +lto always meaning thin (or fat), regardless of Cargo.toml's contents, and having a separate mechanism (ENV var, cargo wrapper parameter) to force the value.<br></div><div><br></div></div></div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="msg7646337196901425628"><div><div>note that dh-cargo is mainly (meant to be) used via debcargo, so it probably makes sense to implement the option throughout the stack, so that packages that are not using debcargo/dh-cargo (like firefox, librsvg, ..) but only the wrapper directly can also profit: </div></div></div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="msg7646337196901425628"><div><div> <br></div></div></div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="msg7646337196901425628"><div><div></div><div>debcargo<br></div><div>- generates d/rules<br></div><div>d/rules<br></div><div>- sets up and calls dh-cargo<br></div><div>dh-cargo<br></div><div>- calls the packaged cargo's cargo wrapper in /usr/share/cargo/bin/cargo , which in turn might call the real cargo<br></div><div><br></div><div>my guess is that this means that actually the cargo wrapper is the one that implements handling of DEB_BUILD_OPTIONS (else we need to parse it twice) and the override env var (if one exists), and debcargo handles setting the override env var (if we want to implement one), with dh-cargo doing nothing at all. if you find an approach that seems better, feel free to just propose it as an MR on salsa in the respective repository, or discuss it here or in #debian-rust on IRC. given the freeze, it will likely be a while until it gets uploaded in any case.<br></div></div></div></blockquote><div><br></div><div>I guess we could make the Cargo wrapper at /usr/share/cargo/bin/cargo to handle the LTO switches since it's closer to the real Cargo and easier to override in the packaging configurations.</div><div>I will post my patches on the Debian Salsa when I finish the implementation.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="msg7646337196901425628"><div><div></div><div><br></div><div>Ximin is currently semi-afk, so I am not sure if he'll respond (soon).<br></div></div></div></blockquote><div><br></div><div>Understood. I will ping them on the Merge Request regarding the review process.</div><div><br></div><div>Thanks,</div><div>Zixing</div></div></div>