psm: allow manual opt out of #cfg[link("psm_s")]

[thoughtpolice]

Currenty, when using the asm configuration, the lib.rs file will manually add a link("psm_s") attribute to the build causing a flag like -lpsm_s to appear on the link line. psm_s is the library of assembly code, built by build.rs in Cargo projects.

However, when using Cargo packages with build systems like Buck2, we have to manually replace the build.rs script, and build the bits of C/assembly code that come with psm and stacker as a separate build item (an actual library), then link them into the Rust libraries.

The name of the library often can't be easily made identical to psm_s as desired by the link() call, but we're building and explicitly linking the library anyway, removing the need for this.

Therefore, introduce a new asm_manual_link cfg option to give "expert" users the ability to link in code manually.

[nagisa]

Since you're replacing the build.rs script anyway, is there any reason why you aren't modifying the build.rs script to also not set the cfg(asm)? Or rather, do you think it would be reasonable enough to keep this all inside the build.rs, possibly by splitting cfg(asm) into two cfgs, one of which would only control emission of the #[link] attribute?

Err, well, I shouldn't be looking at code when its one past midnight outside. I'll look at this again tomorrow.

[nagisa]

The only thing I think would be an improvement here is to invert the newly added cfg here, have the build.rs script enable it whenever this link(...) attribute has to be emitted.

So the code in psm/src/lib.rs looks more like

#![cfg_attr(link_asm, link(name="psm_s"))]

and then in your buck2-specific build.rs you can remove the print that enables the cfg. Since build.rs is going to have a println! to enable emission of this attribute, that is going to be a perfect location to also add a short comment motivating the split of attributes (just one sentence is fine.)

---

That said, there is one important detail. Not using link(...) on extern is actually depriving rustc of sufficient information to correctly invoke the linker. Unless you're invoking the linker directly, omitting this isn't actually correct.

[thoughtpolice]

OK, thanks! I wasn't sure if you wanted it on or off by default, but I can make that change.

That said, there is one important detail. Not using link(...) on extern is actually depriving rustc of sufficient information to correctly invoke the linker. Unless you're invoking the linker directly, omitting this isn't actually correct.

Yeah, speaking briefly, in Buck the dependency graph might look something like cxx_library() -> rust_library() -> rust_binary() to show that libpsm_s is depended on by (rust crate) psm and then used by some app. Buck and Bazel are smart enough to have the rust_library() "propagate" the cxx_library() dependencies it has, so even if you only list the rust library, you will also link against the cxx library — which is exactly what happens here. So by simply depending on the psm crate I'll get the proper bits built and installed.

The real thing is mainly just that the library name can't be made identical to psm_s for "reasons" and this is probably the easiest fix among a handful of places to do it (e.g. maybe I could fix it in the upstream Buck2 rule? But it's a lot harder and this works with older versions of Buck2.) So this has to be disabled for that reason alone, if nothing else.

[thoughtpolice]

Fixed with your review comments.